Skip to content

Commit 56cbc80

Browse files
syi0808targos
authored andcommittedOct 2, 2024
test_runner: make mock_loader not confuse CJS and ESM resolution
PR-URL: #53846 Fixes: #53807 Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
1 parent 4e7edc4 commit 56cbc80

File tree

4 files changed

+39
-4
lines changed

4 files changed

+39
-4
lines changed
 

Diff for: ‎lib/test/mock_loader.js

+15-4
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,8 @@ let debug = require('internal/util/debuglog').debuglog('test_runner', (fn) => {
2323
debug = fn;
2424
});
2525
const { createRequire, isBuiltin } = require('module');
26+
const { defaultGetFormatWithoutErrors } = require('internal/modules/esm/get_format');
27+
const { defaultResolve } = require('internal/modules/esm/resolve');
2628

2729
// TODO(cjihrig): This file should not be exposed publicly, but register() does
2830
// not handle internal loaders. Before marking this API as stable, one of the
@@ -88,11 +90,20 @@ async function resolve(specifier, context, nextResolve) {
8890
if (isBuiltin(specifier)) {
8991
mockSpecifier = ensureNodeScheme(specifier);
9092
} else {
91-
// TODO(cjihrig): This try...catch should be replaced by defaultResolve(),
92-
// but there are some edge cases that caused the tests to fail on Windows.
93+
let format;
94+
95+
if (context.parentURL) {
96+
format = defaultGetFormatWithoutErrors(pathToFileURL(context.parentURL));
97+
}
98+
9399
try {
94-
const req = createRequire(context.parentURL);
95-
specifier = pathToFileURL(req.resolve(specifier)).href;
100+
if (format === 'module') {
101+
specifier = defaultResolve(specifier, context).url;
102+
} else {
103+
specifier = pathToFileURL(
104+
createRequire(context.parentURL).resolve(specifier),
105+
).href;
106+
}
96107
} catch {
97108
const parentURL = normalizeReferrerURL(context.parentURL);
98109
const parsedURL = URL.parse(specifier, parentURL)?.href;
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
export let string = 'original esm string';
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
import { mock } from 'node:test';
2+
3+
try {
4+
mock.module?.('Whatever, this is not significant', { namedExports: {} });
5+
} catch {}
6+
7+
const { string } = await import('./basic-esm-without-extension');
8+
console.log(`Found string: ${string}`); // prints 'original esm string'

Diff for: ‎test/parallel/test-runner-module-mocking.js

+15
Original file line numberDiff line numberDiff line change
@@ -638,3 +638,18 @@ test('defaultExports work with ESM mocks in both module systems', async (t) => {
638638
assert.strictEqual((await import(fixture)).default, defaultExport);
639639
assert.strictEqual(require(fixture), defaultExport);
640640
});
641+
642+
test('wrong import syntax should throw error after module mocking.', async () => {
643+
const { stdout, stderr, code } = await common.spawnPromisified(
644+
process.execPath,
645+
[
646+
'--experimental-test-module-mocks',
647+
'--experimental-default-type=module',
648+
fixtures.path('module-mocking/wrong-import-after-module-mocking.js'),
649+
]
650+
);
651+
652+
assert.strictEqual(stdout, '');
653+
assert.match(stderr, /Error \[ERR_MODULE_NOT_FOUND\]: Cannot find module/);
654+
assert.strictEqual(code, 1);
655+
});

0 commit comments

Comments
 (0)