test: support auto-mock for jest.mock(module) / jest.requireMock - #29836
test: support auto-mock for jest.mock(module) / jest.requireMock#29836robobun wants to merge 36 commits into
Conversation
|
Updated 9:40 PM PT - Aug 16th, 2026
✅ @robobun, your commit b81ecb8d49585b1bc9bbed7500d7d4ad047e695e passed in 🧪 To try this PR locally: bunx bun-pr 29836That installs a local version of the PR into your bun-29836 --bun |
|
Found 2 issues this PR may fix:
🤖 Generated with Claude Code |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds factory-less auto-mocking to Bun’s test runner: Changes
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
Warning Review ran into problems🔥 ProblemsGit: Failed to clone repository. Please run the Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/bun-types/test.d.ts`:
- Around line 114-118: Change the return type of requireMock<T = unknown>(id:
string) and vi.requireMock to wrap T with the project's mock helper type (e.g.,
Mocked<T> or the existing MockedFunction/MockedObject helpers defined in this
file) so consumers can access .mock and mockReturnValue with correct types;
locate the requireMock declarations and replace the bare T return type with the
appropriate Mocked<T> wrapper used by the other test helpers in this file to
enable type-safe mock methods and properties.
In `@src/bun.js/bindings/BunPlugin.cpp`:
- Around line 843-856: Cached JSModuleMock results are returned directly from
the JSModuleMock path (via moduleMock->executeOnce) which bypasses the promise
normalization applied earlier for factory mocks; change the branch that handles
JSModuleMock (the code using virtualModules.get(specifier), JSModuleMock, and
moduleMock->executeOnce) to take the executeOnce() result and pass it through
the same fulfilled/rejected JSPromise normalization used elsewhere (the
normalization block applied to factory/mock returns) — preserving
RETURN_IF_EXCEPTION checks — and only then encode/return the normalized JSValue
instead of returning executeOnce() directly.
In `@src/bun.js/bindings/JSMockFunction.cpp`:
- Around line 1740-1755: The loop currently calls
object->get(lexicalGlobalObject, name) which invokes property getters; instead
fetch the property descriptor/slot (e.g., use the object's
getOwnPropertySlot/getOwnProperty or equivalent descriptor API) to detect
accessor properties and avoid calling getters: if the slot/descriptor indicates
an accessor (has getter/setter) then either copy/preserve the accessor
descriptor to mockFn (putDirect with the descriptor) or skip synthesizing a
value for that property; only call autoMockValue and object->get when the
descriptor indicates a data property. Apply the same change to the other
occurrence around the auto-mock code (the region referenced at 1790-1805) and
keep the existing exception clearing (scope.tryClearException) behavior for
safety.
- Around line 1731-1738: The loop that skips vm.propertyNames->prototype causes
mocked class constructors to lose instance methods; update the handling in the
for (auto& name : names) loop so that when name == vm.propertyNames->prototype
you do not continue, but instead call autoMockValue() on the original prototype
value and assign that mocked prototype to the mock constructor (i.e., replace
the unconditional skip with logic to retrieve the original Class.prototype, run
autoMockValue(originalProto), and set the returned value as the mock's prototype
so instances inherit mocked methods from the mock constructor).
In `@test/js/bun/test/mock/auto-mock.test.ts`:
- Around line 50-72: Add an assertion that the vi alias of requireMock returns
the same auto-mocked module as jest.requireMock: call
vi.requireMock("./auto-mock-fixture") in the tests alongside jest.requireMock
and assert that the returned object has plainFunction.mock and MyClass.mock
defined and behaves like the jest handle (including the mockReturnValue behavior
test); update the second and third tests to include a vi.requireMock(...) call
and corresponding expect checks to cover the alias wiring for vi.requireMock and
ensure it mirrors jest.requireMock.
- Around line 56-72: The second test currently reuses the same module specifier
"./auto-mock-fixture" that was auto-mocked earlier, so make the test independent
by restoring mocks after each test or by using a unique fixture; add an
afterEach hook that calls mock.restore() (or call mock.restore() at the end of
the first test) so beginModuleMockScope()/endModuleMockScope() state is cleared,
or change the second test to requireMock a different module specifier to ensure
it exercises the fresh auto-mock path.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 62aec59a-0086-4928-bb7f-f5764e9e92e8
📒 Files selected for processing (8)
packages/bun-types/test.d.tssrc/bun.js/bindings/BunPlugin.cppsrc/bun.js/bindings/JSMockFunction.cppsrc/bun.js/bindings/JSMockFunction.hsrc/bun.js/test/jest.zigtest/js/bun/test/mock/auto-mock-fixture.tstest/js/bun/test/mock/auto-mock.test.tstest/js/bun/test/mock/mock-module-non-string.test.ts
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/bun.js/bindings/JSMockFunction.cpp`:
- Around line 1725-1726: The auto-mock property walkers currently instantiate
JSC::PropertyNameArrayBuilder with PropertyNameMode::Strings (see
JSC::PropertyNameArrayBuilder and the call to
object->methodTable()->getOwnPropertyNames), which excludes symbol-keyed
properties and drops well-known symbols like
Symbol.iterator/asyncIterator/dispose; update the PropertyNameMode argument from
PropertyNameMode::Strings to PropertyNameMode::StringsAndSymbols in each walker
(the occurrences around the JSC::PropertyNameArrayBuilder construction and
subsequent object->methodTable()->getOwnPropertyNames call) so symbol-keyed
exports and prototype members are included in the mocked module shape.
In `@test/js/bun/test/mock/auto-mock.test.ts`:
- Around line 115-126: The test must assert the real module's accessor
side-effect counter and that the accessor property was not copied onto the mock:
use jest.requireActual("./auto-mock-fixture-accessor") to get the real module
and assert its getter counter (e.g., real.getterHits or real.getterHits.mock as
appropriate) has not increased after mocking, and assert on the mocked object
that the accessor-containing object/property was skipped (e.g.,
expect(mocked.obj).toBeUndefined() or
expect(mocked).not.toHaveProperty("obj.sneaky")), while keeping the existing
checks for mocked.getterHits.mock and mocked.plain.mock.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 5073af0d-ad8a-4b53-bf5a-e9e9b8a38941
📒 Files selected for processing (5)
src/bun.js/bindings/BunPlugin.cppsrc/bun.js/bindings/JSMockFunction.cpptest/js/bun/test/mock/auto-mock-fixture-accessor.tstest/js/bun/test/mock/auto-mock-fixture-ondemand.tstest/js/bun/test/mock/auto-mock.test.ts
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
test/js/bun/test/mock/auto-mock.test.ts (1)
52-86:⚠️ Potential issue | 🟡 MinorMake these alias/parity checks independent of earlier mock registrations.
These cases still reuse
./auto-mock-fixture, and the mock scope lives for the whole file. Ifjest.mock(),vi.mock(), or the latermock.module()call regresses into a no-op, the tests can still pass by reading the mock installed by the first test. AddafterEach(() => mock.restore())or give each case its own specifier/fixture so it proves its own registration path. Based on learnings:beginModuleMockScope()/endModuleMockScope()run once per test file, andmock.restore()is what clears file-local mock entries.Also applies to: 101-113
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/js/bun/test/mock/auto-mock.test.ts` around lines 52 - 86, The tests reuse the same fixture "./auto-mock-fixture" so earlier mock registrations can leak into later specs; make each test independent by either restoring file-local mocks after each test or using unique module specifiers per test: add an afterEach hook that calls mock.restore() (to clear file-local mock entries created by jest.mock/vi.mock/mock.module) or change the specifiers used in the tests that call jest.mock, vi.mock, jest.requireMock and vi.requireMock so each case registers a fresh module (e.g., "./auto-mock-fixture-1", "./auto-mock-fixture-2") to ensure each assertion verifies its own registration path.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/bun.js/bindings/JSMockFunction.cpp`:
- Around line 1774-1819: Seed the visited map with the prototype mapping
immediately after creating mockProto so recursive autoMockValue calls won't
create a second mock for the same prototype: after constructing mockProto (the
result of JSC::constructEmptyObject) insert a visited[originalProtoObj] ->
mockProto entry (use the same map API used elsewhere for visited) before
iterating protoNames and before any calls to autoMockValue; then continue
filling mockProto and finally call mockFn->putDirect(vm,
vm.propertyNames->prototype, mockProto, 0) as existing code does.
---
Duplicate comments:
In `@test/js/bun/test/mock/auto-mock.test.ts`:
- Around line 52-86: The tests reuse the same fixture "./auto-mock-fixture" so
earlier mock registrations can leak into later specs; make each test independent
by either restoring file-local mocks after each test or using unique module
specifiers per test: add an afterEach hook that calls mock.restore() (to clear
file-local mock entries created by jest.mock/vi.mock/mock.module) or change the
specifiers used in the tests that call jest.mock, vi.mock, jest.requireMock and
vi.requireMock so each case registers a fresh module (e.g.,
"./auto-mock-fixture-1", "./auto-mock-fixture-2") to ensure each assertion
verifies its own registration path.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 480e4ed6-a203-42b3-92ca-097a446f8f4b
📒 Files selected for processing (3)
src/bun.js/bindings/JSMockFunction.cpptest/js/bun/test/mock/auto-mock.test.tstest/regression/issue/ENG-24434.test.ts
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/bun.js/bindings/JSMockFunction.cpp`:
- Line 1785: The mock creation uses mockFn->putDirect(vm, name, mockedProp, 0)
which writes every copied member as an enumerable, writable data property and
loses original attributes; change the call to reuse the source property
slot/descriptor attributes when setting the property (read the source's property
slot/descriptor and pass the appropriate flags/attributes instead of literal 0),
so properties like non-enumerable class methods, prototype, ReadOnly/DontDelete
constants retain their original attributes; update all similar sites calling
mockFn->putDirect (including the other putDirect call sites that synthesize
mocks) to fetch and apply the source descriptor/slot flags when writing
mockedProp.
- Around line 1916-1921: Add a regression test and fixture that exports a
primitive (e.g., export default 42) and assert that both the jest.mock() path
(via require("./module")) and jest.requireMock("module") return the primitive 42
(not an object { default: 42 }); locate the auto-mock creation code in
createAutoMockFromExports / JSMockFunction.cpp where a JSObject* wrapper is
created and ensure tests fail if that wrapper leaks into CommonJS, then adjust
implementation so CommonJS require paths unwrap such primitive-wrappers (or
avoid wrapping primitives for auto-mocks) and re-run tests to confirm round-trip
safety.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: fc846d13-6395-4640-895d-608aae9dc0ac
📒 Files selected for processing (6)
src/bun.js/bindings/JSMockFunction.cpptest/js/bun/test/mock/auto-mock-fixture-jest.tstest/js/bun/test/mock/auto-mock-fixture-requiremock.tstest/js/bun/test/mock/auto-mock-fixture-vi.tstest/js/bun/test/mock/auto-mock-fixture-virequiremock.tstest/js/bun/test/mock/auto-mock.test.ts
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (1)
src/bun.js/bindings/JSMockFunction.cpp (1)
1785-1785:⚠️ Potential issue | 🟠 MajorPreserve the source property descriptors when copying members.
These
putDirect(..., 0)calls re-emit every copied member as a plain enumerable/writable data property. That changes the mocked surface for non-enumerable class methods,prototype, and any read-only constants copied through the walker. Please carry over the source slot/descriptor attributes instead of hard-coding0.Also applies to: 1826-1832, 1893-1893
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/bun.js/bindings/JSMockFunction.cpp` at line 1785, The code is overwriting source property descriptors by always calling mockFn->putDirect(vm, name, mockedProp, 0); instead of preserving attributes; fix by reading the original property's slot/descriptor attributes from the source (the property's attributes/flags or PropertyDescriptor retrieved for the source object/member) and pass those attribute flags into putDirect rather than 0, so non-enumerable, non-writable, accessor, and prototype flags are preserved; apply the same change at the other occurrences where putDirect(..., 0) is used for copying members.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/bun.js/bindings/BunPlugin.cpp`:
- Around line 597-637: The auto-mock path calls the bound require (via
Bun::JSCommonJSModule::createBoundRequireFunction / JSC::profiledCall) which can
resolve an existing entry in onLoadPlugins.virtualModules (populated by
addModuleMock / builder.module), causing the mock to be built from a prior
virtual module instead of the real module; change the auto-mock bootstrap to
bypass virtual-module entries when resolving the real module—either by invoking
a require/resolver variant that ignores onLoadPlugins.virtualModules or by
temporarily removing/isolating the specifier entry from
onLoadPlugins.virtualModules around the profiledCall, then call
Bun::createAutoMockFromExports with the true realExports as before and restore
the virtual-module state. Ensure this change is applied to both places where the
factory-less auto-mock path constructs realExports (the shown block and the
similar block at the other location).
In `@src/bun.js/bindings/JSMockFunction.cpp`:
- Around line 1863-1864: The current plain-object own-property collection uses
PropertyNameMode::StringsAndSymbols with DontEnumPropertiesMode::Exclude which
drops non-enumerable own properties; change the collection to use the same
own-property walk as the function/prototype paths (i.e., include DontEnum
properties and private symbols as those paths do) by adjusting the
PropertyName/PropertyMode flags passed to
JSC::PropertyNameArrayBuilder/getOwnPropertyNames and iterate the original
object's own properties rather than only enumerable ones; when creating
properties on the mock, copy the full property attributes
(writable/configurable/enumerable, getters/setters) from the original (use the
original property descriptor and define the property with the same attributes)
so non-enumerable/hidden methods and constants are preserved on the mocked
object.
In `@test/js/bun/test/mock/auto-mock.test.ts`:
- Around line 1-6: The new test cases in auto-mock.test.ts should be merged into
the existing mock-module test file instead of being a new file: open the
existing mock tests for module mocking and move the tests and any helper imports
from auto-mock.test.ts into that file (preserve the "Auto-mock" describe/it
blocks or adapt them to the existing describe hierarchy), then delete
auto-mock.test.ts; ensure you retain test names and assertions (e.g., any
"jest.mock/vi.mock" and "requireMock" cases) and update imports/exports so the
moved cases compile and run in the original mock test suite.
---
Duplicate comments:
In `@src/bun.js/bindings/JSMockFunction.cpp`:
- Line 1785: The code is overwriting source property descriptors by always
calling mockFn->putDirect(vm, name, mockedProp, 0); instead of preserving
attributes; fix by reading the original property's slot/descriptor attributes
from the source (the property's attributes/flags or PropertyDescriptor retrieved
for the source object/member) and pass those attribute flags into putDirect
rather than 0, so non-enumerable, non-writable, accessor, and prototype flags
are preserved; apply the same change at the other occurrences where
putDirect(..., 0) is used for copying members.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 50e21da0-15d4-495d-b4a6-5fbf7f5af715
📒 Files selected for processing (3)
src/bun.js/bindings/BunPlugin.cppsrc/bun.js/bindings/JSMockFunction.cpptest/js/bun/test/mock/auto-mock.test.ts
|
CI (build #48564) finished with 3 Windows-only failures, all unrelated to this PR:
Neither test file contains Can a maintainer retry the failing Windows jobs? Also happy to push a no-op to rebuild. |
|
CI (build #49041) failed with 6 errors — the same Windows / Linux-ASAN flakes that appeared on every build of this PR across multiple commits, none of which touch any of the code this PR adds:
Confirmed unrelated: Happy to push a no-op to retry. |
|
Build #49053 hit the same 3 pre-existing flakes (down from 6 on the previous retry):
Zero mock/plugin-related content in any of them. |
There was a problem hiding this comment.
Everything I flagged earlier has been addressed and nothing new turned up on this pass — but given the scope (~300 lines of new JSC binding code across BunPlugin.cpp/JSMockFunction.cpp, new GC-rooted cache, exception-safe stash/restore around module loading), this warrants a human pass before landing.
Extended reasoning...
Overview
This PR implements Jest's factory-less jest.mock(id) auto-mock and adds jest.requireMock() / vi.requireMock(). It touches:
src/bun.js/bindings/BunPlugin.cpp(~200 new lines): an auto-mock branch inJSMock__jsModuleMockthat synchronously requires the real module, stashes/restores priorvirtualModulesandrequireMapentries around the require, and a newJSMock__jsRequireMockhost function with its own side-cache lookup.src/bun.js/bindings/JSMockFunction.cpp(~300 new lines): a recursiveautoMockValuewalker over module exports (functions →jest.fn(), prototypes mocked, objects recursed, accessors skipped, integer keys routed viaputDirectIndex, cycle detection via a visited map, depth cap).src/bun.js/bindings/JSMockFunction.h: newrequireMockCacheStrong<JSMap>field onJSMockModuleand two new function declarations.src/bun.js/test/jest.zig: wiresrequireMockontojestandvi.packages/bun-types/test.d.ts: typings.- 8 new fixture files + ~215 new test lines in
mock-module.test.ts, plus updates to two existing tests whose expectations changed.
Security risks
None identified. This is test-runner infrastructure (only reachable via bun:test), operates on user-supplied module specifiers that already flow through the existing mock.module() resolver, and adds no new privilege boundaries.
Level of scrutiny
High. This is non-trivial new C++ in JSC bindings that interacts with module loading, the CJS require cache, the virtual-module map, JSC exception-scope discipline, and GC roots. The PR went through ~10 review rounds (CodeRabbit + my own inline comments) fixing real issues: accessor invocation, prototype mocking, symbol-keyed properties, attribute preservation, on-demand cache idempotency, cache isolation from global resolution, exception-safe stash/restore, integer-key putDirect asserts, and requireMockCache lifecycle. All of those are now addressed and resolved, and the bug hunter found nothing on the latest commit (a6a2261). But the volume and subtlety of issues found during review is itself a signal that the surface area is large enough to merit human sign-off on the final shape — particularly the restoreStash exception-juggling lambda in BunPlugin.cpp and the requireMockCache clearing semantics in JSMock__resetSpies.
Other factors
- Test coverage is thorough: every review-driven fix has a matching regression test in
mock-module.test.ts. - CI failures on the latest builds are documented pre-existing flakes (websocket-server, serve-stream-reject-flush-leak, rspack NAPI) unrelated to mock code.
- Two existing tests (
mock-module-non-string.test.ts,ENG-24434.test.ts) had their expectations changed becausemock.module(id)without a callback no longer throws a TypeError — that's an intentional behaviour change, not a regression, but worth a human confirming the removed "does not run the resolver when callback is missing" subprocess test is acceptable to drop.
|
CI (build #49132) failed with 1 error — |
There was a problem hiding this comment.
No bugs found in the current revision, but this is a substantial new feature (~600 lines of new JSC bindings for the auto-mock walker + requireMock host function, new GC-rooted cache, exception-safe stash/restore around module re-evaluation) that also changes the existing behavior of mock.module(id) with no factory — worth a human pass before merge.
Extended reasoning...
Overview
This PR implements Jest-style auto-mocking: jest.mock(id) / mock.module(id) without a factory now synchronously loads the real module and synthesises a mock (functions → jest.fn(), classes → mock constructors with mocked prototypes, plain objects recursed, primitives/arrays/builtins preserved), and adds jest.requireMock / vi.requireMock. The implementation spans ~330 new lines in BunPlugin.cpp (auto-mock branch in JSMock__jsModuleMock with stash/restore of virtualModules + requireMap entries around an internal require(), plus a new JSMock__jsRequireMock host function with a side-cache), ~300 new lines in JSMockFunction.cpp (the recursive autoMockValue walker with cycle detection, accessor skipping, indexed-key handling, attribute preservation), header/Zig wiring, type declarations, and ~215 lines of new tests across 8 fixtures.
Security risks
None identified. This is test-runner-only surface (jest/vi/mock globals), takes string specifiers that flow through the existing module resolver, and doesn't touch auth/crypto/permissions/network. The recursive walker is depth-limited (16) and cycle-safe.
Level of scrutiny
High. This is non-trivial new C++ in the JSC bindings layer with several subtle correctness concerns that surfaced and were fixed across ~15 review iterations: exception-safety of the stash/restore block (4 separate commits to close gaps), putDirect vs putDirectIndex for integer keys (would have tripped a debug assert), requireMockCache lifecycle (would have leaked across test files), accessor-getter invocation, prototype seeding order, promise unwrapping, and the virtualModules vs side-cache distinction for Jest-compat. It also changes existing behavior: mock.module(id) with no second arg previously threw TypeError and now auto-mocks — two existing tests (mock-module-non-string.test.ts, ENG-24434.test.ts) were rewritten to accommodate this.
Other factors
All CodeRabbit and claude[bot] inline comments are resolved. CI has been retried multiple times for unrelated Windows/ASAN flakes; the mock-specific tests pass on all lanes. Test coverage for the new feature is thorough (parity across mock.module/jest.mock/vi.mock, requireMock caching/on-demand, accessor protection, indexed keys, error-path stash restoration, restoreAllMocks cache clearing). Given the volume of new bindings code, the behavioral change to an existing API, and the number of correctness issues found and fixed during review, a human maintainer should sign off on the final shape.
8cd5999 to
bbec484
Compare
bbec484 to
9e579fd
Compare
The stash/restore locals (stashedVirtualEntry, restoreStash lambda) are block-scoped to the `if (isAutoMock)` branch. They go out of scope at the block's closing brace — but the shared post-block code has several more RETURN_IF_EXCEPTION guards (the ESM-namespace-patching block in particular) before `addModuleMock()` finally re-installs the new mock. If any of those guards fired the prior factory-mock / builder.module() entry would stay permanently dropped. Re-insert stashedVirtualEntry into virtualModules just before the block closes. `addModuleMock()` unconditionally `set()`s the new mock on the success path, so this re-insert is a no-op there. Any post-block early-return now leaves the previous entry intact.
…aky, no-validate-*)
The Rust rewrite in 23427db ported src/bun.js/test/jest.zig's createMockObjects to src/runtime/test_runner/jest.rs but didn't carry over the requireMock bindings from this PR. Result: jest.requireMock and vi.requireMock were undefined after the rebase. Add JSMock__jsRequireMock as a require_mock_fn under both jest.* and vi.*, bump the createEmptyObject capacities from 9→10 and 6→7 to match, and declare the extern.
The class-prototype walker skipped `constructor` when copying members but never wrote the reciprocal back-reference, so `MockedClass.prototype.constructor` resolved to `Object` (inherited from `Object.prototype`) instead of `MockedClass`. Jest's auto-mock explicitly sets this pointer in `_generateMock`, and the ES2015 class invariant `Class.prototype.constructor === Class` is how consumer code branches on instance identity. Add the one-line `mockProto->putDirect(vm, propertyNames->constructor, mockFn, DontEnum)` after the loop. Regression asserted in the existing "mock.module without a factory auto-mocks exported functions" test.
Both restoreStash() and the post-block re-seat write to onLoadPlugins.virtualModules->set() guarded only by the stashedVirtualEntry Strong<> — which stays truthy regardless of what happened to the map pointer while boundRequire() ran user JS. If a module under boundRequire() calls Bun.plugin.clearAll() (which does `delete virtualModules; virtualModules = nullptr`), both set() calls would deref null. Re-check hasVirtualModules() at both sites. If the map has been destroyed we don't try to resurrect the stashed entry — it's already gone from the user's perspective.
The specifier-resolution logic (file: URL handling, Bun__resolveSyncWithSource with the relative-URL fallback, mustDoExpensiveRelativeLookup flag) was duplicated between JSMock__jsModuleMock's resolveSpecifier lambda and the inline block in JSMock__jsRequireMock. The two must produce identical resolved keys — jest.requireMock(id) has to find the same virtualModules entry jest.mock(id) installed — so the copy was a drift hazard. Extract a static resolveModuleMockSpecifier(...) that mutates specifier / specifierString in place and returns false if it threw. Both host functions now call it, so there's a single source of truth.
…k path
resolveModuleMockSpecifier set onLoadPlugins.mustDoExpensiveRelativeLookup
unconditionally. That flag's invariant is that it's only true while
virtualModules is non-null — the module loader asserts
!mustDoExpensiveRelativeLookup whenever virtualModules == nullptr
(ZigGlobalObject.cpp:3399). jest.mock upholds it (it always reaches
addModuleMock, which allocates the map), but jest.requireMock caches in a
separate side-map and never allocates virtualModules. So
jest.requireMock("file:./x") in a file that never called jest.mock left
the flag set with a null map, tripping the assert on the next ESM import.
Gate the flag behind a setExpensiveLookupFlag param: true for jest.mock,
false for jest.requireMock. Also drops the helper's dead bool return (both
callers use RETURN_IF_EXCEPTION) in favor of void.
Regression test spawns a fresh process (virtualModules starts null),
jest.requireMock("file:./real"), then import()s a real module — fails with
the exact assert before this fix, passes after.
The latest WebKit drop no longer transitively provides JSMap's full definition where Strong<JSMap>::set is instantiated, so the requireMockCache field failed to compile with 'incomplete type JSC::JSMap used in type trait expression'. Include it directly.
… unmocked modules Two findings on the jest.mock auto-mock branch: 1. resolveModuleMockSpecifier sets mustDoExpensiveRelativeLookup for ./ and file: specifiers assuming addModuleMock() (which allocates virtualModules) will follow. When the internal require() throws — a typo'd relative path — none of the failure paths reached it, leaving the flag set with virtualModules == nullptr and tripping ASSERT(!mustDoExpensiveRelativeLookup) in moduleLoaderResolve on the next ESM import. restoreStash() now re-establishes the invariant: clear the flag whenever virtualModules doesn't exist. This is the jest.mock sibling of the requireMock variant fixed earlier. 2. The requireMap removal before the internal require() fired for any previously-require()'d module, forcing a re-evaluation of the source (double-running top-level side effects) even when the module was never mocked. Only a prior mock (stashedVirtualEntry non-empty) can have patched the cached .exports, so gate the removal on it; an unmocked cached entry is reused as-is. Both paths get spawned regression tests that reproduce the exact failure on the unfixed build (the ZigGlobalObject assert, and __sideEffectRuns == 2 respectively).
…ock shapes
- Replace the three own-property loops with one chain-walking collector
matching jest-mock's _getSlots: own string names up the prototype chain
(stopping before Object.prototype/Function.prototype), so subclass
statics, inherited prototype methods and exported instances keep their
API. Accessors are read when the owner has __esModule (esbuild/tsc CJS
output), mock slots are written as plain properties, and exceptions
propagate instead of being cleared.
- Give JSMockFunction a [[Construct]] path: instances inherit
new.target.prototype and are recorded in mock.instances, so
'new MockedClass()' works.
- Synthesize a 'default' export when the real module was not an ES
namespace, and give primitive carriers the { default, __esModule }
interop shape so require() keeps returning the raw value.
- Only stash JSModuleMock entries when auto-mocking; builder.module()
plugin entries stay in place so the internal require() can find them.
- Move mustDoExpensiveRelativeLookup into addModuleMock so the flag can
only be set once virtualModules exists.
- Drop the requireMock cache clear from resetSpies (Jest's
restoreAllMocks does not touch the module registry) and remove
vi.requireMock (vitest has no such method).
- Update docs, types, and tests; add fixtures for subclass/instance,
CJS, esbuild-shaped, primitive and plugin-provided modules, plus a
builtin fs consumer-behavior test.
…ock prototype, reify lazy prototypes
- Auto-mocks of object exports already match the real require() shape, so
a new JSModuleMock.suppressESModuleInterop flag keeps the loader, the
cached-module patch and requireMock from unwrapping { __esModule,
default } out of them; the primitive carrier and factory mocks keep the
interop. require() of an auto-mocked esbuild/tsc module returns the
full mocked exports object again.
- The property walk stops at JSMockFunctionPrototype so re-mocking an
already-mocked value (repeat jest.mock() of an imported ESM module)
doesn't copy mockReturnValue/mockClear/... onto the new mock as stubs.
- Read the source function's prototype through a PropertySlot instead of
getDirect so a plain 'export function Foo() {}' with a lazily reified
prototype still mocks with one.
- Tests: esbuild fixture now covers jest.mock + require + requireMock,
repeat-mock over a statically imported fixture, LegacyCtor instanceof,
factory { __esModule, default } unwrap in both cache states; stale
flag-mechanism comments in the spawned tests trimmed to the invariant.
clearAll() deletes virtualModules, so leaving the flag set primed the module loader's flag-without-map assert on the next ESM import. Covered by a spawned test (jest.mock file: specifier, clearAll, then import).
The promise-unwrap loop in mock.module's cached-module patch exited the switch but not the loop on Status::Pending, so an async factory re-mocking an already-required module hung the process. Surface the pending promise instead, matching the requireMock path. Spawned regression test.
e323b0d to
6670956
Compare
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
|
Rebased onto today's main (6670956); the clearAll flag reset folded into main's #32261 during the rebase, with the spawned regression test kept. Two additions since your last look, both noted in the new "Deliberate behavior changes" section of the body:
Full mock suite is green on a debug build of the rebased head. |
Extract unwrapSynchronouslySettledPromise for the two mock unwrap sites, rename the unused interop out-param on the ESM fetch path, run the six independent spawned tests with test.concurrent, and give the async re-mock fixture a real assertion.
JSMockFunction extends InternalFunction, which has no default prototype property, so 'new (jest.fn())() instanceof fn' threw a TypeError. Install the ordinary-function pair (prototype with a constructor back-reference); the auto-mock class path overwrites it with the mocked source prototype. Also mark a rejected promise handled before rethrowing its reason in unwrapSynchronouslySettledPromise, matching the other rejected-to-throw sites, so the error doesn't surface again as an unhandledRejection.
2 participants
What does this PR do?
Fixes #29834
Fixes #11018
Refs #16140 (checks off the
vi.mock()no-factory form; the rest of the tracker stays open)Before
After
jest.mock(specifier)without a factory synthesises an auto-mock: the realmodule is loaded synchronously and a mocked copy of its exports is generated,
following jest-mock's
_getSlotsrules:Object.prototype/Function.prototype), so subclass statics, inheritedprototype methods, and exported class instances keep their API
jest.fn()-style mocks;new MockedClass()returns an instance inheriting the mocked prototype and records it in
mock.instancesexcept on
__esModuleinterop objects, where the getters are the exports(esbuild/tsc-compiled CJS)
jest.requireMock(specifier)returns the mocked exports for a module: theregistered mock from a prior
jest.mock(specifier)if there is one, or anon-demand auto-mock cached in a side map that leaves
import/requireresolution untouched. (jest only; vitest has no
vi.requireMock.)Consumer behavior, asserted by tests:
require(id)andimport(named and default) of a mocked local module seethe mock; CJS modules get a synthesized
defaultmirroringrequire-to-import interop, and primitive modules keep their raw value shape
import "fs"/import "node:fs"andjest.requireMock("fs")see the mock;
require("fs")bypasses the mock registry and keepsreturning the real module (pre-existing builtin require behavior, same as
the factory form)
How did you verify your code works?
bun bd test test/js/bun/test/mock/— 39 pass, 1 todo, 0 fail on thedebug/ASAN build, including new coverage for: subclass/instance prototype
chains, esbuild-shaped
__esModulegetter modules, CJS default-exportsynthesis, primitive module shape across require/import/requireMock,
newon mocked classes (mock.instances), plugin-provided modules(
builder.module()entries are no longer removed by the auto-mock stash),and a spawned builtin-fs consumer matrix
BUN_JSC_validateExceptionChecks=1run of the mock suite passes (thewalker now propagates exceptions with RETURN_IF_EXCEPTION instead of
clearing them)
USE_SYSTEM_BUN=1 bun teston the same file: 17 fail (auto-mock absent),confirming the tests exercise the new code
Deliberate behavior changes beyond the no-factory form
{ __esModule: true, default, ... }:require()nowreturns the
defaultin both cache states. Previously only a freshrequire()unwrapped; re-mocking an already-required specifier left thewhole factory object as
module.exports, so the shape depended on loadorder. A factory-form test pins both orders.
Bun.plugin.clearAll()now resetsmustDoExpensiveRelativeLookupwhen itdeletes the virtual module map; leaving it set primed a debug-build assert
in the module loader on the next ESM import (spawned regression test).
mock.module's cached-module patch no longerspins forever on a still-pending promise (async factory re-mocking an
already-required module); it now surfaces the pending promise, matching the
requireMock path (spawned regression test).
Background
virtualModulesmap consulted by the moduleloader;
jest.mock(id, factory)has stored aJSModuleMockthere for awhile. The auto-mock form requires the real module first, so any prior
mock entry is stashed and restored around that internal require. That map
also holds
Bun.pluginbuilder.module()callbacks, which is why thestash is now restricted to
JSModuleMockentries.{ __esModule: true, default: X }as "module.exports is X". The primitivecarrier uses that shape so
require()of a mocked primitive module stillreturns the raw value.
JSMockFunctionpreviously routed[[Construct]]through its[[Call]]handler, sonewon any mock returnedundefined; it now hasa proper construct path (instance from
new.target.prototype, recorded inmock.instances).[review] gate passed · iteration 45 · 24 files touched
fails on main (without fix)
passes on PR (with fix)
diff hotspot
gate history · 5 passed · 0 rejected · iteration 45
evidence per changed file
root cause · written by the author bot
The mock module binding unconditionally validated its second argument as a callable factory, so calling
jest.mock(id)with no factory threw a TypeError instead of auto-mocking the module as Jest does. The fix makes the factory optional: when it is omitted, the runtime synchronously requires the real module, walks its exports to generate an auto-mock that replaces callable exports with mock functions while preserving names, prototypes, and non-callable values, and registers the result in the mock cache. It also addsjest.requireMockandvi.requireMockto return the cached auto-mock or …