test(glossary): fix flaky "Add and Remove Assets" mutex batch#27707
test(glossary): fix flaky "Add and Remove Assets" mutex batch#27707
Conversation
The first batch in this test added two siblings from a mutuallyExclusive glossary and relied on the server rejecting the save so the UI stayed empty and `add-tag` remained clickable for the next batch. Since #25313, TreeAsyncSelectList.handleChange drops mutex siblings client-side once its `/api/v1/glossaries?fields=mutuallyExclusive` fetch resolves, so only one term reaches the server, it saves successfully, tags become non-empty, and `add-tag` is replaced by `edit-button`. The test then times out waiting for `add-tag`. Whether the filter kicks in depends on a race between the dropdown's glossaries fetch and the user clicks, which is why the test sometimes passed (retries masking it on CI) and sometimes timed out at 3 minutes under CI load. Remove the mutex batch (and the now-unused glossary1/term1/term2 fixtures). The mutex flow was never actually asserted here — it was only used as a side channel to keep tags empty. The remaining non-mutex batch + chart-level tagging + asset verification still cover the add/remove-assets behavior end-to-end. Local: 5/5 passes in ~15-23s each (was timing out at 180s). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
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! |
Code Review ✅ ApprovedStabilizes the 'Add and Remove Assets' mutex batch test by properly handling concurrency, effectively resolving the observed flakiness. OptionsDisplay: compact → Showing less information. Comment with these commands to change:
Was this helpful? React with 👍 / 👎 | Gitar |
There was a problem hiding this comment.
Pull request overview
This PR fixes flakiness in the Playwright E2E test Glossary.spec.ts › Add and Remove Assets by removing a mutually-exclusive glossary/term selection “batch” that depended on a server-side rejection (and became race-prone after client-side mutex enforcement was added).
Changes:
- Removed creation and cleanup of a mutually-exclusive glossary and its terms from
Add and Remove Assets. - Removed the initial UI interaction batch that selected mutex siblings and relied on the save being rejected.
- Kept the non-mutex add/remove-assets flow (including chart-level tagging and assets-tab verification) as the core end-to-end coverage.
|
Superseded by a less-invasive fix that keeps the multi-glossary tagging coverage — see the replacement PR. |
Summary
The
Pages/Glossary.spec.ts › Add and Remove AssetsE2E test (shard 6 on CI) is flaky — it times out at 180s on line 580 waiting for[data-testid="add-tag"]after the first save.Root cause. The test's first batch added two siblings from a
mutuallyExclusiveglossary and relied on the server rejecting the save so the entity'stagsstayed empty andadd-tagremained clickable for the next batch. Since #25313,TreeAsyncSelectList.handleChangedrops mutex siblings client-side once its/api/v1/glossaries?fields=mutuallyExclusivefetch resolves, so only one term reaches the server, the save succeeds,tagsbecomes non-empty, andTagsContainerV2swapsadd-tagforedit-button. The flake is the race between that dropdown fetch and the user's clicks — CI retries (retries: 2) masked it at merge time.Fix. Remove the mutex batch and its fixtures. The mutex flow was never actually asserted in this test — it was only used as a side channel to keep
tagsempty. The remaining non-mutex batch plus chart-level tagging and asset-tab verification still cover add/remove-assets end-to-end.Verification
mainbefore the fix.Test plan
yarn playwright test --project=chromium -g "Add and Remove Assets" --repeat-each=5— all 5 Glossary iterations pass.🤖 Generated with Claude Code