Skip to content

test_runner: make mock.module's specifier consistent with import() #54416

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Aug 20, 2024
Merged

test_runner: make mock.module's specifier consistent with import() #54416

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
1a6e2f2
test_runner: make `mock.module`'s `specifier` consistent with `import()`
aduh95 Aug 16, 2024
6855057
fixup! test_runner: make `mock.module`'s `specifier` consistent with …
aduh95 Aug 17, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion doc/api/test.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2050,7 +2050,7 @@ added: v22.3.0

> Stability: 1.0 - Early development

* `specifier` {string} A string identifying the module to mock.
* `specifier` {string|URL} A string identifying the module to mock.
* `options` {Object} Optional configuration options for the mock module. The
following properties are supported:
* `cache` {boolean} If `false`, each call to `require()` or `import()`
Expand Down
9 changes: 6 additions & 3 deletions lib/internal/test_runner/mock/mock.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ const {
} = require('internal/errors');
const esmLoader = require('internal/modules/esm/loader');
const { getOptionValue } = require('internal/options');
const { fileURLToPath, toPathIfFileURL, URL } = require('internal/url');
const { fileURLToPath, toPathIfFileURL, URL, isURL } = require('internal/url');
const {
emitExperimentalWarning,
getStructuredStack,
Expand All @@ -49,7 +49,6 @@ const {
validateInteger,
validateObject,
validateOneOf,
validateString,
} = require('internal/validators');
const { MockTimers } = require('internal/test_runner/mock/mock_timers');
const { strictEqual, notStrictEqual } = require('assert');
Expand Down Expand Up @@ -488,7 +487,11 @@ class MockTracker {

module(specifier, options = kEmptyObject) {
emitExperimentalWarning('Module mocking');
validateString(specifier, 'specifier');
if (typeof specifier !== 'string') {
if (!isURL(specifier))
throw new ERR_INVALID_ARG_TYPE('specifier', ['string', 'URL'], specifier);
specifier = `${specifier}`;
}
validateObject(options, 'options');
debug('module mock entry, specifier = "%s", options = %o', specifier, options);

Expand Down
59 changes: 32 additions & 27 deletions test/parallel/test-runner-module-mocking.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ const fixtures = require('../common/fixtures');
const assert = require('node:assert');
const { relative } = require('node:path');
const { test } = require('node:test');
const { fileURLToPath, pathToFileURL } = require('node:url');
const { pathToFileURL } = require('node:url');

test('input validation', async (t) => {
await t.test('throws if specifier is not a string', (t) => {
Expand Down Expand Up @@ -154,7 +154,7 @@ test('CJS mocking with namedExports option', async (t) => {
assert.strictEqual(original.string, 'original cjs string');
assert.strictEqual(original.fn, undefined);

t.mock.module(fixture, {
t.mock.module(pathToFileURL(fixture), {
namedExports: { fn() { return 42; } },
});
const mocked = require(fixture);
Expand All @@ -174,7 +174,7 @@ test('CJS mocking with namedExports option', async (t) => {
assert.strictEqual(original.string, 'original cjs string');
assert.strictEqual(original.fn, undefined);

t.mock.module(fixture, {
t.mock.module(pathToFileURL(fixture), {
namedExports: { fn() { return 42; } },
cache: true,
});
Expand All @@ -195,7 +195,7 @@ test('CJS mocking with namedExports option', async (t) => {
assert.strictEqual(original.string, 'original cjs string');
assert.strictEqual(original.fn, undefined);

t.mock.module(fixture, {
t.mock.module(pathToFileURL(fixture), {
namedExports: { fn() { return 42; } },
cache: false,
});
Expand All @@ -219,7 +219,7 @@ test('CJS mocking with namedExports option', async (t) => {

const defaultExport = { val1: 5, val2: 3 };

t.mock.module(fixture, {
t.mock.module(pathToFileURL(fixture), {
defaultExport,
namedExports: { val1: 'mock value' },
});
Expand All @@ -242,7 +242,7 @@ test('CJS mocking with namedExports option', async (t) => {

const defaultExport = null;

t.mock.module(fixture, {
t.mock.module(pathToFileURL(fixture), {
defaultExport,
namedExports: { val1: 'mock value' },
});
Expand All @@ -256,7 +256,7 @@ test('CJS mocking with namedExports option', async (t) => {

test('ESM mocking with namedExports option', async (t) => {
await t.test('does not cache by default', async (t) => {
const fixture = fixtures.path('module-mocking', 'basic-esm.mjs');
const fixture = fixtures.fileURL('module-mocking', 'basic-esm.mjs');
const original = await import(fixture);

assert.strictEqual(original.string, 'original esm string');
Expand All @@ -276,7 +276,7 @@ test('ESM mocking with namedExports option', async (t) => {
});

await t.test('explicitly enables caching', async (t) => {
const fixture = fixtures.path('module-mocking', 'basic-esm.mjs');
const fixture = fixtures.fileURL('module-mocking', 'basic-esm.mjs');
const original = await import(fixture);

assert.strictEqual(original.string, 'original esm string');
Expand All @@ -297,7 +297,7 @@ test('ESM mocking with namedExports option', async (t) => {
});

await t.test('explicitly disables caching', async (t) => {
const fixture = fixtures.path('module-mocking', 'basic-esm.mjs');
const fixture = fixtures.fileURL('module-mocking', 'basic-esm.mjs');
const original = await import(fixture);

assert.strictEqual(original.string, 'original esm string');
Expand All @@ -318,7 +318,8 @@ test('ESM mocking with namedExports option', async (t) => {
});

await t.test('named exports are not applied to defaultExport', async (t) => {
const fixture = fixtures.path('module-mocking', 'basic-esm.mjs');
const fixturePath = fixtures.path('module-mocking', 'basic-esm.mjs');
const fixture = pathToFileURL(fixturePath);
const original = await import(fixture);

assert.strictEqual(original.string, 'original esm string');
Expand All @@ -338,11 +339,11 @@ test('ESM mocking with namedExports option', async (t) => {
assert.strictEqual(mocked.default, 'mock default');
assert.strictEqual(mocked.val1, 'mock value');
t.mock.reset();
common.expectRequiredModule(require(fixture), original);
common.expectRequiredModule(require(fixturePath), original);
});

await t.test('throws if named exports cannot be applied to defaultExport as CJS', async (t) => {
const fixture = fixtures.path('module-mocking', 'basic-cjs.js');
const fixture = fixtures.fileURL('module-mocking', 'basic-cjs.js');
const original = await import(fixture);

assert.strictEqual(original.default.string, 'original cjs string');
Expand All @@ -366,13 +367,14 @@ test('ESM mocking with namedExports option', async (t) => {
test('modules cannot be mocked multiple times at once', async (t) => {
await t.test('CJS', async (t) => {
const fixture = fixtures.path('module-mocking', 'basic-cjs.js');
const fixtureURL = pathToFileURL(fixture).href;

t.mock.module(fixture, {
t.mock.module(fixtureURL, {
namedExports: { fn() { return 42; } },
});

assert.throws(() => {
t.mock.module(fixture, {
t.mock.module(fixtureURL, {
namedExports: { fn() { return 55; } },
});
}, {
Expand All @@ -386,7 +388,7 @@ test('modules cannot be mocked multiple times at once', async (t) => {
});

await t.test('ESM', async (t) => {
const fixture = fixtures.path('module-mocking', 'basic-esm.mjs');
const fixture = fixtures.fileURL('module-mocking', 'basic-esm.mjs').href;

t.mock.module(fixture, {
namedExports: { fn() { return 42; } },
Expand All @@ -409,10 +411,10 @@ test('modules cannot be mocked multiple times at once', async (t) => {

test('mocks are automatically restored', async (t) => {
const cjsFixture = fixtures.path('module-mocking', 'basic-cjs.js');
const esmFixture = fixtures.path('module-mocking', 'basic-esm.mjs');
const esmFixture = fixtures.fileURL('module-mocking', 'basic-esm.mjs');

await t.test('CJS', async (t) => {
t.mock.module(cjsFixture, {
t.mock.module(pathToFileURL(cjsFixture), {
namedExports: { fn() { return 42; } },
});

Expand Down Expand Up @@ -442,9 +444,9 @@ test('mocks are automatically restored', async (t) => {

test('mocks can be restored independently', async (t) => {
const cjsFixture = fixtures.path('module-mocking', 'basic-cjs.js');
const esmFixture = fixtures.path('module-mocking', 'basic-esm.mjs');
const esmFixture = fixtures.fileURL('module-mocking', 'basic-esm.mjs');

const cjsMock = t.mock.module(cjsFixture, {
const cjsMock = t.mock.module(pathToFileURL(cjsFixture), {
namedExports: { fn() { return 42; } },
});

Expand Down Expand Up @@ -511,18 +513,19 @@ test('node:- core module mocks can be used by both module systems', async (t) =>

test('CJS mocks can be used by both module systems', async (t) => {
const cjsFixture = fixtures.path('module-mocking', 'basic-cjs.js');
const cjsMock = t.mock.module(cjsFixture, {
const cjsFixtureURL = pathToFileURL(cjsFixture);
const cjsMock = t.mock.module(cjsFixtureURL, {
namedExports: { fn() { return 42; } },
});
let esmImpl = await import(pathToFileURL(cjsFixture));
let esmImpl = await import(cjsFixtureURL);
let cjsImpl = require(cjsFixture);

assert.strictEqual(esmImpl.fn(), 42);
assert.strictEqual(cjsImpl.fn(), 42);

cjsMock.restore();

esmImpl = await import(pathToFileURL(cjsFixture));
esmImpl = await import(cjsFixtureURL);
cjsImpl = require(cjsFixture);

assert.strictEqual(esmImpl.default.string, 'original cjs string');
Expand All @@ -532,7 +535,7 @@ test('CJS mocks can be used by both module systems', async (t) => {
test('relative paths can be used by both module systems', async (t) => {
const fixture = relative(
__dirname, fixtures.path('module-mocking', 'basic-esm.mjs')
);
).replaceAll('\\', '/');
const mock = t.mock.module(fixture, {
namedExports: { fn() { return 42; } },
});
Expand Down Expand Up @@ -597,24 +600,26 @@ test('mocked modules do not impact unmocked modules', async (t) => {

test('defaultExports work with CJS mocks in both module systems', async (t) => {
const fixture = fixtures.path('module-mocking', 'basic-cjs.js');
const fixtureURL = pathToFileURL(fixture);
const original = require(fixture);
const defaultExport = Symbol('default');

assert.strictEqual(original.string, 'original cjs string');
t.mock.module(fixture, { defaultExport });
t.mock.module(fixtureURL, { defaultExport });
assert.strictEqual(require(fixture), defaultExport);
assert.strictEqual((await import(pathToFileURL(fixture))).default, defaultExport);
assert.strictEqual((await import(fixtureURL)).default, defaultExport);
});

test('defaultExports work with ESM mocks in both module systems', async (t) => {
const fixture = fixtures.fileURL('module-mocking', 'basic-esm.mjs');
const fixturePath = fixtures.path('module-mocking', 'basic-esm.mjs');
const fixture = pathToFileURL(fixturePath);
const original = await import(fixture);
const defaultExport = Symbol('default');

assert.strictEqual(original.string, 'original esm string');
t.mock.module(`${fixture}`, { defaultExport });
assert.strictEqual((await import(fixture)).default, defaultExport);
assert.strictEqual(require(fileURLToPath(fixture)), defaultExport);
assert.strictEqual(require(fixturePath), defaultExport);
});

test('wrong import syntax should throw error after module mocking.', async () => {
Expand Down
Loading