Skip to content

test(ui): make glossary listing tests resilient to active-glossary drift#28988

Open
siddhant1 wants to merge 3 commits into
mainfrom
pw-fix-glossary-active-glossary-drift
Open

test(ui): make glossary listing tests resilient to active-glossary drift#28988
siddhant1 wants to merge 3 commits into
mainfrom
pw-fix-glossary-active-glossary-drift

Conversation

@siddhant1

Copy link
Copy Markdown
Member

Problem

Several Glossary.spec.ts listing-page tests (e.g. "Approve and reject glossary term from Glossary Listing") are flaky. The glossary listing page intentionally falls back to the first glossary (glossaries[0]) when the URL's glossary isn't present in the freshly loaded sidebar page (GlossaryPage.component.tsx: setActiveGlossary(find(fqn) || glossaries[0])).

Under parallel test load the sidebar glossary list churns (other workers create/delete glossaries; infinite-scroll pagination re-sets the list), so the in-memory active glossary drifts to another worker's glossary even though the browser URL stays correct. The terms table then renders the wrong glossary's children, and the just-created term's row ([data-row-key="<fqn>"]) never appears — the assertion times out.

Trace evidence from the failing run: after the 2nd term POST (201) into glossary Zany, the table refetched directChildrenOf="…Quiet…" (a different glossary); the URL never left Zany. Pure in-memory state drift.

Note: the find(fqn) || glossaries[0] fallback is intended product behavior, so the fix is test-side.

Change (test-only)

A shared assertWithReloadRecovery helper that, on an assertion miss, reloads (which re-derives the active glossary from the URL FQN) and retries under toPass. Applied at the two drift-exposed points:

  • selectActiveGlossary — the universal re-pin chokepoint. The sidebar highlight is URL-driven, so a re-click is a no-op when drifted; it now verifies the header shows the requested glossary and reloads to re-pin if it drifted.
  • validateGlossaryTerm — the actual failure point. The term-creation loop drifts between terms without re-selecting, so the term-row check now self-heals via reload.

No app/product code touched.

Testing

  • yarn playwright:run --grep "Approve and reject glossary term from Glossary Listing"
  • yarn lint (playwright) — only pre-existing warnings
  • no new tsc errors in the changed file

Candidate for cherry-pick to 1.12.11 (same code path / same flake reported there).

🤖 Generated with Claude Code

The glossary listing page falls back to the first glossary (glossaries[0])
when the URL's glossary isn't present in the freshly loaded sidebar page.
Under parallel test load the sidebar list churns, so the in-memory active
glossary can drift to another worker's glossary even though the URL stays
correct (the sidebar highlight is URL-driven, so a re-click is a no-op),
leaving the terms table showing the wrong glossary. This flaked tests such
as "Approve and reject glossary term from Glossary Listing" at the term-row
visibility assertion.

Recover by reloading -- which re-derives the active glossary from the URL
FQN -- in selectActiveGlossary and validateGlossaryTerm.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@siddhant1 siddhant1 requested a review from a team as a code owner June 12, 2026 06:16
Copilot AI review requested due to automatic review settings June 12, 2026 06:16

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@github-actions

github-actions Bot commented Jun 12, 2026

Copy link
Copy Markdown
Contributor

❌ PR checklist incomplete

This PR cannot be merged until the following are addressed on its linked issue:

  • No GitHub issue is linked. Link an issue in the Development section of the PR (or add Fixes #12345 to the description). For a same-org cross-repo issue, add Fixes open-metadata/<repo>#123 to the description.

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 skip-pr-checks label.

@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 Jun 12, 2026
Comment thread openmetadata-ui/src/main/resources/ui/playwright/utils/glossary.ts
@github-actions

Copy link
Copy Markdown
Contributor

Jest test Coverage

UI tests summary

Lines Statements Branches Functions
Coverage: 62%
62.44% (67087/107430) 43.94% (37140/84513) 45.82% (11474/25038)

@github-actions

github-actions Bot commented Jun 12, 2026

Copy link
Copy Markdown
Contributor

🔴 Playwright Results — 1 failure(s), 17 flaky

✅ 4296 passed · ❌ 1 failed · 🟡 17 flaky · ⏭️ 88 skipped

Shard Passed Failed Flaky Skipped
🟡 Shard 1 299 0 2 4
🟡 Shard 2 811 0 1 9
🟡 Shard 3 810 0 2 8
🟡 Shard 4 847 0 3 12
🟡 Shard 5 732 0 2 47
🔴 Shard 6 797 1 7 8

Genuine Failures (failed on all attempts)

Pages/Glossary.spec.ts › Verify Glossary Deny Permission (shard 6)
Error: �[2mexpect(�[22m�[31mlocator�[39m�[2m).�[22mtoContainText�[2m(�[22m�[32mexpected�[39m�[2m)�[22m failed

Locator: getByTestId('entity-header-display-name')
Expected substring: �[32m"PW % 4091cdf8 Eager661186c8"�[39m
Timeout: 10000ms
Error: element(s) not found

Call log:
�[2m  - Expect "toContainText" with timeout 10000ms�[22m
�[2m  - waiting for getByTestId('entity-header-display-name')�[22m


Call Log:
- Test timeout of 60000ms exceeded
🟡 17 flaky test(s) (passed on retry)
  • Pages/Roles.spec.ts › Roles page should work properly (shard 1, 1 retry)
  • Pages/SearchSettings.spec.ts › Preview config reflects reverted n-gram weight after save (shard 1, 1 retry)
  • Features/Glossary/LargeGlossaryPerformance.spec.ts › should handle drag and drop for term reordering (shard 2, 1 retry)
  • Features/RTL.spec.ts › Verify Following widget functionality (shard 3, 1 retry)
  • Flow/ExploreAggregationCountsMatching.spec.ts › should verify left panel counts and tab search results for normal search (shard 3, 1 retry)
  • Pages/CustomProperties.spec.ts › Enum (shard 4, 1 retry)
  • Pages/CustomProperties.spec.ts › Timestamp (shard 4, 1 retry)
  • Pages/DataContractInheritance.spec.ts › Edit Inherited Contract - Creates new asset contract instead of modifying parent (shard 4, 1 retry)
  • Pages/ExplorePageRightPanel_KnowledgeCenter.spec.ts › Should remove user owner for knowledgeCenter (shard 5, 1 retry)
  • Pages/ExplorePageRightPanel.spec.ts › Should verify deleted user not visible in owner selection for dashboardDataModel (shard 5, 1 retry)
  • Pages/Glossary.spec.ts › Column dropdown drag-and-drop functionality for Glossary Terms table (shard 6, 1 retry)
  • Pages/Lineage/LineageFilters.spec.ts › Verify lineage service type filter selection (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/Lineage/PlatformLineage.spec.ts › Verify domain platform view (shard 6, 1 retry)
  • Pages/Login.spec.ts › Refresh should work (shard 6, 1 retry)
  • Pages/UserDetails.spec.ts › Create team with domain and verify visibility of inherited domain in user profile after team removal (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

Copilot AI review requested due to automatic review settings June 12, 2026 09:17

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@gitar-bot

gitar-bot Bot commented Jun 12, 2026

Copy link
Copy Markdown
Code Review ✅ Approved 1 resolved / 1 findings

Adds a assertWithReloadRecovery helper to resolve flaky glossary tests caused by in-memory active-glossary drift. Note that the recovery path can extend total assertion runtime up to 40 seconds during a reload.

✅ 1 resolved
Quality: Recovery path can extend assertion runtime up to ~40s

📄 openmetadata-ui/src/main/resources/ui/playwright/utils/glossary.ts:66-79 📄 openmetadata-ui/src/main/resources/ui/playwright/utils/glossary.ts:110-115 📄 openmetadata-ui/src/main/resources/ui/playwright/utils/glossary.ts:745-747
assertWithReloadRecovery first runs verify() (which itself carries a 10s expect timeout), and only on failure enters the toPass retry with a 30s timeout. In the worst case a single drifted assertion now costs ~10s + ~30s ≈ 40s before failing. Across the term-creation loop in validateGlossaryTerm and every selectActiveGlossary re-pin, this can noticeably inflate total spec runtime when drift is frequent, and could push individual steps toward the global test timeout.

This is acceptable for the resilience goal, but consider trimming the inner verify() timeout (e.g. to 3-5s) so the fast path fails quickly and hands off to the reload-based recovery sooner, keeping the worst-case bounded. Not blocking — the helper is otherwise correct and genuine failures still surface after the budget expires.

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