ci(playwright): move import/export specs into a dedicated lane - #30617
Merged
Conversation
Add an `import-export` planner lane for the ~61 minutes of bulk
import/export test content that currently rides in the chromium lane.
Chromium is running at (or just under) its shard-budget ceiling in
merge_queue; peeling this content off gives the common lane real
headroom instead of relying on the 20-min bump alone.
- Planner: add `ImportExport` project → `import-export` lane, 2 workers,
same (1, 8) shard bounds as ingestion / search-rbac.
- playwright.config.ts: new `ImportExport` project (grep `@import-export`,
`fullyParallel: true`, `workers: 2`, entity setup + teardown). Chromium's
`grepInvert` excludes `@import-export` when the dedicated lane is on,
mirroring the `@ingestion` pattern.
- Workflow: set `PW_DEDICATED_IMPORT_EXPORT: "true"` on discover +
seed-state steps.
- Tag 12 specs with `@import-export` (TestCaseImportExport{Basic,E2eFlow},
ODCSImportExport{,Permissions}, GlossaryImportExport, BulkImport{,
WithDotInName}, MetricBulkImportExportEdit, BulkEditImportPermissions,
CSVImportWithQuotesAndCommas, SearchExport, LineageExportPNGSnapshot).
Verified locally:
- `pytest .github/scripts/tests/test_playwright_ci_planning.py` — 56 passed.
- `playwright test --list --project=ImportExport` enumerates all 12 files.
- `playwright test --list --project=chromium` — 0 import/export leaks;
3637 tests remain in chromium.
chirag-madlani
requested review from
a team,
akash-jain-10,
harshach and
tutte
as code owners
July 29, 2026 06:10
Contributor
❌ 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 |
github-actions
Bot
added
safe to test
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Running the full ui-checkstyle-changed chain (organize-imports → eslint --fix → prettier --write) re-indents the describe bodies of the specs whose top-level test.describe I converted to multi-line when adding the @Import-Export tag. Local prettier --check accepted both single-line and multi-line describe forms, but CI's checkstyle wants the prettier-canonical form (body at 4 spaces relative to file when the arrow function opens on its own line).
Greptile P1: when the dedicated lane is on, chromium's grepInvert excludes @Import-Export specs, but the planner discovery command didn't request the ImportExport project — so those specs were absent from the plan entirely and would never run in CI. Add --project=ImportExport to the discovery list, matching the Ingestion / DomainIsolation / SearchRBAC / etc pattern.
ShaileshParmar11
approved these changes
Jul 29, 2026
Rohit0301
approved these changes
Jul 29, 2026
Contributor
✅ Playwright Results — workflow succeededValidated commit ✅ 673 passed · ❌ 0 failed · 🟡 2 flaky · ⏭️ 11 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) 49m 46s ⏱️ Max setup 2m 56s · max shard execution 14m 17s · max shard-job elapsed before upload 17m 11s · reporting 6s 🌐 199.26 requests/attempt · 2.78 app boots/UI scenario · 16.80% 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 |
pmbrull
approved these changes
Jul 29, 2026
Code Review ✅ ApprovedMoves 12 bulk import/export Playwright specs into a dedicated CI lane to relieve shard pressure on the main chromium workflow. 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 | Powered by Gitar — free for open source |
harshach
added a commit
that referenced
this pull request
Jul 30, 2026
… lane Playwright never ran on this PR: plan-playwright aborted with "Playwright selectors matched no runnable tests: playwright/e2e/Pages/ GlossaryImportExport.spec.ts", so no shard matrix was produced and every shard was skipped. #30617 moved @Import-Export tagged specs into a dedicated ImportExport project and made chromium grepInvert that tag. The impact map still listed only chromium/Basic for mappings whose spec globs match those files, so build_playwright_shards.py found the spec under project ImportExport, failed to match the selector, and treats an unmatched selector as fatal. Auditing every mapping against the tagged specs found three such combinations, not one — the glossary mapping this PR tripped plus the search and permissions mappings, which would break any PR touching those sources: Features/*Search*.spec.ts -> SearchExport.spec.ts Pages/*Glossary*.spec.ts -> GlossaryImportExport.spec.ts **/*Permission*.spec.ts -> BulkEditImportPermissions.spec.ts, ODCSImportExportPermissions.spec.ts Verified by reproducing the pipeline locally: playwright --list with CI's project set, select_playwright_tests.py over this branch's changed files, then build_playwright_shards.py. Before: exit 1 on the unmatched selector. After: exit 0, 17 shards across 9 lanes, GlossaryImportExport scheduled in the import-export lane. A re-audit reports zero remaining drift for both the ImportExport and Ingestion lanes. Co-Authored-By: Claude <noreply@anthropic.com>
ShaileshParmar11
added a commit
that referenced
this pull request
Jul 30, 2026
#30617 moved every `@import-export` spec into a dedicated ImportExport project and grepInvert-ed them out of chromium/Basic/Ingestion/Reindex, but impact-map.json was never taught about that lane. The DQ/observability mapping's `Features/DataQuality/**/*.spec.ts` glob still swept TestCaseImportExportBasic and TestCaseImportExportE2eFlow in and requested them under the four lanes that can no longer run them, so build_playwright_shards.py aborted with "Playwright selectors matched no runnable tests". This surfaced here because this branch added playwright/utils/incidentManager.ts to that mapping's sources, which is what makes the mapping fire. selector_matches_unit is any-of-projects, so adding the lane resolves the two tagged specs without pulling the other 24 DataQuality specs into it. Co-Authored-By: Claude <noreply@anthropic.com>
Rohit0301
added a commit
that referenced
this pull request
Jul 30, 2026
…right test (#30680) * fix(ui): center domain/data-product list row items & fix DP detail header title width (#30510) * fix(ui): vertically center domain & data product list row items The AI-mode Domain and Data Product list tables rendered their name cells with `<Box align="start">` (align-items: flex-start), pinning the icon + name to the top of the row instead of centering it — most visible when the name wraps to multiple lines. Use `align="center"` on the name/Domains cells so the icon lines up with the middle of the (single- or multi-line) name. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(ui): don't reserve badge space in the DP header when the status badge is empty DataProductsDetailsPage passed `badge={statusBadge}` to EntityHeader. A truthy `badge` applies `w-max-full-200` (max-width: calc(100% - 200px)) to the title column — reserving 200px even when EntityStatusBadge renders nothing (the common `Unprocessed` status), which squeezed the title into a narrow box and forced long names to wrap/break mid-word. The Domain header passes no badge and is unaffected. Move `statusBadge` into `suffix` (next to the LearningIcon): it still renders inline when a status exists, but no longer reserves 200px, so the title uses its natural width. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * ci(playwright): give the domain impact-map mapping the Reindex lane The domain/dataProduct mapping selects SearchSeparation `*Domain*` specs (e.g. DomainRenamePrefixCascade.spec.ts) via its `**/*Domain*.spec.ts` glob, but assigned only [chromium, Basic, DomainIsolation]. Those specs are testIgnored from chromium/Basic and only run under the `Reindex` project, so build_playwright_shards.py found no runnable unit and aborted plan-playwright with "matched no runnable tests" on every PR touching Domain/DataProduct UI sources. Add `Reindex` to the mapping's projects so the spec resolves to its real lane and runs. Side-effect is contained: of this mapping's specs, only the SearchSeparation one yields a Reindex unit. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * Fixed the knowledge center translations (#30521) * Fixed the knowledge center translations * minor fix * playwright fix * lint fix * ci(playwright): move import/export specs into a dedicated lane (#30617) * ci(playwright): move import/export specs into a dedicated lane Add an `import-export` planner lane for the ~61 minutes of bulk import/export test content that currently rides in the chromium lane. Chromium is running at (or just under) its shard-budget ceiling in merge_queue; peeling this content off gives the common lane real headroom instead of relying on the 20-min bump alone. - Planner: add `ImportExport` project → `import-export` lane, 2 workers, same (1, 8) shard bounds as ingestion / search-rbac. - playwright.config.ts: new `ImportExport` project (grep `@import-export`, `fullyParallel: true`, `workers: 2`, entity setup + teardown). Chromium's `grepInvert` excludes `@import-export` when the dedicated lane is on, mirroring the `@ingestion` pattern. - Workflow: set `PW_DEDICATED_IMPORT_EXPORT: "true"` on discover + seed-state steps. - Tag 12 specs with `@import-export` (TestCaseImportExport{Basic,E2eFlow}, ODCSImportExport{,Permissions}, GlossaryImportExport, BulkImport{, WithDotInName}, MetricBulkImportExportEdit, BulkEditImportPermissions, CSVImportWithQuotesAndCommas, SearchExport, LineageExportPNGSnapshot). Verified locally: - `pytest .github/scripts/tests/test_playwright_ci_planning.py` — 56 passed. - `playwright test --list --project=ImportExport` enumerates all 12 files. - `playwright test --list --project=chromium` — 0 import/export leaks; 3637 tests remain in chromium. * ci: fix ui-checkstyle formatting on tagged import/export specs Running the full ui-checkstyle-changed chain (organize-imports → eslint --fix → prettier --write) re-indents the describe bodies of the specs whose top-level test.describe I converted to multi-line when adding the @Import-Export tag. Local prettier --check accepted both single-line and multi-line describe forms, but CI's checkstyle wants the prettier-canonical form (body at 4 spaces relative to file when the arrow function opens on its own line). * ci(playwright): include ImportExport project in discovery command Greptile P1: when the dedicated lane is on, chromium's grepInvert excludes @Import-Export specs, but the planner discovery command didn't request the ImportExport project — so those specs were absent from the plan entirely and would never run in CI. Add --project=ImportExport to the discovery list, matching the Ingestion / DomainIsolation / SearchRBAC / etc pattern. * ci(playwright): restore fork PR support on knowledge-graph workflow (#30561) * ci(playwright): restore fork PR support on knowledge-graph workflow Mirrors the pattern #30485 established on playwright-postgresql-e2e.yml to the RDF/knowledge-graph workflow, which #30310 also regressed to pull_request-only. Fork PRs against RDF/ontology paths were previously short-circuited by the `!fork` check on the second job — they built the distribution but silently skipped the entire Playwright run, so RDF work from external contributors ran zero E2E coverage. Changes: - Add pull_request_target trigger with the same paths filter used by pull_request so the scope stays RDF-focused. - Partition the concurrency group by github.event_name so pull_request and pull_request_target runs on the same PR don't cancel each other. - Introduce a `gate` job that waits for Team Label reconciliation on fork synchronize events, reads the reconciled live PR labels via the GitHub API, and computes a should_run decision: workflow_dispatch → run draft → skip labeled event, label != "safe to test" → skip same-repo PR + pull_request → run fork PR + pull_request_target + reconciled "safe to test" label → run everything else → skip - Wire `build` and `playwright-rdf-postgresql` through `needs: gate` with a `needs.gate.outputs.should_run == 'true'` guard. Removes the now-redundant `!github.event.pull_request.head.repo.fork` short- circuit from the RDF job. - Fix the two `actions/checkout@v7` steps to explicitly check out `github.event.pull_request.head.sha` under pull_request_target (base.sha is wrong for testing fork code) and set `allow-unsafe-pr-checkout: true` — the gate already restricted this event to safe-to-test-labelled forks, and the build/test steps consume the fork tree inside docker containers. The workflow doesn't have a playwright-summary job or a named required-check consumer, so no name-collision rename (#30476) is needed here. `pull_request` vs `pull_request_target` runs on the same PR each report their own separate check-run status; gate-skipped runs finish trivially with `should_run=false` and don't perform any docker work. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * add merge_group trigger + name-collision facade for required check Per @chirag Madlani review — two follow-up gaps confirmed: 1. This workflow is required in the merge queue. 2. `Playwright RDF (Knowledge Graph + Ontology)` is a required branch-protection check. Add merge_group support: - Add `merge_group:` to on: triggers. - Add merge_group to the gate's always-run case: workflow_dispatch | schedule | merge_group. Add name-collision handling (same pattern #30476 established for playwright-postgresql-e2e.yml): - Rename the existing execution job's display name from `Playwright RDF (Knowledge Graph + Ontology)` to `RDF Playwright execution` — internal identity only, no longer the required check name. - Add a `playwright-rdf-summary` job with `if: always() && !cancelled()` that publishes a dynamic check name: gate succeeds, should_run=true → `Playwright RDF (Knowledge Graph + Ontology)` gate fails (crash/cancelled) → `Playwright RDF (Knowledge Graph + Ontology)` + hard fail labeled event, non-safe-to-test → `... (label ignored)` gate succeeds, should_run=false → `... (skipped)` - Three steps inside the summary job: "Guard against missing gate decision" — fails loudly when gate itself did not produce a valid should_run "Report gate-skipped run as green" — no-op success for legitimate skips (redundant sibling event, fork without safe-to-test) "Fail on upstream failure" — propagates build / execution job failures under the required check name so the PR fails visibly Without this, both events on a same-repo PR would publish check-runs under `Playwright RDF (Knowledge Graph + Ontology)`; the pull_request_target sibling finishes in ~15s with a green skip while the real pull_request run is still compiling, making the PR appear mergeable before any RDF test executes — the same class of hole #30476 fixed for playwright-summary on postgresql-e2e. No branch-protection update needed — the required check name is preserved end-to-end; only its publisher moved from the underlying execution job to the always-running facade. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * resolve entity schema path from the classpath scan instead of a hardcoded map (#30618) * Fixes 30533: derive entity schema subdirectory from the classpath scan instead of a hardcoded map * gitarbot feedback: make duplicate entity type schema resolution independent of classpath scan order * fix(settings): don't log an unset optional setting as an error (#28960) (#30625) * fix(ui): keep GenericTab's widget grid inside its container (#30622) react-grid-layout derives each grid item's `left` and `width` in pixels and rounds the two independently. Their exact values always sum to the container width, but each rounding can add just under half a pixel, so the right edge of the last column can land 1px outside the container. That pixel is the widget card's right border. An ancestor that scrolls turns it into a horizontal scrollbar; one that clips swallows the border outright. On entity detail pages both happen, because the active tab pane scrolls vertically and CSS promotes the cross axis to `auto` once one axis is not `visible`. Give the grid 1px of horizontal containerPadding. That pulls the exact right edge in to `width - 1`, which the rounding -- bounded at 1px, since the two exact edges sum to the width and each rounding adds under half a pixel -- can never exceed. Across widths 600-2400 and every column split that reaches the right edge, the last column escapes the container at 1575 of 1801 widths today and at none of them with this change. For the 6 + 2 split the entity detail pages actually use, it is one width in four. Adjusting `margin` instead does not work: it is the gutter between items and sits inside the sum that equals the container width, so changing it only moves which widths collide (still 175/701 at every integer value from 0 to 24). * Improve bulk export action stats (#30615) * Improve bulk export action stats * Add auto opened when the export jobs is success * nit * fix stylling * nit * addressed comments * fix lint checks * nit * fix failing unit test * Fixes #30117: hide soft-deleted owners on the Test Suite detail page (#30520) * Fixes #30117: keep soft-deleted owners in PATCH base to fix index crash Editing owners failed with "array item index out of range" when an owner user was soft-deleted. Detail pages fetch with include=all (soft-deleted owner present) and compute a positional JSON Patch, but the server loaded the PATCH base with NON_DELETED, which dropped that owner and shrank the array so the positional op referenced an index past the end. The dangling ownership also could not be removed. Both patch() entry points now load the original with a RelationIncludes that keeps the entity at NON_DELETED but resolves owners/experts/reviewers/ followers with ALL, so the positional patch stays in range and the ID-based updateOwners diff removes the dangling OWNS row. Adds two integration tests reproducing the reported crash (deleted owner sorting last) and the silent wrong-owner removal (deleted owner in the middle). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * Fixes #30117: tolerate existing soft-deleted owners on PATCH; generic test The first pass loaded the PATCH base with owner/expert/reviewer/follower refs resolved as ALL. That fixed the positional "array item index out of range" crash on owner removal, but getValidatedOwners (which runs on every PATCH) re-validates the whole owner list against NON_DELETED, so ANY edit (e.g. a description change) on an entity carrying a dangling soft-deleted owner then failed with 404 "user instance ... not found". - Narrow the base override to owners only. Experts/reviewers/followers are validated on update against NON_DELETED and would hit the same 404; they are left unchanged as a separate follow-up, per the issue. - Add getValidatedOwnersForPatch: retain owners already assigned in the base (a soft-deleted user is still a valid, existing assignment) and validate only newly added owners against NON_DELETED, so a deleted user still cannot be freshly assigned. - Add a generic BaseEntityIT test so every owner-supporting entity is covered, plus a TestSuite regression test asserting a non-owner edit succeeds and preserves the soft-deleted owner. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * Fixes #30117: address review - retain authoritative owner refs; document tradeoff - getValidatedOwnersForPatch: retained (already-assigned) owners now use the authoritative ref from the base (resolved with ALL, so names are populated) instead of the client-supplied ref, and the sort uses a null-safe comparator. Prevents a possible NPE/500 when a client sends a minimal (id+type, no name) ref for an already-assigned owner, and keeps response metadata authoritative. - Document the known trade-off on PATCH_ORIGINAL_INCLUDES: callers that diff owners against a NON_DELETED view (a GET sending owners:non-deleted) can misalign against the ALL base; the representation-aware fix is the follow-up the issue flags as a design call. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * Fixes #30117: hide soft-deleted owners on Test Suite page (align with data assets) Revert the server-side PATCH-base change. Loading the base with owners=ALL fixed the Test Suite crash but regressed the 17+ data-asset detail pages that fetch owners with includeRelations=owners:non-deleted: a positional owner remove computed against the filtered array removed the wrong owner when applied to the longer ALL base. A single server base cannot align with both client owner representations, so the patch itself must carry it - a separate design call the issue flags. Fix the reported crash where the divergence actually is: the Test Suite detail page was the only owner-editing page fetching owners with include=all and no includeRelations override. getTestSuiteByName now defaults owners/experts to non-deleted like every data-asset page, so a soft-deleted owner is not surfaced and the owner-edit diff stays aligned with the server's NON_DELETED PATCH base - no "array item index out of range". Showing and removing soft-deleted owners remains the issue's separate design call. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * Fixes #30117: honor includeRelations on Test Suite getByName (frontend fix was a no-op) getTestSuiteByName sends includeRelations=owners:non-deleted, but the Test Suite getByName endpoint - unlike getById and every other entity's getByName (e.g. TableResource) - did not accept the param, so the server still resolved owners with include=all and surfaced the soft-deleted owner. The owner-edit crash therefore remained. Wire includeRelations through getByName (same pattern as getById / TableResource.getByName), and add an IT that proves it: an include=all fetch still surfaces the soft-deleted owner, but includeRelations=owners:non-deleted hides it. Verified RED (without the wiring the assertion fails - owner still surfaced) and GREEN. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Co-authored-by: Sid <30566406+siddhant1@users.noreply.github.com> * Fix glossary export playwright (#30585) * fix flaky searchIndexApplication * fix flaky glossary terms popover playwright * revert unwanted changes --------- Co-authored-by: Shrabanti Paul <shrabantipaul@Shrabantis-MacBook-Pro.local> Co-authored-by: Rohit Jain <60229265+Rohit0301@users.noreply.github.com> * ISSUE #30151 - Incident Grouping Endpoint (#30367) * feat: added incident group endpoint * feat: added owner, status, assignee, severiuty and bucket for client metrics * feat: added owner, status, assignee, severiuty and bucket for client metrics * empty commit for CI trigger * feat: remove n+1 in entity ref. retrieval when resolving group references * feat: ignore optional argumewnt static check error * feat: address comments and fix failing integration tests * feat: revert pre existing migrations to 2.0 * feat: address first pass review * feat: move endpoint data to summary table * feat: address comments * feat: address comments * chore: fix migration * fix Column Bulk Operations tag & glossary dropdowns not selectable in the edit drawer (#30634) * fix Column Bulk Operations tag & glossary dropdowns not selectable in the edit drawer * fix lint checks * fix(test): correct EditAll-deny test assertions in EntityPermissions When EditAll is in a DENY rule it subsumes all Edit* operations via matchesBySubsumption(), so EditTier/EditOwners/EditCertification buttons are not visible even when those operations have explicit ALLOW rules. Updated assertions from toBeVisible() to not.toBeVisible() and unskipped the test to reflect actual backend deny-first policy semantics. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> --------- Co-authored-by: Sid <30566406+siddhant1@users.noreply.github.com> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Co-authored-by: Chirag Madlani <12962843+chirag-madlani@users.noreply.github.com> Co-authored-by: Vishnu Jain <121681876+Vishnuujain@users.noreply.github.com> Co-authored-by: Anujkumar Yadav <anujf0510@gmail.com> Co-authored-by: Mohit Yadav <105265192+mohityadav766@users.noreply.github.com> Co-authored-by: shrabantipaul-collate <shrabanti.paul@getcollate.io> Co-authored-by: Shrabanti Paul <shrabantipaul@Shrabantis-MacBook-Pro.local> Co-authored-by: Teddy <teddy.crepineau@gmail.com>
Laveshjadon
pushed a commit
to Laveshjadon/OpenMetadata
that referenced
this pull request
Jul 30, 2026
…rms (open-metadata#30614) * fix(intake-forms): render every custom-property type correctly in create forms Intake-form custom properties were rendered by a private switch in AddDomainForm that hardcoded entityReference to a user/team picker and had cases only for enum/number/string. A property configured for ['glossaryTerm'] therefore listed users, hyperlink-cp fell through to a plain text input, and every other type produced a payload shape the backend rejects. - Add AddDomainFormExtensionFields (react-aria/RHF) covering all custom property types for Create Domain / Data Product, driving the entity search from customPropertyConfig so allowed types are honored. - Add GlossaryTermIntakeFields (antd) and wire intake forms into the Create Glossary Term modal, which had no intake support at all even though the backend enforces it on POST /v1/glossaryTerms. - Centralize serialization in serializeExtensionValue so all submit paths emit backend-valid shapes (numbers coerced, enum wrapped in an array, hyperlink as {url, displayText?}, real EntityReference type). - Let intake forms include optional fields: add formFields with a per-field required flag, keep requiredFields as a deprecated compatibility view, and backfill existing rows via migration. - Remove a deleted custom property from intake forms instead of leaving a dangling field reference. - Group intake custom properties under a "Custom Properties" section with a property-type badge, matching the bulk-edit inline editor. Co-Authored-By: Claude <noreply@anthropic.com> * Update generated TypeScript types * fix(intake-forms): version the custom-property prune via EntityUpdater removeCustomPropertyField mutated the form and called store() directly, skipping the version bump, updatedBy, changeDescription and change event. Because the version stayed put, a client holding a pre-prune copy could PUT it back and still pass the optimistic-lock check, silently restoring the deleted field with no audit trail. Route the prune through getUpdater(...).update() and attribute it to the user who deleted the custom property. IntakeFormUpdater already records formFields/requiredFields, so the cascade now produces a proper change description. assertDeletedPropertyPruned asserts the version increment, updatedBy and the recorded formFields change, which fails on the store()-directly path. Co-Authored-By: Claude <noreply@anthropic.com> * fix(intake-forms): guard non-finite numbers, empty tables, markdown value Address PR review findings on the shared extension serializer and the domain intake widgets: - integer/number/timestamp and timeInterval bounds now emit only finite numbers, so a non-numeric value is omitted rather than POSTed as NaN (invalid JSON for the backend). The empty-bound check is kept ahead of the coercion because Number('') is 0, not NaN. - table-cp is serialized explicitly and omitted when no row is populated, so an optional table no longer ships an empty {columns, rows: []}. - MarkdownExtensionField seeds RichTextEditor from the current field value instead of always '', so a pre-populated form no longer renders an empty editor the user can overwrite by accident. - Replace the hardcoded tw:gap-[6px] with the tw:gap-1.5 design token that the CI Tailwind audit maps it to. Co-Authored-By: Claude <noreply@anthropic.com> * fix(intake-forms): render glossary intake fields with UntitledUI, not antd The Glossary Term intake fields were built on antd, which the deprecation guard blocks for new UI work. Re-render them with the shared react-hook-form/UntitledUI field set already used by Domain and Data Product, so all three create forms now share one widget implementation. The RHF form is owned by GlossaryTermIntakeFields rather than the parent: calling useForm in the component that hosts the antd Form re-renders it mid-validation and its required rules silently stop resolving (this broke the native-required tests). The parent keeps only a ref, validates through it, and merges the serialized extension at submit. The fields render as a sibling of the antd Form because nesting <form> elements is invalid HTML; the modal's Save button lives in the footer outside both. Also stop rendering a plain text input for a required custom property whose definition failed to load — that submitted an untyped value and reproduced the very 400 this PR fixes. It now shows an inline error and blocks submit. Co-Authored-By: Claude <noreply@anthropic.com> * style(intake-forms): apply prettier formatting Co-Authored-By: Claude <noreply@anthropic.com> * fix(migrations): survive a non-array requiredFields when backfilling formFields The 1.13.3 backfill selected rows by key presence but then unconditionally expanded requiredFields as an array. Verified against a live PostgreSQL: a row storing JSON null (or any scalar/object) fails the statement with "cannot extract elements from a scalar", which aborts the upgrade for the whole deployment over one malformed row. MySQL degraded gracefully and did not reproduce, but is given the same guard so both dialects behave identically. Both scripts now expand requiredFields only when it is genuinely an array and carry over only object entries, so a scalar mixed into the array is skipped instead of producing a malformed formFields entry. Rows that already have formFields are still left untouched. Verified on PostgreSQL 15 and the project's MySQL image against rows with requiredFields set to null, a string, an object, an array containing a junk scalar, and a row that already had formFields: both dialects now return [] for the malformed shapes, migrate the valid entry with required=true, and leave the pre-populated row alone. Co-Authored-By: Claude <noreply@anthropic.com> * style(intake-forms): satisfy the src ESLint + organize-imports checkstyle CI's src checkstyle step re-runs organize-imports, eslint --fix and prettier --write over the changed files and fails if that leaves a diff. Two things did: an unused EntityReference import left behind when the glossary serialization test was rewritten to drive the RHF widgets, and a missing blank line before a statement. My earlier local runs missed this because they invoked organize-imports through yarn, which passes the file list as a single argument — it silently matched nothing and reported no changes. Running the binary with the list word-split reproduces CI exactly. Co-Authored-By: Claude <noreply@anthropic.com> * fix(intake-forms): emit a change event for the custom-property prune Change events are inserted by ChangeEventHandler, a REST response filter that builds them from the HTTP response entity. This cascade runs inside the Type update request, so the filter only ever sees the Type — the IntakeForm gained a version and a changeDescription with no event for subscribers to consume, leaving event-driven clients on a stale form. Emit one explicitly via createAndInsertChangeEvent, the EntityRepository helper DataProductRepository already uses for the same situation; it skips empty change descriptions and carries the previous/current version. The cascade IT now asserts an intakeForm ENTITY_UPDATED event is recorded for the pruned form. Co-Authored-By: Claude <noreply@anthropic.com> * Removed alert based helperText and nit picks * nit * fix ui checkstyles * fix route glossary import/export spec to the ImportExport lane * ci(playwright): let impact-map select specs moved to the ImportExport lane Playwright never ran on this PR: plan-playwright aborted with "Playwright selectors matched no runnable tests: playwright/e2e/Pages/ GlossaryImportExport.spec.ts", so no shard matrix was produced and every shard was skipped. open-metadata#30617 moved @Import-Export tagged specs into a dedicated ImportExport project and made chromium grepInvert that tag. The impact map still listed only chromium/Basic for mappings whose spec globs match those files, so build_playwright_shards.py found the spec under project ImportExport, failed to match the selector, and treats an unmatched selector as fatal. Auditing every mapping against the tagged specs found three such combinations, not one — the glossary mapping this PR tripped plus the search and permissions mappings, which would break any PR touching those sources: Features/*Search*.spec.ts -> SearchExport.spec.ts Pages/*Glossary*.spec.ts -> GlossaryImportExport.spec.ts **/*Permission*.spec.ts -> BulkEditImportPermissions.spec.ts, ODCSImportExportPermissions.spec.ts Verified by reproducing the pipeline locally: playwright --list with CI's project set, select_playwright_tests.py over this branch's changed files, then build_playwright_shards.py. Before: exit 1 on the unmatched selector. After: exit 0, 17 shards across 9 lanes, GlossaryImportExport scheduled in the import-export lane. A re-audit reports zero remaining drift for both the ImportExport and Ingestion lanes. Co-Authored-By: Claude <noreply@anthropic.com> * fix(e2e): stabilize multi-enum Autocomplete selection in IntakeForm spec After clicking the first option the chip renders, which shifts the combobox input position and may leave the dropdown in a transitional state (closing animation + focus-restoration reopen via menuTrigger). Wait for the chip to be visible before pressing Escape to cleanly settle the dropdown, then re-click the input to select the second value. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * Removed custom properties label * fix lint issue * fix unit test * fix(e2e): drop stale Custom Properties heading assertions in IntakeForm spec Commit 5ded29b intentionally removed the 'Custom Properties' Typography heading from AddDomainFormExtensionFields. Remove the two toContainText assertions that depended on it so the test matches the current component. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(e2e): fix two more flaky interactions in IntakeForm custom-property test 1. Date picker popup didn't close before the dateTime picker opened, so getByRole('button', { name: 'Today' }) resolved to 2 elements and the strict-mode locator threw. Wait for 'Today' to be hidden after each Apply click before opening the next picker. 2. Multi-enum second selection: pressing Escape returns focus to the combobox input; with menuTrigger="focus" the Autocomplete immediately reopens the dropdown, causing a re-render cycle that detaches the input before the next click lands. Click the name input to defocus and wait for the listbox to disappear before re-clicking the multiEnum input. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * nit * fix lint checks * fix playwright nit and lint checks --------- Co-authored-by: Claude <noreply@anthropic.com> Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: anuj-kumary <anujf0510@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Peel ~61 minutes of bulk import/export test content off the chromium
lane and into a dedicated
import-exportlane.Chromium is running against its shard-budget ceiling in
merge_queue(see #30616 for the short-term 19 → 20 min bump). The knife-edge is
here because chromium currently absorbs ~1h of import/export content
on top of everything else. Moving that content into its own lane
gives chromium real headroom instead of relying on the budget bump
alone, and lets the two workloads scale independently.
.github/scripts/build_playwright_shards.py): registerImportExportproject →import-exportlane, 2 workers,(1, 8)shard bounds (same shape as
ingestion/search-rbac).playwright.config.ts: newImportExportproject(
grep: /@import-export/,fullyParallel: true,workers: 2,entity dependencies + teardown). Chromium's
grepInvertexcludes@import-exportwhen the dedicated lane is on — mirrors the existing@ingestionpattern.PW_DEDICATED_IMPORT_EXPORT: "true"on discover +seed-state steps.
@import-export:Features/DataQuality/TestCaseImportExport{Basic,E2eFlow}.spec.tsPages/ODCSImportExport{,Permissions}.spec.tsPages/GlossaryImportExport.spec.tsPages/CSVImportWithQuotesAndCommas.spec.tsFeatures/BulkImport{,WithDotInName}.spec.tsFeatures/MetricBulkImportExportEdit.spec.tsFeatures/BulkEditImportPermissions.spec.tsFeatures/SearchExport.spec.tsFeatures/LineageExportPNGSnapshot.spec.tsFull mode (
merge_group) still runs every project — this is purelyabout how the planner packs shards.
Test plan
pytest .github/scripts/tests/test_playwright_ci_planning.py— 56 passed (addstest_import_export_runs_in_its_own_lane_with_two_workers)playwright test --list --project=ImportExport— enumerates all 12 filesplaywright test --list --project=chromium— 0 import/export leaks; 3637 tests remain in chromiumprettier --check+eslint— clean on all touched filesimport-exportlane and reduced chromium shard countGreptile Summary
The PR moves tagged import/export Playwright coverage into a dedicated CI lane.
ImportExportproject andimport-exportplanner lane with two workers and bounded shard counts.@import-exporttests from Chromium while routing them through the new project.Confidence Score: 5/5
The PR appears safe to merge.
No blocking failure remains; the current workflow explicitly discovers ImportExport, the planner preserves it as an executable lane, and shard execution re-enables the conditional project through the shard plan.
Important Files Changed
Flowchart
%%{init: {'theme': 'neutral'}}%% flowchart LR Specs[Tagged import/export specs] --> Discovery[Playwright project discovery] Discovery --> Planner[Shard planner] Planner --> ImportExport[import-export lane] Planner --> Chromium[chromium lane] ImportExport --> Matrix[Workflow matrix] Chromium --> Matrix Matrix --> Execution[Playwright shard execution]Reviews (2): Last reviewed commit: "ci(playwright): include ImportExport pro..." | Re-trigger Greptile