test(teams): fix TeamsDragAndDrop on 1.13 (backport #31932) - #31933
Conversation
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>
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>
❌ 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! |
❌ UI Checkstyle Failed❌ Core Components - I18n SyncCore-components ❌ Antd + Less Deprecation GuardA new Affected filesat Function._resolveFilename (node:internal/modules/cjs/loader:1401:15) Fix locally (fast - only checks files changed in this branch): make ui-checkstyle-changed |
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>
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>
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>
Code Review ✅ ApprovedBackports team hierarchy hardening and drag-and-drop test fixes to stabilize the 1.13 branch. 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 |
Backport of #31932 to 1.13, plus the earlier
utils/team.tshardening 1.13 never received.Fixes
TeamsDragAndDrop.spec.tson AUT EKS/RDS MySQL 1.11.13 ➡ 1.13.Why this isn't a plain cherry-pick
#31932 cherry-picks cleanly, but on its own it does not make 1.13 green.
git log --oneline origin/main ^origin/1.13 -- playwright/utils/team.tsshows three commits 1.13 is missing that main and 2.0 both have:51ecf4502f(team.tshunk only)b481bcc629af609a62371.13's
addTeamHierarchyis still the original:No method or status filter, no wait for the
?parentTeam=…&fields=…GET that repaints the table, no modal-hidden wait, no row-visible wait — so the helper returns while the table is still refetching. That is the retry-#1/#2 failure atTeamsDragAndDrop.spec.ts:119, wheretoContainText(description)runs against a row that hasn't painted.#25894 is a 500-file redesign, so its
team.tshunk is hand-backported rather than cherry-picked. #30334 and #31734 fold in on top.Commits
test(teams): backport addTeamHierarchy hardening to 1.13— the three commits above,team.tsonly. The delete-modal changes from Replaced old entity delete modal with untitles delete modal #29760 / Replaced entity delete modal with hard delete modal #29851 are deliberately not included; 1.13 still ships the radio-based delete modal.test(teams): stop the teams landing from eating the test budget— clean cherry-pick of test(teams): stop the teams landing from eating the test budget #31932. RCA in that PR.Verification
tsc --noEmit -p playwright/tsconfig.json,eslint, andprettier --checkare clean on the touched files. The spec itself needs a live deployment and was not run locally.🤖 Generated with Claude Code
Greptile Summary
The backport hardens team hierarchy navigation and creation synchronization while adding explicit cleanup for teams created by the drag-and-drop suite.
Confidence Score: 5/5
The PR appears safe to merge.
No blocking failure remains; the cleanup helper now surfaces both unsuccessful DELETE responses and non-404 lookup failures through its aggregated assertion.
Important Files Changed
Reviews (4): Last reviewed commit: "test(teams): attempt every cleanup delet..." | Re-trigger Greptile