revert: retry a hung visual shard on the label and scheduled visual runs - #3107
Merged
Conversation
This reverts commit 2aed064. The premise did not hold. #3090 read the whole-shard failure — every test in one shard reporting "Could not capture a stable screenshot within 5000ms", 63 of 64 blocks on firefox-linux — as a hung browser process, and gave the label path three attempts against it. Its first real exercise, run 33633352648 on #3104, ran all three: 118 of 120 tests failed each time, in both browsers, at 1110s total. Shard 1 of that PR's `test.yml` run did the same, 85 of 86 tests over 474s, 472s and 472s. The attempts share the runner, so a machine that produces this failure produces it three times. So the retry buys nothing here and costs the label path 15 minutes of timeout and up to 3x the runner time on a genuine diff. The failure itself is #3106, where the stability budget #3090 already flagged is the lead. The scheduled run keeps `attempts: "3"` from #3006 — untouched here, because this reverts one PR and that one predates it. It is worth the same question. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This was referenced Sep 2, 2026
Contributor
Coverage Report for ./packages/components/
File CoverageNo changed files found. |
Same evidence, and this is where the retry started. The three attempts predate the sharding of #3006 — they were a step-level loop over the whole unsharded suite, on the same reasoning #3090 later carried to the label path: a hung firefox process needs a fresh browser, which vitest's per-test `retry` cannot give it. The failure that actually shows up does not need a fresh browser, it needs a different runner. Run 33633352648 ran all three attempts of a shard to 118 of 120 failed tests, in both browsers, at 1110s; the `test.yml` shard beside it did 85 of 86 over 474s, 472s and 472s. Nightly runner time is cheaper than a PR's, but three identical failures buy nothing at any price, and the red they report is the same either way. Timeout 45 -> 30, the label path's number for this same shape at one attempt. 45 was sized for the unsharded job, where one attempt alone took 24-34 minutes. `attempts` stays on the action with no caller. It documents a real mechanism for a hang that shows up on its own, and deleting it would reach past this revert into #3006. Its description now says the failure it does not cover, and points at #3106. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
ins0
approved these changes
Sep 2, 2026
Contributor
🚀 Preview DeploymentPreview environments are ready:
Images:
|
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.
Reverts #3090, and removes the same retry from the scheduled run.
The premise did not hold. #3090 read the whole-shard failure — every test in one shard reporting
Could not capture a stable screenshot within 5000ms, 63 of 64 blocks onfirefox-linux— as a hung browser process, and gave the label path three attempts against it.The first real exercise of that retry was run 33633352648 on #3104. It ran all three attempts: 118 of 120 tests failed each time, in both browsers, 1110s in total. Shard 1 of the same PR's
test.ymlrun did the same — 85 of 86 tests, at 474s, 472s and 472s. The attempts share the runner, so a machine that produces this failure produces it three times.So the retry buys nothing on the observed evidence, and it costs the label path 15 minutes of
timeout-minutesplus up to 3× the runner time whenever a diff is genuine — the case where a reviewer wants the diff artifacts quickly.Two commits, because only one of them is a revert
Revert #3090 —
attempts: "3"andtimeout-minutes: 45off the label path, back to 30. A plaingit revert, one file.The scheduled run is not #3090's doing, so removing its retry is a change rather than a revert. It is where the retry started: three attempts as a step-level loop over the whole unsharded suite, from before #3006 sharded it, on the same firefox-hang reasoning. Same evidence applies, so it goes too. Nightly runner time is cheaper than a PR's, but three identical failures buy nothing at any price, and the red they report is unchanged.
Its timeout goes 45 → 30 as well, the label path's number for this same shape at one attempt. 45 was sized for the unsharded job, where a single attempt took 24-34 minutes.
What stays
attemptsstays on.github/actions/run-visual-shard, now with no caller. It documents a real mechanism for a hang that shows up on its own, and deleting the loop would reach past this revert into #3006. Its description now names the failure it does not cover and points at #3106. Say the word and I strip it.What remains open
The failure itself is #3106, with the five
mainruns it hit and the lead #3090 already noted:toMatchScreenshotruns on vitest's default 5000ms stability budget, whilewaitForPaintedContentright beside it deliberately carries 20s for slower CI hardware.After this, nothing retries a visual shard anywhere, so #3106 reports as a plain red shard with no diff artifacts until it is fixed. #3104, which wired the same retry into
test.yml, is closed for the same reason.related #3090, #3106
🤖 Generated with Claude Code