Improve API Provider Test Cases#14168
Merged
PasanT9 merged 4 commits intowso2:masterfrom Apr 22, 2026
Merged
Conversation
nimsara66
requested review from
AnuGayan,
Arshardh,
chamilaadhi and
tharindu1st
as code owners
Contributor
📝 WalkthroughSummaryThis PR enhances API provider-change integration tests by introducing a secondary-user-store test class and expanding the existing provider-change test suite to improve coverage, robustness, and validation across multiple API types. ChangesNew Test Class: ChangeApiProviderSecondaryUserStoreTestCase
Enhanced ChangeApiProviderTestCase
Test Suite Configuration
Impact
WalkthroughAdds a new integration test class that validates changing an API's provider to a user from a secondary user store, and enhances the existing provider-change tests with GraphQL coverage, tenant-aware provider handling, revision-based validations, and expanded post-change checks (documents, scopes, endpoints, policies). Tests exercise REST, SOAP, SOAP-to-REST, and GraphQL flows, including creating a secondary user, redeploying gateway revisions around provider changes, and verifying artifact accessibility by current API ID and prior revision UUIDs. TestNG suite updated to run both classes; teardown restores userstore and server configuration. Sequence Diagram(s)sequenceDiagram
participant Test as Test Runner
participant US as UserStoreService
participant Admin as API Admin API
participant Registry as Artifact Store
participant GW as Gateway
participant Backend as Backend Service
Test->>US: add secondary user with roles
Test->>Admin: create/import API (REST/SOAP/GraphQL)
Admin->>Registry: persist API + create revision UUID
Admin->>GW: deploy revision
GW->>Backend: invoke pre-change API request
Backend-->>GW: response
GW-->>Test: return pre-change response
Test->>Admin: change API provider to SECONDARY/user
Admin->>US: resolve provider identity in secondary store
Admin->>Registry: update API provider metadata
Admin->>GW: undeploy old revision
Admin->>GW: deploy new revision
GW->>Backend: invoke post-change API request
Backend-->>GW: response
GW-->>Test: return post-change response
Test->>Registry: fetch API by current ID
Registry-->>Test: API artifact
Test->>Registry: fetch API by prior revision UUID
Registry-->>Test: revision artifact
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 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: 6
🧹 Nitpick comments (5)
all-in-one-apim/modules/integration/tests-integration/tests-backend/src/test/java/org/wso2/am/integration/tests/admin/ChangeApiProviderTestCase.java (3)
1109-1116: Minor: use try-with-resources for the writer.
BufferedWriteris closed explicitly, but an exception thrown betweenwriteandclosewill leak the file handle. A try-with-resources block avoids this without requiringfinallyboilerplate. Also,FileWriteruses the platform default charset; specifying UTF-8 (viaFiles.newBufferedWriter) makes the written schema encoding deterministic across platforms. The same applies to the copy of this helper inChangeApiProviderSecondaryUserStoreTestCase.java(Lines 1111–1118).Proposed tweak
- private File getTempFileWithContent(String schema) throws Exception { - File temp = File.createTempFile("schema", ".graphql"); - temp.deleteOnExit(); - BufferedWriter out = new BufferedWriter(new FileWriter(temp)); - out.write(schema); - out.close(); - return temp; - } + private File getTempFileWithContent(String schema) throws Exception { + File temp = File.createTempFile("schema", ".graphql"); + temp.deleteOnExit(); + try (BufferedWriter out = java.nio.file.Files.newBufferedWriter( + temp.toPath(), java.nio.charset.StandardCharsets.UTF_8)) { + out.write(schema); + } + return temp; + }🤖 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/admin/ChangeApiProviderTestCase.java` around lines 1109 - 1116, The helper getTempFileWithContent should use a try-with-resources writer and an explicit UTF-8 charset to avoid resource leaks and platform-dependent encoding; replace the BufferedWriter/FileWriter usage in getTempFileWithContent with Files.newBufferedWriter(temp.toPath(), StandardCharsets.UTF_8) inside a try-with-resources block, keep temp.deleteOnExit(), write the schema, and apply the same change to the duplicate helper in ChangeApiProviderSecondaryUserStoreTestCase.
1160-1163: GuarduserManagementClientdeletion against@BeforeClassfailure.If
@BeforeClassfails beforeuserManagementClientis initialized (e.g., duringsuper.init),destroy()will NPE at Line 1161. Add a null check onuserManagementClient(matching the pattern used forapiID,soapApiId, etc.) to ensure cleanup remains idempotent.Proposed fix
- if (newUser != null) { - userManagementClient.deleteUser(newUser); - } + if (newUser != null && userManagementClient != null) { + userManagementClient.deleteUser(newUser); + }🤖 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/admin/ChangeApiProviderTestCase.java` around lines 1160 - 1163, The cleanup currently calls userManagementClient.deleteUser(newUser) without checking whether userManagementClient was initialized, which can NPE if `@BeforeClass` failed; update the cleanup/destroy method (where userManagementClient and newUser are referenced) to guard deletion with a null check on userManagementClient (similar to existing guards for apiID/soapApiId/etc.), i.e., only call userManagementClient.deleteUser(newUser) when userManagementClient != null (and optionally newUser != null) before calling super.cleanUp().
320-323: Captured redeploy revision UUID is never used.
revisionUUID_2(Line 321) is assigned but never referenced. The same dead-local pattern is repeated at Line 678 (soapApiRevisionUUID_2) and Line 840 (soapToRestRevisionUUID_2). Either use them in subsequent revision-based retrievals/assertions or drop the assignment.🤖 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/admin/ChangeApiProviderTestCase.java` around lines 320 - 323, The variable revisionUUID_2 (and the similar soapApiRevisionUUID_2 and soapToRestRevisionUUID_2) are assigned from createAPIRevisionAndDeployUsingRest but never used; either remove these dead local assignments or use them in subsequent assertions/retrievals. Locate the calls to createAPIRevisionAndDeployUsingRest where revisionUUID_2, soapApiRevisionUUID_2, and soapToRestRevisionUUID_2 are returned and either (A) propagate and assert against the returned UUID in later revision-based checks (e.g., verify deployment/state by revision UUID) or (B) remove the unused local variables and just call createAPIRevisionAndDeployUsingRest(...) for its side effects.all-in-one-apim/modules/integration/tests-integration/tests-backend/src/test/java/org/wso2/am/integration/tests/admin/ChangeApiProviderSecondaryUserStoreTestCase.java (2)
329-337: Unused revision UUID captured after redeploy.
revisionUUID_2is assigned but never referenced again in this method. The same pattern recurs at Line 684 (soapApiRevisionUUID_2) and Line 844 (soapToRestRevisionUUID_2). Either use the new revision UUID in follow-up assertions (e.g., retrieve the API/Swagger by the new revision UUID to verify post-change revision state) or replace the assignment with a plain call to avoid dead locals.Proposed tweak
- String revisionUUID_2 = createAPIRevisionAndDeployUsingRest(apiID, restAPIPublisher); + createAPIRevisionAndDeployUsingRest(apiID, restAPIPublisher);🤖 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/admin/ChangeApiProviderSecondaryUserStoreTestCase.java` around lines 329 - 337, The local variable revisionUUID_2 is assigned from createAPIRevisionAndDeployUsingRest(apiID, restAPIPublisher) but never used; remove the dead local or use it to assert post-deploy state. Fix by either replacing the assignment with a direct call (createAPIRevisionAndDeployUsingRest(apiID, restAPIPublisher);) or add assertions that reference revisionUUID_2 (e.g., call a helper to fetch the revision or Swagger by revision UUID and verify expected changes) so the value is consumed; apply the same fix for soapApiRevisionUUID_2 and soapToRestRevisionUUID_2.
193-581: Significant duplication withChangeApiProviderTestCase.The bodies of
ChangeApiProvider,ChangeSoapApiProvider,ChangeSoapToRestApiProvider, andChangeGraphQLApiProviderin this class are nearly identical to the corresponding methods inChangeApiProviderTestCase.java(the only material differences being the target provider —SECONDARY_USERvs.newProviderName— and the absence of the cross-tenant negative check). This roughly doubles the test-code surface for future maintenance.Consider extracting the shared test flow (API creation, deployment, revision handling, post-change validations) into a protected helper/base class parameterized by the target provider, and keeping only the secondary-user-store-specific wiring in this subclass. This will keep the two suites consistent when assertions or flows evolve.
🤖 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/admin/ChangeApiProviderSecondaryUserStoreTestCase.java` around lines 193 - 581, The four test methods ChangeApiProvider, ChangeSoapApiProvider, ChangeSoapToRestApiProvider, and ChangeGraphQLApiProvider duplicate the same end-to-end flow already implemented in ChangeApiProviderTestCase; extract the shared flow (API creation, createAPIRevisionAndDeployUsingRest, waitForAPIDeploymentSync/waitForAPIDeployment, publish/undeploy/redeploy, swagger/document/scope/business/policy/comment/resource/endpoint/scope validations) into a protected helper or base test class (e.g., ApiProviderChangeBase) that accepts the target provider and any small variances (provider name, extra negative check) as parameters, then refactor the methods in ChangeApiProviderSecondaryUserStoreTestCase to call that helper with SECONDARY_USER and keep only the secondary-user-specific setup (restAPIAdminClient instantiation) in this subclass; ensure to reference and reuse functions like restAPIPublisher.getAPIByID, createAPIRevisionAndDeployUsingRest, restAPIPublisher.updateAPI, restAPIPublisher.updateSwagger, restAPIPublisher.addDocument, restAPIPublisher.getDocuments, restAPIPublisher.addComment, and the assertion checks so behavior remains identical.
🤖 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/admin/ChangeApiProviderSecondaryUserStoreTestCase.java`:
- Around line 701-704: Update the assertion messages to match the expected value
being checked: change the failure message for the assertion comparing
apiByRevisionSoap.getProvider() to originalProvider (in the assertion around
restAPIPublisher.getAPIByID(soapApiRevisionUUID) where variable APIDTO
apiByRevisionSoap is used) so it reads "SOAP API provider from revision should
match original provider"; likewise update the analogous assertion for the
SOAPTOREST case (the assertion comparing getProvider() against originalProvider
at the lines around the SOAPTOREST check) to also say "original provider" for
consistency.
- Around line 149-191: The test initializes an unused userManagementClient and
may not be forcing a server restart when applying the secondary userstore,
causing inconsistent reloads; remove the unused userManagementClient
initialization (the variable initialized via new UserManagementClient(...)) and
change the serverConfigurationManager.applyConfiguration(...) invocation to
enable restart so the server picks up the new userstores (set the restart
parameter to true in the applyConfiguration call that uses
secondaryUserStoreFile/targetSecondaryUserStoreFile).
- Line 23: Remove the internal JDK import
jdk.internal.joptsimple.internal.Strings and replace all usages of Strings.EMPTY
with the empty string literal "" in the
ChangeApiProviderSecondaryUserStoreTestCase class; specifically remove the
import line and update every occurrence (6 places) of Strings.EMPTY to "" in
methods within that class so the code no longer depends on the internal JDK
package.
In
`@all-in-one-apim/modules/integration/tests-integration/tests-backend/src/test/java/org/wso2/am/integration/tests/admin/ChangeApiProviderTestCase.java`:
- Around line 695-698: The assertion messages are inconsistent: in
ChangeApiProviderTestCase the assertion comparing
apiByRevisionSoap.getProvider() to originalProvider uses the message "should
match new provider"; update that message to "should match original provider" to
match the compared value and the REST case; also locate the similar SOAPTOREST
assertion (the assert comparing provider to originalProvider at the other block)
and change its message from "should match new provider" to "should match
original provider" so both assertions (the APIDTO apiByRevisionSoap check and
the SOAPTOREST provider check) have messages that reflect the actual compared
variable originalProvider.
- Around line 294-308: The negative test in ChangeApiProviderTestCase constructs
crossTenantProvider incorrectly for non-super tenant mode causing it to point to
the local admin; change the crossTenantProvider assignment to use a qualified
cross-tenant user (e.g., set crossTenantProvider = firstUserName + "@wso2.com"
when tenantDomain != "carbon.super") and call
restAPIAdminClient.changeApiProvider(crossTenantProvider, apiID) as before; also
replace brittle substring assertions on e.getResponseBody() for "901409" and
"Tenant mismatch" by parsing the error JSON from e.getResponseBody() and
asserting on the structured error fields (error code and message) instead.
- Line 156: The call to userManagementClient.addUser(...) is passing the
username variable newUser as the 4th parameter (profileName); change the 4th
argument in the addUser invocation to a valid profile name (e.g., "default") or
null instead of newUser so profileName is correct; update the call where
userManagementClient.addUser(newUser, newUserPass, newUserRoles, newUser) to use
"default" or null as the last parameter (referencing
userManagementClient.addUser, newUser, newUserPass, newUserRoles).
---
Nitpick comments:
In
`@all-in-one-apim/modules/integration/tests-integration/tests-backend/src/test/java/org/wso2/am/integration/tests/admin/ChangeApiProviderSecondaryUserStoreTestCase.java`:
- Around line 329-337: The local variable revisionUUID_2 is assigned from
createAPIRevisionAndDeployUsingRest(apiID, restAPIPublisher) but never used;
remove the dead local or use it to assert post-deploy state. Fix by either
replacing the assignment with a direct call
(createAPIRevisionAndDeployUsingRest(apiID, restAPIPublisher);) or add
assertions that reference revisionUUID_2 (e.g., call a helper to fetch the
revision or Swagger by revision UUID and verify expected changes) so the value
is consumed; apply the same fix for soapApiRevisionUUID_2 and
soapToRestRevisionUUID_2.
- Around line 193-581: The four test methods ChangeApiProvider,
ChangeSoapApiProvider, ChangeSoapToRestApiProvider, and ChangeGraphQLApiProvider
duplicate the same end-to-end flow already implemented in
ChangeApiProviderTestCase; extract the shared flow (API creation,
createAPIRevisionAndDeployUsingRest,
waitForAPIDeploymentSync/waitForAPIDeployment, publish/undeploy/redeploy,
swagger/document/scope/business/policy/comment/resource/endpoint/scope
validations) into a protected helper or base test class (e.g.,
ApiProviderChangeBase) that accepts the target provider and any small variances
(provider name, extra negative check) as parameters, then refactor the methods
in ChangeApiProviderSecondaryUserStoreTestCase to call that helper with
SECONDARY_USER and keep only the secondary-user-specific setup
(restAPIAdminClient instantiation) in this subclass; ensure to reference and
reuse functions like restAPIPublisher.getAPIByID,
createAPIRevisionAndDeployUsingRest, restAPIPublisher.updateAPI,
restAPIPublisher.updateSwagger, restAPIPublisher.addDocument,
restAPIPublisher.getDocuments, restAPIPublisher.addComment, and the assertion
checks so behavior remains identical.
In
`@all-in-one-apim/modules/integration/tests-integration/tests-backend/src/test/java/org/wso2/am/integration/tests/admin/ChangeApiProviderTestCase.java`:
- Around line 1109-1116: The helper getTempFileWithContent should use a
try-with-resources writer and an explicit UTF-8 charset to avoid resource leaks
and platform-dependent encoding; replace the BufferedWriter/FileWriter usage in
getTempFileWithContent with Files.newBufferedWriter(temp.toPath(),
StandardCharsets.UTF_8) inside a try-with-resources block, keep
temp.deleteOnExit(), write the schema, and apply the same change to the
duplicate helper in ChangeApiProviderSecondaryUserStoreTestCase.
- Around line 1160-1163: The cleanup currently calls
userManagementClient.deleteUser(newUser) without checking whether
userManagementClient was initialized, which can NPE if `@BeforeClass` failed;
update the cleanup/destroy method (where userManagementClient and newUser are
referenced) to guard deletion with a null check on userManagementClient (similar
to existing guards for apiID/soapApiId/etc.), i.e., only call
userManagementClient.deleteUser(newUser) when userManagementClient != null (and
optionally newUser != null) before calling super.cleanUp().
- Around line 320-323: The variable revisionUUID_2 (and the similar
soapApiRevisionUUID_2 and soapToRestRevisionUUID_2) are assigned from
createAPIRevisionAndDeployUsingRest but never used; either remove these dead
local assignments or use them in subsequent assertions/retrievals. Locate the
calls to createAPIRevisionAndDeployUsingRest where revisionUUID_2,
soapApiRevisionUUID_2, and soapToRestRevisionUUID_2 are returned and either (A)
propagate and assert against the returned UUID in later revision-based checks
(e.g., verify deployment/state by revision UUID) or (B) remove the unused local
variables and just call createAPIRevisionAndDeployUsingRest(...) for its side
effects.
🪄 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: 96907e2b-8bbf-46f7-a820-3d02e283c927
📒 Files selected for processing (3)
all-in-one-apim/modules/integration/tests-integration/tests-backend/src/test/java/org/wso2/am/integration/tests/admin/ChangeApiProviderSecondaryUserStoreTestCase.javaall-in-one-apim/modules/integration/tests-integration/tests-backend/src/test/java/org/wso2/am/integration/tests/admin/ChangeApiProviderTestCase.javaall-in-one-apim/modules/integration/tests-integration/tests-backend/src/test/resources/testng.xml
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
all-in-one-apim/modules/integration/tests-integration/tests-backend/src/test/java/org/wso2/am/integration/tests/admin/ChangeApiProviderTestCase.java (1)
81-82: Unused private fields.
serverConfigurationManagerandremoteUserStoreManagerServiceClientare declared but never instantiated or referenced in this class. Consider removing them to avoid confusion with the near-identicalChangeApiProviderSecondaryUserStoreTestCasewhere they are actually used.Proposed fix
- private ServerConfigurationManager serverConfigurationManager; - private RemoteUserStoreManagerServiceClient remoteUserStoreManagerServiceClient; private RestAPIAdminImpl restAPIAdminClient;The corresponding imports on lines 35 and 66 can also be dropped.
🤖 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/admin/ChangeApiProviderTestCase.java` around lines 81 - 82, Remove the unused private fields serverConfigurationManager and remoteUserStoreManagerServiceClient from the ChangeApiProviderTestCase class, and delete the now-unnecessary imports that brought in ServerConfigurationManager and RemoteUserStoreManagerServiceClient so the class no longer contains dead declarations or unused imports; ensure no other references to these symbols remain in the file before committing.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In
`@all-in-one-apim/modules/integration/tests-integration/tests-backend/src/test/java/org/wso2/am/integration/tests/admin/ChangeApiProviderTestCase.java`:
- Around line 81-82: Remove the unused private fields serverConfigurationManager
and remoteUserStoreManagerServiceClient from the ChangeApiProviderTestCase
class, and delete the now-unnecessary imports that brought in
ServerConfigurationManager and RemoteUserStoreManagerServiceClient so the class
no longer contains dead declarations or unused imports; ensure no other
references to these symbols remain in the file before committing.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 48e91478-1ef8-4b65-916f-83651266a525
📒 Files selected for processing (2)
all-in-one-apim/modules/integration/tests-integration/tests-backend/src/test/java/org/wso2/am/integration/tests/admin/ChangeApiProviderSecondaryUserStoreTestCase.javaall-in-one-apim/modules/integration/tests-integration/tests-backend/src/test/java/org/wso2/am/integration/tests/admin/ChangeApiProviderTestCase.java
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
all-in-one-apim/modules/integration/tests-integration/tests-backend/src/test/java/org/wso2/am/integration/tests/admin/ChangeApiProviderTestCase.java (1)
321-323: Unused captured revision UUID.
revisionUUID_2(and similarlysoapApiRevisionUUID_2at Line 678 andsoapToRestRevisionUUID_2at Line 840) is assigned but never referenced later. Either use it (e.g., in post-change revision-based retrieval assertions or teardown) or drop the assignment to avoid implying intent that isn't followed through.Proposed fix
- String revisionUUID_2 = createAPIRevisionAndDeployUsingRest(apiID, restAPIPublisher); + createAPIRevisionAndDeployUsingRest(apiID, restAPIPublisher);🤖 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/admin/ChangeApiProviderTestCase.java` around lines 321 - 323, The variable revisionUUID_2 (and likewise soapApiRevisionUUID_2 and soapToRestRevisionUUID_2) is captured from createAPIRevisionAndDeployUsingRest/createAPIRevisionAndDeployUsingSOAP but never used; either remove the unused assignment or use the UUID in subsequent assertions/teardown (for example validate revision existence or undeploy/remove by revision). Locate the calls to createAPIRevisionAndDeployUsingRest, createAPIRevisionAndDeployUsingSOAP and the variables revisionUUID_2, soapApiRevisionUUID_2, soapToRestRevisionUUID_2 and either delete the redundant "= create..." assignment or pass/save the returned UUID into the existing verification/cleanup logic so the captured revision is actually referenced.all-in-one-apim/modules/integration/tests-integration/tests-backend/src/test/java/org/wso2/am/integration/tests/admin/ChangeApiProviderSecondaryUserStoreTestCase.java (1)
328-330: Unused captured revision UUIDs after redeploy.
revisionUUID_2here (andsoapApiRevisionUUID_2at Line 683,soapToRestRevisionUUID_2at Line 843) are assigned but never used. The GraphQL flow at Line 1067 correctly discards the return value. Either drop the assignments or consume them in a post-change revision-based assertion for consistency.Proposed fix
- String revisionUUID_2 = createAPIRevisionAndDeployUsingRest(apiID, restAPIPublisher); + createAPIRevisionAndDeployUsingRest(apiID, restAPIPublisher);🤖 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/admin/ChangeApiProviderSecondaryUserStoreTestCase.java` around lines 328 - 330, The variables revisionUUID_2, soapApiRevisionUUID_2, and soapToRestRevisionUUID_2 are assigned the return value of createAPIRevisionAndDeployUsingRest (and similar revision-creating calls) but never used; either remove the unused assignments and call createAPIRevisionAndDeployUsingRest(...) without capturing the result, or consume the returned revision UUID by adding a post-deploy assertion (e.g., verifyRevisionExists/revision list check) that uses the returned value; update the calls to createAPIRevisionAndDeployUsingRest and any corresponding SOAP-to-REST revision call sites to either drop the assignment or pass the returned UUID into the chosen assertion helper so the variables are no longer unused.
🤖 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/admin/ChangeApiProviderSecondaryUserStoreTestCase.java`:
- Around line 1162-1171: Wrap the teardown steps so
serverConfigurationManager.restoreToLastConfiguration() always executes even if
deleteUser or super.cleanUp throws: move the
deleteUser(remoteUserStoreManagerServiceClient.deleteUser(SECONDARY_USER)) and
super.cleanUp() calls into a try block and call
serverConfigurationManager.restoreToLastConfiguration() from a finally block
(checking serverConfigurationManager for null), ensuring
remoteUserStoreManagerServiceClient and super.cleanUp errors are still
propagated or logged but the configuration is restored regardless.
---
Nitpick comments:
In
`@all-in-one-apim/modules/integration/tests-integration/tests-backend/src/test/java/org/wso2/am/integration/tests/admin/ChangeApiProviderSecondaryUserStoreTestCase.java`:
- Around line 328-330: The variables revisionUUID_2, soapApiRevisionUUID_2, and
soapToRestRevisionUUID_2 are assigned the return value of
createAPIRevisionAndDeployUsingRest (and similar revision-creating calls) but
never used; either remove the unused assignments and call
createAPIRevisionAndDeployUsingRest(...) without capturing the result, or
consume the returned revision UUID by adding a post-deploy assertion (e.g.,
verifyRevisionExists/revision list check) that uses the returned value; update
the calls to createAPIRevisionAndDeployUsingRest and any corresponding
SOAP-to-REST revision call sites to either drop the assignment or pass the
returned UUID into the chosen assertion helper so the variables are no longer
unused.
In
`@all-in-one-apim/modules/integration/tests-integration/tests-backend/src/test/java/org/wso2/am/integration/tests/admin/ChangeApiProviderTestCase.java`:
- Around line 321-323: The variable revisionUUID_2 (and likewise
soapApiRevisionUUID_2 and soapToRestRevisionUUID_2) is captured from
createAPIRevisionAndDeployUsingRest/createAPIRevisionAndDeployUsingSOAP but
never used; either remove the unused assignment or use the UUID in subsequent
assertions/teardown (for example validate revision existence or undeploy/remove
by revision). Locate the calls to createAPIRevisionAndDeployUsingRest,
createAPIRevisionAndDeployUsingSOAP and the variables revisionUUID_2,
soapApiRevisionUUID_2, soapToRestRevisionUUID_2 and either delete the redundant
"= create..." assignment or pass/save the returned UUID into the existing
verification/cleanup logic so the captured revision is actually referenced.
🪄 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: c05f8614-e82c-4c99-8167-b74db2428a73
📒 Files selected for processing (2)
all-in-one-apim/modules/integration/tests-integration/tests-backend/src/test/java/org/wso2/am/integration/tests/admin/ChangeApiProviderSecondaryUserStoreTestCase.javaall-in-one-apim/modules/integration/tests-integration/tests-backend/src/test/java/org/wso2/am/integration/tests/admin/ChangeApiProviderTestCase.java
PasanT9
approved these changes
Apr 22, 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
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.
Description
${subject}