Fixed flaky timeout-threshold tests racing real setTimeout#27690
Fixed flaky timeout-threshold tests racing real setTimeout#27690
Conversation
no ref
- {{#get}} and {{#recommendations}} timeout tests asserted that the
helper's setTimeout(threshold=1ms) fired before the API stub's
setTimeout(5ms), which is fundamentally a Node scheduler race
- under CI load, both timers can land in the same macrotask and the
ordering becomes unreliable, e.g. recommendations.test.js:173 in
run 25404408183 and get.test.js:562 in run 25399938361
- install sinon fake timers in each test, kick off the helper, then
tickAsync(2) — that fires the 1ms threshold timer but not the 5ms
API timer, so the timeout branch wins deterministically
- limited toFake to [setTimeout, clearTimeout] so Date and other
timers aren't affected (the get notify test still uses real time)
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
WalkthroughTwo frontend helper unit tests were modified to use Sinon fake timers for deterministic timeout behavior. Timeout tests were rewritten to capture response promises before advancing a fake clock, use 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
no ref - replaced two-line "advance past threshold" comment with the single-line bound check that explains the magic number 2
Summary
The
{{#get}}and{{#recommendations}}"timeout threshold" tests asserted that the helper'ssetTimeout(threshold=1ms)fires before the API stub'ssetTimeout(5ms). That's a Node scheduler race, not a behavior assertion.Switch each test to
sinon.useFakeTimers({toFake: ['setTimeout', 'clearTimeout']}), kick off the helper, thenclock.tickAsync(2)— fires the 1ms threshold timer but not the 5ms stub timer, so the timeout branch wins deterministically.Why
Under CI load both timers can land in the same macrotask and the ordering becomes unreliable. Recent hits:
recommendations.test.js:173— run 25404408183 (main)get.test.js:562— run 25399938361 (renovate-vite)The pattern dates back to commit
270f288c48forget.test.js; PR #27571 tried to harden the recommendations variant on 2026-04-27 by awaitingcachePartials, but that fixed setup, not the timer race.toFakeis scoped tosetTimeout/clearTimeoutsoDateand other clocks aren't affected — thenotifytest in the same file still relies on realDate.now()and is unchanged (it isn't a race; it just measures elapsed time after the API resolves).Test plan
recommendations.test.js(4 passing) andget.test.js(47 passing) clean.