From eb0fe1993fffc1c35c3e6bcab87ee8d81c3281ca Mon Sep 17 00:00:00 2001 From: Antoine du Hamel Date: Wed, 21 Aug 2024 17:44:29 +0200 Subject: [PATCH] test_runner: refactor `mock_loader` PR-URL: https://github.com/nodejs/node/pull/54223 Reviewed-By: Yagiz Nizipli Reviewed-By: Colin Ihrig Reviewed-By: Chemi Atlow Reviewed-By: Moshe Atlow Reviewed-By: James M Snell --- lib/internal/test_runner/mock/loader.js | 48 ++++----------------- lib/internal/test_runner/mock/mock.js | 10 ++++- test/parallel/test-runner-module-mocking.js | 6 +++ 3 files changed, 22 insertions(+), 42 deletions(-) diff --git a/lib/internal/test_runner/mock/loader.js b/lib/internal/test_runner/mock/loader.js index a434248bcced16..25d9b31d0cb74d 100644 --- a/lib/internal/test_runner/mock/loader.js +++ b/lib/internal/test_runner/mock/loader.js @@ -10,21 +10,16 @@ const { }, } = primordials; const { - ensureNodeScheme, kBadExportsMessage, kMockSearchParam, kMockSuccess, kMockExists, kMockUnknownMessage, } = require('internal/test_runner/mock/mock'); -const { pathToFileURL, URL } = require('internal/url'); -const { normalizeReferrerURL } = require('internal/modules/helpers'); +const { URL, URLParse } = require('internal/url'); 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): The mocks need to be thread aware because the exports are // evaluated on the thread that creates the mock. Before marking this API as @@ -79,46 +74,18 @@ async function initialize(data) { async function resolve(specifier, context, nextResolve) { debug('resolve hook entry, specifier = "%s", context = %o', specifier, context); - let mockSpecifier; - if (isBuiltin(specifier)) { - mockSpecifier = ensureNodeScheme(specifier); - } else { - let format; - - if (context.parentURL) { - format = defaultGetFormatWithoutErrors(pathToFileURL(context.parentURL)); - } - - try { - 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; - - if (parsedURL) { - specifier = parsedURL; - } - } - - mockSpecifier = specifier; - } + const nextResolveResult = await nextResolve(specifier, context); + const mockSpecifier = nextResolveResult.url; const mock = mocks.get(mockSpecifier); debug('resolve hook, specifier = "%s", mock = %o', specifier, mock); if (mock?.active !== true) { - return nextResolve(specifier, context); + return nextResolveResult; } const url = new URL(mockSpecifier); - url.searchParams.set(kMockSearchParam, mock.localVersion); if (!mock.cache) { @@ -127,13 +94,14 @@ async function resolve(specifier, context, nextResolve) { mock.localVersion++; } - debug('resolve hook finished, url = "%s"', url.href); - return nextResolve(url.href, context); + const { href } = url; + debug('resolve hook finished, url = "%s"', href); + return { __proto__: null, url: href, format: nextResolveResult.format }; } async function load(url, context, nextLoad) { debug('load hook entry, url = "%s", context = %o', url, context); - const parsedURL = URL.parse(url); + const parsedURL = URLParse(url); if (parsedURL) { parsedURL.searchParams.delete(kMockSearchParam); } diff --git a/lib/internal/test_runner/mock/mock.js b/lib/internal/test_runner/mock/mock.js index a0de3d2dd41909..5d3bce816aa69b 100644 --- a/lib/internal/test_runner/mock/mock.js +++ b/lib/internal/test_runner/mock/mock.js @@ -34,7 +34,13 @@ const { } = require('internal/errors'); const esmLoader = require('internal/modules/esm/loader'); const { getOptionValue } = require('internal/options'); -const { fileURLToPath, toPathIfFileURL, URL, isURL } = require('internal/url'); +const { + fileURLToPath, + isURL, + pathToFileURL, + toPathIfFileURL, + URL, +} = require('internal/url'); const { emitExperimentalWarning, getStructuredStack, @@ -511,7 +517,7 @@ class MockTracker { // Get the file that called this function. We need four stack frames: // vm context -> getStructuredStack() -> this function -> actual caller. - const caller = getStructuredStack()[3]?.getFileName(); + const caller = pathToFileURL(getStructuredStack()[3]?.getFileName()).href; const { format, url } = sharedState.moduleLoader.resolveSync( mockSpecifier, caller, null, ); diff --git a/test/parallel/test-runner-module-mocking.js b/test/parallel/test-runner-module-mocking.js index a9a5c33a7c26b4..45345815c69db4 100644 --- a/test/parallel/test-runner-module-mocking.js +++ b/test/parallel/test-runner-module-mocking.js @@ -407,6 +407,12 @@ test('modules cannot be mocked multiple times at once', async (t) => { assert.strictEqual(mocked.fn(), 42); }); + + await t.test('Importing a Windows path should fail', { skip: !common.isWindows }, async (t) => { + const fixture = fixtures.path('module-mocking', 'wrong-path.js'); + t.mock.module(fixture, { namedExports: { fn() { return 42; } } }); + await assert.rejects(import(fixture), { code: 'ERR_UNSUPPORTED_ESM_URL_SCHEME' }); + }); }); test('mocks are automatically restored', async (t) => {