ci: retry acceptance tests on CI to absorb runner flake - #819
Merged
Conversation
The acceptance matrix ran with `fullyParallel: true` and no `retries`, so the Playwright default of 0 applied and any single transient failure took down a whole matrix row. That was happening regularly. Three runs in one four-minute window failed on three different specs, one of them on master, and every failure was a timeout rather than a failed assertion - twice specifically a `page.goto` that never finished loading a fixture page within the 30s default: master select-in-submenu.js:108 timeout after 35.0s fix/issue-809 nested-triggers-autohide.js:49 page.goto timeout fix/issue-812 menu-title-icon-alignment.js:70 page.goto timeout Specs that normally finish in 2-6s were taking 10-23s in those runs, so the runner was simply oversubscribed. Re-running the failed job passed 9/9 with no code change. Retry twice on CI only. Assertions stay exactly as strict, this just stops a loaded runner from being reported as a broken build. Locally retries would only mask a genuinely flaky spec, so they stay off there.
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
1 participant
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.
Problem
playwright.config.jssetsfullyParallel: truebut never setsretries, so Playwright's default of0applies. A single transient failure therefore takes down a whole matrix row and reports the build as broken.This is happening regularly. Three runs in one four-minute window failed, on three different specs, one of them on
master:masterselect-in-submenu.js:108fix/issue-809-context-elementnested-triggers-autohide.js:49page.gototimeoutfix/issue-812-xy-overloadmenu-title-icon-alignment.js:70page.gototimeoutA different spec each time, and every failure is a timeout rather than a failed assertion. Twice it was specifically
page.gotonever finishing the fixture page load inside the 30s default:The runner is simply oversubscribed: specs that normally finish in 2-6s were taking 10-23s in those runs, and other tests in the very same file, sharing the same
beforeEach, passed. Re-running the failed job on #817 passed 9/9 with no code change at all.Change
GitHub Actions sets
CI=trueautomatically, so this is 2 on CI and 0 locally. Verified both ways, and--liststill discovers all 38 tests in 12 files.This is not loosening a tolerance. Every assertion stays exactly as strict, and a spec that genuinely fails still fails after three attempts. It only stops a loaded runner from being reported as a broken build. Retries stay off locally, where they would just mask a genuinely flaky spec.
Possible follow-up, deliberately not done here
Capping
workerson CI would attack the oversubscription directly rather than absorbing it. That is a bigger tuning decision with a wall-clock cost, so it is left out of this one-line fix.No changelog entry: internal test tooling, nothing user-facing.