fix(playwright): stabilize flaky CustomMetric, TestSuiteMultiPipeline, BulkEditEntity tests#30207
Open
ShaileshParmar11 wants to merge 2 commits into
Conversation
…fresh table reads
These two specs intermittently fail in AUT because a child mutation
(custom metric via PUT /tables/{id}/customMetric, or an executable test
suite created when the first test case is added) does not bump the parent
table's version/updatedAt. The entity ETag is hash(version + "-" + updatedAt),
so it is unchanged after the write, the client's conditional refetch matches
If-None-Match, and the UI renders a stale table body — the new custom metric
or the add-pipeline CTA never appears and the test times out.
Add a `forceFreshTableReads` helper that strips the `If-None-Match` header on
`/api/v1/tables/**` requests at the Playwright network layer, so table reads
always return the current body. Scoped to table reads only; no app or backend
changes.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Contributor
❌ PR checklist incompleteThis PR cannot be merged until the following are addressed on its linked issue:
The fields live on the linked issue in the Shipping project (open the issue → right sidebar → Projects). After you set them, re-run this check (or push a commit) — issue/project changes do not re-trigger it automatically. Maintainers can bypass this check by adding the |
github-actions
Bot
added
safe to test
Contributor
🟡 Playwright Results — all passed (30 flaky)✅ 4541 passed · ❌ 0 failed · 🟡 30 flaky · ⏭️ 95 skipped
🟡 30 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 |
The Nested bulk-edit test asserted processed:1 but intermittently saw 5/2/4 because the bulk-edit grid export is scoped by the activeGlossary store value, which could be stale when bulk-edit-table is clicked — exporting a wrong, larger term set. - Wait for the nested term to be indexed before bulk-edit (matches sibling). - Scope the export wait to the parent term's FQN so the grid can't load a wrong scope. - Assert the grid has exactly one row before importing, so a wrong-scope export fails fast instead of surfacing as a confusing processed-row count. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
ShaileshParmar11
changed the title
Code Review ✅ ApprovedAdds a helper to bypass stale ETag-based responses for table API calls, effectively de-flaking the CustomMetric and TestSuiteMultiPipeline Playwright tests. No issues found. OptionsDisplay: compact → Showing less information. Comment with these commands to change the behavior for this request:
Was this helpful? React with 👍 / 👎 | Gitar |
ShaileshParmar11
changed the title
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.
De-flakes three Playwright specs that intermittently fail in AUT (worse under load). All fixes are test-side only — no app or backend changes.
1. CustomMetric & TestSuiteMultiPipeline — stale table reads
Cause: a child mutation —
PUT /tables/{id}/customMetric, or the executable test suite created when the first test case is added — does not bump the parent table'sversion/updatedAt. The entity ETag ishash(version + "-" + updatedAt), so it is unchanged after the write. The client's conditional refetch (If-None-Match, viaetagInterceptor.ts) then gets a not-modified response and the UI renders the stale table body — the new custom metric / theadd-pipeline-buttonnever appears and the test times out.Trace evidence: the failing
/api/v1/tables/name/...GETs carryIf-None-Matchand returnbodySize: 0, and the served body is the pre-write one (customMetrics: 0while thePUTresponse hascustomMetrics: 1, both at tableversion 0.1/ sameupdatedAt).Fix: a
forceFreshTableReads(page)helper stripsIf-None-Matchon**/api/v1/tables/**at the Playwright network layer, so table reads always return the current body. Scoped to table reads only.2. BulkEditEntity › Glossary Term (Nested) — wrong export scope
Cause: the test asserts
processed: 1but intermittently saw5 / 2 / 4. The bulk-edit grid export is scoped by theactiveGlossarystore value (GlossaryTermTab.handleEditGlossary), which could be stale whenbulk-edit-tableis clicked — exporting a wrong, larger term set. The backend export for a term is correctly FQN-scoped, so a fresh parent-with-one-child should be exactly1.Fix:
processed-rowcount.Validation
Ran locally against a live server:
CustomMetric/TestSuiteMultiPipeline: pass.BulkEditEntity › Glossary Term (Nested): 3/3 green (--repeat-each+ reruns),--workers=1 --retries=0.Notes
These contain the test flakes. The underlying product behavior in (1) — a table GET can serve a stale body after a child mutation that doesn't bump the table version — is left as a separate follow-up.
🤖 Generated with Claude Code
Greptile Summary
This test-only PR adds two targeted de-flaking mechanisms for Playwright E2E tests: a
forceFreshTableReadsroute interceptor that stripsIf-None-Matchheaders on/api/v1/tables/**requests so stale ETag cache hits cannot cause the UI to render pre-mutation table state, and awaitForSearchIndexed+ scopedwaitForResponsepair inBulkEditEntityto ensure the glossary search index is current and the bulk-edit export is scoped to the correct parent term before assertions run.entity.ts: NewforceFreshTableReads(page)utility registers a Playwright route handler that deletes theif-none-matchheader from all table API requests, so the server always returns a fresh body instead of a304 Not Modifiedwhen table child mutations (custom metrics, executable test suites) don't bump the ETag.CustomMetric.spec.ts/TestSuiteMultiPipeline.spec.ts: Both specs callforceFreshTableReadsimmediately afterredirectToHomePage, scoping the bypass to table reads only.BulkEditEntity.spec.ts: Adds awaitForSearchIndexedcall before navigating to the bulk-edit view and an FQN-scopedwaitForResponsewait after clicking bulk-edit, plus a.rdg-rowcount assertion to fail fast when the wrong scope is used.Confidence Score: 5/5
Test-only change with no production code modifications; safe to merge.
All changes are confined to Playwright test utilities and specs. The
forceFreshTableReadshelper is narrowly scoped to table API requests and uses a well-established Playwright route interception pattern already present throughout the codebase. No application or backend code is touched.No files require special attention.
Important Files Changed
forceFreshTableReadsutility that intercepts all/api/v1/tables/**requests and strips theIf-None-Matchheader, preventing stale 304 responses after child mutations; implementation is correct and well-documented.forceFreshTableReadsafterredirectToHomePagein both table-level and column-level custom metric tests; minimal, correct change.forceFreshTableReadsafterredirectToHomePagein both multi-pipeline test cases; follows the same pattern as CustomMetric.spec.ts.Sequence Diagram
%%{init: {'theme': 'neutral'}}%% sequenceDiagram participant PW as Playwright Test participant RT as Route Interceptor participant UI as Browser / UI participant API as /api/v1/tables/** Note over PW: forceFreshTableReads registered PW->>RT: "page.route('**/api/v1/tables/**')" PW->>UI: navigate / trigger mutation (PUT customMetric) UI->>API: "PUT /tables/{id}/customMetric" API-->>UI: 200 OK (customMetrics: 1, version still 0.1) UI->>RT: GET /tables/name/... (If-None-Match: etag_v0.1) Note over RT: delete headers['if-none-match'] RT->>API: GET /tables/name/... (no If-None-Match) API-->>RT: 200 OK (full body, customMetrics: 1) RT-->>UI: fresh response Note over PW: Without interceptor UI->>API: GET /tables/name/... (If-None-Match: etag_v0.1) API-->>UI: 304 Not Modified (bodySize: 0) Note over UI: stale body rendered, assertion timeout%%{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 PW as Playwright Test participant RT as Route Interceptor participant UI as Browser / UI participant API as /api/v1/tables/** Note over PW: forceFreshTableReads registered PW->>RT: "page.route('**/api/v1/tables/**')" PW->>UI: navigate / trigger mutation (PUT customMetric) UI->>API: "PUT /tables/{id}/customMetric" API-->>UI: 200 OK (customMetrics: 1, version still 0.1) UI->>RT: GET /tables/name/... (If-None-Match: etag_v0.1) Note over RT: delete headers['if-none-match'] RT->>API: GET /tables/name/... (no If-None-Match) API-->>RT: 200 OK (full body, customMetrics: 1) RT-->>UI: fresh response Note over PW: Without interceptor UI->>API: GET /tables/name/... (If-None-Match: etag_v0.1) API-->>UI: 304 Not Modified (bodySize: 0) Note over UI: stale body rendered, assertion timeoutReviews (2): Last reviewed commit: "test(playwright): de-flake Glossary Term..." | Re-trigger Greptile