From bbd568c7808737b6bb9593b70b8b160328233d1c Mon Sep 17 00:00:00 2001 From: Marco Falkenberg Date: Tue, 14 Jul 2026 13:39:40 +0200 Subject: [PATCH 1/2] ci(visual-tests): retry scheduled visual tests to absorb flaky firefox 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 --- .github/workflows/test-visual-scheduled.yml | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-) diff --git a/.github/workflows/test-visual-scheduled.yml b/.github/workflows/test-visual-scheduled.yml index 51b725bd69..02953cb206 100644 --- a/.github/workflows/test-visual-scheduled.yml +++ b/.github/workflows/test-visual-scheduled.yml @@ -27,9 +27,24 @@ jobs: run: pnpm test:browser:prepare --only-shell - name: Run visual tests - # run without file parallelism, because firefox has issues with parallel visual tests + # Run without file parallelism, because firefox has issues with parallel visual tests. + # Retry the whole step because firefox intermittently *hangs* (not a snapshot diff) on a + # single visual test file in headless CI, which aborts the run with exit code 1 even though + # every test that runs passes. A hang can't be recovered by vitest's per-test `retry`, so we + # re-run the step with a fresh vitest/browser process. Successful `^build` deps are served + # from the nx cache, so retries only re-execute test:visual. run: | - pnpm nx run-many -t test:visual --parallel=1 --browser.fileParallelism=false + for attempt in 1 2 3; do + echo "::group::Visual tests (attempt $attempt/3)" + if pnpm nx run-many -t test:visual --parallel=1 --browser.fileParallelism=false; then + echo "::endgroup::" + exit 0 + fi + echo "::endgroup::" + echo "::warning::Visual tests failed on attempt $attempt/3 (likely a flaky firefox hang), retrying..." + done + echo "::error::Visual tests still failing after 3 attempts." + exit 1 - name: Send alerting message to Slack if: failure() From 338f05422eeb69cb15cb4563cb2affc094ca5a34 Mon Sep 17 00:00:00 2001 From: Marco Falkenberg Date: Tue, 14 Jul 2026 13:55:20 +0200 Subject: [PATCH 2/2] test(Modal): wait for close animation instead of fixed sleep `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 --- .../src/components/Modal/Modal.browser.test.tsx | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/packages/components/src/components/Modal/Modal.browser.test.tsx b/packages/components/src/components/Modal/Modal.browser.test.tsx index 12d1b28c6e..3eb79dc4a2 100644 --- a/packages/components/src/components/Modal/Modal.browser.test.tsx +++ b/packages/components/src/components/Modal/Modal.browser.test.tsx @@ -274,8 +274,12 @@ test("useOnClosed is called when the closing animation has finished", async () = expect(modalText).toBeInTheDocument(); expect(onClosed).not.toHaveBeenCalledOnce(); - await sleep(500); - expect(modalText).not.toBeInTheDocument(); + // Wait for the close animation to finish and the Modal to unmount instead of + // relying on a fixed delay: the animation duration varies and a hard-coded + // sleep races against it under CI load, which made this test flaky. + await vitest.waitFor(() => expect(modalText).not.toBeInTheDocument(), { + timeout: 2000, + }); // onClosed is just called, when the Modal has unmounted (after close animation) expect(onClosed).toHaveBeenCalledOnce();