fallback for datacontract status check

[shrabantipaul-collate]

Describe your changes:

DataQuality fallback for quality validation timeout

The test suite executes and has results, but the contract's latestResult is sometimes not propagated back in time, causing waitForDataContractExecution to time out at 10 minutes.

Added waitForContractExecutionWithFallback in playwright/utils/dataContracts.ts: if the contract poll times out, it falls back to the DataQuality → Bundle Suites page, finds the Data Contract - test suite, clicks into it, and verifies the test case statuses directly from the API response — asserting a terminal status (Success/Failed/Aborted) to confirm execution completed.
Fixes

I worked on ... because ...

Type of change:

• Bug fix
• Improvement
• New feature
• Breaking change (fix or feature that would cause existing functionality to not work as expected)
• Documentation

Checklist:

• I have read the CONTRIBUTING document.
• My PR title is Fixes <issue-number>: <short explanation>
• I have commented on my code, particularly in hard-to-understand areas.
• For JSON Schema changes: I updated the migration scripts or explained why it is not needed.

---

Summary by Gitar

• Test Refactoring:
  • Updated DataContracts.spec.ts and DataContractsSemanticRules.spec.ts to use user.getUserDisplayName() instead of direct property access for improved maintainability.
• Test Infrastructure:
  • Increased test.setTimeout in DataContracts.spec.ts to 900_000ms (15 minutes) to accommodate slower data contract execution cycles.

_{This will update automatically on new commits.}

[github-actions]

Jest test Coverage

UI tests summary

Lines	Statements	Branches	Functions
	62.06% (61924/99777)	42.15% (33109/78550)	45.23% (9802/21667)

[github-actions]

🟡 Playwright Results — all passed (14 flaky)

✅ 3959 passed · ❌ 0 failed · 🟡 14 flaky · ⏭️ 86 skipped

Shard	Passed	Failed	Flaky	Skipped
🟡 Shard 1	298	0	1	4
🟡 Shard 2	753	0	6	8
🟡 Shard 3	731	0	1	7
🟡 Shard 4	757	0	2	18
🟡 Shard 5	686	0	1	41
🟡 Shard 6	734	0	3	8

🟡 14 flaky test(s) (passed on retry)

• Pages/UserCreationWithPersona.spec.ts › Create user with persona and verify on profile (shard 1, 1 retry)
• Features/ActivityAPI.spec.ts › Activity event is created when description is updated (shard 2, 1 retry)
• Features/ActivityAPI.spec.ts › Activity event shows the actor who made the change (shard 2, 1 retry)
• Features/ColumnBulkOperations.spec.ts › should discard changes when closing drawer without saving (shard 2, 1 retry)
• Features/DomainFilterQueryFilter.spec.ts › Domain filter should use exact match and prefix with dot to prevent false positives (shard 2, 1 retry)
• Features/IncidentManager.spec.ts › Verify filters in Incident Manager's page (shard 2, 1 retry)
• Features/LandingPageWidgets/DomainWidgetFilter.spec.ts › Domains widget should show only selected domain when domain filter is active (shard 2, 1 retry)
• Features/RTL.spec.ts › Verify Following widget functionality (shard 3, 1 retry)
• Pages/DataContracts.spec.ts › Create Data Contract and validate for File (shard 4, 1 retry)
• Pages/DataContracts.spec.ts › Create Data Contract and validate for Spreadsheet (shard 4, 1 retry)
• Pages/ExplorePageRightPanel.spec.ts › Should verify deleted glossary term not visible in selection for table (shard 5, 1 retry)
• Pages/Lineage/LineageFilters.spec.ts › Verify lineage schema filter selection (shard 6, 1 retry)
• Pages/Lineage/LineageRightPanel.spec.ts › Verify custom properties tab IS visible for supported type: searchIndex (shard 6, 1 retry)
• Pages/UserDetails.spec.ts › Create team with domain and verify visibility of inherited domain in user profile after team removal (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

[gitar-bot]

Code Review ⚠️ Changes requested 1 resolved / 3 findings

Adds a datacontract status check fallback while resolving a duplicate type annotation. The implementation currently ignores non-timeout errors in the catch block and incorrectly retains a 'Running' status when test cases lack results.

⚠️ Edge Case: Catch-all in fallback swallows non-timeout errors silently

📄 openmetadata-ui/src/main/resources/ui/playwright/utils/dataContracts.ts:148-155

The catch block in waitForContractExecutionWithFallback catches all errors from waitForDataContractExecution, not just timeouts. That function can also throw when maxConsecutiveErrors is exceeded (indicating persistent API failures). By catching everything and falling back to the DQ page, a real infrastructure or API issue could be masked, making the test pass when it shouldn't.

Consider narrowing the catch to timeout-related errors only, or at minimum logging the caught error so flaky-test investigations have context.

Suggested fix

} catch (error) {
  // Only fall back for timeout-related errors; re-throw unexpected failures.
  if (
    !(error instanceof Error) ||
    !error.message.includes('Timeout')
  ) {
    throw error;
  }
  // The test suite has results but the contract's latestResult was not updated in time.

💡 Edge Case: suiteStatus stays 'Running' if all test cases lack results

📄 openmetadata-ui/src/main/resources/ui/playwright/utils/dataContracts.ts:187-199

If testCases is non-empty but none have a testCaseResult with a terminal status, suiteStatus remains 'Running', which doesn't match the terminalStatusPattern regex — causing the assertion to fail. This is probably the desired behavior (execution not complete), but it's worth confirming this edge case won't produce a confusing error message. A more descriptive assertion message would help debugging.

Suggested fix

expect(suiteStatus).toEqual(
  expect.stringMatching(terminalStatusPattern)
);
// Consider adding a message:
// expect(suiteStatus, `Expected terminal status but got '${suiteStatus}' for contract '${contractName}'`).toMatch(terminalStatusPattern);

✅ 1 resolved

✅ Quality: Duplicated type annotation for test case status check

📄 openmetadata-ui/src/main/resources/ui/playwright/utils/dataContracts.ts:174-185
The inline type { testCaseResult?: { testCaseStatus?: string } } is repeated three times (lines 175, 179, 183). Extract it into a local type alias to reduce duplication and improve readability.

🤖 Prompt for agents

Code Review: Adds a datacontract status check fallback while resolving a duplicate type annotation. The implementation currently ignores non-timeout errors in the catch block and incorrectly retains a 'Running' status when test cases lack results.

1. ⚠️ Edge Case: Catch-all in fallback swallows non-timeout errors silently
   Files: openmetadata-ui/src/main/resources/ui/playwright/utils/dataContracts.ts:148-155

   The `catch` block in `waitForContractExecutionWithFallback` catches all errors from `waitForDataContractExecution`, not just timeouts. That function can also throw when `maxConsecutiveErrors` is exceeded (indicating persistent API failures). By catching everything and falling back to the DQ page, a real infrastructure or API issue could be masked, making the test pass when it shouldn't.
   
   Consider narrowing the catch to timeout-related errors only, or at minimum logging the caught error so flaky-test investigations have context.

   Suggested fix:
   } catch (error) {
     // Only fall back for timeout-related errors; re-throw unexpected failures.
     if (
       !(error instanceof Error) ||
       !error.message.includes('Timeout')
     ) {
       throw error;
     }
     // The test suite has results but the contract's latestResult was not updated in time.

2. 💡 Edge Case: suiteStatus stays 'Running' if all test cases lack results
   Files: openmetadata-ui/src/main/resources/ui/playwright/utils/dataContracts.ts:187-199

   If `testCases` is non-empty but none have a `testCaseResult` with a terminal status, `suiteStatus` remains `'Running'`, which doesn't match the `terminalStatusPattern` regex — causing the assertion to fail. This is probably the desired behavior (execution not complete), but it's worth confirming this edge case won't produce a confusing error message. A more descriptive assertion message would help debugging.

   Suggested fix:
   expect(suiteStatus).toEqual(
     expect.stringMatching(terminalStatusPattern)
   );
   // Consider adding a message:
   // expect(suiteStatus, `Expected terminal status but got '${suiteStatus}' for contract '${contractName}'`).toMatch(terminalStatusPattern);

Options

Display: compact → Showing less information.

Comment with these commands to change:

Compact

gitar display:verbose

_{Was this helpful? React with 👍 / 👎 | Gitar}

[sonarqubecloud]

Quality Gate passed for 'open-metadata-ui'

Issues
1063 New issues
0 Accepted issues

Measures
17 Security Hotspots
0.0% Coverage on New Code
3.9% Duplication on New Code

See analysis details on SonarQube Cloud