fix(test): stop fixtures re-reading the clock per timestamp, and add a checker so it stays fixed - #9958
Merged
Merged
Conversation
…a checker so it stays fixed
A test fixture that calls Date.now() once PER TIMESTAMP produces timestamps
that disagree with each other by however long elapsed between the calls. Where
the code under test derives a boundary from one of them, a single millisecond
flips the result.
queue-trends.test.ts had exactly that:
function atDaysAgo(daysAgo: number) { return new Date(Date.now() - ...); }
buildWindow anchors on the newest snapshot:
targetMs = latestMs - windowDays * day
baseline = newest snapshot with fetchedAt <= targetMs
`atDaysAgo(0)` evaluated at T0 and `atDaysAgo(7)` a moment later at T1 put the
"7 days ago" row at T1-7d -- NEWER than the target T0-7d. No baseline, every
window "unavailable", assertion fails. Reproduced deterministically with a 2ms
offset. It fired for real on #9950, a PR whose only changed file was a GitHub
workflow, which is how it was spotted.
This matters more than a flaky test usually would: reviews are one-shot for
everyone but the maintainer, so a false red on a contributor PR is not a re-run
away from fine -- it auto-closes correct work the contributor cannot reopen.
Every offset helper in the suite is now anchored to one instant per file (11
files). scripts/check-fixture-clock-races.ts keeps it that way, wired into
test:ci.
The checker only reports helpers that PROJECT a timestamp from an offset the
caller varies. Reading the clock live stays correct where the passage of time
is itself under test -- a polling waitFor, a lock-expiry comparison -- and
those are not reported. Its own tests pin both directions, because a checker
that cries wolf gets muted, and a muted checker is worse than none.
Writing those tests caught two bugs in the checker itself: it undercounted
arrow-form helpers by one (`const name = (` does not match the call shape the
`-1` assumed), which silently exempted the exactly-two-call case -- the
smallest set that can race -- and its operator-to-parameter span crossed commas,
so a token-expiry helper read as a fixture race.
Closes #9955
Contributor
|
Important 🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨 ⏳ LoopOver is waiting…LoopOver has seen this pull request and is waiting on CI checks to finish before reviewing it. This comment will update once the review runs. 🟩 Safe / merged · 🟦 Advisory · 🟨 Held for review · 🟥 Blocked / closed · 🟨 Waiting |
Contributor
|
Superagent didn't find any vulnerabilities or security issues in this PR. |
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
loopover-ui | caebc0b | Commit Preview URL Branch Preview URL |
Jul 30 2026, 09:18 PM |
Bundle ReportBundle size has no change ✅ |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #9958 +/- ##
=======================================
Coverage 91.88% 91.88%
=======================================
Files 928 928
Lines 113675 113675
Branches 27412 27412
=======================================
Hits 104445 104445
Misses 7931 7931
Partials 1299 1299
Flags with carried forward coverage won't be shown. Click here to find out more. |
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.
Closes #9955
The bug
A fixture that calls
Date.now()once per timestamp produces timestamps that disagree with each other by however long elapsed between the calls. Where the code under test derives a boundary from one of them, a single millisecond flips the result.queue-trends.test.tshad exactly that.buildWindowanchors on the newest snapshot:atDaysAgo(0)evaluated at T0 andatDaysAgo(7)a moment later at T1 put the "7 days ago" row atT1-7d— newer than the targetT0-7d. No baseline, every windowunavailable, assertion fails. It passed only when both calls landed in the same millisecond.Reproduced deterministically with a 2 ms offset. It fired for real on #9950 — a PR whose only changed file was a GitHub workflow, which is how it got noticed.
Why this is worse than an ordinary flake
Reviews are one-shot for everyone but the maintainer. A false red on a contributor PR is not a re-run away from fine — it auto-closes correct work the contributor cannot reopen. A test that fails on timing alone is a gate-correctness problem, not CI noise.
The sweep
Every offset helper in the suite is now anchored to one instant per file — 11 files. The production code was correct throughout; the fixtures were inconsistent with themselves.
scripts/check-fixture-clock-races.tskeeps it that way, wired intotest:ci.The checker is deliberately narrow
It reports only helpers that project a timestamp from an offset the caller varies. Reading the clock live stays correct where the passage of time is itself under test — a polling
waitFor, a lock-expiry comparison — and those are not reported. Both directions are pinned by its own tests, because a checker that cries wolf gets muted, and a muted checker is worse than none.Writing those tests caught two bugs in the checker itself:
const name = (does not match thename(call shape the-1assumed), which silently exempted the exactly-two-call case — the smallest set that can actually race.Date.now() + 60 * 60_000, bodyreached an unrelatedbodyidentifier and reported a token-expiry helper as a fixture race.Full suite green: 25,883 passing, zero failures. No
src/**changes, socodecov/patchis unaffected.