Skip to content

Commit 403df21

Browse files
GeoffreyBoothjuanarbol
authored andcommitted
module: move test reporter loading
Move the logic for handling --test-reporter out of the general module loader and into the test_runner subsystem. PR-URL: #45923 Backport-PR-URL: #46839 Reviewed-By: Moshe Atlow <[email protected]> Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Jacob Smith <[email protected]> Reviewed-By: Antoine du Hamel <[email protected]>
1 parent 4d50db1 commit 403df21

File tree

10 files changed

+87
-61
lines changed

10 files changed

+87
-61
lines changed

doc/api/test.md

+4-1
Original file line numberDiff line numberDiff line change
@@ -470,7 +470,7 @@ The following built-reporters are supported:
470470
The `spec` reporter outputs the test results in a human-readable format.
471471

472472
* `dot`
473-
The `dot` reporter outputs the test results in a comact format,
473+
The `dot` reporter outputs the test results in a compact format,
474474
where each passing test is represented by a `.`,
475475
and each failing test is represented by a `X`.
476476

@@ -591,6 +591,9 @@ module.exports = async function * customReporter(source) {
591591
};
592592
```
593593

594+
The value provided to `--test-reporter` should be a string like one used in an
595+
`import()` in JavaScript code.
596+
594597
### Multiple reporters
595598

596599
The [`--test-reporter`][] flag can be specified multiple times to report test

lib/internal/modules/run_main.js

+23-2
Original file line numberDiff line numberDiff line change
@@ -2,12 +2,12 @@
22

33
const {
44
ObjectCreate,
5+
StringPrototypeEndsWith,
56
} = primordials;
67
const CJSLoader = require('internal/modules/cjs/loader');
7-
const { Module, toRealPath } = CJSLoader;
8+
const { Module, toRealPath, readPackageScope } = CJSLoader;
89
const { getOptionValue } = require('internal/options');
910
const path = require('path');
10-
const { shouldUseESMLoader } = require('internal/modules/utils');
1111
const {
1212
handleProcessExit,
1313
} = require('internal/modules/esm/handle_process_exit');
@@ -27,6 +27,27 @@ function resolveMainPath(main) {
2727
return mainPath;
2828
}
2929

30+
function shouldUseESMLoader(mainPath) {
31+
/**
32+
* @type {string[]} userLoaders A list of custom loaders registered by the user
33+
* (or an empty list when none have been registered).
34+
*/
35+
const userLoaders = getOptionValue('--experimental-loader');
36+
if (userLoaders.length > 0)
37+
return true;
38+
const esModuleSpecifierResolution =
39+
getOptionValue('--experimental-specifier-resolution');
40+
if (esModuleSpecifierResolution === 'node')
41+
return true;
42+
// Determine the module format of the main
43+
if (mainPath && StringPrototypeEndsWith(mainPath, '.mjs'))
44+
return true;
45+
if (!mainPath || StringPrototypeEndsWith(mainPath, '.cjs'))
46+
return false;
47+
const pkg = readPackageScope(mainPath);
48+
return pkg && pkg.data.type === 'module';
49+
}
50+
3051
function runMainESM(mainPath) {
3152
const { loadESM } = require('internal/process/esm_loader');
3253
const { pathToFileURL } = require('internal/url');

lib/internal/modules/utils.js

-54
This file was deleted.

lib/internal/test_runner/utils.js

+13-2
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
'use strict';
22
const {
33
ArrayPrototypePush,
4+
ObjectCreate,
45
ObjectGetOwnPropertyDescriptor,
56
SafePromiseAllReturnArrayLike,
67
RegExp,
@@ -9,9 +10,9 @@ const {
910
} = primordials;
1011
const { basename } = require('path');
1112
const { createWriteStream } = require('fs');
13+
const { pathToFileURL } = require('internal/url');
1214
const { createDeferredPromise } = require('internal/util');
1315
const { getOptionValue } = require('internal/options');
14-
const { requireOrImport } = require('internal/modules/utils');
1516

1617
const {
1718
codes: {
@@ -103,7 +104,17 @@ const kDefaultDestination = 'stdout';
103104
async function getReportersMap(reporters, destinations) {
104105
return SafePromiseAllReturnArrayLike(reporters, async (name, i) => {
105106
const destination = kBuiltinDestinations.get(destinations[i]) ?? createWriteStream(destinations[i]);
106-
let reporter = await requireOrImport(kBuiltinReporters.get(name) ?? name);
107+
108+
// Load the test reporter passed to --test-reporter
109+
const reporterSpecifier = kBuiltinReporters.get(name) ?? name;
110+
let parentURL;
111+
try {
112+
parentURL = pathToFileURL(process.cwd() + '/').href;
113+
} catch {
114+
parentURL = 'file:///';
115+
}
116+
const { esmLoader } = require('internal/process/esm_loader');
117+
let reporter = await esmLoader.import(reporterSpecifier, parentURL, ObjectCreate(null));
107118

108119
if (reporter?.default) {
109120
reporter = reporter.default;

test/fixtures/test-runner/node_modules/reporter-cjs/index.js

+8
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

test/fixtures/test-runner/node_modules/reporter-cjs/package.json

+4
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

test/fixtures/test-runner/node_modules/reporter-esm/index.mjs

+8
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

test/fixtures/test-runner/node_modules/reporter-esm/package.json

+4
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

test/parallel/test-bootstrap-modules.js

-1
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,6 @@ const expectedModules = new Set([
7575
'NativeModule internal/mime',
7676
'NativeModule internal/modules/cjs/helpers',
7777
'NativeModule internal/modules/cjs/loader',
78-
'NativeModule internal/modules/utils',
7978
'NativeModule internal/modules/esm/assert',
8079
'NativeModule internal/modules/esm/create_dynamic_module',
8180
'NativeModule internal/modules/esm/fetch_module',

test/parallel/test-runner-reporters.js

+23-1
Original file line numberDiff line numberDiff line change
@@ -86,10 +86,32 @@ describe('node:test reporters', { concurrency: true }, () => {
8686
it(`should support a '${ext}' file as a custom reporter`, async () => {
8787
const filename = `custom.${ext}`;
8888
const child = spawnSync(process.execPath,
89-
['--test', '--test-reporter', fixtures.path('test-runner/custom_reporters/', filename),
89+
['--test', '--test-reporter', fixtures.fileURL('test-runner/custom_reporters/', filename),
9090
testFile]);
9191
assert.strictEqual(child.stderr.toString(), '');
9292
assert.strictEqual(child.stdout.toString(), `${filename} {"test:start":5,"test:pass":2,"test:fail":3,"test:plan":3,"test:diagnostic":7}`);
9393
});
9494
});
95+
96+
it('should support a custom reporter from node_modules', async () => {
97+
const child = spawnSync(process.execPath,
98+
['--test', '--test-reporter', 'reporter-cjs', 'reporters.js'],
99+
{ cwd: fixtures.path('test-runner') });
100+
assert.strictEqual(child.stderr.toString(), '');
101+
assert.match(
102+
child.stdout.toString(),
103+
/^package: reporter-cjs{"test:start":5,"test:pass":2,"test:fail":3,"test:plan":3,"test:diagnostic":\d+}$/,
104+
);
105+
});
106+
107+
it('should support a custom ESM reporter from node_modules', async () => {
108+
const child = spawnSync(process.execPath,
109+
['--test', '--test-reporter', 'reporter-esm', 'reporters.js'],
110+
{ cwd: fixtures.path('test-runner') });
111+
assert.strictEqual(child.stderr.toString(), '');
112+
assert.match(
113+
child.stdout.toString(),
114+
/^package: reporter-esm{"test:start":5,"test:pass":2,"test:fail":3,"test:plan":3,"test:diagnostic":\d+}$/,
115+
);
116+
});
95117
});

0 commit comments

Comments
 (0)