Add integration test: MCPAssociatedAPIResourceUpdateTestCase

[kavindasr]

Purpose

Related to: wso2/api-manager#5110
Carbon-apimgt PR: wso2/carbon-apimgt#13891

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: f9993182-9f5b-4128-806c-f1b3d2d3ca92

📥 Commits

Reviewing files that changed from the base of the PR and between 68db5d6 and 8e0127c.

📒 Files selected for processing (1)

• all-in-one-apim/modules/integration/tests-integration/tests-backend/src/test/java/org/wso2/am/integration/tests/mcp/MCPAssociatedAPIResourceUpdateTestCase.java

🚧 Files skipped from review as they are similar to previous changes (1)

• all-in-one-apim/modules/integration/tests-integration/tests-backend/src/test/java/org/wso2/am/integration/tests/mcp/MCPAssociatedAPIResourceUpdateTestCase.java

---

📝 Walkthrough

Walkthrough

Adds a TestNG integration test for MCP-associated API resource updates, including API creation, MCP server generation, resource addition verification, and rejection of removing the MCP-mapped resource. The test is registered in the backend TestNG suite.

Changes

MCP Resource Update Integration Test

Layer / File(s)	Summary
Environment setup and suite registration ...mcp/MCPAssociatedAPIResourceUpdateTestCase.java, ...resources/testng.xml	Registers the new test class and starts a WireMock backend on an available port with a /res.* stub.
API and MCP server creation ...mcp/MCPAssociatedAPIResourceUpdateTestCase.java	Creates and deploys the initial API with /res1, then generates an MCP server with a TOOL operation mapped to that backend resource.
Resource addition and gateway invocation ...mcp/MCPAssociatedAPIResourceUpdateTestCase.java	Updates the API to add /res2, redeploys it, creates subscription and token, and verifies both /res1 and /res2 succeed at the gateway without error code 900906.
Removal blocking, cleanup, and helpers ...mcp/MCPAssociatedAPIResourceUpdateTestCase.java	Attempts to remove /res1 and expects HTTP 403 with error code 904001, then cleans up created resources and adds helper methods for operations, assertions, and port selection.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 30.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and concisely describes the main change: adding the MCPAssociatedAPIResourceUpdateTestCase integration test.
Description check	✅ Passed	The description is related to the changeset by linking the relevant issue and upstream PR.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

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/mcp/MCPAssociatedAPIResourceUpdateTestCase.java (1)

196-200: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win

Replace the fixed sleep with a bounded retry on /res2.

waitForAPIDeploymentSync(..., IS_API_EXISTS) can return before the newly deployed revision is the active one, and the extra Thread.sleep(5000) just makes this test timing-sensitive in slower CI. Polling /res2 until it returns 200 without 900906 would make the assertion both revision-aware and less flaky. Based on learnings, waitForAPIDeploymentSync(..., IS_API_EXISTS) only proves that some revision exists, not that the new one is active.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@all-in-one-apim/modules/integration/tests-integration/tests-backend/src/test/java/org/wso2/am/integration/tests/mcp/MCPAssociatedAPIResourceUpdateTestCase.java`
around lines 196 - 200, The test still relies on a fixed sleep after
createAPIRevisionAndDeployUsingRest and waitForAPIDeploymentSync(...,
APIMIntegrationConstants.IS_API_EXISTS), but that only proves an API exists, not
that the newly deployed revision is active. Replace the
Thread.sleep(WAIT_FOR_DEPLOYMENT_IN_MILLISECONDS) in
MCPAssociatedAPIResourceUpdateTestCase with a bounded retry that repeatedly
calls the /res2 endpoint until it returns HTTP 200 and no 900906 error, using
the existing API deployment/rest client helpers to keep the assertion
revision-aware and less flaky.

Source: Learnings

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In
`@all-in-one-apim/modules/integration/tests-integration/tests-backend/src/test/java/org/wso2/am/integration/tests/mcp/MCPAssociatedAPIResourceUpdateTestCase.java`:
- Around line 258-274: Teardown in this test should be best-effort and not let
subscription/application cleanup failures skip MCP/API deletion. Update the
cleanup block in MCPAssociatedAPIResourceUpdateTestCase to catch and continue
past exceptions around restAPIStore.removeSubscription and
restAPIStore.deleteApplication, while still attempting
restAPIPublisher.deleteMCPServer and restAPIPublisher.deleteAPI. Also,
explicitly assert the result of restAPIPublisher.deleteMCPServer(...) is
non-null so MCP server deletion failure is detected, since deleteMCPServer only
returns a non-null response on HTTP 200.

---

Nitpick comments:
In
`@all-in-one-apim/modules/integration/tests-integration/tests-backend/src/test/java/org/wso2/am/integration/tests/mcp/MCPAssociatedAPIResourceUpdateTestCase.java`:
- Around line 196-200: The test still relies on a fixed sleep after
createAPIRevisionAndDeployUsingRest and waitForAPIDeploymentSync(...,
APIMIntegrationConstants.IS_API_EXISTS), but that only proves an API exists, not
that the newly deployed revision is active. Replace the
Thread.sleep(WAIT_FOR_DEPLOYMENT_IN_MILLISECONDS) in
MCPAssociatedAPIResourceUpdateTestCase with a bounded retry that repeatedly
calls the /res2 endpoint until it returns HTTP 200 and no 900906 error, using
the existing API deployment/rest client helpers to keep the assertion
revision-aware and less flaky.

🪄 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: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: e82600e6-3067-4930-a6da-b9dd79337f16

📥 Commits

Reviewing files that changed from the base of the PR and between 18c63e5 and 68db5d6.

📒 Files selected for processing (2)

• all-in-one-apim/modules/integration/tests-integration/tests-backend/src/test/java/org/wso2/am/integration/tests/mcp/MCPAssociatedAPIResourceUpdateTestCase.java
• all-in-one-apim/modules/integration/tests-integration/tests-backend/src/test/resources/testng.xml