Skip to content

Commit

Permalink
test_runner: centralize CLI option handling
Browse files Browse the repository at this point in the history
The test runner relies on a few CLI options. That code was spread
across a few locations. This commit centralizes that logic.

PR-URL: #46707
Backport-PR-URL: #46839
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: James M Snell <jasnell@gmail.com>
  • Loading branch information
cjihrig authored and juanarbol committed Mar 3, 2023
1 parent 638bbe9 commit d44f83d
Show file tree
Hide file tree
Showing 3 changed files with 65 additions and 27 deletions.
22 changes: 14 additions & 8 deletions lib/internal/test_runner/harness.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,12 +14,13 @@ const {
},
} = require('internal/errors');
const { kEmptyObject } = require('internal/util');
const { getOptionValue } = require('internal/options');
const { kCancelledByParent, Test, ItTest, Suite } = require('internal/test_runner/test');
const { setupTestReporters } = require('internal/test_runner/utils');
const {
parseCommandLine,
setupTestReporters,
} = require('internal/test_runner/utils');
const { bigint: hrtime } = process.hrtime;

const isTestRunnerCli = getOptionValue('--test');
const testResources = new SafeMap();
const wasRootSetup = new SafeWeakSet();

Expand Down Expand Up @@ -54,8 +55,8 @@ function createProcessEventHandler(eventName, rootTest) {
};
}

function configureCoverage(rootTest) {
if (!getOptionValue('--experimental-test-coverage')) {
function configureCoverage(rootTest, globalOptions) {
if (!globalOptions.coverage) {
return null;
}

Expand Down Expand Up @@ -96,6 +97,11 @@ function setup(root) {
if (wasRootSetup.has(root)) {
return root;
}

// Parse the command line options before the hook is enabled. We don't want
// global input validation errors to end up in the uncaughtException handler.
const globalOptions = parseCommandLine();

const hook = createHook({
init(asyncId, type, triggerAsyncId, resource) {
if (resource instanceof Test) {
Expand All @@ -120,7 +126,7 @@ function setup(root) {
createProcessEventHandler('uncaughtException', root);
const rejectionHandler =
createProcessEventHandler('unhandledRejection', root);
const coverage = configureCoverage(root);
const coverage = configureCoverage(root, globalOptions);
const exitHandler = () => {
root.coverage = collectCoverage(root, coverage);
root.postRun(new ERR_TEST_FAILURE(
Expand All @@ -140,8 +146,8 @@ function setup(root) {
process.on('uncaughtException', exceptionHandler);
process.on('unhandledRejection', rejectionHandler);
process.on('beforeExit', exitHandler);
// TODO(MoLow): Make it configurable to hook when isTestRunnerCli === false.
if (isTestRunnerCli) {
// TODO(MoLow): Make it configurable to hook when isTestRunner === false.
if (globalOptions.isTestRunner) {
process.on('SIGINT', terminationHandler);
process.on('SIGTERM', terminationHandler);
}
Expand Down
15 changes: 2 additions & 13 deletions lib/internal/test_runner/test.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
'use strict';
const {
ArrayPrototypeMap,
ArrayPrototypePush,
ArrayPrototypeReduce,
ArrayPrototypeShift,
Expand Down Expand Up @@ -31,13 +30,12 @@ const {
},
AbortError,
} = require('internal/errors');
const { getOptionValue } = require('internal/options');
const { MockTracker } = require('internal/test_runner/mock');
const { TestsStream } = require('internal/test_runner/tests_stream');
const {
convertStringToRegExp,
createDeferredCallback,
isTestFailureError,
parseCommandLine,
} = require('internal/test_runner/utils');
const {
createDeferredPromise,
Expand Down Expand Up @@ -65,22 +63,13 @@ const kTestTimeoutFailure = 'testTimeoutFailure';
const kHookFailure = 'hookFailed';
const kDefaultTimeout = null;
const noop = FunctionPrototype;
const isTestRunner = getOptionValue('--test');
const testOnlyFlag = !isTestRunner && getOptionValue('--test-only');
const testNamePatternFlag = isTestRunner ? null :
getOptionValue('--test-name-pattern');
const testNamePatterns = testNamePatternFlag?.length > 0 ?
ArrayPrototypeMap(
testNamePatternFlag,
(re) => convertStringToRegExp(re, '--test-name-pattern')
) : null;
const kShouldAbort = Symbol('kShouldAbort');
const kFilename = process.argv?.[1];
const kHookNames = ObjectSeal(['before', 'after', 'beforeEach', 'afterEach']);
const kUnwrapErrors = new SafeSet()
.add(kTestCodeFailure).add(kHookFailure)
.add('uncaughtException').add('unhandledRejection');

const { testNamePatterns, testOnlyFlag } = parseCommandLine();

function stopTest(timeout, signal) {
if (timeout === kDefaultTimeout) {
Expand Down
55 changes: 49 additions & 6 deletions lib/internal/test_runner/utils.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
'use strict';
const {
ArrayPrototypeMap,
ArrayPrototypePush,
ObjectCreate,
ObjectGetOwnPropertyDescriptor,
Expand Down Expand Up @@ -149,8 +150,27 @@ async function getReportersMap(reporters, destinations) {


async function setupTestReporters(testsStream) {
const { reporters, destinations } = parseCommandLine();
const reportersMap = await getReportersMap(reporters, destinations);
for (let i = 0; i < reportersMap.length; i++) {
const { reporter, destination } = reportersMap[i];
compose(testsStream, reporter).pipe(destination);
}
}

let globalTestOptions;

function parseCommandLine() {
if (globalTestOptions) {
return globalTestOptions;
}

const isTestRunner = getOptionValue('--test');
const coverage = getOptionValue('--experimental-test-coverage');
const destinations = getOptionValue('--test-reporter-destination');
const reporters = getOptionValue('--test-reporter');
let testNamePatterns;
let testOnlyFlag;

if (reporters.length === 0 && destinations.length === 0) {
ArrayPrototypePush(reporters, kDefaultReporter);
Expand All @@ -161,15 +181,37 @@ async function setupTestReporters(testsStream) {
}

if (destinations.length !== reporters.length) {
throw new ERR_INVALID_ARG_VALUE('--test-reporter', reporters,
'must match the number of specified \'--test-reporter-destination\'');
throw new ERR_INVALID_ARG_VALUE(
'--test-reporter',
reporters,
'must match the number of specified \'--test-reporter-destination\'',
);
}

const reportersMap = await getReportersMap(reporters, destinations);
for (let i = 0; i < reportersMap.length; i++) {
const { reporter, destination } = reportersMap[i];
compose(testsStream, reporter).pipe(destination);
if (isTestRunner) {
testOnlyFlag = false;
testNamePatterns = null;
} else {
const testNamePatternFlag = getOptionValue('--test-name-pattern');
testOnlyFlag = getOptionValue('--test-only');
testNamePatterns = testNamePatternFlag?.length > 0 ?
ArrayPrototypeMap(
testNamePatternFlag,
(re) => convertStringToRegExp(re, '--test-name-pattern'),
) : null;
}

globalTestOptions = {
__proto__: null,
isTestRunner,
coverage,
testOnlyFlag,
testNamePatterns,
reporters,
destinations,
};

return globalTestOptions;
}

module.exports = {
Expand All @@ -178,5 +220,6 @@ module.exports = {
doesPathMatchFilter,
isSupportedFileType,
isTestFailureError,
parseCommandLine,
setupTestReporters,
};

0 comments on commit d44f83d

Please sign in to comment.