test(solid): deterministic clock in client-recovery-record.spec.ts - #3635
Conversation
The "already rejected at hydration" case asserted `waitedMs < 5` on the
wall clock. `waitedMs` is the delta between two `performance.now()`
reads in the runtime — at boundary registration and when the queued
`recover` microtask runs — and on a loaded, coverage-instrumented CI
worker that hop read 5.3ms, 7.3ms and 13.3ms today. A failed assertion
also skipped the test's own `off()`, leaking the listener into the next
test, whose `observed("recovery") === false` check then failed too.
`performance.now()` is now a scripted clock (the #3598 pattern): it
stands still unless the test moves it, so every duration is asserted
exactly — `waitedMs: 0` for the already-rejected case, `30` for the
streaming case where the test advances the clock before rejecting, and
`renderMs: 2` because rendering the children advances it by 2. The
subscription is torn down in `afterEach`, so a failure cannot cascade.
|
Coverage Report for CI Build 35984972807Coverage remained the same at 73.134%Details
Uncovered ChangesNo uncovered changes found. Coverage RegressionsNo coverage regressions found. Coverage Stats
💛 - Coveralls |
Merging this PR will not alter performance
Comparing Footnotes
|
The flake
packages/solid/test/client-recovery-record.spec.ts— "a fragment already rejected at hydration: no wait, a fresh render" — assertedwaitedMs < 5on the wall clock.waitedMsis the delta between twoperformance.now()reads inside the runtime: one at boundary registration (registeredAt, read only when a"recovery"listener exists) and one when the queuedrecovermicrotask fires. That hop covers the rest of thecreateRootbody, aflush(), and the microtask checkpoint — sub-millisecond locally, but on the coverage-instrumented CI job under load it read:nextpush after feat(signals): OPTIMISTIC_REVERTED — an optimistic value the screen showed was replaced #3607 merged, https://github.com/solidjs/solid/actions/runs/35972694277/job/107545856942Each time the failed assertion also skipped the test's own
off(), so the"recovery"listener leaked into the following test, "with no listener the boundary still recovers, and nothing is recorded", whoseobserved("recovery") === falsecheck then failed withexpected true to be false. Reproduced locally by inserting a 6ms synchronous stall betweenboundary()and theawait: same two failures, same lines.The fix (test only)
performance.now()is a scripted clock viavi.spyOn(the test(web): run the performance-tracks spec on a controllable engine clock #3598 pattern, as inattribution-interactions.test.ts): it stands still unless the test moves it, so every duration is asserted exactly instead of against a threshold.waitedMs: 0(the clock never moves between registration and recovery — the person saw no fallback)waitedMs: 30renderMs: 2in both, proving the fresh render sits between the runtime's last two readsperformance.nowis asserted never called (the doc comment's "no listener, no clock read")"recovery"subscription is stored module-level and torn down inafterEachalongsidevi.restoreAllMocks(), so an assertion failure can no longer cascade into the next test.Real timers still drive the promise settles; only the stamps are the test's. Verified: spec 20/20 green in a loop; the stall no longer affects the result; a forced failure in test 2 leaves test 4 green; full
solid-jssuite (780) and the@solidjs/webclient suite native +JSX_COMPILER=babel(944 each) pass.Public API
None.
— Claude via Cursor