Skip to content

Commit

Permalink
module: move test reporter loading
Browse files Browse the repository at this point in the history
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 <moshe@atlow.co.il>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Jacob Smith <jacob@frende.me>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
  • Loading branch information
GeoffreyBooth authored and ruyadorno committed Jan 31, 2023
1 parent fbdc3f7 commit 3a3a6d8
Show file tree
Hide file tree
Showing 10 changed files with 91 additions and 60 deletions.
6 changes: 5 additions & 1 deletion doc/api/test.md
Original file line number Diff line number Diff line change
Expand Up @@ -470,7 +470,7 @@ The following built-reporters are supported:
The `spec` reporter outputs the test results in a human-readable format.

* `dot`
The `dot` reporter outputs the test results in a comact format,
The `dot` reporter outputs the test results in a compact format,
where each passing test is represented by a `.`,
and each failing test is represented by a `X`.

Expand Down Expand Up @@ -591,6 +591,9 @@ module.exports = async function * customReporter(source) {
};
```

The value provided to `--test-reporter` should be a string like one used in an
`import()` in JavaScript code, or a value provided for [`--import`][].

### Multiple reporters

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

[TAP]: https://testanything.org/
[`--import`]: cli.md#--importmodule
[`--test-name-pattern`]: cli.md#--test-name-pattern
[`--test-only`]: cli.md#--test-only
[`--test-reporter-destination`]: cli.md#--test-reporter-destination
Expand Down
28 changes: 27 additions & 1 deletion lib/internal/modules/run_main.js
Original file line number Diff line number Diff line change
@@ -1,8 +1,11 @@
'use strict';

const {
StringPrototypeEndsWith,
} = primordials;

const { getOptionValue } = require('internal/options');
const path = require('path');
const { shouldUseESMLoader } = require('internal/modules/utils');

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

function shouldUseESMLoader(mainPath) {
/**
* @type {string[]} userLoaders A list of custom loaders registered by the user
* (or an empty list when none have been registered).
*/
const userLoaders = getOptionValue('--experimental-loader');
/**
* @type {string[]} userImports A list of preloaded modules registered by the user
* (or an empty list when none have been registered).
*/
const userImports = getOptionValue('--import');
if (userLoaders.length > 0 || userImports.length > 0)
return true;
const { readPackageScope } = require('internal/modules/cjs/loader');
// Determine the module format of the main
if (mainPath && StringPrototypeEndsWith(mainPath, '.mjs'))
return true;
if (!mainPath || StringPrototypeEndsWith(mainPath, '.cjs'))
return false;
const pkg = readPackageScope(mainPath);
return pkg && pkg.data.type === 'module';
}

function runMainESM(mainPath) {
const { loadESM } = require('internal/process/esm_loader');
const { pathToFileURL } = require('internal/url');
Expand Down
54 changes: 0 additions & 54 deletions lib/internal/modules/utils.js

This file was deleted.

14 changes: 12 additions & 2 deletions lib/internal/test_runner/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,9 @@ const {
} = primordials;
const { basename } = require('path');
const { createWriteStream } = require('fs');
const { pathToFileURL } = require('internal/url');
const { createDeferredPromise } = require('internal/util');
const { getOptionValue } = require('internal/options');
const { requireOrImport } = require('internal/modules/utils');

const {
codes: {
Expand Down Expand Up @@ -103,7 +103,17 @@ const kDefaultDestination = 'stdout';
async function getReportersMap(reporters, destinations) {
return SafePromiseAllReturnArrayLike(reporters, async (name, i) => {
const destination = kBuiltinDestinations.get(destinations[i]) ?? createWriteStream(destinations[i]);
let reporter = await requireOrImport(kBuiltinReporters.get(name) ?? name);

// Load the test reporter passed to --test-reporter
const reporterSpecifier = kBuiltinReporters.get(name) ?? name;
let parentURL;
try {
parentURL = pathToFileURL(process.cwd() + '/').href;
} catch {
parentURL = 'file:///';
}
const { esmLoader } = require('internal/process/esm_loader');
let reporter = await esmLoader.import(reporterSpecifier, parentURL, { __proto__: null });

if (reporter?.default) {
reporter = reporter.default;
Expand Down
8 changes: 8 additions & 0 deletions test/fixtures/test-runner/node_modules/reporter-cjs/index.js

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

8 changes: 8 additions & 0 deletions test/fixtures/test-runner/node_modules/reporter-esm/index.mjs

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 0 additions & 1 deletion test/parallel/test-bootstrap-modules.js
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,6 @@ const expectedModules = new Set([
'NativeModule internal/idna',
'NativeModule internal/linkedlist',
'NativeModule internal/modules/cjs/loader',
'NativeModule internal/modules/utils',
'NativeModule internal/modules/esm/utils',
'NativeModule internal/modules/helpers',
'NativeModule internal/modules/package_json_reader',
Expand Down
24 changes: 23 additions & 1 deletion test/parallel/test-runner-reporters.js
Original file line number Diff line number Diff line change
Expand Up @@ -86,10 +86,32 @@ describe('node:test reporters', { concurrency: true }, () => {
it(`should support a '${ext}' file as a custom reporter`, async () => {
const filename = `custom.${ext}`;
const child = spawnSync(process.execPath,
['--test', '--test-reporter', fixtures.path('test-runner/custom_reporters/', filename),
['--test', '--test-reporter', fixtures.fileURL('test-runner/custom_reporters/', filename),
testFile]);
assert.strictEqual(child.stderr.toString(), '');
assert.strictEqual(child.stdout.toString(), `${filename} {"test:start":5,"test:pass":2,"test:fail":3,"test:plan":3,"test:diagnostic":7}`);
});
});

it('should support a custom reporter from node_modules', async () => {
const child = spawnSync(process.execPath,
['--test', '--test-reporter', 'reporter-cjs', 'reporters.js'],
{ cwd: fixtures.path('test-runner') });
assert.strictEqual(child.stderr.toString(), '');
assert.match(
child.stdout.toString(),
/^package: reporter-cjs{"test:start":5,"test:pass":2,"test:fail":3,"test:plan":3,"test:diagnostic":\d+}$/,
);
});

it('should support a custom ESM reporter from node_modules', async () => {
const child = spawnSync(process.execPath,
['--test', '--test-reporter', 'reporter-esm', 'reporters.js'],
{ cwd: fixtures.path('test-runner') });
assert.strictEqual(child.stderr.toString(), '');
assert.match(
child.stdout.toString(),
/^package: reporter-esm{"test:start":5,"test:pass":2,"test:fail":3,"test:plan":3,"test:diagnostic":\d+}$/,
);
});
});

0 comments on commit 3a3a6d8

Please sign in to comment.