Skip to content

Commit 6f8bafa

Browse files
committed
test_runner: fix virtual mock to resolve canonical URL before mocking
1 parent c1881e2 commit 6f8bafa

File tree

4 files changed

+100
-47
lines changed

4 files changed

+100
-47
lines changed

lib/internal/test_runner/mock/loader.js

Lines changed: 19 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -18,23 +18,28 @@ const mocks = new SafeMap();
1818
function resolve(specifier, context, nextResolve) {
1919
debug('resolve hook entry, specifier = "%s", context = %o', specifier, context);
2020

21-
// Virtual mocks - skip resolution, the module may not exist on disk.
22-
const virtualMock = mocks.get(specifier);
23-
if (virtualMock?.virtual && virtualMock?.active === true) {
24-
const url = new URL(virtualMock.url);
25-
url.searchParams.set(kMockSearchParam, virtualMock.localVersion);
26-
if (!virtualMock.cache) {
27-
virtualMock.localVersion++;
21+
// Try normal resolution first. If it fails, check for a virtual mock
22+
// of a non-existent module.
23+
let nextResolveResult;
24+
try {
25+
nextResolveResult = nextResolve(specifier, context);
26+
} catch (resolveError) {
27+
// Resolution failed - check if there's a virtual mock for this specifier.
28+
const virtualMock = mocks.get(specifier);
29+
if (virtualMock?.active === true && virtualMock?.virtual) {
30+
const url = new URL(virtualMock.url);
31+
url.searchParams.set(kMockSearchParam, virtualMock.localVersion);
32+
if (!virtualMock.cache) {
33+
virtualMock.localVersion++;
34+
}
35+
const { href } = url;
36+
debug('resolve hook finished (virtual), url = "%s"', href);
37+
return { __proto__: null, url: href, format: virtualMock.format, shortCircuit: true };
2838
}
29-
30-
const { href } = url;
31-
debug('resolve hook finished (virtual), url = "%s"', href);
32-
return { __proto__: null, url: href, format: virtualMock.format, shortCircuit: true };
39+
throw resolveError;
3340
}
3441

35-
const nextResolveResult = nextResolve(specifier, context);
3642
const mockSpecifier = nextResolveResult.url;
37-
3843
const mock = mocks.get(mockSpecifier);
3944
debug('resolve hook, specifier = "%s", mock = %o', specifier, mock);
4045

@@ -53,7 +58,7 @@ function resolve(specifier, context, nextResolve) {
5358

5459
const { href } = url;
5560
debug('resolve hook finished, url = "%s"', href);
56-
return { __proto__: null, url: href, format: nextResolveResult.format };
61+
return { __proto__: null, url: href, format: mock.format || nextResolveResult.format };
5762
}
5863

5964
function load(url, context, nextLoad) {

lib/internal/test_runner/mock/mock.js

Lines changed: 33 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -209,7 +209,8 @@ class MockModuleContext {
209209
if (fullPath) {
210210
sharedState.mockMap.set(fullPath, config);
211211
} else if (virtual) {
212-
// Virtual mock - store under raw specifier for CJS resolution fallback.
212+
// Virtual mock for a non-existent module - store under raw specifier
213+
// so CJS resolution fallback can find it.
213214
sharedState.mockMap.set(specifier, config);
214215
}
215216

@@ -278,7 +279,7 @@ class MockModuleContext {
278279

279280
this.#sharedState.mockMap.delete(this.#restore.fullPath);
280281
} else if (this.#restore.specifier !== undefined) {
281-
// Virtual mock - clean up specifier key.
282+
// Virtual mock for non-existent module - clean up specifier key.
282283
this.#sharedState.mockMap.delete(this.#restore.specifier);
283284
}
284285

@@ -673,35 +674,27 @@ class MockTracker {
673674
// If the caller is already a file URL, use it as is. Otherwise, convert it.
674675
const hasFileProtocol = StringPrototypeStartsWith(filename, 'file://');
675676
const caller = hasFileProtocol ? filename : pathToFileURL(filename).href;
676-
if (virtual) {
677-
const url = `mock:///${encodeURIComponent(mockSpecifier)}`;
678-
const format = 'module';
679-
const baseURL = URLParse(url);
680-
const ctx = new MockModuleContext({
681-
__proto__: null,
682-
baseURL: baseURL.href,
683-
cache,
684-
caller,
685-
defaultExport,
686-
format,
687-
fullPath: null,
688-
hasDefaultExport,
689-
namedExports,
690-
sharedState,
691-
specifier: mockSpecifier,
692-
virtual,
693-
});
677+
const request = { __proto__: null, specifier: mockSpecifier, attributes: kEmptyObject };
694678

695-
ArrayPrototypePush(this.#mocks, {
696-
__proto__: null,
697-
ctx,
698-
restore: restoreModule,
699-
});
700-
return ctx;
679+
// Try to resolve the specifier. For virtual mocks, if the module exists
680+
// on disk, use its canonical URL so that all resolution paths to that
681+
// module are properly intercepted. Only fall back to a synthetic URL
682+
// if the module truly doesn't exist.
683+
let format, url;
684+
if (virtual) {
685+
try {
686+
({ url } = sharedState.moduleLoader.resolveSync(caller, request));
687+
} catch {
688+
// Module doesn't exist - use a synthetic URL.
689+
url = `mock:///${encodeURIComponent(mockSpecifier)}`;
690+
}
691+
// Virtual mocks always use 'module' format since the generated source
692+
// is ESM regardless of the original module's format.
693+
format = 'module';
694+
} else {
695+
({ format, url } = sharedState.moduleLoader.resolveSync(caller, request));
701696
}
702697

703-
const request = { __proto__: null, specifier: mockSpecifier, attributes: kEmptyObject };
704-
const { format, url } = sharedState.moduleLoader.resolveSync(caller, request);
705698
debug('module mock, url = "%s", format = "%s", caller = "%s"', url, format, caller);
706699
if (format) { // Format is not yet known for ambiguous files when detection is enabled.
707700
validateOneOf(format, 'format', kSupportedFormats);
@@ -896,13 +889,20 @@ function setupSharedModuleState() {
896889
function cjsMockModuleLoad(request, parent, isMain) {
897890
let resolved;
898891

899-
// Virtual mock - skip resolution, the module doesn't exist on disk.
900-
if (this.mockMap.get(request)?.virtual) {
901-
resolved = request;
902-
} else if (isBuiltin(request)) {
892+
if (isBuiltin(request)) {
903893
resolved = ensureNodeScheme(request);
904894
} else {
905-
resolved = _resolveFilename(request, parent, isMain);
895+
try {
896+
resolved = _resolveFilename(request, parent, isMain);
897+
} catch (resolveError) {
898+
// Resolution failed - check if there's a virtual mock for this specifier.
899+
const virtualConfig = this.mockMap.get(request);
900+
if (virtualConfig?.virtual) {
901+
resolved = request;
902+
} else {
903+
throw resolveError;
904+
}
905+
}
906906
}
907907

908908
const config = this.mockMap.get(resolved);
Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
'use strict';
2+
const assert = require('node:assert');
3+
const { test } = require('node:test');
4+
5+
// This test verifies that a virtual mock of an existing package intercepts
6+
// ALL resolution paths to that module, not just the exact specifier used
7+
// to create the mock. This is critical to avoid multiple sources of truth.
8+
9+
test('virtual mock intercepts all resolution paths to the same module', async (t) => {
10+
t.mock.module('reporter-cjs', {
11+
virtual: true,
12+
namedExports: { fn() { return 'mocked'; } },
13+
});
14+
15+
// Access via package name (the specifier used to create the mock).
16+
const byName = require('reporter-cjs');
17+
assert.strictEqual(byName.fn(), 'mocked');
18+
19+
// Access via relative path to the actual file on disk. This resolves to
20+
// the same canonical file:// URL, so it must also return the mock.
21+
const byRelativePath = require('./node_modules/reporter-cjs/index.js');
22+
assert.strictEqual(byRelativePath.fn(), 'mocked');
23+
24+
// Access via import() by package name.
25+
const byImport = await import('reporter-cjs');
26+
assert.strictEqual(byImport.fn(), 'mocked');
27+
});

test/parallel/test-runner-module-mocking.js

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -759,9 +759,30 @@ test('virtual mock overrides an existing module', async (t) => {
759759
namedExports: { custom() { return 'virtual'; } },
760760
});
761761

762+
// Both 'readline' and 'node:readline' should resolve to the mock
763+
// because the specifier resolves to the same canonical URL.
762764
const mocked = await import('readline');
763765
assert.strictEqual(mocked.custom(), 'virtual');
764766
assert.strictEqual(mocked.cursorTo, undefined);
767+
768+
const mockedWithPrefix = await import('node:readline');
769+
assert.strictEqual(mockedWithPrefix.custom(), 'virtual');
770+
assert.strictEqual(mockedWithPrefix.cursorTo, undefined);
771+
});
772+
773+
test('virtual mock intercepts all resolution paths to the same module', async (t) => {
774+
const cwd = fixtures.path('test-runner');
775+
const fixture = fixtures.path('test-runner', 'mock-virtual-paths.js');
776+
const args = ['--experimental-test-module-mocks', fixture];
777+
const {
778+
code,
779+
stdout,
780+
signal,
781+
} = await common.spawnPromisified(process.execPath, args, { cwd });
782+
783+
assert.strictEqual(code, 0);
784+
assert.strictEqual(signal, null);
785+
assert.match(stdout, /pass 1/);
765786
});
766787

767788
test('input validation for virtual option', async (t) => {

0 commit comments

Comments
 (0)