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.

PR-URL: nodejs#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 MoLow committed Jan 26, 2023
1 parent a160a3d commit 9df0311
Show file tree
Hide file tree
Showing 10 changed files with 89 additions and 63 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 @@ -1537,6 +1540,7 @@ added: v18.7.0
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
27 changes: 24 additions & 3 deletions lib/internal/modules/run_main.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,12 @@

const {
ObjectCreate,
StringPrototypeEndsWith,
} = primordials;
const CJSLoader = require('internal/modules/cjs/loader');
const { Module, toRealPath } = CJSLoader;
const { Module, toRealPath, readPackageScope } = CJSLoader;
const { getOptionValue } = require('internal/options');
const path = require('path');
const { shouldUseESMLoader } = require('internal/modules/utils');
const {
handleProcessExit,
} = require('internal/modules/esm/handle_process_exit');
Expand All @@ -27,6 +27,27 @@ 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');
if (userLoaders.length > 0)
return true;
const esModuleSpecifierResolution =
getOptionValue('--experimental-specifier-resolution');
if (esModuleSpecifierResolution === 'node')
return true;
// 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 Expand Up @@ -64,4 +85,4 @@ function executeUserEntryPoint(main = process.argv[1]) {
module.exports = {
executeUserEntryPoint,
handleMainPromise,
};
};
55 changes: 0 additions & 55 deletions lib/internal/modules/utils.js

This file was deleted.

15 changes: 13 additions & 2 deletions lib/internal/test_runner/utils.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
'use strict';
const {
ArrayPrototypePush,
ObjectCreate,
ObjectGetOwnPropertyDescriptor,
SafePromiseAllReturnArrayLike,
RegExp,
Expand All @@ -9,9 +10,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 +104,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, ObjectCreate(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 @@ -75,7 +75,6 @@ const expectedModules = new Set([
'NativeModule internal/mime',
'NativeModule internal/modules/cjs/helpers',
'NativeModule internal/modules/cjs/loader',
'NativeModule internal/modules/utils',
'NativeModule internal/modules/esm/assert',
'NativeModule internal/modules/esm/create_dynamic_module',
'NativeModule internal/modules/esm/fetch_module',
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 9df0311

Please sign in to comment.