test(lib): make idle deadline reset timing deterministic

[luvs01]

Summary

The idleDeadline reset/postpone test assumes each requested 40 ms sleep resumes before a 120 ms idle deadline. Under scheduling delay, the production timer can correctly fire before the next reset, making the test fail even though the primitive's contract is satisfied. This occurred in the macOS control for #4036: the test reported 432.21 ms and observed one firing where it expected zero. The primitive and this test were unchanged by that PR; individual callback timings were not logged, so the exact delayed interval is unknown.

Make only this boundary case synchronous with a scoped timer fixture. The fixture models generic timer scheduling/cancellation, while the real idleDeadline implementation handles resets and terminal state. Advance 40 ms and reset four times, verify no firing through another 119 ms, then verify exactly one firing at the next millisecond and no later repeat. Individual spies are restored in nested finally blocks. The five other cases still exercise Bun's real timers. Runtime code and user behavior are unchanged, so no documentation update is needed.

Verification

• Head 9aa3e9204c12c1bbd9068e77115501e16203bb60, based on dev 7dc7dc99e65268bc8764e19840952256b030bce9; Bun 1.4.0 on Windows. Only tests/lib/abort-idle-deadline.test.ts changes.
• A controlled copy of the original reset test that resumes requested 40 ms waits after 160 ms failed with expected 0 / received 1 (644.95 ms). This demonstrates the scheduling assumption; it does not claim to reproduce the exact hosted-CI scheduling event.
• bun run test -- --timeout 60000 --parallel=1 tests/lib/abort-idle-deadline.test.ts: 6 tests / 12 assertions passed, 678 ms, including all five unchanged real-timer cases. Cases following the modified test run after spy restoration.
• Two isolated source ablations were rejected by the new fixture: removing cancellation before rearming, and making repeated resets no-ops while a timer exists. Each produced the expected 0 / 1 assertion failure. The selected real-timer pause/rearm control still passed after the failed assertion, checking that the spies were restored on the failure path. Canonical runtime source was not modified; temporary ablation/reproduction files were removed after verification.
• bun run typecheck, bun run privacy:scan, and git diff --check passed. Independent read-only review found no required corrections in mock lifetime, timer handles, or assertion coverage.
• CodeRabbit reviewed the current patch and found no blocking issue after checking spy lifetime, deadline boundaries, and regression sensitivity. No inline findings are outstanding at this update. Full contributor CI passed 26/26 jobs on exact head 9aa3e9204, including all Windows shards and the macOS control. Current dev remains 7dc7dc99e.

Checklist

• Scope stays focused and avoids unrelated cleanup.
• Docs or release notes were updated when needed.
• Security-sensitive changes were reviewed for secrets, auth, and unsafe defaults.

Review readiness checklist

• All CI tests are green on my local testing.
• I pushed my PR to the latest dev commit.
• I resolved all correct Codex and CodeRabbit findings.
• My PR is ready for review.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Advanced

Run ID: b8f9eabe-0cdd-4eaa-80f6-bd3d2f74da37

📥 Commits

Reviewing files that changed from the base of the PR and between 7dc7dc9 and 9aa3e92.

📒 Files selected for processing (1)

• tests/lib/abort-idle-deadline.test.ts

Included review availability: Your plan provides up to 10 included reviews per hour; 7 remain after this review.

---

📝 Walkthrough

Walkthrough

Changes

Idle deadline test validation

Layer / File(s)	Summary
Virtual clock reset validation tests/lib/abort-idle-deadline.test.ts	The reset re-arm test uses mocked setTimeout and clearTimeout calls with a virtual clock. It verifies that repeated resets postpone firing, firing occurs once after 120 ms of silence, cancel() remains permanent, and mocked globals are restored.

Priority: ⬇️ Low — Defer this test-only change because it is limited to deterministic idle-deadline timing coverage with no runtime behavior changes.

Estimated code review effort: 2 (Simple) | ~10 minutes

Merge Risk: 🟡 Moderate · up to 9aa3e

The timer-test rewrite does not change runtime behavior, but a reported duplicate declaration may prevent the test file from compiling. Confirming a clean typecheck is needed before merge.

Suggested reviewers: lidge-jun

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 1…
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and concisely describes the main change: making the idleDeadline reset timing test deterministic. It matches the test-only scope and uses specific, meaningful terminology.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[github-actions]

✅ Deterministic PR hygiene checks passed.

[github-actions]

✅ READY

• all PR quality gates passed; the review readiness checklist is complete.

Review readiness checklist

• ✅ All CI tests are green on my local testing.
• ✅ I pushed my PR to the latest dev commit.
• ✅ I resolved all correct Codex and CodeRabbit findings.
• ✅ My PR is ready for review.

✅ 4/4 boxes ticked.

This pull request has been marked Ready for Review.
The review-ready label marks this PR as ready; review automation runs independently.
Maintainers notified: @lidge-jun @Ingwannu

Hygiene

✅ Deterministic PR hygiene checks passed.

[luvs01]

@coderabbitai review

Please review head 9aa3e9204c12c1bbd9068e77115501e16203bb60 against 7dc7dc99e65268bc8764e19840952256b030bce9, focusing on timer-spy lifetime and whether the synchronous fixture preserves meaningful reset/deadline coverage.

Only the reset/postpone test changes behavior; runtime code and five real-timer tests stay unchanged. The virtual fixture models scheduling/cancellation only, runs without await while global spies are installed, and restores those individual spies in nested finally blocks. Six tests / twelve assertions pass. Controlled late resumption reproduces the old assertion failure, and isolated mutations removing cancellation or disabling repeated reset both fail the new test; the real-timer pause/rearm control still passes after each failed assertion, exercising cleanup. Typecheck, privacy scan, diff check, and independent review passed. Full contributor CI is separate and pending.

[coderabbitai]

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

base='7dc7dc99e65268bc8764e19840952256b030bce9'
head='9aa3e9204c12c1bbd9068e77115501e16203bb60'

echo '=== Changed files and diff ==='
git diff --stat "$base" "$head"
git diff --check "$base" "$head"
git diff --find-renames --unified=100 "$base" "$head" -- tests/lib/abort-idle-deadline.test.ts

echo '=== Changed test outline ==='
ast-grep outline tests/lib/abort-idle-deadline.test.ts --items all || true

echo '=== Changed test, numbered ==='
nl -ba tests/lib/abort-idle-deadline.test.ts

echo '=== idleDeadline definitions and references ==='
rg -n -C 5 --glob '!tests/lib/abort-idle-deadline.test.ts' \
  '\bidleDeadline\b|function\s+idleDeadline|const\s+idleDeadline|export.*idleDeadline' .

echo '=== Timer API declarations and imports in relevant source candidates ==='
rg -n -C 4 --glob '*.{ts,tsx,js,mjs,cjs}' \
  '\b(setTimeout|clearTimeout|setInterval|clearInterval)\b' \
  src lib packages tests 2>/dev/null || true

Length of output: 6116

---

🏁 Script executed:

#!/bin/bash
set -euo pipefail

head='9aa3e9204c12c1bbd9068e77115501e16203bb60'

echo '=== Numbered changed test at reviewed head ==='
git show "${head}:tests/lib/abort-idle-deadline.test.ts" | awk '{ printf "%6d  %s\n", NR, $0 }'

echo '=== Locate abort implementation candidates at reviewed head ==='
git ls-tree -r --name-only "$head" | grep -E '(^|/)(abort|abort\.(ts|tsx|js|mjs|cjs))$|/abort\.(ts|tsx|js|mjs|cjs)$' || true

echo '=== Production implementation: src/lib/abort.ts ==='
git show "${head}:src/lib/abort.ts" | awk '{ printf "%6d  %s\n", NR, $0 }'

echo '=== Timer API and idleDeadline context in tracked source ==='
git grep -n -E '\bidleDeadline\b|\b(setTimeout|clearTimeout)\b' "$head" -- 'src/**' \
  | head -n 300 || true

echo '=== Parent test version for direct comparison ==='
git show '7dc7dc99e65268bc8764e19840952256b030bce9:tests/lib/abort-idle-deadline.test.ts' \
  | awk '{ printf "%6d  %s\n", NR, $0 }'

Length of output: 53673

---

@luvs01 Review complete. I found no blocking issue.

tests/lib/abort-idle-deadline.test.ts Lines 51-71 contain both global timer spies within the synchronous test path. The test does not await while the spies are active. The nested finally blocks first cancel the pending idleDeadline timer, then restore clearTimeout, then restore setTimeout. This order is correct for src/lib/abort.ts Lines 45-67, where cancel() can call clearTimeout.

The synchronous fixture preserves the meaningful reset/deadline coverage:

• Lines 53-57 verify that four resets at 40 ms intervals prevent the original 120 ms deadline from firing.
• Lines 59-62 verify that the final reset does not fire at 119 ms and fires at exactly 120 ms.
• Lines 63-64 verify the terminal one-shot behavior.
• The fixture removes a fired timer before it invokes its callback. This matches the one-shot scheduling model required by idleDeadline.
• The fixture processes timers created by callbacks during advanceBy(). This is sufficient for the current primitive and avoids hiding re-arming defects.

The test directly detects both key regressions: failure to cancel the old handle before re-arming, and failure to re-arm when a pending timer exists. The remaining five cases still use real timers.

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[lidge-jun]

리뷰 · 우선순위 63 / 80

이 PR은 런타임 코드가 아니라 테스트만 고친다. 대상은 tests/lib/abort-idle-deadline.test.ts의 idleDeadline reset() re-arms and postpones firing 한 케이스다. 지금 dev HEAD는 7dc7dc99e(#4037 provider-runtime 스택 기록 머지 뒤)이고, 실제 구현은 src/lib/abort.ts의 idleDeadline이다. 동작은 단순하다. 조용한 시간이 idleMs를 넘으면 콜백을 한 번만 부르고, reset()은 타이머를 지운 뒤 다시 건다. pause()는 끄기만 하고, cancel()은 끝난 상태로 만든다.

문제는 테스트가 “40ms씩 네 번 자고 매번 reset하면, 전체는 160ms를 넘어도 한 번도 안 터져야 한다”고 가정한 점이다. 스케줄러가 밀리면 실제 sleep(40)이 120ms 창보다 늦게 깨어날 수 있다. 그때 프로덕션 타이머가 맞게 먼저 울리면, 구현은 올바른데 테스트만 “expected 0 / received 1”로 깨진다. 작성자가 말한 macOS control(#4036 관련)에서 432ms로 관측된 게 그 형태다. 즉 flaky test이지 idleDeadline 버그가 아니다.

이 PR은 그 경계 케이스만 spyOn(globalThis, "setTimeout"/"clearTimeout")으로 가짜 시계를 만들고, advanceBy로 시간을 직접 민다. 40ms 네 번 reset 뒤에도 0, 119ms 더 가도 0, 1ms 더 가면 정확히 1, 그 뒤 240ms를 더 가도 1인 채로 남는지 본다. finally에서 cancel과 spy 복원을 중첩해, 중간에 실패해도 다른 테스트의 진짜 타이머를 더럽히지 않게 했다. 나머지 다섯 케이스는 그대로 Bun 실제 타이머를 쓴다. 런타임·문서 변경은 없다.

지금 dev가 쫓는 2.49.x 제품 팁(#4002 근처)이나 Qoder/Hermes/Gemini 스택과는 직교한다. types.ts/config.ts 분할에도 안 걸린다. CI 쪽 gate(hygiene / enforce-target / label / CodeRabbit)는 통과했고, 작성자 포크 전체 CI도 head 9aa3e9204에서 초록이라고 적혀 있다. 범위가 한 파일이라 리뷰 부담도 작다.

라인 쪽:
라인 20-34 - 가짜 setTimeout이 delay를 now + delay로만 쌓고, 인자(...args)는 fire 때 넘긴다. idleDeadline은 인자를 안 쓰니 지금은 충분하다.
라인 35-45 - advanceBy는 due 타이머를 하나씩 꺼내 시간 순으로 발사한다. 같은 시각에 여러 개가 있어도 루프가 이어서 처리한다.
라인 55-65 - spy 복원이 안쪽 finally에 있어서, idle.cancel()이 抛더라도 mock이 남기지 않는다. 좋은 패턴이다.
경로 src/lib/abort.ts - 구현 자체는 손대지 않았다. 테스트만 계약(무음 구간 기준 re-arm)을 고정한다.

메인테이너의 판단이 필요한 지점

• 이 한 케이스만 가짜 시계로 빼는 게 맞는지, 아니면 다섯 케이스 전부를 같은 픽스처로 통일할지(지금은 의도적으로 혼합)
• upstream Actions에서 포크 PR full suite를 다시 돌릴지, 작성자 포크 초록만으로 머지할지
• #4036(port reclaim)과 같은 레인에 묶인 flaky인지, 이 테스트만 독립으로 머지해도 되는지

너의 추천
review-ready이고 범위가 테스트 flake 제거라서 지금 머지해도 된다. 런타임 위험이 없고, CI 노이즈만 줄인다. 머지 후 leftover 이슈는 없다. 따로 닫을 이유도 없다.

이 댓글은 grok-bot이 작성했습니다

[lidge-jun]

Landed on dev in cfff026 via #4049, which carried all nine PRs of this train as one sequentially reviewed integration branch. Your commit is preserved in the squash body with a Co-authored-by trailer, so the contribution stays attributed to you.

Verification at the exact merged head 8c58bac0863f1f2ca2b430e1177a0e417b474c85: Cross-platform CI run 34256853348 finished with 19 jobs successful and 0 failures, bun x tsc --noEmit exit 0, and 312 focused tests passing across the ten affected files. An independent review confirmed the stacked tree is byte-identical to the union of the nine PR diffs.

Closing this PR because the change is already on dev. Thank you.

[lidge-jun]

Landed via #4049 at cfff026