chore(playwright): make retries: 0 explicit + document fail-fast intent#1567
Merged
renemadsen merged 1 commit intostablefrom May 7, 2026
Merged
Conversation
Shard b's playwright specs form a chain of cumulative state mutations (dashboard-assert → dashboard-edit-a → dashboard-edit-b) operating on the same worker "c d" and same week ranges with no per-spec DB reset. When a test fails partway and the suite retries, the chain runs twice; cumulative writes stack; downstream specs see corrupted baselines and fail with confusing deltas (e.g. expected 88.36, got 156.38 — exactly +68 hours of doubled accumulation in WorkingHours). This commit doesn't change current behavior — Playwright's default is already retries: 0. It just makes the value explicit and documents *why* nobody should ever raise it to mask flakes. Future stabilization work belongs in fixing the actual flake source (worker isolation OR adding a test-only reset endpoint OR direct SQL cleanup from a beforeAll hook), not in retries-through-cumulative-state. Fail-fast surfaces the actual flaky test on the first run; otherwise the cascade buries the real culprit under collateral damage. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Makes the Playwright E2E configuration’s “no retries” intent explicit and documents the rationale to avoid masking stateful-test failures via retries.
Changes:
- Adds an explanatory comment in
playwright.config.tsabout why retries should remain disabled. - Sets
retries: 0explicitly (matching Playwright’s default).
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+6
to
+10
| // Explicit fail-fast — do NOT raise this. Shard b's specs form a chain of | ||
| // cumulative state mutations on a shared worker / week range with no | ||
| // per-spec DB reset; retrying through a partial failure stacks writes and | ||
| // produces confusing cascade failures downstream (e.g. expected 88.36, got | ||
| // 156.38 from a doubled WorkingHours pass). The actual fix for the |
Comment on lines
+6
to
+11
| // Explicit fail-fast — do NOT raise this. Shard b's specs form a chain of | ||
| // cumulative state mutations on a shared worker / week range with no | ||
| // per-spec DB reset; retrying through a partial failure stacks writes and | ||
| // produces confusing cascade failures downstream (e.g. expected 88.36, got | ||
| // 156.38 from a doubled WorkingHours pass). The actual fix for the | ||
| // underlying flake source belongs in test isolation (worker isolation OR |
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.
Summary
One-line config change to
eform-client/playwright.config.tsplus a leading comment documenting why retries should never be raised. No behavior change — Playwright's default is alreadyretries: 0. Just makes intent explicit.Why
Shard b's specs form a chain of cumulative state mutations (
dashboard-assert→dashboard-edit-a→dashboard-edit-b) on a shared worker and shared week ranges with no per-spec DB reset. If retries get enabled to "stabilize" CI, a partial failure in one spec stacks its writes on top of the prior pass — downstream specs then see corrupted cumulative baselines (e.g. expected88.36, got156.38from a doubled-pass WorkingHours accumulation). The cascade buries the actual culprit under collateral damage.Fail-fast forces the first flaky test to surface immediately, where triage is focused. Real flake-source fix (worker isolation / test-only reset endpoint / direct SQL cleanup) is a follow-up.
Risk
None. Default is already 0; this change just blocks future "let's add retries: 1 to stabilize" attempts via the explicit comment.
Test plan
🤖 Generated with Claude Code