Skip to content

Commit

Permalink
test_runner: refactor and simplify internals
Browse files Browse the repository at this point in the history
This commit refactors some of the internals of the test runner.

PR-URL: #53921
Reviewed-By: Chemi Atlow <chemi@atlow.co.il>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
  • Loading branch information
cjihrig authored and targos committed Jul 28, 2024
1 parent e907236 commit f45edb4
Show file tree
Hide file tree
Showing 4 changed files with 23 additions and 29 deletions.
42 changes: 20 additions & 22 deletions lib/internal/test_runner/harness.js
Original file line number Diff line number Diff line change
Expand Up @@ -208,7 +208,7 @@ function setup(root) {
};
},
counters: null,
shouldColorizeTestFiles: false,
shouldColorizeTestFiles: shouldColorizeTestFiles(globalOptions.destinations),
teardown: exitHandler,
snapshotManager: null,
};
Expand All @@ -218,48 +218,46 @@ function setup(root) {
}

let globalRoot;
let reportersSetup;
function getGlobalRoot() {
let asyncBootstrap;
function lazyBootstrapRoot() {
if (!globalRoot) {
globalRoot = createTestTree({ __proto__: null, entryFile: process.argv?.[1] });
globalRoot.reporter.on('test:fail', (data) => {
if (data.todo === undefined || data.todo === false) {
process.exitCode = kGenericUserError;
}
});
reportersSetup = setupTestReporters(globalRoot.reporter);
globalRoot.harness.shouldColorizeTestFiles ||= shouldColorizeTestFiles(globalRoot);
asyncBootstrap = setupTestReporters(globalRoot.reporter);
}
return globalRoot;
}

async function startSubtest(subtest) {
if (reportersSetup) {
if (asyncBootstrap) {
// Only incur the overhead of awaiting the Promise once.
await reportersSetup;
reportersSetup = undefined;
}

const root = getGlobalRoot();
if (!root.harness.bootstrapComplete) {
root.harness.bootstrapComplete = true;
queueMicrotask(() => {
root.harness.allowTestsToRun = true;
root.processPendingSubtests();
});
await asyncBootstrap;
asyncBootstrap = undefined;
if (!subtest.root.harness.bootstrapComplete) {
subtest.root.harness.bootstrapComplete = true;
queueMicrotask(() => {
subtest.root.harness.allowTestsToRun = true;
subtest.root.processPendingSubtests();
});
}
}

await subtest.start();
}

function runInParentContext(Factory) {
function run(name, options, fn, overrides) {
const parent = testResources.get(executionAsyncId()) || getGlobalRoot();
const parent = testResources.get(executionAsyncId()) || lazyBootstrapRoot();
const subtest = parent.createSubtest(Factory, name, options, fn, overrides);
if (!(parent instanceof Suite)) {
return startSubtest(subtest);
if (parent instanceof Suite) {
return PromiseResolve();
}
return PromiseResolve();

return startSubtest(subtest);
}

const test = (name, options, fn) => {
Expand All @@ -286,7 +284,7 @@ function runInParentContext(Factory) {

function hook(hook) {
return (fn, options) => {
const parent = testResources.get(executionAsyncId()) || getGlobalRoot();
const parent = testResources.get(executionAsyncId()) || lazyBootstrapRoot();
parent.createHook(hook, fn, {
__proto__: null,
...options,
Expand Down
2 changes: 0 additions & 2 deletions lib/internal/test_runner/runner.js
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,6 @@ const {
convertStringToRegExp,
countCompletedTest,
kDefaultPattern,
shouldColorizeTestFiles,
} = require('internal/test_runner/utils');
const { Glob } = require('internal/fs/glob');
const { once } = require('events');
Expand Down Expand Up @@ -552,7 +551,6 @@ function run(options = kEmptyObject) {
}

const root = createTestTree({ __proto__: null, concurrency, timeout, signal });
root.harness.shouldColorizeTestFiles ||= shouldColorizeTestFiles(root);

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
3 changes: 1 addition & 2 deletions lib/internal/test_runner/test.js
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,6 @@ const kHookFailure = 'hookFailed';
const kDefaultTimeout = null;
const noop = FunctionPrototype;
const kShouldAbort = Symbol('kShouldAbort');
const kFilename = process.argv?.[1];
const kHookNames = ObjectSeal(['before', 'after', 'beforeEach', 'afterEach']);
const kUnwrapErrors = new SafeSet()
.add(kTestCodeFailure).add(kHookFailure)
Expand Down Expand Up @@ -508,7 +507,7 @@ class Test extends AsyncResource {
this.diagnostic(warning);
}

if (loc === undefined || kFilename === undefined) {
if (loc === undefined) {
this.loc = undefined;
} else {
this.loc = {
Expand Down
5 changes: 2 additions & 3 deletions lib/internal/test_runner/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -131,10 +131,9 @@ function tryBuiltinReporter(name) {
return require(builtinPath);
}

function shouldColorizeTestFiles(rootTest) {
function shouldColorizeTestFiles(destinations) {
// This function assumes only built-in destinations (stdout/stderr) supports coloring
const { reporters, destinations } = parseCommandLine();
return ArrayPrototypeSome(reporters, (_, index) => {
return ArrayPrototypeSome(destinations, (_, index) => {
const destination = kBuiltinDestinations.get(destinations[index]);
return destination && shouldColorize(destination);
});
Expand Down

0 comments on commit f45edb4

Please sign in to comment.