Skip to content

Commit

Permalink
test_runner: make mock.module's specifier consistent with import()
Browse files Browse the repository at this point in the history 
The previous implementation was trying to follow both `require` and
`import` conventions. It is not practical to try to follow both,
and aligning with `import()` seems to be what makes the most sense.

PR-URL: #54416
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Moshe Atlow <[email protected]>
  • Loading branch information
@aduh95 @RafaelGSS
aduh95 authored and RafaelGSS committed Aug 25, 2024
1 parent 3a74c40 commit 1c29e74
Show file tree
Hide file tree
Showing 3 changed files with 39 additions and 31 deletions.
2 changes: 1 addition & 1 deletion doc/api/test.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2040,7 +2040,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
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
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

0 comments on commit 1c29e74

Please sign in to comment.