Skip to content

feat(dq): add testSuiteNames to CreateTestCaseRequest #26973

Closed
SaaiAravindhRaja wants to merge 128 commits intoopen-metadata:mainfrom
SaaiAravindhRaja:feat/issue-21203-test-case-test-suites-api
Closed

feat(dq): add testSuiteNames to CreateTestCaseRequest #26973
SaaiAravindhRaja wants to merge 128 commits intoopen-metadata:mainfrom
SaaiAravindhRaja:feat/issue-21203-test-case-test-suites-api

Conversation

@SaaiAravindhRaja
Copy link
Copy Markdown
Contributor

Fixes #21203

Adds an optional testSuiteNames field (array of logical test suite FQNs) to CreateTestCaseRequest. This lets callers attach a new test case to one or more logical test suites in a single API call — no separate PUT /logicalTestCases needed.

Changes

  • createTestCase.json: add optional testSuiteNames string array
  • TestCaseResource: call addTestCaseToNamedTestSuites after create and createOrUpdate; throws if a basic test suite is passed

…21203)

Adds an optional testSuiteNames field (array of FQNs) to CreateTestCaseRequest
so users can attach a new test case to logical test suites in a single API call,
without needing a separate PUT /logicalTestCases request.
Copilot AI review requested due to automatic review settings April 2, 2026 11:10
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Apr 2, 2026

Hi there 👋 Thanks for your contribution!

The OpenMetadata team will review the PR shortly! Once it has been labeled as safe to test, the CI workflows
will start executing and we'll be able to make sure everything is working as expected.

Let us know if you need any help!

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Apr 2, 2026

Hi there 👋 Thanks for your contribution!

The OpenMetadata team will review the PR shortly! Once it has been labeled as safe to test, the CI workflows
will start executing and we'll be able to make sure everything is working as expected.

Let us know if you need any help!

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds the ability for API callers to attach a newly created/updated TestCase to one or more logical TestSuites in the same request, by introducing a new optional request field and wiring it into the TestCase create/update flows.

Changes:

  • Extend CreateTestCaseRequest schema with optional testSuiteNames: string[] (logical TestSuite FQNs).
  • Update TestCaseResource create and createOrUpdate endpoints to attach the resulting TestCase to the specified logical suites.
  • Add helper logic in TestCaseResource to resolve suite FQNs and perform the attachment via TestCaseRepository.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File Description
openmetadata-spec/src/main/resources/json/schema/api/tests/createTestCase.json Adds testSuiteNames optional array to the CreateTestCase request schema.
openmetadata-service/src/main/java/org/openmetadata/service/resources/dqtests/TestCaseResource.java Hooks new request field into create/createOrUpdate and adds helper to attach the test case to named logical suites.

Comment on lines +1602 to +1606
for (String testSuiteName : testSuiteNames) {
TestSuite testSuite =
Entity.getEntityByName(Entity.TEST_SUITE, testSuiteName, "domains,owners", ALL);
if (Boolean.TRUE.equals(testSuite.getBasic())) {
throw new IllegalArgumentException(
Copy link

Copilot AI Apr 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

addTestCaseToNamedTestSuites fetches the TestSuite and mutates it via repository.addTestCasesToLogicalTestSuite(...), but it does not perform any authorization checks (unlike validateTestSuiteOps, which enforces EDIT_TESTS/EDIT_ALL). This allows callers who can create/update a test case to attach it to logical suites they may not have permission to edit. Consider passing SecurityContext into this helper and calling validateTestSuiteOps(testSuite, securityContext) (or equivalent) before the mutation. Also, using Include.ALL here can resolve soft-deleted suites; Include.NON_DELETED (or null) would be safer/consistent with other add-to-suite flows in this resource.

Copilot uses AI. Check for mistakes.
new AuthRequest(testCaseOpContext, testCaseResourceContext));
authorizer.authorizeRequests(securityContext, requests, AuthorizationLogic.ANY);
test = addHref(uriInfo, repository.create(uriInfo, test));
addTestCaseToNamedTestSuites(test, create.getTestSuiteNames());
Copy link

Copilot AI Apr 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This call runs after repository.create(...). If addTestCaseToNamedTestSuites throws (e.g., basic suite passed, suite not found, or future auth checks), the API will return an error even though the test case has already been created, leaving the system in a partial state. Consider validating all testSuiteNames (existence, non-basic, permissions) before creating the test case, and/or making the create+attach operation atomic (single transaction or compensating delete on failure).

Suggested change
addTestCaseToNamedTestSuites(test, create.getTestSuiteNames());
try {
addTestCaseToNamedTestSuites(test, create.getTestSuiteNames());
} catch (Exception ex) {
LOG.error(
"Failed to add test case {} to named test suites {} after creation",
test.getFullyQualifiedName(),
create.getTestSuiteNames(),
ex);
}

Copilot uses AI. Check for mistakes.
Comment on lines 868 to 869
addTestCaseToNamedTestSuites(response.getEntity(), create.getTestSuiteNames());
addHref(uriInfo, response.getEntity());
Copy link

Copilot AI Apr 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As with create(...), this call runs after repository.createOrUpdate(...). If addTestCaseToNamedTestSuites fails, the request can return an error despite the test case having been created/updated, which is a surprising partial-success outcome for PUT. Consider pre-validating testSuiteNames (and authorizations) before the write, and/or ensuring the update + attachments are applied atomically or with a clear, consistent error strategy.

Suggested change
addTestCaseToNamedTestSuites(response.getEntity(), create.getTestSuiteNames());
addHref(uriInfo, response.getEntity());
TestCase persistedTestCase = response.getEntity();
try {
addTestCaseToNamedTestSuites(persistedTestCase, create.getTestSuiteNames());
} catch (RuntimeException ex) {
log.error(
"Failed to add test case {} to named test suites {} after createOrUpdate completed",
persistedTestCase.getId(),
create.getTestSuiteNames(),
ex);
}
addHref(uriInfo, persistedTestCase);

Copilot uses AI. Check for mistakes.
@harshach harshach added the safe to test Add this label to run secure Github workflows on PRs label Apr 2, 2026
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Apr 2, 2026

The Java checkstyle failed.

Please run mvn spotless:apply in the root of your repository and commit the changes to this PR.
You can also use pre-commit to automate the Java code formatting.

You can install the pre-commit hooks with make install_test precommit_install.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Apr 2, 2026

⚠️ TypeScript Types Need Update

The generated TypeScript types are out of sync with the JSON schema changes.

Since this is a pull request from a forked repository, the types cannot be automatically committed.
Please generate and commit the types manually:

cd openmetadata-ui/src/main/resources/ui
./json2ts-generate-all.sh -l true
git add src/generated/
git commit -m "Update generated TypeScript types"
git push

After pushing the changes, this check will pass automatically.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Apr 2, 2026

🟡 Playwright Results — all passed (16 flaky)

✅ 3455 passed · ❌ 0 failed · 🟡 16 flaky · ⏭️ 223 skipped

Shard Passed Failed Flaky Skipped
🟡 Shard 1 454 0 1 2
🟡 Shard 2 617 0 2 32
🟡 Shard 3 618 0 4 27
🟡 Shard 4 620 0 5 47
✅ Shard 5 587 0 0 67
🟡 Shard 6 559 0 4 48
🟡 16 flaky test(s) (passed on retry)
  • Pages/UserCreationWithPersona.spec.ts › Create user with persona and verify on profile (shard 1, 1 retry)
  • Features/BulkEditEntity.spec.ts › Glossary (shard 2, 1 retry)
  • Features/DataQuality/BundleSuiteBulkOperations.spec.ts › Bulk selection operations (shard 2, 1 retry)
  • Features/Permissions/GlossaryPermissions.spec.ts › Team-based permissions work correctly (shard 3, 1 retry)
  • Features/Table.spec.ts › Table page should show schema tab with count (shard 3, 1 retry)
  • Features/UserProfileOnlineStatus.spec.ts › Should show online status badge on user profile for active users (shard 3, 1 retry)
  • Flow/ExploreDiscovery.spec.ts › Should display deleted assets when showDeleted is checked and deleted is not present in queryFilter (shard 3, 1 retry)
  • Pages/Customproperties-part2.spec.ts › entityReferenceList shows item count, scrollable list, no expand toggle (shard 4, 1 retry)
  • Pages/DataContracts.spec.ts › Create Data Contract and validate for Table (shard 4, 1 retry)
  • Pages/DataContracts.spec.ts › Create Data Contract and validate for SearchIndex (shard 4, 1 retry)
  • Pages/DataContracts.spec.ts › Create Data Contract and validate for Database (shard 4, 1 retry)
  • Pages/Domains.spec.ts › Verify Domain entity API calls do not include invalid domains field in glossary term assets (shard 4, 1 retry)
  • Pages/Glossary.spec.ts › Add and Remove Assets (shard 6, 1 retry)
  • Pages/InputOutputPorts.spec.ts › Lineage section collapse/expand (shard 6, 1 retry)
  • Pages/ProfilerConfigurationPage.spec.ts › Non admin user (shard 6, 1 retry)
  • Pages/Users.spec.ts › Permissions for table details page for Data Consumer (shard 6, 1 retry)

📦 Download artifacts

How to debug locally
# Download playwright-test-results-<shard> artifact and unzip
npx playwright show-trace path/to/trace.zip    # view trace

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Apr 3, 2026

The Java checkstyle failed.

Please run mvn spotless:apply in the root of your repository and commit the changes to this PR.
You can also use pre-commit to automate the Java code formatting.

You can install the pre-commit hooks with make install_test precommit_install.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Apr 3, 2026

⚠️ TypeScript Types Need Update

The generated TypeScript types are out of sync with the JSON schema changes.

Since this is a pull request from a forked repository, the types cannot be automatically committed.
Please generate and commit the types manually:

cd openmetadata-ui/src/main/resources/ui
./json2ts-generate-all.sh -l true
git add src/generated/
git commit -m "Update generated TypeScript types"
git push

After pushing the changes, this check will pass automatically.

@sonarqubecloud
Copy link
Copy Markdown

sonarqubecloud Bot commented Apr 3, 2026

@SaaiAravindhRaja SaaiAravindhRaja marked this pull request as draft April 4, 2026 05:35
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Apr 4, 2026

The Java checkstyle failed.

Please run mvn spotless:apply in the root of your repository and commit the changes to this PR.
You can also use pre-commit to automate the Java code formatting.

You can install the pre-commit hooks with make install_test precommit_install.

@SaaiAravindhRaja SaaiAravindhRaja changed the title feat(dq): add testSuiteNames to CreateTestCaseRequest feat(dq): add testSuiteNames to CreateTestCaseRequest [WIP] Apr 4, 2026
@SaaiAravindhRaja SaaiAravindhRaja marked this pull request as ready for review April 4, 2026 11:16
@SaaiAravindhRaja SaaiAravindhRaja requested a review from a team as a code owner April 4, 2026 11:16
Copilot AI review requested due to automatic review settings April 4, 2026 11:16
Rohit0301 and others added 9 commits April 12, 2026 20:40
…ed tests (open-metadata#26561)

* feat: migrate DeleteModal to new component structure and update related tests

* fixed typography changes

* fixed unit test

* fixed lint issue
…en-metadata#27219)

Bumps [axios](https://github.com/axios/axios) from 1.13.5 to 1.15.0.
- [Release notes](https://github.com/axios/axios/releases)
- [Changelog](https://github.com/axios/axios/blob/v1.x/CHANGELOG.md)
- [Commits](axios/axios@v1.13.5...v1.15.0)

---
updated-dependencies:
- dependency-name: axios
  dependency-version: 1.15.0
  dependency-type: direct:production
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
…PI (open-metadata#27139)

* fix: action buttons visible immediately despite slow pipelineStatus API

* fixed the recent run overlapping issue

* addressed gitar comment

* addressed PR comment

* addressed gitar comment

* fixed lint checks
* Fix loading effect for ontology scroll

* nit

* nit

* fix translated key
…tion spec (open-metadata#27262)

* fix(playwright): Fix flaky ActivityFeed mention notification and reaction tests

* remove afterAll
…t timeouts in E2E specs (open-metadata#27291)

* fix: refactor user initialization and applyFixture usage in ProfilerConfigurationPage tests

* fix: update ESLint rules and refactor test page fixture usage in ProfilerConfigurationPage tests

* fix: update test case incident page navigation to ensure API response is validated

* fix: refactor user and table initialization in IncidentManager tests and add notification response validation

* fix: add slow test condition for AUT Global test case list page in Data Quality redirection test

* addressing comments
Copilot AI review requested due to automatic review settings April 12, 2026 12:40
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 205 out of 810 changed files in this pull request and generated 2 comments.

Comments suppressed due to low confidence (2)

openmetadata-service/src/main/java/org/openmetadata/service/resources/dqtests/TestCaseResource.java:1

  • If testSuiteNames contains an invalid/basic/non-permitted suite, addTestCaseToNamedTestSuites(...) throws after repository.create(...) already persists the TestCase. That leaves the system in a partial state (client sees a 400, but the TestCase exists). To keep the endpoint behavior consistent, validate/resolve and authorize the testSuiteNames before creating the TestCase (or execute create + attaches in a single transaction / roll back on failure).
package org.openmetadata.service.resources.dqtests;

openmetadata-service/src/main/java/org/openmetadata/service/resources/dqtests/TestCaseResource.java:1

  • Similar to create, this can partially apply changes: createOrUpdate(...) may commit the update, then addTestCaseToNamedTestSuites(...) can throw and return an error response. Consider resolving/authorizing suites before calling repository.createOrUpdate(...), or ensure the entire operation is transactional so the response accurately reflects the persisted state.
package org.openmetadata.service.resources.dqtests;

Comment on lines 1605 to 1606
limit,
offset,
Copy link

Copilot AI Apr 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If testSuiteNames includes duplicates, this will validate and attach the same suite multiple times, causing redundant lookups and potentially duplicate relationship writes (depending on repository behavior). Consider de-duplicating input first (while preserving order if needed) and/or de-duplicating validatedSuites before the attach loop.

Copilot uses AI. Check for mistakes.
Comment on lines +76 to 82
"testSuiteNames": {
"description": "Fully qualified names of the logical test suites to add this test case to upon creation.",
"type": "array",
"items": {
"$ref": "../../type/basic.json#/definitions/fullyQualifiedEntityName"
}
}
Copy link

Copilot AI Apr 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider adding uniqueItems: true to testSuiteNames to make the API contract explicit and prevent accidental duplicate suite names. This also aligns with the server-side need to avoid duplicate attachments/work.

Copilot uses AI. Check for mistakes.
@github-actions
Copy link
Copy Markdown
Contributor

The Java checkstyle failed.

Please run mvn spotless:apply in the root of your repository and commit the changes to this PR.
You can also use pre-commit to automate the Java code formatting.

You can install the pre-commit hooks with make install_test precommit_install.

@gitar-bot
Copy link
Copy Markdown

gitar-bot Bot commented Apr 12, 2026

Code Review ⚠️ Changes requested 2 resolved / 3 findings

Adds testSuiteNames to CreateTestCaseRequest with proper validation fixes (404→400 for missing suites, partial attachment rollback), but profiler settings fetch failures abort table sampling without graceful degradation.

⚠️ Edge Case: Profiler settings fetch failure aborts entire table sampling

In processor.py, the call to self.metadata.get_profiler_config_settings() at line 142 is not wrapped in its own try/except. If this API call fails (e.g., transient network error, permission issue), the exception propagates to the outer catch block and the entire sampling record is reported as a StackTraceError — meaning no sample data is collected for that table.

By contrast, messaging_service.py (lines 167–183) correctly wraps the same call in a dedicated try/except and falls back to False (default behavior) on failure. The processor should follow the same defensive pattern so that a settings-fetch failure doesn't prevent sampling from running with defaults.

Suggested fix
try:
    settings = self.metadata.get_profiler_config_settings()
except Exception as exc:
    logger.debug(f"Could not fetch global profiler config: {exc}")
    settings = None

profiler_global_config = (
    cast(ProfilerConfiguration, settings.config_value)
    if settings and settings.config_value
    else None
)
✅ 2 resolved
Edge Case: Non-existent testSuiteNames yields 404 instead of 400

📄 openmetadata-service/src/main/java/org/openmetadata/service/resources/dqtests/TestCaseResource.java:1603-1604
When a caller passes a suite name that doesn't exist, Entity.getEntityByName will throw an EntityNotFoundException (mapped to 404). From the caller's perspective, a 400 Bad Request with a clear message ("test suite 'X' not found") would be more appropriate since the error is in the request payload, not in the URL path. This is a minor UX issue but could confuse API consumers.

Edge Case: Partial attachment not rolled back in createOrUpdate UPDATE path

📄 openmetadata-service/src/main/java/org/openmetadata/service/resources/dqtests/TestCaseResource.java:894-908
In createOrUpdate (lines 894-913), if attachTestCaseToSuites succeeds for the first suite but fails on a subsequent one, the rollback guard only triggers when response.getStatus() == CREATED. For the UPDATE path, the already-persisted update and the first suite's attachment are left in place while the caller receives an error. This creates a partial-write scenario where some suites are attached and others are not, with no indication to the caller of which succeeded.

This is a minor concern because: (1) validation already passed so runtime failures here would be rare infrastructure issues, and (2) the previous code had the same gap. A full fix would require wrapping everything in a DB transaction.

🤖 Prompt for agents
Code Review: Adds testSuiteNames to CreateTestCaseRequest with proper validation fixes (404→400 for missing suites, partial attachment rollback), but profiler settings fetch failures abort table sampling without graceful degradation.

1. ⚠️ Edge Case: Profiler settings fetch failure aborts entire table sampling

   In `processor.py`, the call to `self.metadata.get_profiler_config_settings()` at line 142 is not wrapped in its own try/except. If this API call fails (e.g., transient network error, permission issue), the exception propagates to the outer catch block and the entire sampling record is reported as a `StackTraceError` — meaning no sample data is collected for that table.
   
   By contrast, `messaging_service.py` (lines 167–183) correctly wraps the same call in a dedicated try/except and falls back to `False` (default behavior) on failure. The processor should follow the same defensive pattern so that a settings-fetch failure doesn't prevent sampling from running with defaults.

   Suggested fix:
   try:
       settings = self.metadata.get_profiler_config_settings()
   except Exception as exc:
       logger.debug(f"Could not fetch global profiler config: {exc}")
       settings = None
   
   profiler_global_config = (
       cast(ProfilerConfiguration, settings.config_value)
       if settings and settings.config_value
       else None
   )

Options

Display: compact → Showing less information.

Comment with these commands to change:

Compact
gitar display:verbose         

Was this helpful? React with 👍 / 👎 | Gitar

@github-actions
Copy link
Copy Markdown
Contributor

The Java checkstyle failed.

Please run mvn spotless:apply in the root of your repository and commit the changes to this PR.
You can also use pre-commit to automate the Java code formatting.

You can install the pre-commit hooks with make install_test precommit_install.

@TeddyCr
Copy link
Copy Markdown
Collaborator

TeddyCr commented Apr 14, 2026

closing this as a duplicate of #26960

@TeddyCr TeddyCr closed this Apr 14, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

safe to test Add this label to run secure Github workflows on PRs

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Adding Test Case to Bundle Test Suite w/ API