Skip to content

Conversation

@KurtGokhan
Copy link

What does this PR do?

The vi object has nearly every function in the new Jest fake timers APIs. But setSystemTime was missing. This PR adds the TS declaration, Zig binding, and some unit tests for it.

Related issue: #16142

How did you verify your code works?

Added some unit tests for the setSystemTime function. Built the bun-debug binary locally and ran the tests with it. Made sure all tests passed.

@KurtGokhan KurtGokhan requested a review from alii as a code owner December 7, 2025 22:16
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Dec 7, 2025

Walkthrough

Adds a new setSystemTime member to the Bun vi mocking namespace that exposes jest.setSystemTime, including TypeScript type definitions, runtime implementation in the test module setup, and comprehensive test coverage for setting system time with both Date objects and numeric timestamps.

Changes

Cohort / File(s) Summary
Type definitions
packages/bun-types/test.d.ts
Adds setSystemTime member to the vi namespace in the "bun:test" module, typing it as typeof jest.setSystemTime
Runtime implementation
src/bun.js/test/jest.zig
Initializes vi.setSystemTime as a new property in the Jest mock object, increasing the vi object allocation from 6 to 7 slots
Test coverage
test/js/bun/test/fake-timers/fake-timers.test.ts
Adds three test cases verifying vi.setSystemTime functionality with Date objects, numeric timestamps, and sequential time updates

Possibly related PRs

Pre-merge checks

✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately summarizes the main change: adding the setSystemTime function to the vi object for Vitest/Jest compatibility.
Description check ✅ Passed The description follows the required template with both sections completed: it explains what the PR does and how verification was performed.

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

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: 1

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Disabled knowledge base sources:

  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 73c3f00 and 99d46b5.

📒 Files selected for processing (3)
  • packages/bun-types/test.d.ts (1 hunks)
  • src/bun.js/test/jest.zig (1 hunks)
  • test/js/bun/test/fake-timers/fake-timers.test.ts (1 hunks)
🧰 Additional context used
📓 Path-based instructions (6)
src/**/*.{cpp,zig}

📄 CodeRabbit inference engine (.cursor/rules/building-bun.mdc)

src/**/*.{cpp,zig}: Use bun bd or bun run build:debug to build debug versions for C++ and Zig source files; creates debug build at ./build/debug/bun-debug
Run tests using bun bd test <test-file> with the debug build; never use bun test directly as it will not include your changes
Execute files using bun bd <file> <...args>; never use bun <file> directly as it will not include your changes
Enable debug logs for specific scopes using BUN_DEBUG_$(SCOPE)=1 environment variable
Code generation happens automatically as part of the build process; no manual code generation commands are required

Files:

  • src/bun.js/test/jest.zig
src/**/*.zig

📄 CodeRabbit inference engine (.cursor/rules/building-bun.mdc)

Use bun.Output.scoped(.${SCOPE}, .hidden) for creating debug logs in Zig code

Implement core functionality in Zig, typically in its own directory in src/

src/**/*.zig: Private fields in Zig are fully supported using the # prefix: struct { #foo: u32 };
Use decl literals in Zig for declaration initialization: const decl: Decl = .{ .binding = 0, .value = 0 };
Prefer @import at the bottom of the file (auto formatter will move them automatically)

Be careful with memory management in Zig code - use defer for cleanup with allocators

Files:

  • src/bun.js/test/jest.zig
**/*.zig

📄 CodeRabbit inference engine (.cursor/rules/zig-javascriptcore-classes.mdc)

**/*.zig: Expose generated bindings in Zig structs using pub const js = JSC.Codegen.JS<ClassName> with trait conversion methods: toJS, fromJS, and fromJSDirect
Use consistent parameter name globalObject instead of ctx in Zig constructor and method implementations
Use bun.JSError!JSValue return type for Zig methods and constructors to enable proper error handling and exception propagation
Implement resource cleanup using deinit() method that releases resources, followed by finalize() called by the GC that invokes deinit() and frees the pointer
Use JSC.markBinding(@src()) in finalize methods for debugging purposes before calling deinit()
For methods returning cached properties in Zig, declare external C++ functions using extern fn and callconv(JSC.conv) calling convention
Implement getter functions with naming pattern get<PropertyName> in Zig that accept this and globalObject parameters and return JSC.JSValue
Access JavaScript CallFrame arguments using callFrame.argument(i), check argument count with callFrame.argumentCount(), and get this with callFrame.thisValue()
For reference-counted objects, use .deref() in finalize instead of destroy() to release references to other JS objects

Files:

  • src/bun.js/test/jest.zig
test/**/*.{js,ts,jsx,tsx}

📄 CodeRabbit inference engine (.cursor/rules/writing-tests.mdc)

test/**/*.{js,ts,jsx,tsx}: Write tests as JavaScript and TypeScript files using Jest-style APIs (test, describe, expect) and import from bun:test
Use test.each and data-driven tests to reduce boilerplate when testing multiple similar cases

Files:

  • test/js/bun/test/fake-timers/fake-timers.test.ts
test/**/*.test.{ts,js,jsx,tsx,mjs,cjs}

📄 CodeRabbit inference engine (test/CLAUDE.md)

test/**/*.test.{ts,js,jsx,tsx,mjs,cjs}: Use bun:test with files that end in *.test.{ts,js,jsx,tsx,mjs,cjs}
Do not write flaky tests. Never wait for time to pass in tests; always wait for the condition to be met instead of using an arbitrary amount of time
Never use hardcoded port numbers in tests. Always use port: 0 to get a random port
Prefer concurrent tests over sequential tests using test.concurrent or describe.concurrent when multiple tests spawn processes or write files, unless it's very difficult to make them concurrent
When spawning Bun processes in tests, use bunExe and bunEnv from harness to ensure the same build of Bun is used and debug logging is silenced
Use -e flag for single-file tests when spawning Bun processes
Use tempDir() from harness to create temporary directories with files for multi-file tests instead of creating files manually
Prefer async/await over callbacks in tests
When callbacks must be used and it's just a single callback, use Promise.withResolvers to create a promise that can be resolved or rejected from a callback
Do not set a timeout on tests. Bun already has timeouts
Use Buffer.alloc(count, fill).toString() instead of 'A'.repeat(count) to create repetitive strings in tests, as ''.repeat is very slow in debug JavaScriptCore builds
Use describe blocks for grouping related tests
Always use await using or using to ensure proper resource cleanup in tests for APIs like Bun.listen, Bun.connect, Bun.spawn, Bun.serve, etc
Always check exit codes and test error scenarios in error tests
Use describe.each() for parameterized tests
Use toMatchSnapshot() for snapshot testing
Use beforeAll(), afterEach(), beforeEach() for setup/teardown in tests
Track resources (servers, clients) in arrays for cleanup in afterEach()

Files:

  • test/js/bun/test/fake-timers/fake-timers.test.ts
test/**/*.test.{ts,tsx}

📄 CodeRabbit inference engine (CLAUDE.md)

test/**/*.test.{ts,tsx}: For single-file tests in Bun test suite, prefer using -e flag over tempDir
For multi-file tests in Bun test suite, prefer using tempDir and Bun.spawn
Always use port: 0 when spawning servers in tests - do not hardcode ports or use custom random port functions
Use normalizeBunSnapshot to normalize snapshot output in tests instead of manual output comparison
Never write tests that check for no 'panic', 'uncaught exception', or similar strings in test output - that is not a valid test
Use tempDir from harness to create temporary directories in tests - do not use tmpdirSync or fs.mkdtempSync
In tests, call expect(stdout).toBe(...) before expect(exitCode).toBe(0) when spawning processes for more useful error messages on failure
Do not write flaky tests - do not use setTimeout in tests; instead await the condition to be met since you're testing the CONDITION, not TIME PASSING
Verify your test fails with USE_SYSTEM_BUN=1 bun test <file> and passes with bun bd test <file> - tests are not valid if they pass with USE_SYSTEM_BUN=1
Avoid shell commands in tests - do not use find or grep; use Bun's Glob and built-in tools instead
Test files must end in .test.ts or .test.tsx and be created in the appropriate test folder structure

Files:

  • test/js/bun/test/fake-timers/fake-timers.test.ts
🧠 Learnings (21)
📓 Common learnings
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: .cursor/rules/writing-tests.mdc:0-0
Timestamp: 2025-11-24T18:35:50.422Z
Learning: Applies to test/**/*.{js,ts,jsx,tsx} : Write tests as JavaScript and TypeScript files using Jest-style APIs (`test`, `describe`, `expect`) and import from `bun:test`
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-02T05:59:51.485Z
Learning: Applies to test/**/*.test.{ts,tsx} : Verify your test fails with `USE_SYSTEM_BUN=1 bun test <file>` and passes with `bun bd test <file>` - tests are not valid if they pass with `USE_SYSTEM_BUN=1`
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: test/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:37:30.259Z
Learning: Applies to test/**/*.test.{ts,js,jsx,tsx,mjs,cjs} : Do not set a timeout on tests. Bun already has timeouts
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-02T05:59:51.485Z
Learning: Applies to test/**/*.test.{ts,tsx} : For multi-file tests in Bun test suite, prefer using `tempDir` and `Bun.spawn`
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: .cursor/rules/registering-bun-modules.mdc:0-0
Timestamp: 2025-11-24T18:35:39.205Z
Learning: Add tests for new Bun runtime functionality
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: test/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:37:30.259Z
Learning: Applies to test/**/*.test.{ts,js,jsx,tsx,mjs,cjs} : When spawning Bun processes in tests, use `bunExe` and `bunEnv` from `harness` to ensure the same build of Bun is used and debug logging is silenced
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/bun.js/bindings/v8/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:36:59.706Z
Learning: Applies to src/bun.js/bindings/v8/test/v8/v8.test.ts : Add corresponding test cases to test/v8/v8.test.ts using checkSameOutput() function to compare Node.js and Bun output
Learnt from: Jarred-Sumner
Repo: oven-sh/bun PR: 22478
File: test/regression/issue/22475.test.ts:0-0
Timestamp: 2025-09-07T22:26:50.213Z
Learning: The `toBeDate()` matcher is acceptable and supported in Bun's test suite - don't suggest replacing it with `toBeInstanceOf(Date)`.
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: test/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:37:30.259Z
Learning: Applies to test/**/*.test.{ts,js,jsx,tsx,mjs,cjs} : Use `bun:test` with files that end in `*.test.{ts,js,jsx,tsx,mjs,cjs}`
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/bun.js/bindings/v8/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:36:59.706Z
Learning: Applies to src/bun.js/bindings/v8/test/v8/v8-module/main.cpp : Register new V8 API test functions in the Init method using NODE_SET_METHOD with exports object
📚 Learning: 2025-11-24T18:37:30.259Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: test/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:37:30.259Z
Learning: Applies to test/**/*.test.{ts,js,jsx,tsx,mjs,cjs} : Do not set a timeout on tests. Bun already has timeouts

Applied to files:

  • src/bun.js/test/jest.zig
  • packages/bun-types/test.d.ts
  • test/js/bun/test/fake-timers/fake-timers.test.ts
📚 Learning: 2025-11-24T18:36:59.706Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/bun.js/bindings/v8/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:36:59.706Z
Learning: Applies to src/bun.js/bindings/v8/test/v8/v8.test.ts : Add corresponding test cases to test/v8/v8.test.ts using checkSameOutput() function to compare Node.js and Bun output

Applied to files:

  • src/bun.js/test/jest.zig
  • packages/bun-types/test.d.ts
  • test/js/bun/test/fake-timers/fake-timers.test.ts
📚 Learning: 2025-11-24T18:36:59.706Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/bun.js/bindings/v8/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:36:59.706Z
Learning: Applies to src/bun.js/bindings/v8/test/v8/v8-module/main.cpp : Register new V8 API test functions in the Init method using NODE_SET_METHOD with exports object

Applied to files:

  • src/bun.js/test/jest.zig
  • packages/bun-types/test.d.ts
📚 Learning: 2025-11-24T18:37:30.259Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: test/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:37:30.259Z
Learning: Applies to test/**/*.test.{ts,js,jsx,tsx,mjs,cjs} : When spawning Bun processes in tests, use `bunExe` and `bunEnv` from `harness` to ensure the same build of Bun is used and debug logging is silenced

Applied to files:

  • src/bun.js/test/jest.zig
  • packages/bun-types/test.d.ts
📚 Learning: 2025-11-24T18:36:59.706Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/bun.js/bindings/v8/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:36:59.706Z
Learning: Applies to src/bun.js/bindings/v8/test/v8/v8-module/main.cpp : Create test functions in test/v8/v8-module/main.cpp that take FunctionCallbackInfo<Value> parameter, use the test V8 API, print results for comparison with Node.js, and return Undefined

Applied to files:

  • src/bun.js/test/jest.zig
📚 Learning: 2025-11-24T18:35:50.422Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: .cursor/rules/writing-tests.mdc:0-0
Timestamp: 2025-11-24T18:35:50.422Z
Learning: Applies to test/**/*.{js,ts,jsx,tsx} : Write tests as JavaScript and TypeScript files using Jest-style APIs (`test`, `describe`, `expect`) and import from `bun:test`

Applied to files:

  • src/bun.js/test/jest.zig
  • packages/bun-types/test.d.ts
📚 Learning: 2025-12-02T05:59:51.485Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-02T05:59:51.485Z
Learning: Applies to src/bun.js/bindings/**/*.cpp : Cache structures in ZigGlobalObject for C++ JavaScript classes

Applied to files:

  • src/bun.js/test/jest.zig
📚 Learning: 2025-11-24T18:35:39.205Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: .cursor/rules/registering-bun-modules.mdc:0-0
Timestamp: 2025-11-24T18:35:39.205Z
Learning: Applies to src/bun.js/api/BunObject.zig : Implement getter functions in `src/bun.js/api/BunObject.zig` that return your feature, and export them in the `exportAll()` function

Applied to files:

  • src/bun.js/test/jest.zig
📚 Learning: 2025-12-02T05:59:51.485Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-02T05:59:51.485Z
Learning: Applies to test/**/*.test.{ts,tsx} : For multi-file tests in Bun test suite, prefer using `tempDir` and `Bun.spawn`

Applied to files:

  • src/bun.js/test/jest.zig
  • packages/bun-types/test.d.ts
  • test/js/bun/test/fake-timers/fake-timers.test.ts
📚 Learning: 2025-10-25T17:20:19.041Z
Learnt from: theshadow27
Repo: oven-sh/bun PR: 24063
File: test/js/bun/telemetry/server-header-injection.test.ts:5-20
Timestamp: 2025-10-25T17:20:19.041Z
Learning: In the Bun telemetry codebase, tests are organized into two distinct layers: (1) Internal API tests in test/js/bun/telemetry/ use numeric InstrumentKind enum values to test Zig↔JS injection points and low-level integration; (2) Public API tests in packages/bun-otel/test/ use string InstrumentKind values ("http", "fetch", etc.) to test the public-facing BunSDK and instrumentation APIs. This separation allows internal tests to use efficient numeric enums for refactoring flexibility while the public API maintains a developer-friendly string-based interface.

Applied to files:

  • src/bun.js/test/jest.zig
📚 Learning: 2025-12-02T05:59:51.485Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-02T05:59:51.485Z
Learning: Applies to test/**/*.test.{ts,tsx} : Verify your test fails with `USE_SYSTEM_BUN=1 bun test <file>` and passes with `bun bd test <file>` - tests are not valid if they pass with `USE_SYSTEM_BUN=1`

Applied to files:

  • packages/bun-types/test.d.ts
  • test/js/bun/test/fake-timers/fake-timers.test.ts
📚 Learning: 2025-11-24T18:37:30.259Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: test/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:37:30.259Z
Learning: Applies to test/**/*.test.{ts,js,jsx,tsx,mjs,cjs} : Use `bun:test` with files that end in `*.test.{ts,js,jsx,tsx,mjs,cjs}`

Applied to files:

  • packages/bun-types/test.d.ts
📚 Learning: 2025-11-24T18:35:39.205Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: .cursor/rules/registering-bun-modules.mdc:0-0
Timestamp: 2025-11-24T18:35:39.205Z
Learning: Add tests for new Bun runtime functionality

Applied to files:

  • packages/bun-types/test.d.ts
📚 Learning: 2025-10-19T02:44:46.354Z
Learnt from: theshadow27
Repo: oven-sh/bun PR: 23798
File: packages/bun-otel/context-propagation.test.ts:1-1
Timestamp: 2025-10-19T02:44:46.354Z
Learning: In the Bun repository, standalone packages under packages/ (e.g., bun-vscode, bun-inspector-protocol, bun-plugin-yaml, bun-plugin-svelte, bun-debug-adapter-protocol, bun-otel) co-locate their tests with package source code using *.test.ts files. This follows standard npm/monorepo patterns. The test/ directory hierarchy (test/js/bun/, test/cli/, test/js/node/) is reserved for testing Bun's core runtime APIs and built-in functionality, not standalone packages.

Applied to files:

  • packages/bun-types/test.d.ts
📚 Learning: 2025-12-02T05:59:51.485Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-02T05:59:51.485Z
Learning: Applies to test/**/*.test.{ts,tsx} : Do not write flaky tests - do not use `setTimeout` in tests; instead `await` the condition to be met since you're testing the CONDITION, not TIME PASSING

Applied to files:

  • test/js/bun/test/fake-timers/fake-timers.test.ts
📚 Learning: 2025-11-24T18:37:30.259Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: test/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:37:30.259Z
Learning: Applies to test/**/*.test.{ts,js,jsx,tsx,mjs,cjs} : Do not write flaky tests. Never wait for time to pass in tests; always wait for the condition to be met instead of using an arbitrary amount of time

Applied to files:

  • test/js/bun/test/fake-timers/fake-timers.test.ts
📚 Learning: 2025-11-24T18:35:08.612Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: .cursor/rules/dev-server-tests.mdc:0-0
Timestamp: 2025-11-24T18:35:08.612Z
Learning: Applies to test/bake/dev/esm.test.ts : Organize ESM tests in esm.test.ts for tests about various ESM features in development mode

Applied to files:

  • test/js/bun/test/fake-timers/fake-timers.test.ts
📚 Learning: 2025-11-24T18:35:50.422Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: .cursor/rules/writing-tests.mdc:0-0
Timestamp: 2025-11-24T18:35:50.422Z
Learning: See `test/harness.ts` for common test utilities and helpers

Applied to files:

  • test/js/bun/test/fake-timers/fake-timers.test.ts
📚 Learning: 2025-11-24T18:35:08.612Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: .cursor/rules/dev-server-tests.mdc:0-0
Timestamp: 2025-11-24T18:35:08.612Z
Learning: Applies to test/bake/dev/html.test.ts : Organize HTML tests in html.test.ts for tests relating to HTML files themselves

Applied to files:

  • test/js/bun/test/fake-timers/fake-timers.test.ts
📚 Learning: 2025-11-24T18:35:08.612Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: .cursor/rules/dev-server-tests.mdc:0-0
Timestamp: 2025-11-24T18:35:08.612Z
Learning: Applies to test/bake/**/*.test.ts : Use `dev.write()`, `dev.patch()`, and `dev.delete()` to mutate the filesystem instead of `node:fs` APIs, as dev server functions are hooked to wait for hot-reload and notify clients

Applied to files:

  • test/js/bun/test/fake-timers/fake-timers.test.ts
🧬 Code graph analysis (1)
test/js/bun/test/fake-timers/fake-timers.test.ts (1)
packages/bun-types/test.d.ts (4)
  • test (588-588)
  • test (589-589)
  • vi (170-202)
  • expect (612-612)
🔇 Additional comments (2)
src/bun.js/test/jest.zig (1)

242-249: LGTM!

The allocation size increase and addition of setSystemTime to the vi object are correct. The implementation properly mirrors the jest object structure and follows the established pattern for other timer-related functions.

packages/bun-types/test.d.ts (1)

201-201: LGTM!

The TypeScript type definition correctly exposes setSystemTime on the vi namespace by referencing jest.setSystemTime, which maintains consistency with other timer-related APIs.

Comment on lines +451 to +480
describe("vi.setSystemTime", () => {
test("can set system time with Date object", () => {
vi.useFakeTimers();
vi.setSystemTime(new Date("1995-12-19T00:00:00.000Z"));

expect(new Date().toISOString()).toBe("1995-12-19T00:00:00.000Z");
expect(Date.now()).toBe(819331200000);
});

test("can set system time with timestamp", () => {
vi.useFakeTimers();
vi.setSystemTime(new Date("2020-01-01T00:00:00.000Z").getTime());

expect(new Date().toISOString()).toBe("2020-01-01T00:00:00.000Z");
expect(Date.now()).toBe(1577836800000);
});

test("can update system time multiple times", () => {
vi.useFakeTimers();

vi.setSystemTime(new Date("1995-12-19T00:00:00.000Z"));
expect(Date.now()).toBe(819331200000);

vi.setSystemTime(new Date("2020-01-01T00:00:00.000Z"));
expect(Date.now()).toBe(1577836800000);

vi.setSystemTime(new Date("2025-06-15T12:00:00.000Z"));
expect(Date.now()).toBe(1749988800000);
});
});
Copy link
Contributor

Choose a reason for hiding this comment

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

🧹 Nitpick | 🔵 Trivial

Expand test coverage for edge cases and error handling.

The basic functionality tests look good and follow established patterns. However, consider adding tests for:

  1. Error when fake timers are not active (similar to lines 182-184, 199-201):

    test("throws error if fake timers not active", () => {
      expect(() => vi.setSystemTime(new Date())).toThrow("Fake timers are not active");
    });
  2. Calling without arguments to reset time (as documented in test.d.ts line 86):

    test("resets system time when called without arguments", () => {
      vi.useFakeTimers();
      vi.setSystemTime(new Date("2020-01-01T00:00:00.000Z"));
      vi.setSystemTime(); // Should reset to current time
      // Verify time is reset
    });
  3. Interaction with advanceTimersByTime:

    test("advanceTimersByTime works after setSystemTime", () => {
      vi.useFakeTimers();
      vi.setSystemTime(new Date("2020-01-01T00:00:00.000Z"));
      const initial = Date.now();
      vi.advanceTimersByTime(1000);
      expect(Date.now()).toBe(initial + 1000);
    });
  4. Verify performance.now() is not affected (since docs at test.d.ts lines 62-66 only mention Date.now/new Date/Intl):

    test("does not affect performance.now()", () => {
      vi.useFakeTimers();
      const perfBefore = performance.now();
      vi.setSystemTime(new Date("2020-01-01T00:00:00.000Z"));
      const perfAfter = performance.now();
      expect(perfAfter).toBe(perfBefore); // performance.now should be unchanged
    });

Based on learnings, these tests should be verified with USE_SYSTEM_BUN=1 bun test <file> (should fail) and bun bd test <file> (should pass).

🤖 Prompt for AI Agents
In test/js/bun/test/fake-timers/fake-timers.test.ts around lines 451 to 480,
expand the vi.setSystemTime test block to add four new tests: (1) a test that
calling vi.setSystemTime without first calling vi.useFakeTimers() throws the
expected "Fake timers are not active" error; (2) a test that calling
vi.setSystemTime() with no arguments after setting a mocked time resets time to
real current time (use vi.useFakeTimers(), set a fixed time, call
setSystemTime() with no args, then assert Date.now() approximates Date.now()
without fakes or restore and compare); (3) a test that after
vi.setSystemTime(...) calling vi.advanceTimersByTime(1000) advances Date.now()
by 1000ms; and (4) a test that performance.now() is unchanged by
vi.setSystemTime (capture performance.now() before and after setSystemTime and
assert they are equal or within a tiny delta). Ensure each test sets up
vi.useFakeTimers() where needed and restores timers after assertions to avoid
cross-test leakage.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant