Skip to content

(test) ui: fix flaky entity spec#26151

Merged
Rohit0301 merged 8 commits intomainfrom
fix-flaky-entity-spec-2
Mar 5, 2026
Merged

(test) ui: fix flaky entity spec#26151
Rohit0301 merged 8 commits intomainfrom
fix-flaky-entity-spec-2

Conversation

@harsh-vador
Copy link
Copy Markdown
Contributor

@harsh-vador harsh-vador commented Feb 27, 2026

Describe your changes:

I worked on fixing the flakiness in entity.spec

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

  • Replaced generic loader waits with specific element visibility:
    • Removed waitForAllLoadersToDisappear() calls; use targeted waitFor({ state: 'visible' }) or waitForSelector('[data-testid="loader"]', { state: 'detached' })
  • API response matching precision:
    • Changed from string URL patterns to function predicates filtering by HTTP method (PUT/PATCH) to prevent false matches from concurrent requests
  • Test isolation for parallel execution:
    • Scoped all selectors in updateCustomPropertyInRightPanel() to .entity-summary-panel-container to avoid DOM interference across parallel test runs
  • ProseMirror editor interaction reliability:
    • Replaced unreliable clear()/fill() with keyboard shortcuts (Ctrl+A, Backspace) to properly trigger editor state changes
  • Timing adjustments for slow environments:
    • Extended UI verification timeout to 10s; improved comments explaining flakiness root causes

This will update automatically on new commits.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Feb 27, 2026

Jest test Coverage

UI tests summary

Lines Statements Branches Functions
Coverage: 65%
65.86% (56761/86188) 45.41% (29856/65742) 48.09% (8958/18628)

Comment thread openmetadata-ui/src/main/resources/ui/playwright/utils/entity.ts Outdated
Comment thread openmetadata-ui/src/main/resources/ui/playwright/utils/customProperty.ts Outdated
@gitar-bot
Copy link
Copy Markdown

gitar-bot Bot commented Mar 3, 2026

Code Review 👍 Approved with suggestions 2 resolved / 3 findings

Solid flaky test fixes with well-targeted wait strategies. One minor timeout asymmetry remains from the previous review.

💡 Edge Case: Extended timeout only applied to "No Description" branch

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

The 10s timeout was added to the isEmpty(description) branch (line 731) to handle slow re-renders under CPU load during parallel test runs, but the else branch (lines 733-738) still uses Playwright's default 5s timeout. The same CPU load conditions that delay the "No Description" re-render would equally delay a description text re-render, potentially leaving flakiness in the non-empty description path.

Suggested fix
  } else {
    await expect(
      page
        .locator(`[${rowSelector}="${rowId}"]`)
        .getByTestId('viewer-container')
        .getByRole('paragraph')
    ).toContainText(description, { timeout: 10000 });
  }
✅ 2 resolved
Bug: Meta+A does not select all text on Linux CI (ubuntu)

📄 openmetadata-ui/src/main/resources/ui/playwright/utils/entity.ts:682 🔗 Playwright keyboard.press docs
The new clear logic uses page.keyboard.press('Meta+A') to select all text before pressing Backspace. However, Playwright CI runs on ubuntu-latest (see .github/workflows/playwright-integration-tests-postgres.yml), and Playwright's documentation explicitly states that Meta+A maps to the Super/Windows key on Linux — not Ctrl. This means the select-all shortcut will silently do nothing on Linux, leaving the old description text intact instead of clearing it.

The Playwright docs say:

"on Windows and Linux: page.keyboard.press('Control+A') // on macOS: page.keyboard.press('Meta+A')"

Other test files in this repo already use Control+A correctly (e.g., BulkImport.spec.ts, user.ts, DataContracts.spec.ts). The DataProductAndSubdomains.spec.ts file has the same Meta+A bug — but that's a pre-existing issue outside this PR's scope.

This will cause updateDescriptionForChildren to fail when trying to clear/replace a description: the old text won't be selected, Backspace will delete at most one character, and the new text will be appended to the old text (or the field won't be cleared at all when description is empty).

Quality: Inconsistent use of panelContainer vs duplicate locator

📄 openmetadata-ui/src/main/resources/ui/playwright/utils/customProperty.ts:1272
The panelContainer variable is created at line 1255 and used to scope searchContainer and the card count assertion, but at line 1272 a new container is created using page.locator('.entity-summary-panel-container') instead of reusing panelContainer. This is functionally identical but inconsistent with the PR's explicit goal of scoping everything through panelContainer to avoid parallel test interference. Using panelContainer.getByTestId(propertyName) would be more consistent and clearly communicate the intent.

🤖 Prompt for agents
Code Review: Solid flaky test fixes with well-targeted wait strategies. One minor timeout asymmetry remains from the previous review.

1. 💡 Edge Case: Extended timeout only applied to "No Description" branch
   Files: openmetadata-ui/src/main/resources/ui/playwright/utils/entity.ts:728-738

   The 10s timeout was added to the `isEmpty(description)` branch (line 731) to handle slow re-renders under CPU load during parallel test runs, but the `else` branch (lines 733-738) still uses Playwright's default 5s timeout. The same CPU load conditions that delay the "No Description" re-render would equally delay a description text re-render, potentially leaving flakiness in the non-empty description path.

   Suggested fix:
   } else {
     await expect(
       page
         .locator(`[${rowSelector}="${rowId}"]`)
         .getByTestId('viewer-container')
         .getByRole('paragraph')
     ).toContainText(description, { timeout: 10000 });
   }

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

sonarqubecloud Bot commented Mar 3, 2026

@harsh-vador harsh-vador removed their assignment Mar 4, 2026
@Rohit0301 Rohit0301 merged commit 1564233 into main Mar 5, 2026
31 of 32 checks passed
@Rohit0301 Rohit0301 deleted the fix-flaky-entity-spec-2 branch March 5, 2026 07:20
Rohit0301 added a commit that referenced this pull request Apr 21, 2026
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.

3 participants