bun:test: return an object when constructing a mock function

[robobun]

What does this PR do?

Fixes a debug assertion (ASSERTION FAILED: isCell() in JSValue::asCell()) found by fuzzing, triggered by constructing a bun:test mock function whose implementation produces a primitive:

const { spyOn, jest } = Bun.jest(import.meta.path);

// spy on a missing property (original value is undefined), then construct it
const spy = spyOn(Float32Array, "nonexistentprop");
Reflect.construct(spy, []); // ASSERTION FAILED: isCell()

// also reachable with plain mocks
Reflect.construct(jest.fn(() => 42), []); // ASSERTION FAILED: isCell()
new (jest.fn())(); // evaluated to `undefined` instead of an object

JSMockFunction registered jsMockFunctionCall as both its call and construct callback, so [[Construct]] returned whatever the mock implementation produced — including undefined and other primitives. JSC requires native constructors to return an object: Interpreter::executeConstruct() ends with asObject(result), which asserts in debug builds (and is a type confusion in release builds) when the result isn't a cell. Through the bytecode path, new mock() silently evaluated to a primitive, which violates constructor semantics.

How did you fix it?

Gave JSMockFunction a dedicated construct entry point that follows ordinary constructor semantics, matching how Jest's mockConstructor behaves as a plain JS function:

• create the instance object from new.target's .prototype (falling back to Object.prototype),
• run the existing mock-call logic with that instance as this (so mock.contexts records the instance and implementations see it as this),
• return the implementation's result when it is an object, otherwise return the newly created instance.

The call path is unchanged.

Tests

Added mock constructor calls tests to test/js/bun/test/mock-fn.test.js covering new/Reflect.construct on mocks with no implementation, primitive-returning implementations, object-returning implementations, and a spy on a missing property. The first three fail on current main (primitives escape from new); the spy case crashes debug builds. The original fuzzer reproduction runs cleanly with this change.

[robobun]

^{Updated 8:13 PM PT - May 24th, 2026}

❌ @robobun, your commit a170803 has 2 failures in Build #57838 (All Failures):

• scripts/build/ci.ts - build failed on 🐧 aarch64-musl - build-bun
• scripts/build/ci.ts - build failed on 🐧 x64-musl - build-bun
• test/bundler/transpiler/transpiler.test.js - code 1 on 🪟 2019 x64
• test/bundler/transpiler/transpiler.test.js - code 1 on 🪟 2019 x64-baseline
• test/bundler/transpiler/transpiler.test.js - code 1 on 🪟 11 aarch64

---

🧪   To try this PR locally:

bunx bun-pr 31373

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

bun-31373 --bun

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 13abe6fb-e5e1-4d1c-92f0-9a47f34ced66

📥 Commits

Reviewing files that changed from the base of the PR and between 1950a3a and a170803.

📒 Files selected for processing (2)

• src/jsc/bindings/JSMockFunction.cpp
• test/js/bun/test/mock-fn.test.js

---

Walkthrough

This PR adds proper constructor support to Jest mock functions in Bun's JSC implementation. It refactors the mock call logic into a shared internal helper, implements a dedicated construct handler that derives the correct this binding from the constructor's prototype, and ensures constructor calls always return objects with recorded contexts.

Changes

Mock function constructor implementation

Layer / File(s)	Summary
Call logic refactoring and construct declaration src/jsc/bindings/JSMockFunction.cpp	jsMockFunctionCallImpl extracts the core mock call behavior (recording calls, contexts, invocation order, handling implementation kinds, advancing "once" implementations) into a reusable helper. jsMockFunctionConstruct host function is declared for constructor support.
Constructor and call handlers src/jsc/bindings/JSMockFunction.cpp	jsMockFunctionConstruct derives thisObject from new.target.prototype, invokes the shared call logic, and enforces that [[Construct]] returns an object (falling back to thisObject if the implementation returns a primitive). jsMockFunctionCall becomes a thin wrapper to jsMockFunctionCallImpl.
Constructor behavior tests test/js/bun/test/mock-fn.test.js	New test suite validates that new calls on mocks yield objects, record contexts correctly, handle primitive returns properly, preserve object returns, and that Bun's spyOn on missing properties can be constructed without crashing.

🚥 Pre-merge checks | ✅ 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title 'bun:test: return an object when constructing a mock function' clearly and specifically summarizes the main change: fixing mock function constructor semantics to always return an object.
Description check	✅ Passed	The description fully addresses both template sections: 'What does this PR do?' explains the bug and the fix in detail, and 'How did you verify your code works?' describes comprehensive test coverage.
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.

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

---

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

[github-actions]

This PR may be a duplicate of:

1. Fix crash constructing jest.fn() via Reflect.construct #30212 - Both fix the same ASSERTION FAILED: isCell() crash when constructing mock functions via new/Reflect.construct, using the same approach (splitting jsMockFunctionCall into separate call/construct entry points in JSMockFunction.cpp)

🤖 Generated with Claude Code

[claude]

🟣 Pre-existing, not introduced here: mock.instances is exposed on the mock object but is never populated — jsMockFunctionCallImpl pushes to calls/contexts/invocationCallOrder/returnValues but never to fn->instances. Now that mocks have a real [[Construct]] path, users will reach for fn.mock.instances[0] (the canonical Jest API for constructor calls) and find it empty. Might be worth pushing thisValue into instances alongside contexts while you're in here, but it shouldn't block this crash fix.

Extended reasoning...

What the bug is

JSMockFunction declares a WriteBarrier<JSArray> instances field, lazily creates it via getInstances(), exposes it at offset 2 of the mock-object structure (the mock.instances property), clears it in clear(), and visits it in GC. But nothing in jsMockFunctionCallImpl (or anywhere else in the file) ever pushes to it. So fn.mock.instances is always an empty array regardless of how many times the mock is called or constructed.

In Jest, mock.instances records the this value for every invocation — for new fn() that's the newly created instance, and for plain calls it's whatever this was bound to. It is the documented way to assert on constructed instances (e.g. expect(Foo.mock.instances[0]).toBe(a)).

Code path

Walk through const fn = jest.fn(); const inst = new fn(); after this PR:

1. jsMockFunctionConstruct runs, builds thisObject from new.target.prototype.
2. It calls jsMockFunctionCallImpl(globalObject, callframe, thisObject).
3. Inside the impl, fn->calls gets [ [] ], fn->contexts gets [thisObject], fn->invocationCallOrder gets [n], and fn->returnValues gets [{type:'return', value:undefined}].
4. There is no statement of the form fn->instances...->push(...) or instances->initializeIndex(...) — grep for instances in the file and you'll find only the declaration, clear(), getInstances(), the GC visit, and the structure offset.
5. jsMockFunctionConstruct returns thisObject.
6. fn.mock.instances is still []; fn.mock.contexts is [inst].

The PR's own test asserts expect(fn.mock.contexts[0]).toBe(instance) rather than fn.mock.instances[0], which is consistent with instances being empty.

Why existing code doesn't prevent it

There simply is no write site. The array is wired up end-to-end on the read side (lazy creation, structure slot, GC) but the population step was never implemented. Before this PR it mattered less because constructing a mock didn't have proper semantics anyway (it returned primitives / asserted in debug). Now that [[Construct]] is real, the gap becomes user-visible in the exact scenario this PR enables.

Impact

Tests ported from Jest that do things like:

const Ctor = jest.fn();
const a = new Ctor();
expect(Ctor.mock.instances).toContain(a); // fails: instances is []

will fail under bun:test even though new Ctor() now behaves correctly. The workaround is mock.contexts, but that's not what Jest users typically reach for when testing constructors.

How to fix

In jsMockFunctionCallImpl, mirror the contexts push for instances: after pushing thisValue to fn->contexts, also push it to fn->instances (lazily creating the array the same way). Jest records this in instances for every call, not just constructor calls, so doing it unconditionally in the shared impl matches upstream semantics and keeps instances.length === calls.length.

Severity

Pre-existing. instances was never populated before this PR either; this change just makes the omission relevant. It shouldn't block landing the crash fix, but it's a natural follow-up (or a one-line addition here if the author prefers).

[robobun]

Closing as a duplicate of #30212, which fixes the same isCell() assertion when constructing a mock function and is already reviewed with green CI. The extra regression test from this PR (constructing a spy created by spyOn on a missing property) has been added to #30212 in daaa8d9.