Skip to content

Commit 90c99bf

Browse files
harsh-vadorclaude
andauthored
test(teams): fix TeamsDragAndDrop on 1.13 (backport #31932) (#31933)
* 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>
1 parent 965c031 commit 90c99bf

3 files changed

Lines changed: 211 additions & 45 deletions

File tree

openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/TeamsDragAndDrop.spec.ts

Lines changed: 25 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -12,8 +12,8 @@
1212
*/
1313
import { expect, test } from '@playwright/test';
1414
import { PLAYWRIGHT_BASIC_TEST_TAG_OBJ } from '../../constant/config';
15-
import { GlobalSettingOptions } from '../../constant/settings';
1615
import {
16+
createNewPage,
1717
redirectToHomePage,
1818
toastNotification,
1919
uuid,
@@ -23,9 +23,11 @@ import {
2323
dragAndDropElement,
2424
openDragDropDropdown,
2525
} from '../../utils/dragDrop';
26-
import { waitForAllLoadersToDisappear } from '../../utils/entity';
27-
import { settingClick } from '../../utils/sidebar';
28-
import { addTeamHierarchy } from '../../utils/team';
26+
import {
27+
addTeamHierarchy,
28+
hardDeleteTeamsByName,
29+
visitTeamsPage,
30+
} from '../../utils/team';
2931

3032
// use the admin user to login
3133
test.use({ storageState: 'playwright/.auth/admin.json' });
@@ -89,28 +91,32 @@ test.describe(
8991
'Teams drag and drop should work properly',
9092
PLAYWRIGHT_BASIC_TEST_TAG_OBJ,
9193
() => {
94+
// Every test re-enters Settings > Teams, and that landing waits on the
95+
// hierarchy table's asset-count aggregation, which grows with the catalog.
96+
// On a long-lived deployment the hook alone can outlast the 60s default.
97+
test.slow(true);
98+
9299
test.beforeEach(async ({ page }) => {
93100
await redirectToHomePage(page);
101+
await visitTeamsPage(page);
102+
});
94103

95-
const getOrganizationResponse = page.waitForResponse(
96-
(response) =>
97-
response.url().includes('/api/v1/teams/name/') &&
98-
response.status() === 200
99-
);
100-
const permissionResponse = page.waitForResponse(
101-
(response) =>
102-
response.url().includes('/api/v1/permissions/team/name/') &&
103-
response.status() === 200
104-
);
104+
test.afterAll(async ({ browser }) => {
105+
const { apiContext, afterAction } = await createNewPage(browser);
105106

106-
await settingClick(page, GlobalSettingOptions.TEAMS);
107-
await permissionResponse;
108-
await getOrganizationResponse;
109-
await waitForAllLoadersToDisappear(page);
107+
try {
108+
await hardDeleteTeamsByName(apiContext, [
109+
teamNameBusiness,
110+
teamNameDivision,
111+
teamNameDepartment,
112+
teamNameGroup,
113+
]);
114+
} finally {
115+
await afterAction();
116+
}
110117
});
111118

112119
test('Add teams in hierarchy', async ({ page }) => {
113-
test.slow();
114120
for (const teamDetails of DRAG_AND_DROP_TEAM_DETAILS) {
115121
await addTeamHierarchy(page, teamDetails);
116122

@@ -158,7 +164,6 @@ test.describe(
158164
test(`Should drag and drop on ${TEAM_TYPE_BY_NAME[droppableTeamName]} team type`, async ({
159165
page,
160166
}) => {
161-
test.slow();
162167
// Nested team will be shown once anything is moved under it
163168
if (index !== 0) {
164169
await openDragDropDropdown(page, teams[index - 1]);
@@ -182,7 +187,6 @@ test.describe(
182187
}
183188

184189
test(`Should drag and drop team on table level`, async ({ page }) => {
185-
test.slow();
186190
// Open department team dropdown as it is moved under it from last test
187191
await openDragDropDropdown(page, teamNameDepartment);
188192

openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/TeamsHierarchy.spec.ts

Lines changed: 2 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import {
2020
addTeamHierarchy,
2121
getNewTeamDetails,
2222
searchTeam,
23+
visitTeamsPage,
2324
} from '../../utils/team';
2425

2526
// use the admin user to login
@@ -46,17 +47,7 @@ test.describe(
4647

4748
test.beforeEach(async ({ page }) => {
4849
await redirectToHomePage(page);
49-
50-
const getOrganizationResponse = page.waitForResponse(
51-
'/api/v1/teams/name/*'
52-
);
53-
const permissionResponse = page.waitForResponse(
54-
'/api/v1/permissions/team/name/*'
55-
);
56-
57-
await settingClick(page, GlobalSettingOptions.TEAMS);
58-
await permissionResponse;
59-
await getOrganizationResponse;
50+
await visitTeamsPage(page);
6051
});
6152

6253
test('Add teams in hierarchy', async ({ page }) => {
@@ -111,8 +102,6 @@ test.describe(
111102
});
112103

113104
test('Delete Parent Team', async ({ page }) => {
114-
await settingClick(page, GlobalSettingOptions.TEAMS);
115-
116105
await page.getByRole('link', { name: businessTeamName }).click();
117106

118107
await page.click('[data-testid="manage-button"]');

openmetadata-ui/src/main/resources/ui/playwright/utils/team.ts

Lines changed: 184 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,162 @@ import { settingClick } from './sidebar';
3333

3434
const TEAM_TYPES = ['Department', 'Division', 'Group'];
3535

36+
const ADD_TEAM_MODAL = '[role="dialog"].ant-modal';
37+
// A success toast self-dismisses after 3.5s; give it a beat past that.
38+
const TOAST_DISMISS_TIMEOUT = 6_000;
39+
const MODAL_OPEN_TIMEOUT = 10_000;
40+
const MODAL_RETRY_TIMEOUT = 60_000;
41+
42+
type AddTeamTrigger = 'add-team' | 'add-placeholder-button';
43+
44+
/**
45+
* Click an add-team trigger and return the modal it opens.
46+
*
47+
* The backend fans async-delete/job notifications out to every socket of the
48+
* logged-in user, so a parallel worker's toast can drop over the button in the
49+
* window between Playwright's hit-target check and the dispatched click — the
50+
* toast swallows the click and the modal never opens. Success toasts carry no
51+
* close button, so let them expire and click again; clicking with `force` only
52+
* dispatches INTO the toast.
53+
*/
54+
export const openAddTeamModal = async (
55+
page: Page,
56+
trigger: AddTeamTrigger = 'add-team'
57+
) => {
58+
const addButton = page.getByTestId(trigger);
59+
const addTeamModal = page.locator(ADD_TEAM_MODAL).last();
60+
61+
await expect(async () => {
62+
await page
63+
.getByTestId('alert-bar')
64+
.first()
65+
.waitFor({ state: 'detached', timeout: TOAST_DISMISS_TIMEOUT })
66+
.catch(() => undefined);
67+
68+
await expect(addButton).toBeEnabled();
69+
await addButton.click();
70+
71+
await expect(addTeamModal).toBeVisible({ timeout: MODAL_OPEN_TIMEOUT });
72+
}).toPass({ timeout: MODAL_RETRY_TIMEOUT, intervals: [1_000] });
73+
74+
return addTeamModal;
75+
};
76+
77+
/**
78+
* Land on Settings > Teams with the hierarchy table settled.
79+
*
80+
* The table spins on its own child-teams fetch plus the per-team asset-count
81+
* aggregation, so navigation alone is not enough — a caller that acts right
82+
* after the click drags rows that are still being repainted. Wait on the two
83+
* calls that gate the first paint, then on the table itself.
84+
*/
85+
export const visitTeamsPage = async (page: Page) => {
86+
const organizationResponse = page.waitForResponse(
87+
(response) =>
88+
response.url().includes('/api/v1/teams/name/') && response.ok()
89+
);
90+
const permissionResponse = page.waitForResponse(
91+
(response) =>
92+
response.url().includes('/api/v1/permissions/team/name/') && response.ok()
93+
);
94+
95+
await settingClick(page, GlobalSettingOptions.TEAMS);
96+
await Promise.all([permissionResponse, organizationResponse]);
97+
98+
await expect(page.getByTestId('team-hierarchy-table')).toBeVisible();
99+
await waitForAllLoadersToDisappear(page);
100+
};
101+
102+
interface TeamCleanupFailure {
103+
teamName: string;
104+
reason: string;
105+
}
106+
107+
/**
108+
* Hard-delete one team created through the UI, children included.
109+
*
110+
* Reports a failure rather than throwing so a caller cleaning up several teams
111+
* still attempts the rest — a throw here would leave the remaining teams behind
112+
* and recreate the accumulation this cleanup exists to prevent.
113+
*
114+
* Specs that build teams through the UI have no entity handle to call
115+
* `TeamClass.delete` on, so the id is resolved by name first. Delete-by-name is
116+
* not an option: `TeamResource` pins that route to `recursive=false`, and these
117+
* teams are nested by the time cleanup runs.
118+
*
119+
* 404 on the lookup is the one tolerated outcome — the spec may have deleted
120+
* the team as part of what it asserts, and a recursive delete of its parent
121+
* takes its children with it.
122+
*/
123+
const hardDeleteTeamByName = async (
124+
apiContext: APIRequestContext,
125+
teamName: string
126+
): Promise<TeamCleanupFailure | undefined> => {
127+
let failure: TeamCleanupFailure | undefined;
128+
129+
try {
130+
const teamResponse = await apiContext.get(
131+
`/api/v1/teams/name/${encodeURIComponent(teamName)}`
132+
);
133+
134+
if (!teamResponse.ok()) {
135+
if (teamResponse.status() !== 404) {
136+
failure = {
137+
teamName,
138+
reason: `lookup returned ${teamResponse.status()} ${await teamResponse.text()}`,
139+
};
140+
}
141+
} else {
142+
const { id } = await teamResponse.json();
143+
const deleteResponse = await apiContext.delete(
144+
`/api/v1/teams/${id}?hardDelete=true&recursive=true`
145+
);
146+
147+
if (!deleteResponse.ok()) {
148+
failure = {
149+
teamName,
150+
reason: `delete returned ${deleteResponse.status()} ${await deleteResponse.text()}`,
151+
};
152+
}
153+
}
154+
} catch (error) {
155+
failure = { teamName, reason: (error as Error).message };
156+
}
157+
158+
return failure;
159+
};
160+
161+
/**
162+
* Hard-delete teams created through the UI, children included.
163+
*
164+
* Deletes are sequential: a recursive delete takes a team's children with it,
165+
* so issuing them in parallel would race the ones already removed. Every name
166+
* is attempted before anything is asserted, and the assertion then names every
167+
* team that survived — cleanup that fails quietly is what lets teams pile up on
168+
* a long-lived deployment in the first place.
169+
*/
170+
export const hardDeleteTeamsByName = async (
171+
apiContext: APIRequestContext,
172+
teamNames: string[]
173+
) => {
174+
const failures: TeamCleanupFailure[] = [];
175+
176+
for (const teamName of teamNames) {
177+
const failure = await hardDeleteTeamByName(apiContext, teamName);
178+
179+
if (failure) {
180+
failures.push(failure);
181+
}
182+
}
183+
184+
expect(
185+
failures,
186+
`Failed to clean up teams: ${failures
187+
.map(({ teamName, reason }) => `"${teamName}" (${reason})`)
188+
.join(', ')}`
189+
).toEqual([]);
190+
};
191+
36192
interface SearchTeamOptions {
37193
expectEmptyResults?: boolean;
38194
expectNotFound?: boolean;
@@ -252,16 +408,12 @@ export const addTeamHierarchy = async (
252408
index?: number,
253409
isHierarchy = false
254410
) => {
255-
const getTeamsResponse = page.waitForResponse('/api/v1/teams*');
256-
257-
// Fetching the add button and clicking on it
258-
if (index && index > 0) {
259-
await page.click('[data-testid="add-placeholder-button"]');
260-
} else {
261-
await page.click('[data-testid="add-team"]');
262-
}
411+
const addTeamModal = await openAddTeamModal(
412+
page,
413+
index && index > 0 ? 'add-placeholder-button' : 'add-team'
414+
);
263415

264-
await getTeamsResponse;
416+
await expect(page.locator('[data-testid="name"]')).toBeVisible();
265417

266418
// Entering team details
267419
await validateFormNameFieldInput({
@@ -285,9 +437,30 @@ export const addTeamHierarchy = async (
285437
await page.locator(descriptionBox).fill(teamDetails.description);
286438

287439
// Saving the created team
288-
const saveTeamResponse = page.waitForResponse('/api/v1/teams');
440+
const saveTeamResponse = page.waitForResponse(
441+
(response) =>
442+
response.url().includes('/api/v1/teams') &&
443+
response.request().method() === 'POST' &&
444+
response.ok()
445+
);
446+
const teamsListResponse = page.waitForResponse(
447+
(response) =>
448+
response.url().includes('/api/v1/teams?parentTeam=') &&
449+
response.url().includes('fields=') &&
450+
response.request().method() === 'GET'
451+
);
289452
await page.click('[form="add-team-form"]');
290453
await saveTeamResponse;
454+
const teamsListResponseResult = await teamsListResponse;
455+
456+
expect(teamsListResponseResult.status()).toBe(200);
457+
await expect(addTeamModal).toBeHidden({ timeout: 60000 });
458+
await waitForAllLoadersToDisappear(page);
459+
await expect(
460+
page.locator(`[data-row-key="${teamDetails.name}"]`)
461+
).toBeVisible({
462+
timeout: 60000,
463+
});
291464
};
292465

293466
export const removeOrganizationPolicyAndRole = async (
@@ -592,7 +765,7 @@ export const executionOnOwnerTeam = async (
592765

593766
await addEmailTeam(page, data.email);
594767

595-
await page.getByTestId('add-placeholder-button').click();
768+
await openAddTeamModal(page, 'add-placeholder-button');
596769

597770
const newTeamData = await createTeam(page);
598771

0 commit comments

Comments
 (0)