test(ui): harden glossary import/export permission tests#28688
Open
siddhant1 wants to merge 1 commit into
Open
test(ui): harden glossary import/export permission tests#28688siddhant1 wants to merge 1 commit into
siddhant1 wants to merge 1 commit into
Conversation
Isolate the shared mock state with a beforeEach reset so tests no longer depend on source ordering, and make the two ChangeParentHierarchy tests set their own glossary-term context instead of relying on leaked state. Add regression coverage for the entity-level All operand of importExportPermissions and for the case where only a non-edit permission (Delete/ViewAll) is granted. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Contributor
|
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! |
|
Code Review ✅ ApprovedHardens glossary import/export permission tests by isolating mock state and adding coverage for the entity-level All operand. No issues found. OptionsDisplay: compact → Showing less information. Comment with these commands to change:
Was this helpful? React with 👍 / 👎 | Gitar |
Contributor
There was a problem hiding this comment.
Pull request overview
Strengthens Jest coverage and isolation for GlossaryHeader’s import/export visibility logic, especially around conditional policies and entity-level permission fallbacks.
Changes:
- Adds a
beforeEachto reinitialize shared permission/context mocks between tests. - Makes
ChangeParentHierarchytests explicitly set their own entity context. - Adds coverage for entity-level
permissions[Operation.All]and ensures non-edit permissions don’t surface Import/Export.
siddhant1
added
UI
anuj-kumary
approved these changes
Jun 4, 2026
Contributor
Jest test CoverageUI tests summary
|
|
Contributor
🔴 Playwright Results — 2 failure(s), 16 flaky✅ 4255 passed · ❌ 2 failed · 🟡 16 flaky · ⏭️ 88 skipped
Genuine Failures (failed on all attempts)❌
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Follow-up to #27488 (which restored the glossary Import/Export buttons when
EditAllis granted via a conditional policy such asisOwner()). This strengthens the test coverage and isolation around that logic inGlossaryHeader.Changes
beforeEachthat resetsmockGlossaryTermPermissionandmockContext, so tests no longer depend on state leaking between them in source order.ChangeParentHierarchytests self-contained — they previously passed only by inheriting leakedGLOSSARY_TERM+EditAllstate from an earlier test; with the reset in place they now set their own context.Alloperand — thepermissions[Operation.All]branch ofimportExportPermissionswas never exercised; the new test fails if that operand is removed, so it genuinely guards it.Delete/ViewAll) does not surface Import/Export, while the manage menu still renders viaDelete.Test
jest GlossaryHeader.test.tsx→ 11/11 passing. Also verified the newAll-path test fails whenpermissions[Operation.All]is removed from the component, confirming it guards the intended branch.