Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

test_runner: remove problematic uses of parseCommandLine() #54353

Closed
wants to merge 6 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 6 additions & 24 deletions lib/internal/main/test_runner.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,7 @@ const {
} = require('internal/process/pre_execution');
const { isUsingInspector } = require('internal/util/inspector');
const { run } = require('internal/test_runner/runner');
const {
parseCommandLine,
setupTestReporters,
} = require('internal/test_runner/utils');
const { parseCommandLine } = require('internal/test_runner/utils');
const { exitCodes: { kGenericUserError } } = internalBinding('errors');
let debug = require('internal/util/debuglog').debuglog('test_runner', (fn) => {
debug = fn;
Expand All @@ -22,32 +19,17 @@ let debug = require('internal/util/debuglog').debuglog('test_runner', (fn) => {
prepareMainThreadExecution(false);
markBootstrapComplete();

const {
perFileTimeout,
runnerConcurrency,
shard,
watchMode,
} = parseCommandLine();

let concurrency = runnerConcurrency;
let inspectPort;
const options = parseCommandLine();

if (isUsingInspector()) {
process.emitWarning('Using the inspector with --test forces running at a concurrency of 1. ' +
'Use the inspectPort option to run with concurrency');
concurrency = 1;
inspectPort = process.debugPort;
options.concurrency = 1;
options.inspectPort = process.debugPort;
}

const options = {
concurrency,
inspectPort,
watch: watchMode,
setup: setupTestReporters,
timeout: perFileTimeout,
shard,
globPatterns: ArrayPrototypeSlice(process.argv, 1),
};
options.globPatterns = ArrayPrototypeSlice(process.argv, 1);

debug('test runner configuration:', options);
run(options).on('test:fail', (data) => {
if (data.todo === undefined || data.todo === false) {
Expand Down
89 changes: 46 additions & 43 deletions lib/internal/test_runner/harness.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,13 +17,10 @@ const {
},
} = require('internal/errors');
const { exitCodes: { kGenericUserError } } = internalBinding('errors');

const { kEmptyObject } = require('internal/util');
const { kCancelledByParent, Test, Suite } = require('internal/test_runner/test');
const {
parseCommandLine,
reporterScope,
setupTestReporters,
shouldColorizeTestFiles,
} = require('internal/test_runner/utils');
const { queueMicrotask } = require('internal/process/task_queues');
Expand All @@ -34,8 +31,42 @@ let globalRoot;

testResources.set(reporterScope.asyncId(), reporterScope);

function createTestTree(options = kEmptyObject) {
globalRoot = setup(new Test({ __proto__: null, ...options, name: '<root>' }));
function createTestTree(rootTestOptions, globalOptions) {
const harness = {
__proto__: null,
allowTestsToRun: false,
bootstrapPromise: resolvedPromise,
watching: false,
config: globalOptions,
coverage: null,
resetCounters() {
harness.counters = {
__proto__: null,
all: 0,
failed: 0,
passed: 0,
cancelled: 0,
skipped: 0,
todo: 0,
topLevel: 0,
suites: 0,
};
},
counters: null,
shouldColorizeTestFiles: shouldColorizeTestFiles(globalOptions.destinations),
teardown: null,
snapshotManager: null,
};

harness.resetCounters();
globalRoot = new Test({
__proto__: null,
...rootTestOptions,
harness,
name: '<root>',
});
setupProcessState(globalRoot, globalOptions, harness);
globalRoot.startTime = hrtime();
return globalRoot;
}

Expand Down Expand Up @@ -127,15 +158,7 @@ function collectCoverage(rootTest, coverage) {
return summary;
}

function setup(root) {
if (root.startTime !== null) {
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();

function setupProcessState(root, globalOptions) {
const hook = createHook({
__proto__: null,
init(asyncId, type, triggerAsyncId, resource) {
Expand Down Expand Up @@ -195,46 +218,26 @@ function setup(root) {
process.on('SIGTERM', terminationHandler);
}

root.harness = {
__proto__: null,
allowTestsToRun: false,
bootstrapPromise: resolvedPromise,
watching: false,
coverage: FunctionPrototypeBind(collectCoverage, null, root, coverage),
resetCounters() {
root.harness.counters = {
__proto__: null,
all: 0,
failed: 0,
passed: 0,
cancelled: 0,
skipped: 0,
todo: 0,
topLevel: 0,
suites: 0,
};
},
counters: null,
shouldColorizeTestFiles: shouldColorizeTestFiles(globalOptions.destinations),
teardown: exitHandler,
snapshotManager: null,
};
root.harness.resetCounters();
root.startTime = hrtime();
return root;
root.harness.coverage = FunctionPrototypeBind(collectCoverage, null, root, coverage);
root.harness.teardown = exitHandler;
}

function lazyBootstrapRoot() {
if (!globalRoot) {
// This is where the test runner is bootstrapped when node:test is used
// without the --test flag or the run() API.
createTestTree({ __proto__: null, entryFile: process.argv?.[1] });
const rootTestOptions = {
__proto__: null,
entryFile: process.argv?.[1],
};
const globalOptions = parseCommandLine();
createTestTree(rootTestOptions, globalOptions);
globalRoot.reporter.on('test:fail', (data) => {
if (data.todo === undefined || data.todo === false) {
process.exitCode = kGenericUserError;
}
});
globalRoot.harness.bootstrapPromise = setupTestReporters(globalRoot.reporter);
globalRoot.harness.bootstrapPromise = globalOptions.setup(globalRoot.reporter);
}
return globalRoot;
}
Expand Down
11 changes: 10 additions & 1 deletion lib/internal/test_runner/runner.js
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ const {
convertStringToRegExp,
countCompletedTest,
kDefaultPattern,
parseCommandLine,
} = require('internal/test_runner/utils');
const { Glob } = require('internal/fs/glob');
const { once } = require('events');
Expand Down Expand Up @@ -561,7 +562,15 @@ function run(options = kEmptyObject) {
});
}

const root = createTestTree({ __proto__: null, concurrency, timeout, signal });
const rootTestOptions = { __proto__: null, concurrency, timeout, signal };
const globalOptions = {
__proto__: null,
// parseCommandLine() should not be used here. However, The existing run()
// behavior has relied on it, so removing it must be done in a semver major.
...parseCommandLine(),
setup, // This line can be removed when parseCommandLine() is removed here.
};
const root = createTestTree(rootTestOptions, globalOptions);

if (process.env.NODE_TEST_CONTEXT !== undefined) {
process.emitWarning('node:test run() is being called recursively within a test file. skipping running files.');
Expand Down
Loading
Loading