Move application management integration test cases for corresponding test classes#27403
Move application management integration test cases for corresponding test classes#27403sadilchamishka wants to merge 1 commit intoarchive_IS-7.3from
Conversation
|
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)
WalkthroughTestNG configuration reorganization: two test cases ( Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~2 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ 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 |
|
PR builder started |
|
There was a problem hiding this comment.
Pull request overview
This PR reorganizes TestNG suite composition for application management-related integration tests, relocating specific test classes to more appropriate <test> blocks in the main integration TestNG suite.
Changes:
- Moved
ApplicationTemplateMgtTestCaseinto theis-tests-default-configurationblock. - Moved
ApplicationPatchTestinto the main server application management REST API test block. - Removed both classes from
is-tests-saas-app-creation.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| <test name="is-tests-saas-app-creation" preserve-order="true" parallel="false" group-by-instances="true"> | ||
| <classes> | ||
| <class name="org.wso2.identity.integration.test.application.mgt.SaasAppCreationInitializerTestCase"/> | ||
| <class name="org.wso2.identity.integration.test.application.mgt.ApplicationTemplateMgtTestCase"/> | ||
| <class name="org.wso2.identity.integration.test.rest.api.server.application.management.v1.ApplicationPatchTest"/> | ||
| </classes> |
There was a problem hiding this comment.
is-tests-saas-app-creation now contains only SaasAppCreationInitializerTestCase. Since that class enables a config, restarts the server, and restores the config in @AfterTest, this block becomes an expensive no-op (extra restart + potential flakiness) with no tests benefiting from the enabled setting. Consider removing this <test> block entirely, or keep at least one test class in this block that actually requires enable_app_creation=true.
| <class name="org.wso2.identity.integration.test.scim.SCIMServiceProviderUserTestCase"/> | ||
| <class name="org.wso2.identity.integration.test.scim.SCIMServiceProviderGroupTestCase"/> | ||
| <class name="org.wso2.identity.integration.test.application.mgt.ImportExportServiceProviderTest"/> | ||
| <class name="org.wso2.identity.integration.test.application.mgt.ApplicationTemplateMgtTestCase"/> |
There was a problem hiding this comment.
The PR description is currently the placeholder $subject, which doesn’t explain the motivation/impact of moving these test classes between TestNG blocks (e.g., ordering/config assumptions, CI runtime impact). Please update the PR description to briefly state why the move is needed and what suite(s) these tests should belong to.
|
PR builder completed |



$subject
Summary by CodeRabbit