Open
Conversation
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
WalkthroughA single test assertion in ESBJAVA4394 was modified to expect SOAP fault code "101503" instead of "101508" in the Axis fault message check. Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~2 minutes Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
integration/mediation-tests/tests-patches/src/test/java/org/wso2/carbon/esb/passthru/transport/test/ESBJAVA4394.java (1)
53-53: Comments reference stale error code "101508".The assertion now expects fault code
101503, but the comments on lines 28–31 still reference101508. This inconsistency may confuse future maintainers.Consider updating the comments to reflect the corrected error code:
📝 Proposed comment updates
- * the 2nd listeners target port is not the actual service port, because by making the 2nd tcpmon listener to - * connect to a non existing port we could simulate the 101508 error. + * the 2nd listeners target port is not the actual service port, because by making the 2nd tcpmon listener to + * connect to a non existing port we could simulate the 101503 error. * - * Without the fix of ESBJAVA-4394, when a 101508 happens the response is not send back + * Without the fix of ESBJAVA-4394, when a 101503 happens the response is not send back * to the client. (client gets empty response)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@integration/mediation-tests/tests-patches/src/test/java/org/wso2/carbon/esb/passthru/transport/test/ESBJAVA4394.java` at line 53, Update the stale inline comment that references error code "101508" to the corrected code "101503" so it matches the assertion in ESBJAVA4394 (the test containing assertTrue(axisFaultMessage.contains("101503"))); locate the comment block around the test (previously lines 28–31) and change the referenced error code and any explanatory text to reference 101503 consistently.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In
`@integration/mediation-tests/tests-patches/src/test/java/org/wso2/carbon/esb/passthru/transport/test/ESBJAVA4394.java`:
- Line 53: Update the stale inline comment that references error code "101508"
to the corrected code "101503" so it matches the assertion in ESBJAVA4394 (the
test containing assertTrue(axisFaultMessage.contains("101503"))); locate the
comment block around the test (previously lines 28–31) and change the referenced
error code and any explanatory text to reference 101503 consistently.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: f917c62c-b53a-4d4e-b185-94cf90c1417f
📒 Files selected for processing (1)
integration/mediation-tests/tests-patches/src/test/java/org/wso2/carbon/esb/passthru/transport/test/ESBJAVA4394.java
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Purpose
Update the ESBJAVA4394 test case to align with the fix: wso2/wso2-synapse#2431
Summary
In the test, since the request is written through the reachable first hop and the downstream side terminates later,
endOfInput()is triggered while the state isREQUEST_DONE.Previously,
endOfInput()method only invokedconn.close(). As a result,closed()method was triggered later to handle the fault while the state was stillREQUEST_DONE. Then,handleError()was called withCONNECTION_CLOSED(base error code101505) and stateREQUEST_DONE(ordinal3) [1].Inside
handleError(), the final error is computed as:finalError = baseErrorCode + state.ordinal()[2]So the final error became
101508, which was asserted in the test.However, after changing
endOfInput()with PR [3] to directly callhandleError()withSND_IO_ERROR(base error code101500) using the same state, the base error code changed to101500while the state remainedREQUEST_DONE(ordinal3). As a result, the final error became101503.Related PRs
#4312
Summary by CodeRabbit