test_runner: make mock_loader not confuse CJS and ESM resolution

PR-URL: #53846 Fixes: #53807 Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
nodejs · Aug 6, 2024 · 5bfebfe · 5bfebfe
1 parent 24ed890
commit 5bfebfe
Show file tree

Hide file tree

Showing 4 changed files with 39 additions and 4 deletions.
diff --git a/lib/test/mock_loader.js b/lib/test/mock_loader.js
@@ -23,6 +23,8 @@ let debug = require('internal/util/debuglog').debuglog('test_runner', (fn) => {
  debug = fn;
 });
 const { createRequire, isBuiltin } = require('module');
+const { defaultGetFormatWithoutErrors } = require('internal/modules/esm/get_format');
+const { defaultResolve } = require('internal/modules/esm/resolve');
 
 // TODO(cjihrig): This file should not be exposed publicly, but register() does
 // not handle internal loaders. Before marking this API as stable, one of the
@@ -88,11 +90,20 @@ async function resolve(specifier, context, nextResolve) {
  if (isBuiltin(specifier)) {
  mockSpecifier = ensureNodeScheme(specifier);
  } else {
- // TODO(cjihrig): This try...catch should be replaced by defaultResolve(),
- // but there are some edge cases that caused the tests to fail on Windows.
+ let format;
+
+ if (context.parentURL) {
+ format = defaultGetFormatWithoutErrors(pathToFileURL(context.parentURL));
+ }
+
  try {
- const req = createRequire(context.parentURL);
- specifier = pathToFileURL(req.resolve(specifier)).href;
+ if (format === 'module') {
+ specifier = defaultResolve(specifier, context).url;
+ } else {
+ specifier = pathToFileURL(
+ createRequire(context.parentURL).resolve(specifier),
+ ).href;
+ }
  } catch {
  const parentURL = normalizeReferrerURL(context.parentURL);
  const parsedURL = URL.parse(specifier, parentURL)?.href;

diff --git a/test/fixtures/module-mocking/basic-esm-without-extension.js b/test/fixtures/module-mocking/basic-esm-without-extension.js
@@ -0,0 +1 @@
+export let string = 'original esm string';
diff --git a/test/fixtures/module-mocking/wrong-import-after-module-mocking.js b/test/fixtures/module-mocking/wrong-import-after-module-mocking.js
@@ -0,0 +1,8 @@
+import { mock } from 'node:test';
+
+try {
+ mock.module?.('Whatever, this is not significant', { namedExports: {} });
+} catch {}
+
+const { string } = await import('./basic-esm-without-extension');
+console.log(`Found string: ${string}`); // prints 'original esm string'
diff --git a/test/parallel/test-runner-module-mocking.js b/test/parallel/test-runner-module-mocking.js
@@ -638,3 +638,18 @@ test('defaultExports work with ESM mocks in both module systems', async (t) => {
  assert.strictEqual((await import(fixture)).default, defaultExport);
  assert.strictEqual(require(fixture), defaultExport);
 });
+
+test('wrong import syntax should throw error after module mocking.', async () => {
+ const { stdout, stderr, code } = await common.spawnPromisified(
+ process.execPath,
+ [
+ '--experimental-test-module-mocks',
+ '--experimental-default-type=module',
+ fixtures.path('module-mocking/wrong-import-after-module-mocking.js'),
+ ]
+ );
+
+ assert.strictEqual(stdout, '');
+ assert.match(stderr, /Error \[ERR_MODULE_NOT_FOUND\]: Cannot find module/);
+ assert.strictEqual(code, 1);
+});