Skip to content

bun test: keep the owners of a mocked module export in the engine, so mocks and spies end in any order - #48621

Draft
robobun wants to merge 1 commit into
mainfrom
robobun/929034ed/unmock-restores-in-any-order
Draft

robobun wants to merge 1 commit into
mainfrom
robobun/929034ed/unmock-restores-in-any-order

Conversation

@robobun

@robobun robobun commented Nov 11, 2026

Copy link
Copy Markdown
Collaborator

Problem

  • Two module mocks patch one variable when two modules export it (export * from). If the older mock ends first, a mock value stays: the next test file reads "mock of inner" where it expects "real".
  • Each mock stores back its own copy of what it replaced (restoreOverwrittenExports, src/jsc/bindings/BunPlugin.cpp:3579). The copy of the newer mock is the value of the older one. A namespace spyOn (JSMockFunction.cpp:1002) and bun test --aot do the same.

Fix

  • WebKit ([JSC] Overrides of a module export have owners and end in any order WebKit#1033) keeps the owners of an overridden export variable in order, and what each found. The owner on top stores that back. Any other owner hands it to the owner above.
  • mock.module() and a namespace spyOn store as owners, unmockModule and mockRestore end by owner. A displaced preload's mock comes back in place when the file ends.
  • Verified: test/js/bun/test/mock/mock-module.test.ts (19 new tests, main fails 16), the other mock and spy suites, test/aot.

Background

  • A module mock of a loaded ES module stores its values in the module's variables, so existing importers read them.
  • Considered a hand-over between Bun's mocks at removal: it walks every other mock, and spies and --aot stay wrong.

Downsides

  • Behaviour change: a module mock keeps its value when a test restores a spy of the same export. Before, the restore put the real value back.
  • Per export, a mock and unmock costs 2,756 instructions, from 2,497. A namespace spyOn plus mockRestore costs 5,683, from 4,813. .text grows by 14,080 bytes.
  • bun test --aot stays wrong for a builtin module behind a compiled barrel.
Notes

Pin. WEBKIT_VERSION points at the preview build of oven-sh/WebKit#1033. It moves to the merged commit before this PR merges. That WebKit branch starts at the commit that main pins.

Repro. variable.js exports a. inner.js and outer.js export it again with export *. A test mocks ./inner.js, then ./outer.js, then calls vi.doUnmock("./inner.js"). The next test file reads a.

while both stand after doUnmock(inner) next file
before mock of outer real mock of inner
after mock of outer mock of outer real

Same cause, fixed here.

  • vi.doMock() or a second mock.module() over the older patch.
  • A disposed using handle of a mock of a builtin module under the mock of a barrel. node:path stayed mocked in the next file.
  • Two { spy: true } mocks of two barrels of one class.
  • A mock that a hook of a preload lays over the mock of a test file.
  • Two mocks of a preload that a file removes oldest first, and their return when the file ends.
  • spyOn(namespace) and a module mock of one export, in either order. A spy that stands over a mock calls the real function once the mock ends.
  • Two mocks of modules that hand on one namespace with export * as. The variable kept a mock value after the file.
  • bun test --aot: the mocks of two compiled files that export one variable, and a spy over one.

The behaviour change. spyOn(ns, "a"), then mock.module() of a module that exports a, then spy.mockRestore(): the export keeps the value of the mock until the mock ends. Before, the restore stored the real function and the standing mock showed nothing. docs/test/mocks.mdx says that mock.restore() doesn't remove module mocks.

The end of a test file keeps its newest-first order (patchOrder). The order no longer decides the values. It is the cheap order: the owner that stored last ends without a walk.

Left for later.

Earlier work. #48121 (a base plus owner layers per property) and #35669 (an undo record keyed on the leaf binding) keep the record in Bun, above the store. This PR keeps it where every store meets.

Checks.

  • test/js/bun/test/mock/mock-module.test.ts: 332 pass, 1 todo on a debug build with ASAN.
  • mock-hoist, mock-module-non-string, mock-module-resolve-log, mock-fn, mock-disposable, spyMatchers, vi-utils, vitest-apis, vitest-flavor, jest-extended: pass.
  • BUN_JSC_validateExceptionChecks=1 on the repros: clean.
  • test/aot: the fixed-runs, mock-* and spy-* files pass on a release build.

Numbers. Release builds with the same flags, base against this change, each with its side of WebKit. They are from before the rebase onto #48478 (Bun 89452d2, WebKit 873ab3a4). Instructions are user-mode counts (perf_event_open), the minimum of 3 runs. Two runs of the base differ by up to 0.7%.

before after
sizeof AbstractModuleRecord / JSModuleRecord / JSModuleNamespaceObject / VM / Zig::GlobalObject 320 / 416 / 72 / 110,872 / 10,568 the same
sizeof(JSModuleMock) 256 264
.text 109,836,112 109,850,192
GC cells per mocked export while the mock stands (200 exports) 0 0, and 1 table per module
stores for the repro 4 3
one mock of 100 exports and its end: stores, resolveExport calls 200, 309 200, 209
mock and unmock of a loaded module, per export 2,497 2,756
1,000 modules mocked in place, one export each 173.1M 174.3M
the same, and the end of the file 175.1M 175.9M
the same, each removed with vi.doUnmock() 830.5M 833.9M
vm.SyntheticModule setExport, per call 898 898
read of a mocked export, per read 6.5 6.5
spyOn plus mockRestore of a plain object property (no JIT) 3,546 3,538
spyOn plus mockRestore of a namespace export (no JIT) 4,813 5,683
GC cells per standing namespace spy 6 6
1,000 mocks of 1,000 barrels of one variable, removed oldest first 1,152M 1,344M
the same mocks, ended by the end of the file 162.8M 182.7M

…odule export

mock.module() of a loaded ES module and spyOn() of a namespace export store their values
with JSModuleNamespaceObject::layExportOverride() and end with removeExportOverride().
The engine keeps the owners of each export variable and what each of them found there.
So two mocks of one variable (`export * from`), or a mock and a spy, end in any order:
an owner that ends under a newer one hands over what it found, and stores nothing.

patchOrder and the newest-first pass at the end of a test file go. A preload's mock that
a test file displaces is set aside, and comes back where it stood.
@robobun

robobun commented Nov 11, 2026 •

Copy link
Copy Markdown
Collaborator Author
Updated 9:55 AM PT - Nov 11th, 2026

⏳ @robobun, your commit bc8b142 is still building in Build #140235, but has 2 failures so far (All Failures):

@robobun

robobun commented Nov 11, 2026

Copy link
Copy Markdown
Collaborator Author

Status

Reproduced with five modules. variable.js exports a. inner.js and outer.js export it again with export *. A test file mocks ./inner.js, then ./outer.js, then calls vi.doUnmock("./inner.js"). On main the next test file reads "mock of inner" from all three modules. The row the first is removed while the second stays in test/js/bun/test/mock/mock-module.test.ts is this case.

Fix: #48621, with the engine half in oven-sh/WebKit#1033.

This branch has not been deployed

No deployments
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.

1 participant