Skip to content

Fix race in update-check toast test (flaky on CI desktop) - #9

Merged
clouths merged 1 commit into
mainfrom
fix/update-check-test-race
Oct 15, 2026
Merged

clouths merged 1 commit into
mainfrom
fix/update-check-test-race

Conversation

@clouths

@clouths clouths commented Oct 15, 2026

Copy link
Copy Markdown
Contributor

Follow-up to #8: the Tests (Playwright) job failed on two consecutive runs (main #6 push and the #8 PR) with the same single failure — update-check.spec.js › toast appears when the deployed page changed on desktop — while passing on mobile and locally.

Root cause

A race in the test itself:

  • the intercepting route was installed only after page.goto(..., waitUntil: 'load') returned
  • but update-check.js fires its first re-fetch ~2 s after the script parses — that script runs at the end of <body>, i.e. before the load event
  • on slow CI runners the first check therefore fetched the unmodified page, and the checker's 30 s throttle pushed the next check ~60 s out — past the test's 15 s timeout

Fix

Test-only change to tests/update-check.spec.js:

  • install the route before the navigation
  • distinguish the initial navigation by request order (first request passes through) instead of sec-fetch-dest — that header is not populated on Playwright-intercepted requests (verified: undefined for every request in this setup), so the old isNavigation check never fired and would have broken a pre-installed route

Verified: 3× consecutive full spec passes (desktop+mobile), plus a throwaway scenario with a 3 s artificially slowed navigation — passes in 5.3 s.

The route intercepting the checker's no-store re-fetch was installed only
after page.goto('load') returned, but update-check.js fires its first
re-fetch ~2 s after the script parses — before 'load'. On slow CI runners
the first check therefore saw the unmodified page, and the checker's 30 s
throttle pushed the next check ~60 s out, past the test's 15 s timeout
(failed twice on CI desktop, passed on mobile/local where load is fast).

Install the route before the navigation and distinguish the initial
navigation by request order instead of sec-fetch-dest — that header is
not populated on Playwright-intercepted requests, so the old check never
fired. Verified with a 3 s artificially slowed navigation.
@clouths clouths self-assigned this Oct 15, 2026
@clouths
clouths merged commit 532768c into main Oct 15, 2026
7 checks passed
@clouths
clouths deleted the fix/update-check-test-race branch October 15, 2026 15:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants