fix(test): make mock.restore() restore modules mocked with mock.module()

Previously, mock.restore() only reset function spies (spyOn) but had no
effect on modules mocked with mock.module(). This was because
mock.restore() only called JSMock__resetSpies(), which never touched
the virtualModules map or reversed overrideExportValue patches on ESM
namespace objects.

The fix adds snapshot/restore logic to JSModuleMock:
- Before patching ESM exports, snapshot original values into a plain
  JSObject using WriteBarrier on the existing GC-managed JSModuleMock
- On mock.restore(), iterate virtualModules, restore original export
  values via overrideExportValue, and clear the mock entries
- Preserve true originals across re-mocks of the same specifier

Key discovery: JSModuleNamespaceObject::getOwnPropertyNames() returns 0
exports for Bun's synthetic namespace objects (empty m_names). The fix
uses the mock result object's property names to determine which exports
to snapshot from the namespace.

Closes #7823
This commit is contained in:
Dylan Conway
2026-01-28 23:21:38 -08:00
parent fc4624c672
commit a33f58818a
4 changed files with 216 additions and 1 deletions

View File

@@ -407,6 +407,12 @@ public:
mutable WriteBarrier<JSObject> callbackFunctionOrCachedResult;
bool hasCalledModuleMock = false;
// Original export values snapshot (a plain JS object with properties = original exports).
// Used by mock.restore() to reverse overrideExportValue patches.
WriteBarrier<JSObject> originalExportsSnapshot;
// The target to restore to: JSModuleNamespaceObject (ESM) or JSCommonJSModule (CJS).
WriteBarrier<JSObject> restoreTarget;
static JSModuleMock* create(JSC::VM& vm, JSC::Structure* structure, JSC::JSObject* callback);
static Structure* createStructure(JSC::VM& vm, JSC::JSGlobalObject* globalObject, JSC::JSValue prototype);
@@ -586,6 +592,20 @@ extern "C" JSC_DEFINE_HOST_FUNCTION(JSMock__jsModuleMock, (JSC::JSGlobalObject *
JSModuleMock* mock = JSModuleMock::create(vm, globalObject->mockModule.mockModuleStructure.getInitializedOnMainThread(globalObject), callback);
// If this specifier was already mocked, carry over the original snapshot
// so we preserve the true originals across re-mocks.
if (globalObject->onLoadPlugins.virtualModules) {
auto it = globalObject->onLoadPlugins.virtualModules->find(specifier);
if (it != globalObject->onLoadPlugins.virtualModules->end()) {
if (auto* existingMock = jsDynamicCast<JSModuleMock*>(it->value.get())) {
if (existingMock->originalExportsSnapshot) {
mock->originalExportsSnapshot.set(vm, mock, existingMock->originalExportsSnapshot.get());
mock->restoreTarget.set(vm, mock, existingMock->restoreTarget.get());
}
}
}
}
auto* esm = globalObject->esmRegistryMap();
RETURN_IF_EXCEPTION(scope, {});
@@ -639,6 +659,31 @@ extern "C" JSC_DEFINE_HOST_FUNCTION(JSMock__jsModuleMock, (JSC::JSGlobalObject *
auto* object = exportsValue.getObject();
removeFromESM = false;
// Snapshot original export values before patching,
// so mock.restore() can reverse the overrideExportValue calls.
// We use the mock result's property names to know which exports
// to snapshot, since synthetic namespace objects may have empty m_names.
if (!mock->originalExportsSnapshot) {
mock->restoreTarget.set(vm, mock, moduleNamespaceObject);
JSObject* snapshot = constructEmptyObject(globalObject);
if (object) {
JSC::PropertyNameArrayBuilder mockNames(vm, PropertyNameMode::Strings, PrivateSymbolMode::Exclude);
JSObject::getOwnPropertyNames(object, globalObject, mockNames, DontEnumPropertiesMode::Exclude);
RETURN_IF_EXCEPTION(scope, {});
for (auto& name : mockNames) {
JSValue originalValue = moduleNamespaceObject->get(globalObject, name);
RETURN_IF_EXCEPTION(scope, {});
snapshot->putDirect(vm, name, originalValue);
}
} else {
JSValue originalDefault = moduleNamespaceObject->get(globalObject, vm.propertyNames->defaultKeyword);
RETURN_IF_EXCEPTION(scope, {});
snapshot->putDirect(vm, vm.propertyNames->defaultKeyword, originalDefault);
}
mock->originalExportsSnapshot.set(vm, mock, snapshot);
}
if (object) {
JSC::PropertyNameArrayBuilder names(vm, PropertyNameMode::Strings, PrivateSymbolMode::Exclude);
JSObject::getOwnPropertyNames(object, globalObject, names, DontEnumPropertiesMode::Exclude);
@@ -676,6 +721,18 @@ extern "C" JSC_DEFINE_HOST_FUNCTION(JSMock__jsModuleMock, (JSC::JSGlobalObject *
if (entryValue) {
removeFromCJS = true;
if (auto* moduleObject = entryValue ? jsDynamicCast<Bun::JSCommonJSModule*>(entryValue) : nullptr) {
// Snapshot original CJS exports before patching.
if (!mock->originalExportsSnapshot) {
JSValue currentExports = moduleObject->getIfPropertyExists(globalObject, Bun::builtinNames(vm).exportsPublicName());
RETURN_IF_EXCEPTION(scope, {});
if (currentExports) {
JSObject* snapshot = constructEmptyObject(globalObject);
snapshot->putDirect(vm, vm.propertyNames->defaultKeyword, currentExports);
mock->originalExportsSnapshot.set(vm, mock, snapshot);
mock->restoreTarget.set(vm, mock, moduleObject);
}
}
JSValue exportsValue = getJSValue();
RETURN_IF_EXCEPTION(scope, {});
@@ -708,10 +765,63 @@ void JSModuleMock::visitChildrenImpl(JSCell* cell, Visitor& visitor)
Base::visitChildren(mock, visitor);
visitor.append(mock->callbackFunctionOrCachedResult);
visitor.append(mock->originalExportsSnapshot);
visitor.append(mock->restoreTarget);
}
DEFINE_VISIT_CHILDREN(JSModuleMock);
void BunPlugin::OnLoad::restoreModuleMocks(JSC::JSGlobalObject* lexicalGlobalObject)
{
if (!virtualModules)
return;
auto& vm = lexicalGlobalObject->vm();
auto scope = DECLARE_THROW_SCOPE(vm);
// Collect specifiers to remove (can't mutate map while iterating).
Vector<String> toRemove;
for (auto& entry : *virtualModules) {
auto* moduleMock = jsDynamicCast<JSModuleMock*>(entry.value.get());
if (!moduleMock) {
continue;
}
toRemove.append(entry.key);
if (!moduleMock->originalExportsSnapshot || !moduleMock->restoreTarget)
continue;
auto* snapshot = moduleMock->originalExportsSnapshot.get();
auto* target = moduleMock->restoreTarget.get();
if (auto* ns = jsDynamicCast<JSC::JSModuleNamespaceObject*>(target)) {
// ESM: restore each export value on the namespace object.
JSC::PropertyNameArrayBuilder names(vm, PropertyNameMode::Strings, PrivateSymbolMode::Exclude);
JSObject::getOwnPropertyNames(snapshot, lexicalGlobalObject, names, DontEnumPropertiesMode::Exclude);
RETURN_IF_EXCEPTION(scope, );
for (auto& name : names) {
JSValue originalValue = snapshot->get(lexicalGlobalObject, name);
RETURN_IF_EXCEPTION(scope, );
ns->overrideExportValue(lexicalGlobalObject, name, originalValue);
RETURN_IF_EXCEPTION(scope, );
}
} else if (auto* cjsModule = jsDynamicCast<Bun::JSCommonJSModule*>(target)) {
// CJS: restore original exports on the module object.
JSValue originalExports = snapshot->get(lexicalGlobalObject, vm.propertyNames->defaultKeyword);
RETURN_IF_EXCEPTION(scope, );
cjsModule->putDirect(vm, Bun::builtinNames(vm).exportsPublicName(), originalExports, 0);
}
}
for (auto& key : toRemove) {
virtualModules->remove(key);
}
}
EncodedJSValue BunPlugin::OnLoad::run(JSC::JSGlobalObject* globalObject, BunString* namespaceString, BunString* path)
{
Group* groupPtr = this->group(namespaceString ? namespaceString->toWTFString(BunString::ZeroCopy) : String());

View File

@@ -76,6 +76,7 @@ public:
bool hasVirtualModules() const { return virtualModules != nullptr; }
void addModuleMock(JSC::VM& vm, const String& path, JSC::JSObject* mock);
void restoreModuleMocks(JSC::JSGlobalObject* globalObject);
std::optional<String> resolveVirtualModule(const String& path, const String& from);

View File

@@ -1461,7 +1461,9 @@ BUN_DEFINE_HOST_FUNCTION(JSMock__jsSetSystemTime, (JSC::JSGlobalObject * globalO
BUN_DEFINE_HOST_FUNCTION(JSMock__jsRestoreAllMocks, (JSC::JSGlobalObject * globalObject, JSC::CallFrame* callframe))
{
JSMock__resetSpies(jsCast<Zig::GlobalObject*>(globalObject));
auto* zigGlobalObject = jsCast<Zig::GlobalObject*>(globalObject);
JSMock__resetSpies(zigGlobalObject);
zigGlobalObject->onLoadPlugins.restoreModuleMocks(globalObject);
return JSValue::encode(jsUndefined());
}

View File

@@ -0,0 +1,102 @@
import { describe, expect, mock, spyOn, test } from "bun:test";
import { fn, variable } from "./mock-module-fixture";
import * as spyFixture from "./spymodule-fixture";
describe("mock.module restore", () => {
test("mock.restore() restores ESM module exports to original values", () => {
expect(fn()).toBe(42);
expect(variable).toBe(7);
mock.module("./mock-module-fixture", () => ({
fn: () => 999,
variable: 100,
}));
expect(fn()).toBe(999);
expect(variable).toBe(100);
mock.restore();
expect(fn()).toBe(42);
expect(variable).toBe(7);
});
test("re-mocking after restore works", () => {
expect(fn()).toBe(42);
expect(variable).toBe(7);
mock.module("./mock-module-fixture", () => ({
fn: () => 555,
variable: 55,
}));
expect(fn()).toBe(555);
expect(variable).toBe(55);
mock.restore();
expect(fn()).toBe(42);
expect(variable).toBe(7);
});
test("multiple re-mocks then restore goes back to true originals", () => {
expect(fn()).toBe(42);
expect(variable).toBe(7);
mock.module("./mock-module-fixture", () => ({
fn: () => 1,
variable: 1,
}));
expect(fn()).toBe(1);
mock.module("./mock-module-fixture", () => ({
fn: () => 2,
variable: 2,
}));
expect(fn()).toBe(2);
mock.module("./mock-module-fixture", () => ({
fn: () => 3,
variable: 3,
}));
expect(fn()).toBe(3);
mock.restore();
expect(fn()).toBe(42);
expect(variable).toBe(7);
});
test("mock.restore() also restores spyOn alongside mock.module", () => {
const originalSpy = spyFixture.iSpy;
spyOn(spyFixture, "iSpy");
expect(spyFixture.iSpy).not.toBe(originalSpy);
mock.module("./mock-module-fixture", () => ({
fn: () => 777,
}));
expect(fn()).toBe(777);
mock.restore();
expect(spyFixture.iSpy).toBe(originalSpy);
expect(fn()).toBe(42);
});
test("mock.restore() restores builtin modules", async () => {
const origReadFile = (await import("node:fs/promises")).readFile;
mock.module("fs/promises", () => ({
readFile: () => Promise.resolve("mocked-content"),
}));
const { readFile } = await import("node:fs/promises");
expect(await readFile("anything")).toBe("mocked-content");
mock.restore();
const { readFile: restored } = await import("node:fs/promises");
expect(restored).toBe(origReadFile);
});
});