Fix the ServiceIngestion and Entity spec AUT failiures#29176
Fix the ServiceIngestion and Entity spec AUT failiures#29176aniketkatkar97 wants to merge 2 commits into
Conversation
aniketkatkar97
added
the
To release
github-actions
Bot
added
safe to test
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| deletedBadge = page.locator('[data-testid="deleted-badge"]'); | ||
| } | ||
| } | ||
| await page.reload({ waitUntil: 'domcontentloaded' }); |
There was a problem hiding this comment.
waitUntil: 'domcontentloaded', which resolves as soon as the HTML is parsed — well before the SPA's React tree has hydrated or async data requests have completed. The original code always called waitForAllLoadersToDisappear after every reload. While the interval between polls provides some buffer, on slower CI environments the isVisible() check on the very next iteration could fire before the app is ready, causing a spurious extra cycle. Using 'networkidle' (or 'load') would be closer to the original contract and more reliable.
| await page.reload({ waitUntil: 'domcontentloaded' }); | |
| await page.reload({ waitUntil: 'networkidle' }); |
There was a problem hiding this comment.
Pull request overview
This PR targets flaky Playwright E2E behavior by improving entity page navigation reliability (tooltip overlap) and by refactoring the soft-delete “deleted badge” verification into a polling-based retry strategy.
Changes:
- Updated
visitEntityPageto dismiss tooltips by moving the mouse away before clicking the entity name. - Replaced a manual retry loop in
softDeleteEntitywithexpect.pollto make deletion verification more robust and maintainable.
| await page.reload({ waitUntil: 'domcontentloaded' }); | ||
|
|
||
| await expect(deletedBadge).toHaveText('Deleted'); | ||
| return false; | ||
| }, |
Jest test CoverageUI tests summary
|
Code Review ✅ ApprovedRefactors Playwright utility functions to improve test stability by dismissing tooltips and replacing manual retry loops with OptionsDisplay: compact → Showing less information. Comment with these commands to change:
Was this helpful? React with 👍 / 👎 | Gitar |
|
🟡 Playwright Results — all passed (12 flaky)✅ 4302 passed · ❌ 0 failed · 🟡 12 flaky · ⏭️ 88 skipped
🟡 12 flaky test(s) (passed on retry)
How to debug locally# Download playwright-test-results-<shard> artifact and unzip
npx playwright show-trace path/to/trace.zip # view trace |
Fixes #29175
This pull request improves the reliability and readability of Playwright utility functions for entity interactions by refining how UI elements are interacted with and how retry logic is handled. The changes focus on making UI automation less prone to failures caused by tooltips and improving the robustness of the soft deletion verification process.
UI interaction reliability improvements:
visitEntityPage, replaced the previous workaround for tooltip overlap (hovering and force-clicking) with a more reliable approach: moving the mouse to the top-left of the page to dismiss tooltips before performing a standard click on the entity name. This reduces flaky test failures due to tooltip interference.Robust retry logic for deletion checks:
softDeleteEntity, refactored the manual retry loop for checking the "deleted" badge into Playwright'sexpect.pollwith configurable intervals and timeout. This makes the check more robust, readable, and easier to maintain.Greptile Summary
This PR addresses flaky Playwright test failures in
ServiceIngestionand entity spec tests by refining two utility functions inentity.ts. The tooltip-dismissal strategy invisitEntityPageis simplified (replacing hover + force-click with a mouse-to-origin move and a standard click), and the deleted-badge polling loop insoftDeleteEntityis migrated to Playwright's idiomaticexpect.pollAPI.visitEntityPage: Dropsforce: trueand the prior hover workaround; instead moves the mouse tobodyposition(0, 0)before the click, which is a more reliable way to dismiss transient tooltips without bypassing Playwright's overlap detection.softDeleteEntity: Replaces a manualwhileloop (≤ 5 reload attempts, no timeout) withexpect.poll(120 s timeout, ascending retry intervals of 5 s → 10 s → 15 s), giving the check a clear timeout, a descriptive failure message, and making it easier to maintain.Confidence Score: 5/5
Safe to merge; changes are scoped to Playwright test utilities with no production code path affected.
Both changes are self-contained improvements to test utility helpers. The
visitEntityPagetooltip fix removes a linting suppression and the unreliable force-click. ThesoftDeleteEntitymigration toexpect.pollis a strict improvement in clarity and robustness over the raw while loop, with the same eventual correctness guarantees. No application logic is touched.No files require special attention.
Important Files Changed
visitEntityPagesimplified to mouse-origin hover + standard click (removesforce: true);softDeleteEntitymanual retry loop replaced withexpect.poll. One minor concern: the internal reload useswaitUntil: 'domcontentloaded'which resolves before the SPA is fully hydrated, thoughwaitForAllLoadersToDisappearpartially mitigates this.Sequence Diagram
%%{init: {'theme': 'neutral'}}%% sequenceDiagram participant Test participant Page participant Server Note over Test,Server: softDeleteEntity flow Test->>Page: click confirm-button Server-->>Test: deleteResponse (API 200) Test->>Page: toastNotification check Test->>Page: reload() + waitForAllLoadersToDisappear() loop expect.poll (timeout: 120s, intervals: 5s→10s→15s) Test->>Page: isVisible('[deleted-badge]')? alt Badge already visible Page-->>Test: true → poll resolves else Badge not visible Test->>Page: "reload({ waitUntil: 'domcontentloaded' })" Test->>Page: waitForAllLoadersToDisappear() Test->>Page: isVisible('[deleted-badge]')? Page-->>Test: true / false end end Test->>Page: expect toHaveText('Deleted') Test->>Page: deletedEntityCommonChecks (deleted: true) Test->>Page: restoreEntity() Test->>Page: reload() + waitForAllLoadersToDisappear() Test->>Page: deletedEntityCommonChecks (deleted: false)%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%% sequenceDiagram participant Test participant Page participant Server Note over Test,Server: softDeleteEntity flow Test->>Page: click confirm-button Server-->>Test: deleteResponse (API 200) Test->>Page: toastNotification check Test->>Page: reload() + waitForAllLoadersToDisappear() loop expect.poll (timeout: 120s, intervals: 5s→10s→15s) Test->>Page: isVisible('[deleted-badge]')? alt Badge already visible Page-->>Test: true → poll resolves else Badge not visible Test->>Page: "reload({ waitUntil: 'domcontentloaded' })" Test->>Page: waitForAllLoadersToDisappear() Test->>Page: isVisible('[deleted-badge]')? Page-->>Test: true / false end end Test->>Page: expect toHaveText('Deleted') Test->>Page: deletedEntityCommonChecks (deleted: true) Test->>Page: restoreEntity() Test->>Page: reload() + waitForAllLoadersToDisappear() Test->>Page: deletedEntityCommonChecks (deleted: false)Reviews (2): Last reviewed commit: "Worked on comments" | Re-trigger Greptile