fix(playwright): route BulkImport back to import-export lane and audit-split the atomic describe - #30834
Conversation
…t-split the atomic describe Merge-queue plan step for PR #30697 failed with: Atomic Playwright units exceed the 20-minute execution budget; refactor or explicitly audit them for parallel splitting: chromium|Features/BulkImport.spec.ts|Bulk Import Export (25.8m) Two things went wrong when #30458 re-enabled the suite: 1. The re-enable rewrote test.describe.fixme('Bulk Import Export', { tag: '@Import-Export' }, ...) as test.describe('Bulk Import Export', ...) dropping both `.fixme` AND the `@import-export` tag. Without the tag, the suite fails to match `ImportExport` project's grep AND is not excluded by the chromium project's grepInvert — so the whole thing lands on the chromium lane instead of the dedicated import-export lane it was designed for. Restore the tag on the describe. 2. Even after the tag is restored, the entire "Bulk Import Export" describe is a single atomic unit. The six tests inside each set `test.setTimeout(300_000-600_000)` (5-10 min each), so on any real run the aggregate crosses the planner's 20-minute per-unit ceiling (TARGET_MS) regardless of lane. Add the describe to `AUDITED_PARALLEL_SUITES` so `discover_units` splits it into six per-spec parallel units. Module-scoped entity constructors generate unique names per test instance, so each parallel unit brings its own beforeAll state without cross-worker collision. Together: the tests route to the import-export lane (dedicated workers, tuned resources) AND no single unit can trip the atomic-ceiling gate as the suite grows. 64 planning tests pass locally. Prettier + ESLint clean on the spec. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
❌ 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 |
Code Review ✅ ApprovedRestores the OptionsDisplay: compact → Showing less information. Comment with these commands to change the behavior for this request:
Was this helpful? React with 👍 / 👎 | Gitar | Powered by Gitar — free for open source |
There was a problem hiding this comment.
Pull request overview
This PR fixes Playwright CI sharding/planning by ensuring the BulkImport suite is (1) routed to the dedicated Import/Export lane again and (2) split into audited per-spec units so it no longer violates the 20-minute atomic-unit budget enforced by the shard planner.
Changes:
- Restores the
@import-exporttag on theBulk Import Exportdescribe, so the suite is picked up by theImportExportPlaywright project and excluded from the chromium lane whenPW_DEDICATED_IMPORT_EXPORT=true. - Adds
("Features/BulkImport.spec.ts", "Bulk Import Export")toAUDITED_PARALLEL_SUITES, enabling the planner to split the suite into per-spec parallel units instead of treating the wholedescribeas one oversized atomic unit.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/BulkImport.spec.ts | Re-adds the @import-export tag to route the suite back to the ImportExport lane. |
| .github/scripts/build_playwright_shards.py | Audits/splits Bulk Import Export into per-spec units to satisfy the planner’s atomic-unit runtime ceiling. |
✅ Playwright Results — workflow succeededValidated commit ✅ 554 passed · ❌ 0 failed · 🟡 2 flaky · ⏭️ 3 skipped · 🧰 0 lifecycle flaky 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) 52m 11s ⏱️ Max setup 3m 1s · max shard execution 19m 5s · max shard-job elapsed before upload 22m 0s · reporting 5s 🌐 203.56 requests/attempt · 2.81 app boots/UI scenario · 2.55% common-shard skew Optimization targets still in progress:
🟡 2 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 |
🚦 Removed from the merge queue —
|
🚦 Removed from the merge queue —
|
…sized-unit error Two changes to make the tag-drop failure mode (PR #30834) actionable at PR time rather than a merge-queue plan failure. A. Improved `oversized_units` error message The existing gate correctly fires when any unit exceeds TARGET_MS, but the message ("refactor or explicitly audit them for parallel splitting") left developers guessing between two very different fixes. Rewrite it to spell out both: * Restore a `{ tag: '...' }` option that a recent edit dropped (points at FILE_LANE_HINTS and PR #30834 for the pattern). * Add the (file, describe_title) to AUDITED_PARALLEL_SUITES to split the describe into per-spec parallel units. B. FILE_LANE_HINTS — filename → expected project check A small mapping from filename regex → expected project + expected tag. At plan time, for every planned unit whose file matches a hint but whose project doesn't match the expected one, hard-fail with: Features/BulkImport.spec.ts: on `chromium` project, expected `ImportExport` (add `{ tag: '@Import-Export' }` to the top-level describe) This catches the exact PR #30834 pattern (a re-enable rewrote `test.describe.fixme('...', { tag: '@Import-Export' }, ...)` as `test.describe('...', ...)`, dropping the tag along with `.fixme`) at PR review time. Runs before the oversized-unit check so the developer gets the specific error, not the generic one. Seeded with one hint today — BulkImport*.spec.ts + *ImportExport*.spec.ts → `ImportExport` / `@import-export`. That matches 12 of the 13 files currently in the tree with those names; the 13th is the very bug this guard catches (see PR #30834). Add other conventions as they emerge. 7 new pytest cases cover the hint match, the ignore path, dedupe across audit-split units, the improved oversized message, and an end-to-end main() SystemExit reproduction of the tag-drop scenario. 71 total planner tests pass locally. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…pen-metadata#30871) * chore(playwright): refresh timing baseline from run 30802611748 The checked-in `.github/playwright/timing-baseline.json` had drifted 12 days behind the tree — captured 2026-07-22 (sha ccc86fe), but 51 commits since then have added, renamed, or re-tagged specs. Concrete drift as measured on origin/main today: * 30 current spec files have zero baseline coverage (Auth/SSOAuthentication.spec.ts + 25 tests, KnowledgeGraph, the 8 Features/SearchSeparation files, etc.) * 1007 baseline titles (23 %) reference tests that no longer exist — dead weight in load_history * 66 files (20 % of covered) have title drift — leaf renames, @tag additions, describe restructures * Worst offenders — Pages/ExplorePageRightPanel (218/234 orphaned, 93 % dead), Features/AdvancedSearch (107/131, 82 %), Pages/CustomProperties (98/149, 66 %), Pages/Lineage/DataAssetLineage (79/85, 93 %) Refresh from the most recent successful full-mode merge_group run (30802611748, 2026-08-03 09:43 UTC, sha 79df9ae). Its `playwright-timing-history-full` artifact is retained until 2026-09-02 and reflects the CI's actual runtime under the current tree. Metadata delta: * 4384 → 4409 tests (25 net additions) * 322 → 325 files (4 new covered: AssetHealthWidget, ColumnBulkOperationsTagsGlossary, IncidentManagerPagination, PipelineValidation) * 49 → 14 zero-duration entries (35 previously-skipped tests now have real durations captured) * BulkImport correctly recorded under the `ImportExport` project (PR open-metadata#30834 route fix reflected in the fresh capture) Preserved unchanged from the current baseline: * `retainedUnstableTestIds` (68-entry curated allowlist for known flaky tests) * `retainedSourceRunId` / `retainedSourceSha` pointers Normalized entries to the current baseline's schema (short `title` instead of the reporter's ` › proj › file › describe › leaf` breadcrumb; dropped `retryDurationMs`) so the diff stays a one-line JSON reserialization on the payload. Also removed a stale assertion in `test_versioned_baseline_omits_all_zero_ids_from_weights` that required at least one zero-duration entry with outcome=expected — a bug-case sanity check that no longer holds now that the fresh capture only contains legitimately-skipped zero-ms entries. The load-time filtering behavior (the meaningful part of the test) still asserts. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * address Copilot review — guard against vacuous pass `test_versioned_baseline_omits_all_zero_ids_from_weights` builds `zero_tests = [t for t in baseline if durationMs == 0]` and then asserts `all(t.id not in weights for t in zero_tests)`. If a future baseline refresh captures a run with no zero-duration entries, `zero_tests` is empty and the `all(...)` passes vacuously — the test would go green without actually exercising the load_history filter. Add an explicit `assert zero_tests` before the filter check with a message telling the next baseline-refresher to either construct a synthetic fixture or convert the test to synthetic input. Today's refreshed baseline still has 14 skipped 0-ms entries, so the guard is a no-op right now — it just catches the future case. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Summary
The merge-queue plan step for PR #30697 failed with:
Two orthogonal issues from #30458's re-enable of the suite, both fixed here.
Issue 1 — the
@import-exporttag was dropped along with.fixmeBefore #30458:
After #30458 (current
origin/main):.fixmewas removed correctly, but the{ tag: '@import-export' }option went with it. Without the tag:ImportExportproject'sgrep: combineGrep(/@import-export/)doesn't match → tests are excluded from the dedicated project.chromiumproject'sgrepInvert: [.., /@import-export/, ..](active withPW_DEDICATED_IMPORT_EXPORT=true) doesn't exclude them → they land on the chromium lane.Fix: restore the tag on the describe. The whole suite routes back to the dedicated
ImportExportproject →import-exportlane.Issue 2 — the atomic-unit ceiling still trips even in the import-export lane
The
Bulk Import Exportdescribe wraps six tests, each declaringtest.setTimeout(300_000-600_000)(5-10 min per test). The planner'soversized_unitscheck compares each unit'sweight_msagainstTARGET_MS = 20 * 60 * 1000regardless of destination lane — so a >20-min aggregate breaks the plan step wherever it lands.Fix: add
("Features/BulkImport.spec.ts", "Bulk Import Export")toAUDITED_PARALLEL_SUITES.discover_unitsthen splits the describe into six per-spec parallel units. Module-scoped entity constructors (new UserClass(),new GlossaryClass()etc.) generate unique names per test instance, so each parallel unit brings its ownbeforeAllstate without cross-worker collision.Verification
Test plan
import-exportshard with BulkImport specs.Related: #30812 (planner all-zero-history fix), #30822 (glossary hang fix).
🤖 Generated with Claude Code