Skip to content

test(glossary): fix flaky Async Delete multiple deletes with mixed results#27451

Open
siddhant1 wants to merge 5 commits intomainfrom
fix/glossary-async-delete-mixed-results-flaky
Open

test(glossary): fix flaky Async Delete multiple deletes with mixed results#27451
siddhant1 wants to merge 5 commits intomainfrom
fix/glossary-async-delete-mixed-results-flaky

Conversation

@siddhant1
Copy link
Copy Markdown
Member

Summary

  • Stabilize Glossary tests › Async Delete - multiple deletes with mixed results (playwright/e2e/Pages/Glossary.spec.ts:1454), which flaked by timing out on the final expectGlossaryVisible for glossary C.
  • Re-anchor on /glossary after each delete of the currently-selected glossary so the sidebar is guaranteed to be rendered before the next selection / assertion.

Root cause

Observed in CI run 24541918429/1128/1: both attempts failed at asyncDelete.ts:161 waiting for menuitem C to become visible. The error-context page snapshot showed the app on /my-data (home), not /glossary, at the moment of failure — so no glossary menuitems were rendered at all.

Sequence that exposed it:

  1. Select A → /glossary/A, optimistic delete → URL invalid, UI redirects.
  2. Select B → /glossary/B, optimistic delete → URL invalid again.
  3. emitDeleteFailure(B) + refetch restores B on the server side, but the page had already landed on /my-data (because both A's real async completion and B's optimistic removal raced the UI's "active glossary was deleted" redirect).
  4. expectGlossaryVisible(B) caught a transient re-render and passed; by the time expectGlossaryVisible(C) ran the sidebar had already collapsed back to the home layout → timeout.

The single-glossary recovery test (line 1370) avoids this because it emits the failure before any other delete kicks off; the nested all succeed test (line 1415) avoids it because it doesn't hit the failure/refetch path.

Fix

After each delete of the active glossary, call sidebarClick(page, SidebarItem.GLOSSARY) to deterministically land back on the glossary list page before the next step. This removes the race without changing what the test is actually exercising (optimistic delete + mixed success/failure recovery).

Test plan

  • Run locally (needs full stack up): npx playwright test Glossary.spec.ts -g "Async Delete - multiple deletes with mixed results" --repeat-each=5 --workers=1
  • Confirm the three sibling Async Delete tests (single delete success, WebSocket failure triggers recovery, multiple deletes all succeed) still pass.
  • Re-run on CI at least twice to check for flake.

Follow-up (separate issue)

The underlying product behavior — deleting the currently-active glossary when other glossaries exist can land the user on /my-data instead of another glossary in the list — looks like a UX bug worth tracking. Out of scope for this PR.

🤖 Generated with Claude Code

… results"

Re-anchor on the glossary page after deleting the currently-selected
glossary. Optimistically removing the active glossary navigates the page
away from /glossary before the sidebar re-renders the remaining items,
causing the final visibility checks to miss the untouched glossary C.
@siddhant1 siddhant1 requested a review from a team as a code owner April 17, 2026 05:30
Copilot AI review requested due to automatic review settings April 17, 2026 05:30
@github-actions
Copy link
Copy Markdown
Contributor

Hi there 👋 Thanks for your contribution!

The OpenMetadata team will review the PR shortly! Once it has been labeled as safe to test, the CI workflows
will start executing and we'll be able to make sure everything is working as expected.

Let us know if you need any help!

@siddhant1 siddhant1 added UI UI specific issues 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 17, 2026
@siddhant1 siddhant1 enabled auto-merge (squash) April 17, 2026 05:33
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Stabilizes the flaky Playwright test “Glossary tests › Async Delete - multiple deletes with mixed results” by ensuring the UI is re-anchored on the Glossary page between sequential deletes to avoid sidebar/menu race conditions.

Changes:

  • Re-navigate to /glossary after deleting the currently-selected glossary to ensure the glossary sidebar is rendered before subsequent assertions.
  • Adds an explicit “A is not visible” assertion after returning to the glossary page to confirm the delete took effect in the menu.

anuj-kumary
anuj-kumary previously approved these changes Apr 17, 2026
…ssary.spec.ts

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings April 17, 2026 05:35
anuj-kumary
anuj-kumary previously approved these changes Apr 17, 2026
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Apr 17, 2026

Jest test Coverage

UI tests summary

Lines Statements Branches Functions
Coverage: 63%
63.73% (59642/93585) 43.65% (31392/71908) 46.73% (9433/20182)

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Apr 17, 2026

🟡 Playwright Results — all passed (24 flaky)

✅ 3661 passed · ❌ 0 failed · 🟡 24 flaky · ⏭️ 89 skipped

Shard Passed Failed Flaky Skipped
🟡 Shard 1 474 0 6 4
🟡 Shard 2 647 0 4 7
🟡 Shard 3 658 0 2 1
🟡 Shard 4 631 0 3 27
🟡 Shard 5 610 0 1 42
🟡 Shard 6 641 0 8 8
🟡 24 flaky test(s) (passed on retry)
  • Features/DataAssetRulesDisabled.spec.ts › Verify the ApiEndpoint entity item action after rules disabled (shard 1, 1 retry)
  • Features/DataAssetRulesDisabled.spec.ts › Verify the Chart entity item action after rules disabled (shard 1, 2 retries)
  • Features/DataAssetRulesDisabled.spec.ts › Verify the Database entity item action after rules disabled (shard 1, 1 retry)
  • Features/CustomizeDetailPage.spec.ts › Stored Procedure - customization should work (shard 1, 1 retry)
  • Features/MetricCustomUnitFlow.spec.ts › Should create metric and test unit of measurement updates (shard 1, 1 retry)
  • Pages/UserCreationWithPersona.spec.ts › Create user with persona and verify on profile (shard 1, 1 retry)
  • Features/BulkEditEntity.spec.ts › Glossary (shard 2, 1 retry)
  • Features/ChangeSummaryBadge.spec.ts › Automated badge should appear on entity description with Automated source (shard 2, 1 retry)
  • Features/DataQuality/TestCaseImportExportE2eFlow.spec.ts › Admin: Complete export-import-validate flow (shard 2, 1 retry)
  • Features/DataQuality/TestCaseResultPermissions.spec.ts › User with only VIEW cannot PATCH results (shard 2, 1 retry)
  • Features/RTL.spec.ts › Verify Following widget functionality (shard 3, 1 retry)
  • Flow/AddRoleAndAssignToUser.spec.ts › Verify assigned role to new user (shard 3, 1 retry)
  • Pages/Customproperties-part2.spec.ts › entityReferenceList shows item count, scrollable list, no expand toggle (shard 4, 1 retry)
  • Pages/DataContracts.spec.ts › Create Data Contract and validate for Table (shard 4, 1 retry)
  • Pages/DataProductAndSubdomains.spec.ts › Add expert to data product via UI (shard 4, 2 retries)
  • Pages/Glossary.spec.ts › Add and Remove Assets (shard 5, 2 retries)
  • Features/AutoPilot.spec.ts › Create Service and check the AutoPilot status (shard 6, 1 retry)
  • Pages/InputOutputPorts.spec.ts › Output ports section collapse/expand (shard 6, 1 retry)
  • Pages/Lineage/DataAssetLineage.spec.ts › verify create lineage for entity - Worksheet (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/ODCSImportExport.spec.ts › Multi-object ODCS contract - object selector shows all schema objects (shard 6, 1 retry)
  • Pages/Users.spec.ts › Permissions for table details page for Data Consumer (shard 6, 1 retry)
  • VersionPages/EntityVersionPages.spec.ts › Directory (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

waitForGlossaryListRefetch expects GET /api/v1/glossaries with status 200.
Nothing in the optimistic-success path produces that: handleGlossaryDelete
only filters state and navigates, the async DELETE itself returns 202
Accepted, and the mocked WebSocket prevents any backend-driven refetch.
The listener sat until the 180s test timeout on shard 6 of PR 27451.

Keep the sidebarClick re-anchor from the original fix — it already lands
the page back on /glossary before the next selection.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings April 17, 2026 08:41
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.

@gitar-bot
Copy link
Copy Markdown

gitar-bot bot commented Apr 17, 2026

Code Review ✅ Approved

Stabilizes the glossary delete test suite by adjusting expectations for mixed asynchronous results. No issues found.

Options

Display: compact → Showing less information.

Comment with these commands to change:

Compact
gitar display:verbose         

Was this helpful? React with 👍 / 👎 | Gitar

@sonarqubecloud
Copy link
Copy Markdown

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 UI UI specific issues

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants