ci/test: fix two flaky visual/browser test failures (firefox hang retry + Modal deflake)#2641
Open
mfal wants to merge 2 commits into
Open
ci/test: fix two flaky visual/browser test failures (firefox hang retry + Modal deflake)#2641mfal wants to merge 2 commits into
mfal wants to merge 2 commits into
Conversation
…x hangs The scheduled visual regression workflow failed on ~15% of runs (6/40). Investigation of the failed runs showed these are not snapshot/pixel regressions: every test that runs passes and there are zero assertion failures. Instead, firefox intermittently *hangs* on a single visual test file in headless CI (webkit always completes), so exactly one test-file summary is missing and the run aborts with exit code 1. A hang cannot be recovered by vitest's per-test `retry` (the process is killed as a whole, so there is nothing to re-run), so wrap the step in a retry loop that re-runs `test:visual` in a fresh vitest/browser process up to three times. Successful `^build` deps come from the nx cache, so retries only re-execute the visual tests. The Slack alert still fires only when all three attempts fail. Co-Authored-By: Claude Opus 4.8 <[email protected]>
mfal
requested review from
Jan-Eimertenbrink,
Lisa18289,
ins0 and
maaaathis
as code owners
Contributor
Coverage Report for ./packages/components/
File CoverageNo changed files found. |
Contributor
🚀 Preview DeploymentPreview environments are ready:
Images:
|
`useOnClosed is called when the closing animation has finished` intermittently failed in CI (only in the "Run tests" workflow, always this single assertion: `expect(element).not.toBeInTheDocument()`). It is a timing-dependent flaky test, not a regression: it passed and failed on unrelated commits (e.g. a version-bump commit), and the latest main run was green. Root cause: the test waited a fixed 550ms (sleep(50) + sleep(500)) for the modal close animation to finish and unmount, but the animation lasts `--transition--duration--slow` = 400ms, leaving only ~150ms for animationend + React state update + unmount. Under CI load that margin is too tight and the modal is still in the document at 550ms. Replace the fixed sleep with condition-based waiting (`vitest.waitFor`) that polls until the modal has unmounted (2s timeout), removing the race entirely. Verified locally in webkit: test passes reliably across repeated runs. Co-Authored-By: Claude Opus 4.8 <[email protected]>
mfal
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
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.
Fixes two unrelated flaky failures found by analyzing the failed GitHub Actions runs.
1.
Run scheduled visual tests— firefox intermittently hangs (~15%, 6/40 runs)Analysis of the failed runs:
×) failures.Switch.browser.test.tsx); webkit always completes.ResizeObserver loop completed with undelivered notifications.is a red herring — appears identically (4×) in passing runs too.Root cause: firefox intermittently hangs on a single visual test file in headless CI, aborting the run with exit code 1. Same class of browser flakiness for which chromium was already disabled and file parallelism already turned off.
Fix: wrap the
Run visual testsstep in a retry loop (up to 3 attempts). A hang can't be recovered by vitest's per-testretry(the whole process is killed), so we re-runtest:visualin a fresh vitest/browser process. Successful^builddeps come from the nx cache, so retries only re-execute the visual tests. The Slack alert still fires only when all three attempts fail. Expected residual failure rate ≈ 0.15³ ≈ 0.3%.2.
Run tests— flaky Modal animation-timing testAnalysis: the recurring
Run testsfailures were always a single assertion inModal.browser.test.tsx > useOnClosed is called when the closing animation has finished(expect(element).not.toBeInTheDocument()). It is flaky, not a regression: it passed/failed on unrelated commits (including a pure version-bump commit) and the latest main run was green.Root cause: the test waited a fixed 550ms (
sleep(50)+sleep(500)) for the close animation to finish and the modal to unmount, but the animation lasts--transition--duration--slow= 400ms, leaving only ~150ms foranimationend+ React state update + unmount. Under CI load that margin is too tight.Fix: replace the fixed sleep with condition-based waiting (
vitest.waitFor, 2s timeout) that polls until the modal has unmounted, removing the race entirely.Verification: ran the Modal browser test in webkit locally across repeated runs — passes reliably (
8 passed).🤖 Generated with Claude Code