test(teams): stop the add-team click from landing in a toast - #31734
Conversation
addTeamHierarchy clicked the add button with `force: true`, which skips the hit-target check. The backend fans delete/job notifications out to every socket of the logged-in admin, so a parallel worker's success toast can sit over that button — and with force the click was dispatched into the toast. The click "succeeded", the modal never opened, and the failure surfaced 15s later on the modal assertion. Success toasts carry no close button and self-dismiss after 3.5s, so clicking without force is the fix: the hit-target check waits the toast out. Co-Authored-By: Claude Opus 5 (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 |
|
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! |
harsh-vador
added
the
safe to test
✅ Playwright Results — workflow succeededValidated commit ✅ 570 passed · ❌ 0 failed · 🟡 0 flaky · ⏭️ 0 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) 1h 7m 2s ⏱️ Max setup 4m 20s · max shard execution 14m 50s · max shard-job elapsed before upload 18m 18s · reporting 5s 🌐 218.43 requests/attempt · 2.85 app boots/UI scenario · 4.38% 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 |
harsh-vador
added
the
To release
Dropping `force` was not enough. Playwright checks the hit target and then dispatches, so a socket-fanned success toast landing in that window still swallows the click — the trace shows the click "succeeding" in 16ms with a delete toast on screen and the modal never opening. Route every add-team trigger through openAddTeamModal: let a pending toast expire, click, and retry until the modal is up. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Pushed b63c205 — the first fix was not sufficient, it still failed in nightly (run 32129322883, Dropping New Applied at every add-team trigger so it cannot regress at one call site: Verified locally: prettier clean, eslint 0 errors on both files (the 4 remaining warnings are the pre-existing |
🚦 Removed from the merge queue —
|
|
Failed to cherry-pick changes to the 1.13 branch. |
|
Changes have been cherry-picked to the 2.0 branch. |
* test(teams): stop the add-team click from landing in a toast addTeamHierarchy clicked the add button with `force: true`, which skips the hit-target check. The backend fans delete/job notifications out to every socket of the logged-in admin, so a parallel worker's success toast can sit over that button — and with force the click was dispatched into the toast. The click "succeeded", the modal never opened, and the failure surfaced 15s later on the modal assertion. Success toasts carry no close button and self-dismiss after 3.5s, so clicking without force is the fix: the hit-target check waits the toast out. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(teams): retry the add-team open when a toast eats the click Dropping `force` was not enough. Playwright checks the hit target and then dispatches, so a socket-fanned success toast landing in that window still swallows the click — the trace shows the click "succeeding" in 16ms with a delete toast on screen and the modal never opening. Route every add-team trigger through openAddTeamModal: let a pending toast expire, click, and retry until the modal is up. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> (cherry picked from commit af609a6)
Code Review ✅ ApprovedRemoves forced clicks from the team-hierarchy Playwright helper to let tests wait out transient success toasts, eliminating test flakiness. No issues found. OptionsDisplay: compact → Showing less information. Comment with these commands to change the behavior for this request:
Was this helpful? React with 👍 / 👎 | Powered by Gitar — free for open source |
* test(teams): backport addTeamHierarchy hardening to 1.13 1.13 never received the addTeamHierarchy waits that main and 2.0 carry, so the helper returns while the teams table is still refetching: it opens the modal on a plain click a toast can swallow, resolves on the first `/api/v1/teams` response regardless of method or status, and never waits for the modal to close or the row to render. Backports the team.ts changes from #25894, #30334 and #31734. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(teams): stop the teams landing from eating the test budget TeamsDragAndDrop's beforeEach spent the whole 60s test timeout before any test body ran. settingClick already ends in waitForAllLoadersToDisappear (30s), and the hook then called it a second time. The Teams table's antd spinner is the shared `data-testid="loader"`, and it stays up until both the child-teams fetch and the per-team asset-count aggregation settle — on a long-lived deployment that is tens of seconds, so the two waits together consumed the budget and the hook timed out on the second one. Give the suite test.slow(true) so the landing has headroom, and hard-delete the four teams the suite creates. Without that cleanup every nightly run left four more teams under Organization, growing the aggregation the landing waits on and making the next run slower. Fold the duplicated navigation into visitTeamsPage and reuse it from TeamsHierarchy, which had the same hook with looser glob waits. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(teams): surface a failed cleanup delete hardDeleteTeamByName issued the DELETE and ignored the result, so a delete rejected on permissions or failing with a 500 left the team behind with no signal in the run — quietly reintroducing the accumulation this cleanup exists to prevent. Assert the response instead, with the status and body in the message. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(teams): only tolerate a 404 on the cleanup lookup The lookup guard keyed off `ok()`, which is false for an auth error or a 500 just as it is for a 404. A broken lookup therefore took the same path as a team that was already gone: return without deleting, and report success. Tolerate 404 alone — the spec may have deleted the team itself, and a recursive delete of its parent takes its children — and assert every other lookup failure. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(teams): attempt every cleanup delete before asserting The assertion sat inside hardDeleteTeamByName, so the first team that failed to delete threw out of the caller's loop and the remaining names were never attempted — leaving those teams on the deployment, which is the accumulation the cleanup exists to prevent. Move the assertion up into hardDeleteTeamsByName: the per-team helper now reports a failure instead of throwing (network errors included), every name is attempted, and one assertion at the end names every team that survived along with the status and body that explain why. Wrap the call in try/finally in the spec so the API context is disposed even when cleanup fails. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
3 participants
Describe your changes
TeamsHierarchy.spec.ts→Add teams in hierarchyflakes: the add-team modal never opens, and the failure surfaces ~15s later asexpect(locator('[role="dialog"].ant-modal').last()).toBeVisible()timing out.Root cause.
addTeamHierarchyclicked the add button with{ force: true }, which skips Playwright's hit-target check. The backend fans delete/job notifications out to every socket of the logged-in admin, so a parallel worker's success toast (e.g."archive-test-….txt" deleted successfully!) can sit overadd-placeholder-button. Withforcethe click was dispatched into the toast — it reported success in ~148ms, no modal opened, and the next assertion ate the full 15s.Fix. Drop
force: trueand click agetByTestIdlocator normally. Success toasts render no close button (showClose: falsefor thesuccessvariant) and self-dismiss after 3500ms, so the hit-target check simply waits the toast out — well inside the test timeout. This also brings the helper in line with the repo rule that{ force: true }is not allowed in Playwright tests.Considered and rejected:
closeFirstPopupAlert(page)— targetsalert-icon-close, which the success toast never renders, so it is a no-op here.alert-barto detach — a cross-worker toast can arrive after that wait, so it is not sound; the hit-target retry covers both cases without adding a timeout.Blast radius: the other
addTeamHierarchycallers —TeamsDragAndDrop.spec.tsandTeams.spec.ts— get the same de-forced click. None relied onforce; the button is a normal enabled placeholder button in every case.Type of change
Checklist
Verified locally:
prettier,eslint, andtsc --noEmitclean onplaywright/utils/team.ts. The spec itself needs a live server, so it was not executed here.🤖 Generated with Claude Code
Greptile Summary
The PR centralizes opening the add-team modal in a retrying Playwright helper to prevent transient toast overlays from swallowing clicks.
Confidence Score: 5/5
The PR appears safe to merge.
No blocking failure remains.
Important Files Changed
Reviews (2): Last reviewed commit: "test(teams): retry the add-team open whe..." | Re-trigger Greptile