Add test cases for API Product behaviour#14120
Merged
PasanT9 merged 1 commit intowso2:masterfrom Apr 12, 2026
Merged
Conversation
tharikaGitHub
requested review from
AnuGayan,
Arshardh,
chamilaadhi and
tharindu1st
as code owners
|
|
Contributor
WalkthroughTwo new integration test cases were added to verify API product functionality: one tests restoring API revisions in products when operations differ between revisions, and another tests updating the underlying API of an API product with validation of API and operation mappings. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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.
Actionable comments posted: 1
🧹 Nitpick comments (1)
all-in-one-apim/modules/integration/tests-integration/tests-backend/src/test/java/org/wso2/am/integration/tests/apiproduct/APIProductCreationTestCase.java (1)
898-899: Strengthen operation validation beyond count comparison.At Line [898]-Line [899], count equality can still pass with incorrect operation mappings. Consider asserting verb+target membership too.
🔧 Suggested assertion hardening
Assert.assertEquals(matchingApi.getOperations().size(), apiOne.getOperations().size(), "Number of operations in the API Product should match the underlying API"); + Assert.assertTrue( + matchingApi.getOperations().stream().allMatch(productOp -> + apiOne.getOperations().stream().anyMatch(apiOp -> + apiOp.getVerb().equals(productOp.getVerb()) + && apiOp.getTarget().equals(productOp.getTarget()))), + "API Product operations should match underlying API verb/target mappings");🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@all-in-one-apim/modules/integration/tests-integration/tests-backend/src/test/java/org/wso2/am/integration/tests/apiproduct/APIProductCreationTestCase.java` around lines 898 - 899, Current assertion only compares counts between matchingApi.getOperations() and apiOne.getOperations(), which can hide incorrect mappings; update the test in APIProductCreationTestCase to iterate over apiOne.getOperations() and assert that each operation's verb and target (e.g., operation.getVerb(), operation.getTarget()) exist in matchingApi.getOperations() (compare by verb+target string or a composite key) and also assert any response/permission attributes if relevant; keep the existing count assertion but add these membership checks so matchingApi truly contains the same operations as apiOne.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@all-in-one-apim/modules/integration/tests-integration/tests-backend/src/test/java/org/wso2/am/integration/tests/api/revision/APIProductRevisionTestCase.java`:
- Around line 214-216: The local declaration of apisToBeUsed in the test is
shadowing the class-level field used by destroy(), causing created APIs to be
skipped in teardown; remove the local "List<APIDTO> apisToBeUsed = new
ArrayList<>();" and instead use the class field apisToBeUsed (e.g.,
apisToBeUsed.add(apiOne);) so the API created via
apiProductTestHelper.createAPIProductInPublisher(...) is tracked and cleaned up
by destroy().
---
Nitpick comments:
In
`@all-in-one-apim/modules/integration/tests-integration/tests-backend/src/test/java/org/wso2/am/integration/tests/apiproduct/APIProductCreationTestCase.java`:
- Around line 898-899: Current assertion only compares counts between
matchingApi.getOperations() and apiOne.getOperations(), which can hide incorrect
mappings; update the test in APIProductCreationTestCase to iterate over
apiOne.getOperations() and assert that each operation's verb and target (e.g.,
operation.getVerb(), operation.getTarget()) exist in matchingApi.getOperations()
(compare by verb+target string or a composite key) and also assert any
response/permission attributes if relevant; keep the existing count assertion
but add these membership checks so matchingApi truly contains the same
operations as apiOne.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: def78ceb-9c9c-422c-80e4-215cd0e85e3f
📒 Files selected for processing (2)
all-in-one-apim/modules/integration/tests-integration/tests-backend/src/test/java/org/wso2/am/integration/tests/api/revision/APIProductRevisionTestCase.javaall-in-one-apim/modules/integration/tests-integration/tests-backend/src/test/java/org/wso2/am/integration/tests/apiproduct/APIProductCreationTestCase.java
Comment on lines
+214
to
+216
| List<APIDTO> apisToBeUsed = new ArrayList<>(); | ||
| apisToBeUsed.add(apiOne); | ||
| APIProductDTO apiProductDTO = apiProductTestHelper.createAPIProductInPublisher(provider, name, context, version, |
Contributor
There was a problem hiding this comment.
Avoid shadowing apisToBeUsed; it skips teardown tracking.
At Line [214]-Line [216], the local apisToBeUsed shadows the class field used in destroy(). The API created in this test will not be cleaned up, which can pollute the test environment.
🧹 Suggested fix
- List<APIDTO> apisToBeUsed = new ArrayList<>();
- apisToBeUsed.add(apiOne);
+ List<APIDTO> productApisToBeUsed = new ArrayList<>();
+ productApisToBeUsed.add(apiOne);
+ this.apisToBeUsed.add(apiOne);
APIProductDTO apiProductDTO = apiProductTestHelper.createAPIProductInPublisher(provider, name, context, version,
- apisToBeUsed, policies);
+ productApisToBeUsed, policies);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@all-in-one-apim/modules/integration/tests-integration/tests-backend/src/test/java/org/wso2/am/integration/tests/api/revision/APIProductRevisionTestCase.java`
around lines 214 - 216, The local declaration of apisToBeUsed in the test is
shadowing the class-level field used by destroy(), causing created APIs to be
skipped in teardown; remove the local "List<APIDTO> apisToBeUsed = new
ArrayList<>();" and instead use the class field apisToBeUsed (e.g.,
apisToBeUsed.add(apiOne);) so the API created via
apiProductTestHelper.createAPIProductInPublisher(...) is tracked and cleaned up
by destroy().
PasanT9
approved these changes
Apr 12, 2026
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
3 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.
This PR adds test cases for [1].
[1] wso2/api-manager#4801
Summary by CodeRabbit