Skip to content

Conversation

@robobun
Copy link
Collaborator

@robobun robobun commented Jan 27, 2026

Summary

  • Adds advanceTimers option to jest.useFakeTimers() for automatic timer advancement
  • advanceTimers: true advances fake time by 20ms every 20ms of real time (matching Jest's default behavior)
  • advanceTimers: <number> advances fake time by that number of milliseconds
  • Enables compatibility with @testing-library/user-event and similar libraries that wait for timers during interactions

Implementation Details

Uses a real-time EventLoopTimer with a new FakeTimersAutoAdvance tag that:

  • Runs on real time (not affected by fake timers)
  • Periodically advances the fake clock and fires any ready fake timers
  • Properly handles race conditions when useRealTimers() is called during timer callbacks

Test plan

  • New regression tests in test/regression/issue/26037.test.ts covering:
    • advanceTimers: true auto-advances timers
    • advanceTimers: <number> advances by specified amount
    • advanceTimers: false does not auto-advance
    • Default behavior (no advanceTimers option) remains unchanged
    • useRealTimers() properly stops auto-advancing
    • Works with setInterval
    • Works combined with now option
    • Handles race condition when useRealTimers() called during auto-advance
  • Existing fake timer tests pass (test/js/bun/test/fake-timers/fake-timers.test.ts)

Fixes #26037

🤖 Generated with Claude Code

Adds support for the `advanceTimers` option in `jest.useFakeTimers()`,
which automatically advances fake time in real time. This is critical
for compatibility with libraries like @testing-library/user-event that
wait for timers to resolve during interactions.

- `advanceTimers: true` advances fake time by 20ms every 20ms (Jest default)
- `advanceTimers: <number>` advances fake time by that amount every real-time interval
- `advanceTimers: false` (or omitting the option) keeps the existing manual-advance behavior

The implementation uses a real-time EventLoopTimer with a new `FakeTimersAutoAdvance`
tag that fires fake timer callbacks while preserving the mocked time semantics.

Fixes #26037

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@robobun
Copy link
Collaborator Author

robobun commented Jan 27, 2026

Updated 4:08 AM PT - Jan 27th, 2026

❌ Your commit a89dcc1f has 9 failures in Build #35952 (All Failures):


🧪   To try this PR locally:

bunx bun-pr 26493

That installs a local version of the PR into your bun-26493 executable, so you can run:

bun-26493 --bun

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jan 27, 2026

Walkthrough

Selects per-timer time reference (real vs mocked) in timer core, adds a FakeTimersAutoAdvance tag and real-time-driven auto-advance machinery, exposes an advanceTimers option in FakeTimersConfig, and adds regression tests covering auto-advance and fake/real timer interactions.

Changes

Cohort / File(s) Summary
Timer core
src/bun.js/api/Timer.zig
next() extended to accept and track both mocked (now) and real (real_now) timestamps; chooses which to use per-timer via timer.tag.allowFakeTimers(); updates removal to set timer.in_heap = .none; drainTimers() passes and initializes real-time parameters.
EventLoopTimer Tag
src/bun.js/api/Timer/EventLoopTimer.zig
Added FakeTimersAutoAdvance Tag variant and mapped type to jsc.Jest.bun_test.FakeTimers; allowFakeTimers() treats it as disallowing fake timers; fire() handles .FakeTimersAutoAdvance by delegating to fake_timers.onAutoAdvanceTimer(vm).
FakeTimers implementation
src/bun.js/test/timers/FakeTimers.zig
Added auto_advance_timer and auto_advance_interval_ms; activate() accepts auto_advance_ms; new APIs onAutoAdvanceTimer, scheduleAutoAdvanceTimer, stopAutoAdvanceTimer; real-time auto-advance scheduling/execution, in-heap state updates, re-entrancy guards, and cleanup adjustments.
FakeTimers config binding
src/bun.js/test/timers/FakeTimersConfig.bindv2.ts
Added exported advanceTimers field to FakeTimersConfig (type: b.RawAny, internalName: "advanceTimers").
Tests
test/regression/issue/26037.test.ts
New regression test validating auto-advance default and numeric intervals, disabling auto-advance, switching between fake and real timers, setInterval interaction, now integration, and a race where useRealTimers is invoked during an auto-advance cycle.

Possibly related PRs

Suggested reviewers

  • alii
  • nektro
🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately summarizes the main change: adding an advanceTimers option to jest.useFakeTimers() for automatic timer advancement.
Description check ✅ Passed The description follows the template with clear sections covering what the PR does, implementation details, and test plan, providing comprehensive information about the feature.
Linked Issues check ✅ Passed The PR implements all coding requirements from issue #26037: adds advanceTimers option with true/numeric/false support, implements real-time auto-advance mechanism, handles race conditions, and maintains backward compatibility.
Out of Scope Changes check ✅ Passed All changes are directly related to implementing the advanceTimers feature for fake timers and include a comprehensive regression test covering the feature scope.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

Warning

Review ran into problems

🔥 Problems

Errors were encountered while retrieving linked issues.

Errors (1)
  • TIMERS-26037: Entity not found: Issue - Could not find referenced Issue.

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
src/bun.js/api/Timer.zig (1)

291-334: Pass the correct timespec to timers that don't allow fake timers.
Line 54 always calls t.fire(&now, ...), but next() only initializes now for timers where allowFakeTimers() is true. Timers like BunTest, WTFTimer, and EventLoopDelayMonitor (which have allowFakeTimers() == false) use the now parameter in their fire callbacks but receive an uninitialized value.

✅ Suggested fix
        while (this.next(&has_set_now, &now, &has_set_real_now, &real_now)) |t| {
+            const fire_now = if (!t.tag.allowFakeTimers()) &real_now else &now;
-            t.fire(&now, vm);
+            t.fire(fire_now, vm);
        }
🤖 Fix all issues with AI agents
In `@src/bun.js/test/timers/FakeTimers.zig`:
- Around line 8-17: Make the two internal-only fields auto_advance_timer and
auto_advance_interval_ms private by renaming them to `#auto_advance_timer` and
`#auto_advance_interval_ms` (matching the existing private field pattern like
`#active`); update all internal references/usages of auto_advance_timer and
auto_advance_interval_ms within the same struct/module to the new private names,
and if external access is required create explicit accessors or methods instead
of keeping them public.
- Around line 312-329: The numeric-path for config.advanceTimers currently
converts advance_num via `@intFromFloat` after range checks but misses a finite
check, so NaN slips past (since NaN < 1 and NaN > maxInt are both false); update
the branch handling config.advanceTimers.isNumber() (using symbols advance_num,
`@intFromFloat`, auto_advance_ms) to first validate std.math.isFinite(advance_num)
and if not finite call globalObject.throwInvalidArguments("'advanceTimers' must
be a finite number", .{}), then perform the existing range checks and finally
assign auto_advance_ms = `@intFromFloat`(advance_num).

Address review feedback:
- Rename auto_advance_timer to #auto_advance_timer (private)
- Rename auto_advance_interval_ms to #auto_advance_interval_ms (private)
- Add std.math.isFinite check to reject NaN/Infinity for advanceTimers option

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@robobun
Copy link
Collaborator Author

robobun commented Jan 27, 2026

Addressed review feedback in commit 9efc218:

  • Made auto_advance_timer and auto_advance_interval_ms private by renaming to #auto_advance_timer and #auto_advance_interval_ms
  • Added std.math.isFinite() check to properly reject NaN/Infinity values for the advanceTimers option

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Fix all issues with AI agents
In `@src/bun.js/test/timers/FakeTimers.zig`:
- Around line 205-215: After running this.executeUntil(globalObject, target) we
must avoid unconditionally setting current_time back to target because callbacks
may have advanced time past target; instead, while still holding the timers.lock
and after the existing isActive check, read the current time/offset from
current_time (using the existing current_time accessors) and only call
current_time.set(globalObject, .{ .offset = &target }) if target is strictly
greater than the current offset; keep the existing timers.lock, defer unlock,
and preserve the isActive check (symbols: executeUntil, timers.lock, isActive,
current_time.set, target, globalObject).

Address review feedback: callbacks during executeUntil may manually
advance time past target via advanceTimersByTime(), so we should only
set current_time to target if it would actually advance time forward.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@robobun
Copy link
Collaborator Author

robobun commented Jan 27, 2026

Addressed additional review feedback in commit f224b8b:

  • Only advance current_time to target if it's strictly greater than the current offset
  • This prevents overwriting a time that callbacks may have advanced past target via advanceTimersByTime()

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Fix all issues with AI agents
In `@src/bun.js/test/timers/FakeTimers.zig`:
- Around line 242-254: Add a defensive assertion at the start of
stopAutoAdvanceTimer to verify the required timers lock is held before
proceeding: assert the appropriate mutex/lock (the timers heap lock referenced
by this.timers or vm.timer) is currently owned/held, then continue with the
existing logic that manipulates this.#auto_advance_timer, this.timers, and
vm.timer; this prevents accidental misuse when calling stopAutoAdvanceTimer.

Add defensive assertion to verify the timers lock is held before
manipulating timer state, preventing accidental misuse.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@robobun
Copy link
Collaborator Author

robobun commented Jan 27, 2026

Addressed review feedback in commit a481971:

  • Added assertValid(.locked) assertion to stopAutoAdvanceTimer to verify the timers lock is held before manipulating timer state

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Fix all issues with AI agents
In `@src/bun.js/test/timers/FakeTimers.zig`:
- Around line 227-240: scheduleAutoAdvanceTimer currently always calls
vm.timer.insert(&this.#auto_advance_timer), which can double-insert the
intrusive timer when useFakeTimers() is called multiple times; before inserting,
check the timer state (this.#auto_advance_timer.state) and only set next/state
and call vm.timer.insert(...) if the state is not already .PENDING (or otherwise
already scheduled). Update scheduleAutoAdvanceTimer (and the similar block at
351-354) to short-circuit when the timer is already scheduled to prevent
duplicate insertions into the heap.

Add check to scheduleAutoAdvanceTimer to prevent duplicate insertion
when useFakeTimers() is called multiple times. The timer is only
inserted if in_heap is .none.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@robobun
Copy link
Collaborator Author

robobun commented Jan 27, 2026

Addressed review feedback in commit a89dcc1:

  • Added check in scheduleAutoAdvanceTimer to prevent double-insertion when useFakeTimers() is called multiple times
  • The timer is only inserted if in_heap is .none, preventing duplicate heap entries

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Test using useFakeTimers hangs after simulating click event (testing-library/react)

2 participants