Skip to content

Conversation

@ShaileshParmar11
Copy link
Contributor

@ShaileshParmar11 ShaileshParmar11 commented Jan 26, 2026

playwright: fix AUT playwright test for test cases

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

  • Test reliability improvements:
    • Added getFqn() helper with null safety checks for fullyQualifiedName in TestCaseImportExportBasic.spec.ts
    • Environment-specific test.slow() handling and explicit loader detachment waits reduce flakiness
  • Null safety enhancements:
    • Validation for table metadata (service, database, databaseSchema) in testCases.ts prevents runtime errors
  • Code quality refactoring:
    • Consistent test.describe block formatting and removal of template violations in TestDefinitionPermissions.spec.ts

This will update automatically on new commits.


…reliability in data quality import/export tests.
Copy link
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

This PR adjusts Playwright Data Quality E2E utilities/specs to make tests more robust (primarily around table metadata/FQN usage) and tidies up spec formatting.

Changes:

  • Added explicit validation for required table metadata/FQN before using it in Playwright helpers.
  • Refactored/normalized formatting in Data Quality permission specs and tightened API validation flows.
  • Added a local getFqn helper + minor stability tweaks (e.g., conditional test.slow()) in bulk import/export specs.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File Description
openmetadata-ui/src/main/resources/ui/playwright/utils/testCases.ts Adds guards for required table metadata/FQN before asserting breadcrumbs / building test-suite strings.
openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/DataQuality/TestDefinitionPermissions.spec.ts Formatting cleanup and small robustness tweaks around system test definition lookup.
openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/DataQuality/TestCaseImportExportBasic.spec.ts Adds getFqn helper and minor stability/formatting improvements for import/export permission and flow tests.

@gitar-bot
Copy link

gitar-bot bot commented Jan 26, 2026

🔍 CI failure analysis for 3e0f5c6: CI failure persists after retry: ServiceCreationPermissions.spec.ts consistently fails in both attempts, unrelated to PR's Data Quality changes.

Issue

The CI job playwright-ci-postgresql (3, 6) has failed consistently across 2 attempts with the same hard failure, indicating a reproducible test issue rather than random flakiness.

Root Cause

Consistent Failure in Both Attempts:

  • Attempt 1 (Job 61475463732): Test failed after 56.8s, retry within run also failed after 46.9s
  • Attempt 2 (Job 61485958429): Test failed again with identical error

Failed Test:

  • File: playwright/e2e/Flow/ServiceCreationPermissions.spec.ts:303
  • Test: "Different user can view but cannot modify service owned by another user"
  • Error: expect(getByTestId('entity-header-name')).toBeVisible() - element not found after 5000ms timeout

The failure is consistent and reproducible, not a random flaky test. The element entity-header-name never appears when a different user navigates to view a service owned by another user.

Details

Unrelated to PR Changes:

This PR only modifies Data Quality test files:

  • TestCaseImportExportBasic.spec.ts
  • TestDefinitionPermissions.spec.ts
  • testCases.ts

The failing test ServiceCreationPermissions.spec.ts was not modified by this PR.

Flaky Tests (Environmental Issues):

  • Attempt 1: 7 flaky tests (passed on retry)
  • Attempt 2: 9 flaky tests (passed on retry)
  • These include permission tests, widget tests, and explore/discovery tests experiencing timeouts

Pattern Analysis:
The consistent failure across both manual retry attempts (not just internal Playwright retries) suggests:

  1. Race condition in user authentication/session switching
  2. Page navigation not completing before element checks
  3. CI environment-specific issue affecting this permission scenario
Code Review 👍 Approved with suggestions 0 resolved / 1 findings

Good test reliability improvements with proper null safety checks. One minor edge case where displayName validation could be more complete.

💡 Edge Case: Incomplete null check: displayName is checked but not validated

📄 openmetadata-ui/src/main/resources/ui/playwright/utils/testCases.ts:738

The code destructures displayName from table.entityResponseData but only validates service, database, and databaseSchema. However, displayName is used directly in the assertion on line 760 (${displayName}/) without validation.

If displayName is undefined, the test will produce an incorrect assertion string like undefined/ instead of failing early with a clear error message.

Suggested fix:
Add displayName to the validation check:

if (!service || !database || !databaseSchema || !displayName) {
  throw new Error(
    `Table metadata (service, database, databaseSchema, or displayName) is missing for ${table.entity.name}`
  );
}

Or alternatively, if displayName can legitimately be undefined, handle it explicitly (e.g., fall back to table.entity.name).

Tip

Comment Gitar fix CI or enable auto-apply: gitar auto-apply:on

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

@github-actions
Copy link
Contributor

Jest test Coverage

UI tests summary

Lines Statements Branches Functions
Coverage: 65%
65.82% (55592/84464) 44.82% (28665/63953) 47.72% (8734/18301)

@sonarqubecloud
Copy link

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants