Fixes #21203: Attach TestCases to TestSuites upon creation cleanly via Resource layer#27398
Conversation
…ing PATCH regressions (open-metadata#21203)
|
Hi there 👋 Thanks for your contribution! The OpenMetadata team will review the PR shortly! Once it has been labeled as Let us know if you need any help! |
|
Hi there 👋 Thanks for your contribution! The OpenMetadata team will review the PR shortly! Once it has been labeled as Let us know if you need any help! |
There was a problem hiding this comment.
Pull request overview
This PR extends the TestCase creation API to optionally attach newly created/updated TestCases to one or more logical (non-basic) TestSuites, adding schema/UI support plus server-side validation and integration tests.
Changes:
- Add
testSuitestoCreateTestCaserequest schema and generated UI API types. - Implement server-side batch resolution/authorization for requested logical TestSuites and attach created TestCases to them on create/createMany/createOrUpdate.
- Add integration tests covering logical suite attachment, rejection of basic suites, non-existent suites, and PATCH regression safety.
Reviewed changes
Copilot reviewed 3 out of 4 changed files in this pull request and generated 7 comments.
| File | Description |
|---|---|
| openmetadata-ui/src/main/resources/ui/src/generated/api/tests/createTestCase.ts | Adds testSuites to the generated TS CreateTestCase type. |
| openmetadata-spec/src/main/resources/json/schema/api/tests/createTestCase.json | Adds testSuites array field to the CreateTestCase API schema. |
| openmetadata-service/src/main/java/org/openmetadata/service/resources/dqtests/TestCaseResource.java | Resolves/validates logical suites and attaches TestCases during POST/PUT/bulk creation flows. |
| openmetadata-integration-tests/src/test/java/org/openmetadata/it/tests/TestCaseResourceIT.java | Adds IT coverage for logical suite attachment and PATCH non-regression. |
|
Hi there 👋 Thanks for your contribution! The OpenMetadata team will review the PR shortly! Once it has been labeled as Let us know if you need any help! |
|
Hi there 👋 Thanks for your contribution! The OpenMetadata team will review the PR shortly! Once it has been labeled as Let us know if you need any help! |
![gitar-bot[bot]](https://avatars.githubusercontent.com/in/827041?s=60&v=4){kind=small}
|
Hi there 👋 Thanks for your contribution! The OpenMetadata team will review the PR shortly! Once it has been labeled as Let us know if you need any help! |
|
Hi there 👋 Thanks for your contribution! The OpenMetadata team will review the PR shortly! Once it has been labeled as Let us know if you need any help! |
Code Review ✅ Approved 5 resolved / 5 findingsRefactors TestSuite attachment logic to prevent duplicate inserts and partial states during creation. All identified validation and state consistency issues have been addressed. ✅ 5 resolved✅ Bug: createOrUpdate attaches suites unconditionally, causing duplicate insert failures
✅ Performance: createMany calls addTestCasesToLogicalTestSuite per test case per suite
✅ Bug: Partial state if suite attachment fails after entity creation
✅ Edge Case: PUT updates reject invalid testSuites even though they're ignored
✅ Quality: Schema says "best-effort" but code does fail-fast validation
OptionsDisplay: compact → Showing less information. Comment with these commands to change:
Was this helpful? React with 👍 / 👎 | Gitar |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 4 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (1)
openmetadata-service/src/main/java/org/openmetadata/service/resources/dqtests/TestCaseResource.java:905
createOrUpdatealways callsresolveAndValidateLogicalSuites(create.getTestSuites(), ...)before knowing whether the request will create vs update. If a client includestestSuitesduring an update (status OK), the request can now fail due to suite validation/authorization even though the field is effectively ignored for updates (you only attach when status is CREATED). Consider skipping validation unless the entity will be created (e.g., pre-check existence viarepository.getByNameOrNull(...)whentestSuitesis non-empty, or move validation inside the CREATED branch and accept the tradeoff).
EntityLink entityLink = EntityLink.parse(create.getEntityLink());
OperationContext tableOpContext =
new OperationContext(Entity.TABLE, MetadataOperation.EDIT_TESTS);
ResourceContextInterface tableResourceContext =
TestCaseResourceContext.builder().entityLink(entityLink).build();
|
Hi there 👋 Thanks for your contribution! The OpenMetadata team will review the PR shortly! Once it has been labeled as Let us know if you need any help! |
|
Hi @TeddyCr @mohityadav766 All automated review feedback (Gitar & Copilot) regarding batch optimizations, validation safety, and edge-cases has been fully addressed and pushed. Could you please add the |
PubChimps
added
the
safe to test
|
The Java checkstyle failed. Please run You can install the pre-commit hooks with |
|
Jest test CoverageUI tests summary
|
|
Hi @PubChimps👋 I saw the issue #21203 was closed with a note that the team is working on My PR #27398 implements the same feature via the Resource layer with batch Could you let me know:
In the meantime, I'll continue resolving any failing CI checks on my end. Thank you for all the work you're doing on this! 🙏 |
|
|
🟡 Playwright Results — all passed (26 flaky)✅ 3639 passed · ❌ 0 failed · 🟡 26 flaky · ⏭️ 84 skipped
🟡 26 flaky test(s) (passed on retry)
How to debug locally# Download playwright-test-results-<shard> artifact and unzip
npx playwright show-trace path/to/trace.zip # view trace |
Description:
Fixes #21203
I worked on fully enabling
CreateTestCaseentities to be attached to Logical TestSuites during thePOST/PUTcreation workflows.Why this approach is safe and highly optimized:
Unlike previous attempts which introduced massive
N+1queries and caused 34+ failures due toPATCHregressions, this implementation is heavily optimized. I moved the validation logic directly into the REST Resource boundary (TestCaseResource.java).create(),createMany(), andcreateOrUpdate(). It totally ignoresPATCH, cleanly side-stepping the "basic test suite" failure loops.Entity.getEntityByNames()to resolve alltestSuitesin a single query upfront, ensuring high performance even increateMany()bursts.Proof of Verification:
Type of change:
Checklist:
Fixes #21203: Attach TestCases to TestSuites upon creation cleanly via Resource layertestSuiteswas only added to theCreateTestCaserequest schema to generate relationship links, not to the coreTestCasedatabase storage schema itself).