-
Notifications
You must be signed in to change notification settings - Fork 1.6k
Chore(UI): Fix AUT flaky tests in main #25459
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/ExploreQuickFilters.spec.ts
Show resolved
Hide resolved
openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/ImpactAnalysis.spec.ts
Show resolved
Hide resolved
🔍 CI failure analysis for fe64724: CI run had same 3 ColumnBulkOperations hard failures (lines 936, 1075, 1134) persisting across multiple commits + 8 flaky tests; 550 of 561 tests passed in 52.3 minIssueThe Root CauseCRITICAL: Same 3 tests continue to fail consistently across multiple commits: All 3 hard failures are in
These failures have persisted through:
This proves these are NOT flaky tests but consistent, deterministic failures requiring code fixes. From previous analysis:
Flaky Tests (8 total, all passed on retry):
DetailsKey Observations:
Pattern: The recent commits focused on configuration changes but did not address the actual test code issues in ColumnBulkOperations.spec.ts. Code Review 👍 Approved with suggestions 9 resolved / 10 findingsSolid test refactoring PR improving reliability with API-based setup and better synchronization. The previous finding about potential undefined displayName remains partially unresolved - while lodash's get() is now used for safer access, no fallback value is provided. 💡 Edge Case: verifyPipelineDataInDrawer may fail if displayName undefined📄 openmetadata-ui/src/main/resources/ui/playwright/utils/lineage.ts:434-443 In const fromNodeDisplayName = get(fromNode.entity, 'displayName');
const toNodeDisplayName = get(toNode.entity, 'displayName');
...
await expect(
page.locator('.overview-section').getByTestId('Source-value')
).toHaveText(fromNodeDisplayName); // Will fail if undefinedConsider providing a default value: const fromNodeDisplayName = get(fromNode.entity, 'displayName', '');
const toNodeDisplayName = get(toNode.entity, 'displayName', '');Or using optional chaining with nullish coalescing like other parts of the PR do ( ✅ 9 resolved✅ Bug: Missing await on visitEntityPage call
✅ Bug: Removed sqlQuery validation from custom property test
✅ Bug: Missing await on async visitEntityPage call
✅ Bug: Filtering without assertion in verifyPipelineDataInDrawer
✅ Edge Case: Potential undefined access on displayName in drawer verification
...and 4 more resolved from earlier reviews Tip Comment OptionsAuto-apply is off → Gitar will not commit updates to this branch. Comment with these commands to change:
Was this helpful? React with 👍 / 👎 | Gitar |
|



This pull request refactors and improves Playwright E2E tests for the OpenMetadata UI, focusing on code maintainability, test reliability, and clarity. The main changes include replacing UI-based entity setup with direct API patching, removing unnecessary cleanup steps, improving test synchronization with API responses, and refactoring repeated string constructions for clarity and maintainability.
Test setup and teardown improvements:
ExploreQuickFilters.spec.ts, entity setup inbeforeAllnow uses direct API patching for tags, tier, and domain assignments, replacing UI interactions for faster and more reliable test initialization. Unnecessary cleanup steps inafterAllare removed.ImpactAnalysis.spec.ts, redundantafterAllcleanup code is removed since entity lifecycle is now managed more efficiently.Test reliability and synchronization:
ImpactAnalysis.spec.tsto ensure assertions only occur after data is loaded, reducing flakiness. [1] [2] [3]Code clarity and maintainability:
Lineage.spec.tsby introducing variables and using them in edge ID generation, reducing repetition and improving readability. [1] [2] [3] [4] [5] [6]Minor test logic fixes:
replaceAllfor consistency inExploreQuickFilters.spec.ts.