Skip to content

playwright: remove duplicate team deletion test from TeamsDragAndDrop#27668

Merged
harsh-vador merged 2 commits intomainfrom
remove-team-deletion-teams-drag-drop
Apr 23, 2026
Merged

playwright: remove duplicate team deletion test from TeamsDragAndDrop#27668
harsh-vador merged 2 commits intomainfrom
remove-team-deletion-teams-drag-drop

Conversation

@harsh-vador
Copy link
Copy Markdown
Contributor

Describe your changes:

PR description

  • Removes the standalone Delete Teams test from TeamsDragAndDrop.spec.ts since Teams.spec.ts already covers team deletion.
  • Adds an afterAll API cleanup in to hard-delete the teams created by this spec, ensuring cleanup still runs even if earlier steps fail.
Screen.Recording.2026-04-23.at.5.29.01.PM.mov

Type of change:

  • Bug fix
  • Improvement
  • New feature
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation

Checklist:

  • I have read the CONTRIBUTING document.
  • My PR title is Fixes <issue-number>: <short explanation>
  • I have commented on my code, particularly in hard-to-understand areas.
  • For JSON Schema changes: I updated the migration scripts or explained why it is not needed.

@harsh-vador harsh-vador self-assigned this Apr 23, 2026
@harsh-vador harsh-vador requested a review from a team as a code owner April 23, 2026 12:01
@harsh-vador harsh-vador added safe to test Add this label to run secure Github workflows on PRs To release Will cherry-pick this PR into the release branch labels Apr 23, 2026
Comment on lines +93 to +96
test.afterAll('Cleanup', async ({ browser }) => {
const { afterAction } = await createNewPage(browser);
await afterAction();
});
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚨 Bug: afterAll cleanup does not delete any teams

The old Delete Teams test explicitly called hardDeleteTeam for each of the four created teams (teamNameBusiness, teamNameDivision, teamNameDepartment, teamNameGroup). The replacement afterAll block only calls createNewPage and afterAction(), which simply disposes the API context and closes the page — it performs no team deletion at all.

This means every run of this spec will leak four teams in the environment, which can cause name collisions on repeated runs (since team names include a uuid(), this won't fail immediately, but it pollutes the instance) and contradicts the PR description's claim of "hard-delete the teams created by this spec."

The afterAll should use the apiContext from createNewPage to issue hard-delete API calls for all four teams before calling afterAction().

Suggested fix:

test.afterAll('Cleanup', async ({ browser }) => {
  const { apiContext, afterAction } = await createNewPage(browser);
  for (const teamName of [
    teamNameBusiness,
    teamNameDivision,
    teamNameDepartment,
    teamNameGroup,
  ]) {
    await apiContext.delete(
      `/api/v1/teams/name/${teamName}?hardDelete=true&recursive=true`
    );
  }
  await afterAction();
});

Was this helpful? React with 👍 / 👎 | Reply gitar fix to apply this suggestion

@gitar-bot
Copy link
Copy Markdown

gitar-bot Bot commented Apr 23, 2026

Code Review 🚫 Blocked 0 resolved / 1 findings

Removes duplicate team deletion test, but the afterAll cleanup hook fails to delete the teams created in the suite, leading to potential data leakage in the test environment.

🚨 Bug: afterAll cleanup does not delete any teams

📄 openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/TeamsDragAndDrop.spec.ts:93-96

The old Delete Teams test explicitly called hardDeleteTeam for each of the four created teams (teamNameBusiness, teamNameDivision, teamNameDepartment, teamNameGroup). The replacement afterAll block only calls createNewPage and afterAction(), which simply disposes the API context and closes the page — it performs no team deletion at all.

This means every run of this spec will leak four teams in the environment, which can cause name collisions on repeated runs (since team names include a uuid(), this won't fail immediately, but it pollutes the instance) and contradicts the PR description's claim of "hard-delete the teams created by this spec."

The afterAll should use the apiContext from createNewPage to issue hard-delete API calls for all four teams before calling afterAction().

Suggested fix
test.afterAll('Cleanup', async ({ browser }) => {
  const { apiContext, afterAction } = await createNewPage(browser);
  for (const teamName of [
    teamNameBusiness,
    teamNameDivision,
    teamNameDepartment,
    teamNameGroup,
  ]) {
    await apiContext.delete(
      `/api/v1/teams/name/${teamName}?hardDelete=true&recursive=true`
    );
  }
  await afterAction();
});
🤖 Prompt for agents
Code Review: Removes duplicate team deletion test, but the `afterAll` cleanup hook fails to delete the teams created in the suite, leading to potential data leakage in the test environment.

1. 🚨 Bug: afterAll cleanup does not delete any teams
   Files: openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/TeamsDragAndDrop.spec.ts:93-96

   The old `Delete Teams` test explicitly called `hardDeleteTeam` for each of the four created teams (`teamNameBusiness`, `teamNameDivision`, `teamNameDepartment`, `teamNameGroup`). The replacement `afterAll` block only calls `createNewPage` and `afterAction()`, which simply disposes the API context and closes the page — it performs **no team deletion at all**.
   
   This means every run of this spec will leak four teams in the environment, which can cause name collisions on repeated runs (since team names include a `uuid()`, this won't fail immediately, but it pollutes the instance) and contradicts the PR description's claim of "hard-delete the teams created by this spec."
   
   The `afterAll` should use the `apiContext` from `createNewPage` to issue hard-delete API calls for all four teams before calling `afterAction()`.

   Suggested fix:
   test.afterAll('Cleanup', async ({ browser }) => {
     const { apiContext, afterAction } = await createNewPage(browser);
     for (const teamName of [
       teamNameBusiness,
       teamNameDivision,
       teamNameDepartment,
       teamNameGroup,
     ]) {
       await apiContext.delete(
         `/api/v1/teams/name/${teamName}?hardDelete=true&recursive=true`
       );
     }
     await afterAction();
   });

Options

Display: compact → Showing less information.

Comment with these commands to change:

Compact
gitar display:verbose         

Was this helpful? React with 👍 / 👎 | Gitar

@github-actions
Copy link
Copy Markdown
Contributor

Jest test Coverage

UI tests summary

Lines Statements Branches Functions
Coverage: 61%
62% (60413/97427) 42.01% (31680/75394) 44.99% (9509/21133)

@sonarqubecloud
Copy link
Copy Markdown

@harsh-vador harsh-vador enabled auto-merge (squash) April 23, 2026 13:27
@harsh-vador harsh-vador merged commit 331be60 into main Apr 23, 2026
49 checks passed
@harsh-vador harsh-vador deleted the remove-team-deletion-teams-drag-drop branch April 23, 2026 14:11
@github-actions
Copy link
Copy Markdown
Contributor

🟡 Playwright Results — all passed (19 flaky)

✅ 3692 passed · ❌ 0 failed · 🟡 19 flaky · ⏭️ 89 skipped

Shard Passed Failed Flaky Skipped
🟡 Shard 1 479 0 1 4
🟡 Shard 2 655 0 2 7
🟡 Shard 3 662 0 2 1
🟡 Shard 4 641 0 7 27
🟡 Shard 5 610 0 1 42
🟡 Shard 6 645 0 6 8
🟡 19 flaky test(s) (passed on retry)
  • Pages/AuditLogs.spec.ts › should apply both User and EntityType filters simultaneously (shard 1, 1 retry)
  • Features/BulkEditEntity.spec.ts › Glossary (shard 2, 1 retry)
  • Features/ColumnBulkOperations.spec.ts › should restore filters from URL on page load (shard 2, 1 retry)
  • Features/RTL.spec.ts › Verify Following widget functionality (shard 3, 1 retry)
  • Flow/PersonaFlow.spec.ts › Set default persona for team should work properly (shard 3, 1 retry)
  • Pages/Customproperties-part2.spec.ts › entityReferenceList shows item count, scrollable list, no expand toggle (shard 4, 1 retry)
  • Pages/DataContractInheritance.spec.ts › Edit Asset Contract - Add SLA when inheriting SLA from Data Product (PATCH should use /add not /replace) (shard 4, 1 retry)
  • Pages/DataContracts.spec.ts › Create Data Contract and validate for Api Collection (shard 4, 1 retry)
  • Pages/DataContracts.spec.ts › Create Data Contract and validate for File (shard 4, 1 retry)
  • Pages/DataContracts.spec.ts › Create Data Contract and validate for Worksheet (shard 4, 1 retry)
  • Pages/DataContractsSemanticRules.spec.ts › Validate DataProduct Rule Any_In (shard 4, 1 retry)
  • Pages/Domains.spec.ts › User with noDomain() rule cannot access tables without domain (shard 4, 1 retry)
  • Pages/EntityDataConsumer.spec.ts › Tier Add, Update and Remove (shard 5, 1 retry)
  • Pages/Lineage/DataAssetLineage.spec.ts › verify create lineage for entity - Api Endpoint (shard 6, 1 retry)
  • Pages/Lineage/DataAssetLineage.spec.ts › Column lineage for mlModel -> dashboard (shard 6, 1 retry)
  • Pages/Lineage/LineageFilters.spec.ts › Verify lineage schema filter selection (shard 6, 1 retry)
  • Pages/Lineage/LineageRightPanel.spec.ts › Verify custom properties tab IS visible for supported type: searchIndex (shard 6, 1 retry)
  • Pages/Login.spec.ts › Refresh should work (shard 6, 1 retry)
  • Pages/Users.spec.ts › Permissions for table details page for Data Consumer (shard 6, 1 retry)

📦 Download artifacts

How to debug locally
# Download playwright-test-results-<shard> artifact and unzip
npx playwright show-trace path/to/trace.zip    # view trace

@github-actions
Copy link
Copy Markdown
Contributor

Changes have been cherry-picked to the 1.12.7 branch.

github-actions Bot pushed a commit that referenced this pull request Apr 23, 2026
…#27668)

* playwright: remove duplicate team deletion test from TeamsDragAndDrop

* remove unwanted

(cherry picked from commit 331be60)
harsh-vador added a commit that referenced this pull request Apr 24, 2026
…#27668)

* playwright: remove duplicate team deletion test from TeamsDragAndDrop

* remove unwanted

(cherry picked from commit 331be60)
harsh-vador added a commit that referenced this pull request Apr 24, 2026
…#27668)

* playwright: remove duplicate team deletion test from TeamsDragAndDrop

* remove unwanted

(cherry picked from commit 331be60)
jatinmasaram pushed a commit to jatinmasaram/OpenMetadata that referenced this pull request May 2, 2026
…open-metadata#27668)

* playwright: remove duplicate team deletion test from TeamsDragAndDrop

* remove unwanted
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

safe to test Add this label to run secure Github workflows on PRs To release Will cherry-pick this PR into the release branch

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants