Skip to content

Add test for search rbac#25787

Open
siddhant1 wants to merge 4 commits intomainfrom
sid/test-dp
Open

Add test for search rbac#25787
siddhant1 wants to merge 4 commits intomainfrom
sid/test-dp

Conversation

@siddhant1
Copy link
Copy Markdown
Member

@siddhant1 siddhant1 commented Feb 10, 2026

Describe your changes:

Fixes

I worked on ... because ...

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.


Summary by Gitar

  • New test file:
    • Added DomainViewPermissions.spec.ts with end-to-end tests for domain and data product view permission denial scenarios

This will update automatically on new commits.

@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!

@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!

1 similar comment
@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!

@github-actions
Copy link
Copy Markdown
Contributor

Jest test Coverage

UI tests summary

Lines Statements Branches Functions
Coverage: 66%
66.11% (57653/87203) 45.89% (30605/66685) 48.71% (9154/18791)

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Mar 17, 2026

🔴 Playwright Results — 2 failure(s), 25 flaky

✅ 3326 passed · ❌ 2 failed · 🟡 25 flaky · ⏭️ 187 skipped

Shard Passed Failed Flaky Skipped
🟡 Shard 1 452 0 3 2
🔴 Shard 2 301 2 2 1
🟡 Shard 3 652 0 6 37
🟡 Shard 4 723 0 7 47
🟡 Shard 5 590 0 1 67
🟡 Shard 6 608 0 6 33

Genuine Failures (failed on all attempts)

Features/Container.spec.ts › expand / collapse should not appear after updating nested fields for container (shard 2)
�[31mTest timeout of 180000ms exceeded.�[39m
Features/DataQuality/TestCaseImportExportBasic.spec.ts › should upload and validate CSV file (shard 2)
Error: �[2mexpect(�[22m�[31mlocator�[39m�[2m).�[22mtoHaveText�[2m(�[22m�[32mexpected�[39m�[2m)�[22m failed

Locator:  getByTestId('passed-row')
Expected: �[32m"4"�[39m
Received: �[31m"1"�[39m
Timeout:  15000ms

Call log:
�[2m  - Expect "toHaveText" with timeout 15000ms�[22m
�[2m  - waiting for getByTestId('passed-row')�[22m
�[2m    19 × locator resolved to <span data-testid="passed-row" class="font-semibold passed-row">1</span>�[22m
�[2m       - unexpected value "1"�[22m

🟡 25 flaky test(s) (passed on retry)
  • Features/CustomizeDetailPage.spec.ts › Domain - customization should work (shard 1, 1 retry)
  • Pages/AuditLogs.spec.ts › should apply both User and EntityType filters simultaneously (shard 1, 1 retry)
  • Pages/Customproperties-part1.spec.ts › Timestamp (shard 1, 1 retry)
  • Features/ColumnBulkOperations.spec.ts › should discard changes when closing drawer without saving (shard 2, 1 retry)
  • Features/RTL.spec.ts › Verify Following widget functionality (shard 2, 1 retry)
  • Features/DataQuality/TestCaseImportExportE2eFlow.spec.ts › Admin: Complete export-import-validate flow (shard 3, 1 retry)
  • Features/DataQuality/TestCaseImportExportE2eFlow.spec.ts › EditAll User: Complete export-import-validate flow (shard 3, 1 retry)
  • Features/DataQuality/TestCaseIncidentPermissions.spec.ts › User with TEST_CASE.EDIT_ALL can see edit icon on incidents (shard 3, 1 retry)
  • Features/DataQuality/TestCaseIncidentPermissions.spec.ts › User with only VIEW cannot PATCH incidents (shard 3, 1 retry)
  • Features/DataQuality/TestCaseResultPermissions.spec.ts › User with only VIEW cannot PATCH results (shard 3, 1 retry)
  • Features/Permissions/GlossaryPermissions.spec.ts › Team-based permissions work correctly (shard 3, 1 retry)
  • Pages/Customproperties-part2.spec.ts › Table (shard 4, 1 retry)
  • Pages/Customproperties-part2.spec.ts › entityReferenceList shows item count, scrollable list, no expand toggle (shard 4, 1 retry)
  • Pages/DomainDataProductsRightPanel.spec.ts › Should open right panel when clicking data product card in domain (shard 4, 1 retry)
  • Pages/DomainDataProductsRightPanel.spec.ts › Should display overview tab for data product (shard 4, 1 retry)
  • Pages/Domains.spec.ts › Data Product announcement create, edit & delete (shard 4, 1 retry)
  • Pages/Domains.spec.ts › Rename domain with assets (tables, topics, dashboards) preserves associations (shard 4, 1 retry)
  • Pages/Domains.spec.ts › Domain Rbac (shard 4, 1 retry)
  • Pages/EntityDataConsumer.spec.ts › Tag Add, Update and Remove (shard 5, 1 retry)
  • Pages/Glossary.spec.ts › Column dropdown drag-and-drop functionality for Glossary Terms table (shard 6, 1 retry)
  • Pages/HyperlinkCustomProperty.spec.ts › should accept valid http and https URLs (shard 6, 1 retry)
  • Pages/Lineage.spec.ts › Lineage creation from Metric entity (shard 6, 1 retry)
  • Pages/Users.spec.ts › Permissions for table details page for Data Consumer (shard 6, 1 retry)
  • Pages/Users.spec.ts › Check permissions for Data Steward (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

Replace deprecated 'networkidle' with 'domcontentloaded' wait state.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@gitar-bot
Copy link
Copy Markdown

gitar-bot bot commented Mar 17, 2026

Code Review ⚠️ Changes requested 1 resolved / 3 findings

Adds test coverage for domain view permissions but the test suite is skipped with test.describe.skip, making it a no-op. Additionally, the PR title references 'search rbac' while the actual test covers domain permissions, creating a mismatch in documentation.

⚠️ Bug: test.describe.skip makes entire test suite a no-op

📄 openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/Permissions/DomainViewPermissions.spec.ts:102 📄 openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/Permissions/DomainViewPermissions.spec.ts:55 📄 openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/Permissions/DomainViewPermissions.spec.ts:158

The test suite at line 102 uses test.describe.skip(...), which means none of the four test cases will ever run. The beforeAll setup (creating users, domains, policies, roles) and afterAll cleanup will still execute, but all actual assertions are skipped.

If the tests are not ready, this is expected but should be documented with a TODO/reason. If the intent is to merge enabled tests, remove .skip. The commit message "Skip test" suggests this is intentional for now, but shipping permanently-skipped tests adds maintenance burden with no value.

💡 Quality: PR title says 'search rbac' but test is for domain permissions

📄 openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/Permissions/DomainViewPermissions.spec.ts:1

The PR title is "Add test for search rbac" but the test file is DomainViewPermissions.spec.ts and tests domain/data-product view permission denial. This mismatch makes the PR harder to find and understand in the future. Consider updating the PR title to reflect the actual content.

✅ 1 resolved
Quality: Missing newline at end of file

📄 openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/Permissions/DomainViewPermissions.spec.ts:173
The file is missing a trailing newline (noted by the diff marker \ No newline at end of file). Most editors and linters expect a final newline.

🤖 Prompt for agents
Code Review: Adds test coverage for domain view permissions but the test suite is skipped with `test.describe.skip`, making it a no-op. Additionally, the PR title references 'search rbac' while the actual test covers domain permissions, creating a mismatch in documentation.

1. ⚠️ Bug: test.describe.skip makes entire test suite a no-op
   Files: openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/Permissions/DomainViewPermissions.spec.ts:102, openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/Permissions/DomainViewPermissions.spec.ts:55, openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/Permissions/DomainViewPermissions.spec.ts:158

   The test suite at line 102 uses `test.describe.skip(...)`, which means none of the four test cases will ever run. The `beforeAll` setup (creating users, domains, policies, roles) and `afterAll` cleanup will still execute, but all actual assertions are skipped.
   
   If the tests are not ready, this is expected but should be documented with a TODO/reason. If the intent is to merge enabled tests, remove `.skip`. The commit message "Skip test" suggests this is intentional for now, but shipping permanently-skipped tests adds maintenance burden with no value.

2. 💡 Quality: PR title says 'search rbac' but test is for domain permissions
   Files: openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/Permissions/DomainViewPermissions.spec.ts:1

   The PR title is "Add test for search rbac" but the test file is `DomainViewPermissions.spec.ts` and tests domain/data-product view permission denial. This mismatch makes the PR harder to find and understand in the future. Consider updating the PR title to reflect the actual content.

Rules 🎸 1 action taken

Gitar Rules

🎸 Summary Enhancement: PR description contained only template placeholders; appended technical summary with 3 categories covering test coverage, RBAC setup, and validation logic

2 rules not applicable. Show all rules by commenting gitar display:verbose.

Options

Auto-apply is off → Gitar will not commit updates to this branch.
Display: compact → Showing less information.

Comment with these commands to change:

Auto-apply Compact
gitar auto-apply:on         
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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant