Skip to content

Commit 3a3a6d8

Browse files
GeoffreyBoothruyadorno
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. Backport-PR-URL: #46361 PR-URL: #45923 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 fbdc3f7 commit 3a3a6d8

File tree

10 files changed

+91
-60
lines changed

10 files changed

+91
-60
lines changed

doc/api/test.md

+5-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, or a value provided for [`--import`][].
596+
594597
### Multiple reporters
595598

596599
The [`--test-reporter`][] flag can be specified multiple times to report test
@@ -1581,6 +1584,7 @@ added:
15811584
aborted.
15821585

15831586
[TAP]: https://testanything.org/
1587+
[`--import`]: cli.md#--importmodule
15841588
[`--test-name-pattern`]: cli.md#--test-name-pattern
15851589
[`--test-only`]: cli.md#--test-only
15861590
[`--test-reporter-destination`]: cli.md#--test-reporter-destination

lib/internal/modules/run_main.js

+27-1
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,11 @@
11
'use strict';
22

3+
const {
4+
StringPrototypeEndsWith,
5+
} = primordials;
6+
37
const { getOptionValue } = require('internal/options');
48
const path = require('path');
5-
const { shouldUseESMLoader } = require('internal/modules/utils');
69

710
function resolveMainPath(main) {
811
// Note extension resolution for the main entry point can be deprecated in a
@@ -20,6 +23,29 @@ function resolveMainPath(main) {
2023
return mainPath;
2124
}
2225

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

lib/internal/modules/utils.js

-54
This file was deleted.

lib/internal/test_runner/utils.js

+12-2
Original file line numberDiff line numberDiff line change
@@ -9,9 +9,9 @@ const {
99
} = primordials;
1010
const { basename } = require('path');
1111
const { createWriteStream } = require('fs');
12+
const { pathToFileURL } = require('internal/url');
1213
const { createDeferredPromise } = require('internal/util');
1314
const { getOptionValue } = require('internal/options');
14-
const { requireOrImport } = require('internal/modules/utils');
1515

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

108118
if (reporter?.default) {
109119
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
@@ -53,7 +53,6 @@ const expectedModules = new Set([
5353
'NativeModule internal/idna',
5454
'NativeModule internal/linkedlist',
5555
'NativeModule internal/modules/cjs/loader',
56-
'NativeModule internal/modules/utils',
5756
'NativeModule internal/modules/esm/utils',
5857
'NativeModule internal/modules/helpers',
5958
'NativeModule internal/modules/package_json_reader',

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)