test(ui): re-enable Domains, Data Product & domain-rename E2E suites#30451
test(ui): re-enable Domains, Data Product & domain-rename E2E suites#30451siddhant1 wants to merge 5 commits into
Conversation
Re-enables E2E coverage that was disabled by the mass-skip #29909 and a few one-off skips, fixing the tests that had broken from UI migrations. Fixed: - Domains (9 suites): 47/49 already passed once run; fixed the 2 real failures, both migration selector drifts — "Add Assets" moved to the core-ui `asset-selection-modal` Dialog (old `form-heading` testid gone), and the tree toggle is now a `radio`, not a `button`. - DomainDataProductsWidgets: marked `test.describe.serial` (the suite is one sequential scenario: assign widgets -> add assets -> verify -> remove), and fixed the data-product asset-removal count-timing bug — wait for the asset card to render before counting, otherwise count() reads 0 and removes nothing. - GlossaryNavigation empty-state: assert the empty-glossary placeholder (message.no-glossary-term) instead of the "No Glossary Term found" row that only a non-matching status filter shows. Unskipped as-is (passed without changes): - Glossary "Request description task for Glossary Term" - EntityRenameConsolidation domain rename (x2) Left skipped (blocked on product/BE work, still documented): - DataMarketplace x3 (routes removed in #27377) - GlossaryStatusFilterNestedTerms x5 (flat nested status-filter results not implemented; status-only queries route through directChildrenOf) - Glossary "Term should stay approved" (BE approval-workflow reverts it) BulkImport is untouched. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…dataproducts-e2e # Conflicts: # openmetadata-ui/src/main/resources/ui/playwright/e2e/Pages/Domains.spec.ts
❌ PR checklist incompleteThis PR cannot be merged until the following are addressed on its linked issue:
The fields live on the linked issue in the Shipping project (open the issue → right sidebar → Projects). After you set them, re-run this check (or push a commit) — issue/project changes do not re-trigger it automatically. Maintainers can bypass this check by adding the |
|
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! |
siddhant1
added
safe to test
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
🔴 Playwright Results — workflow failedValidated commit ✅ 0 passed · ❌ 0 failed · 🟡 0 flaky · ⏭️ 0 skipped · 🧰 0 lifecycle flaky Pipeline and setup failures (2)
PerformanceBlocking targets: ✅ met · Optimization targets: 🟡 in progress Shard-job maxima below are not the full workflow wall time; the linked run includes build, fixture, planning, and reporting. 🕒 Full workflow signal wall (to summary) 54m 21s ⏱️ Max setup 1m 35s · max shard execution 19m 9s · max shard-job elapsed before upload 22m 51s · reporting 4s 🌐 206.71 requests/attempt · 2.60 app boots/UI scenario · 0.00% common-shard skew Optimization targets still in progress:
How to debug locally# Download playwright-test-results-<shard> artifact and unzip
npx playwright show-trace path/to/trace.zip # view trace |
| await selectActiveGlossary(page, glossary1.data.displayName); | ||
| await selectActiveGlossaryTerm(page, glossaryTerm1.data.displayName); | ||
| test('Request description task for Glossary Term', async ({ browser }) => { | ||
| const { page, afterAction, apiContext } = await performAdminLogin(browser); |
There was a problem hiding this comment.
When the re-enabled description-task test runs, performAdminLogin(browser) takes the API-only path and returns no page, so the first sidebarClick receives undefined and the test fails before exercising the description-task workflow.
Code Review ✅ ApprovedRe-enables the Domains, Data Product, and domain-rename Playwright E2E suites by fixing selector drift, test timing, and assertion issues. No issues found. OptionsDisplay: compact → Showing less information. Comment with these commands to change the behavior for this request:
Was this helpful? React with 👍 / 👎 | Gitar |
What & why
Re-enables Playwright E2E coverage that had been switched off — mostly by the bulk-skip in #29909 ("Skip failing main specs") plus a few one-off skips. When actually run against a current build, the vast majority already pass; this PR fixes the handful that had genuinely broken (all from UI-component migrations) and re-enables everything that is green.
Re-enabled + fixed
describesuites)asset-selection-modalDialog (oldform-headingtestid gone); tree toggle is now aradio, not abutton.test.describe.serial— the suite is one sequential scenario (assign widgets → add assets → verify → remove); parallel execution violated that ordering. Also fixed the data-product asset-removal count-timing bug: wait for the asset card to render before counting (count()doesn't auto-wait, so it read 0 and removed nothing).message.no-glossary-term) instead of the"No Glossary Term found"row — that row only appears when a status filter matches nothing, not for a 0-term glossary.Left skipped (blocked on product/BE work — documented in-code)
/data-marketplace/*sub-routes removed in refactor(ui): render Data Marketplace home page on the main app layout #27377.directChildrenOf(root level); flat nested results would need the frontend to route status-only queries through the flat search API (which the search path already uses).BulkImportis intentionally untouched.Testing
All re-enabled tests verified green locally against a running stack (
localhost:8585): Domains 49/49, DomainDataProductsWidgets 6/6, GlossaryNavigation empty-state, Glossary request-description-task, EntityRenameConsolidation domain-rename ×2.🤖 Generated with Claude Code
Greptile Summary
Re-enables previously skipped Playwright coverage and updates selectors, empty-state assertions, suite ordering, and asset-removal synchronization.
Confidence Score: 5/5
The PR appears safe to merge from this follow-up review because no additional blocking failure remains beyond the issue already covered by the existing inline thread.
No additional blocking failure remains.
Important Files Changed
Reviews (3): Last reviewed commit: "Merge branch 'main' into sid/reenable-do..." | Re-trigger Greptile