Skip to content
Draft
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
2 changes: 2 additions & 0 deletions lib/internal/test_runner/harness.js
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,8 @@ function createTestTree(rootTestOptions, globalOptions) {
buildSuites: [],
isWaitingForBuildPhase: false,
watching: false,
bail: globalOptions.bail,
bailedOut: false,
config: globalOptions,
coverage: null,
resetCounters() {
Expand Down
2 changes: 2 additions & 0 deletions lib/internal/test_runner/reporter/spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,8 @@ class SpecReporter extends Transform {
}
#handleEvent({ type, data }) {
switch (type) {
case 'test:bail':
return `${reporterColorMap['test:bail']}Bailing out! no new test files will be started!${colors.white}\n`;
case 'test:fail':
if (data.details?.error?.failureType !== kSubtestsFailed) {
ArrayPrototypePush(this.#failedTests, data);
Expand Down
3 changes: 3 additions & 0 deletions lib/internal/test_runner/reporter/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,9 @@ const reporterColorMap = {
get 'test:diagnostic'() {
return colors.blue;
},
get 'test:bail'() {
return colors.yellow;
},
get 'info'() {
return colors.blue;
},
Expand Down
114 changes: 99 additions & 15 deletions lib/internal/test_runner/runner.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@ const {
SafePromiseAll,
SafePromiseAllReturnVoid,
SafePromiseAllSettledReturnVoid,
SafePromisePrototypeFinally,
SafePromiseRace,
SafeSet,
StringPrototypeIndexOf,
StringPrototypeSlice,
Expand Down Expand Up @@ -254,6 +256,7 @@ class FileTest extends Test {
if (item.data.details?.error) {
item.data.details.error = deserializeError(item.data.details.error);
}

if (item.type === 'test:pass' || item.type === 'test:fail') {
item.data.testNumber = isTopLevel ? (this.root.harness.counters.topLevel + 1) : item.data.testNumber;
countCompletedTest({
Expand Down Expand Up @@ -604,6 +607,7 @@ function run(options = kEmptyObject) {
} = options;
const {
concurrency,
bail,
timeout,
signal,
files,
Expand Down Expand Up @@ -747,6 +751,7 @@ function run(options = kEmptyObject) {
functionCoverage: functionCoverage,
cwd,
globalSetupPath,
bail,
};
const root = createTestTree(rootTestOptions, globalOptions);
let testFiles = files ?? createTestFileList(globPatterns, cwd);
Expand All @@ -756,10 +761,33 @@ function run(options = kEmptyObject) {
testFiles = ArrayPrototypeFilter(testFiles, (_, index) => index % shard.total === shard.index - 1);
}

if (bail) {
validateBoolean(bail, 'options.bail');
if (watch) {
throw new ERR_INVALID_ARG_VALUE('options.bail', watch, 'bail not supported with watch mode');
}
}

let teardown;
let postRun;
let filesWatcher;
let runFiles;

if (bail) {
root.reporter.on('test:fail', (item) => {
if (root.harness.bail && !root.harness.bailedOut) {
root.harness.bailedOut = true;
queueMicrotask(() => {
root.reporter[kEmitMessage]('test:bail', {
__proto__: null,
file: item.name,
test: item.data,
});
});
}
});
}

const opts = {
__proto__: null,
root,
Expand All @@ -770,6 +798,7 @@ function run(options = kEmptyObject) {
hasFiles: files != null,
globPatterns,
only,
bail,
forceExit,
cwd,
isolation,
Expand All @@ -792,15 +821,53 @@ function run(options = kEmptyObject) {
teardown = () => root.harness.teardown();
}

runFiles = () => {
root.harness.bootstrapPromise = null;
root.harness.buildPromise = null;
return SafePromiseAllSettledReturnVoid(testFiles, (path) => {
const subtest = runTestFile(path, filesWatcher, opts);
filesWatcher?.runningSubtests.set(path, subtest);
return subtest;
});
};
if (bail) {
runFiles = async () => {
root.harness.bootstrapPromise = null;
root.harness.buildPromise = null;

const running = new SafeSet();
let index = 0;

const shouldBail = () => bail && root.harness.bailedOut;

const enqueueNext = () => {
if (index < testFiles.length && !shouldBail()) {
const path = testFiles[index++];
const subtest = runTestFile(path, filesWatcher, opts);
filesWatcher?.runningSubtests.set(path, subtest);
running.add(subtest);
SafePromisePrototypeFinally(subtest, () => running.delete(subtest));
}
};

// Fill initial pool up to root test concurrency
// We use root test concurrency here because concurrency logic is handled at test level.
while (running.size < root.concurrency && index < testFiles.length && !shouldBail()) {
enqueueNext();
}

// As each test completes, enqueue the next one
while (running.size > 0) {
await SafePromiseRace([...running]);

// Refill pool after completion(s)
while (running.size < root.concurrency && index < testFiles.length && !shouldBail()) {
enqueueNext();
}
}
};
} else {
runFiles = () => {
root.harness.bootstrapPromise = null;
root.harness.buildPromise = null;
return SafePromiseAllSettledReturnVoid(testFiles, (path) => {
const subtest = runTestFile(path, filesWatcher, opts);
filesWatcher?.runningSubtests.set(path, subtest);
return subtest;
});
};
}
} else if (isolation === 'none') {
if (watch) {
const absoluteTestFiles = ArrayPrototypeMap(testFiles, (file) => (isAbsolute(file) ? file : resolve(cwd, file)));
Expand All @@ -813,17 +880,21 @@ function run(options = kEmptyObject) {
return subtest;
};
} else {

runFiles = async () => {
const { promise, resolve: finishBootstrap } = PromiseWithResolvers();
// Ensure global bootstrap is completed before running files, then allow
// subtests to start immediately so bail can stop further file imports.
if (root.harness.bootstrapPromise) {
await root.harness.bootstrapPromise;
root.harness.bootstrapPromise = null;
}
root.harness.buildPromise = null;

await root.runInAsyncScope(async () => {
const parentURL = pathToFileURL(cwd + sep).href;
const cascadedLoader = esmLoader.getOrInitializeCascadedLoader();
let topLevelTestCount = 0;

root.harness.bootstrapPromise = root.harness.bootstrapPromise ?
SafePromiseAllReturnVoid([root.harness.bootstrapPromise, promise]) :
promise;
let failedCount = root.harness.counters.failed;

// We need to setup the user modules in the test runner if we are running with
// --test-isolation=none and --test in order to avoid loading the user modules
Expand All @@ -841,7 +912,12 @@ function run(options = kEmptyObject) {
}

for (let i = 0; i < testFiles.length; ++i) {
if (root.harness.bail && root.harness.bailedOut) {
break;
}

const testFile = testFiles[i];
const testFilePath = resolve(cwd, testFile);
const fileURL = pathToFileURL(resolve(cwd, testFile));
const parent = i === 0 ? undefined : parentURL;
let threw = false;
Expand Down Expand Up @@ -872,18 +948,26 @@ function run(options = kEmptyObject) {
}

topLevelTestCount = root.subtests.length;

// Ensure any subtest chains spawned by this file are finished.
if (root.subtestsPromise?.promise) {
await root.subtestsPromise.promise;
}
}
});

debug('beginning test execution');
root.entryFile = null;
finishBootstrap();
if (root.harness.bail && root.harness.bailedOut) {
return;
}
return root.processPendingSubtests();
};
}
}

const runChain = async () => {

if (root.harness?.bootstrapPromise) {
await root.harness.bootstrapPromise;
}
Expand Down
13 changes: 10 additions & 3 deletions lib/internal/test_runner/test.js
Original file line number Diff line number Diff line change
Expand Up @@ -523,7 +523,7 @@ class Test extends AsyncResource {
this.root = this;
this.harness = options.harness;
this.config = this.harness.config;
this.concurrency = 1;
this.concurrency = 1; // <-- is this needed?
this.nesting = 0;
this.only = this.config.only;
this.reporter = new TestsStream();
Expand All @@ -540,7 +540,7 @@ class Test extends AsyncResource {
this.root = parent.root;
this.harness = null;
this.config = config;
this.concurrency = parent.concurrency;
this.concurrency = parent.concurrency; // <-- is this needed?
this.nesting = nesting;
this.only = only;
this.reporter = parent.reporter;
Expand Down Expand Up @@ -580,7 +580,7 @@ class Test extends AsyncResource {
}
}

switch (typeof concurrency) {
switch (typeof concurrency) { // <-- here we are overriding this.concurrency with the value from options!
case 'number':
validateUint32(concurrency, 'options.concurrency', true);
this.concurrency = concurrency;
Expand Down Expand Up @@ -953,6 +953,13 @@ class Test extends AsyncResource {
this.passed = false;
}

// Immediate bail signalling for isolation='none' path: mark harness so the
// runner can stop loading additional files as soon as a top-level test
// fails. This is a no-op when bail is disabled.
if (this.harness?.bail) {
this.harness.bailedOut = true;
}

this.error = err;
}

Expand Down
9 changes: 9 additions & 0 deletions lib/internal/test_runner/tests_stream.js
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,15 @@ class TestsStream extends Readable {
});
}

bail(nesting, loc, test) {
this[kEmitMessage]('test:bail', {
__proto__: null,
nesting,
test,
...loc,
});
}

end() {
this.#tryPush(null);
}
Expand Down
2 changes: 2 additions & 0 deletions lib/internal/test_runner/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -211,6 +211,7 @@ function parseCommandLine() {
}

const isTestRunner = getOptionValue('--test');
const bail = getOptionValue('--test-bail');
const coverage = getOptionValue('--experimental-test-coverage');
const forceExit = getOptionValue('--test-force-exit');
const sourceMaps = getOptionValue('--enable-source-maps');
Expand Down Expand Up @@ -341,6 +342,7 @@ function parseCommandLine() {
globalTestOptions = {
__proto__: null,
isTestRunner,
bail,
concurrency,
coverage,
coverageExcludeGlobs,
Expand Down
6 changes: 6 additions & 0 deletions src/node_options.cc
Original file line number Diff line number Diff line change
Expand Up @@ -861,6 +861,12 @@ EnvironmentOptionsParser::EnvironmentOptionsParser() {
kDisallowedInEnvvar,
false,
OptionNamespaces::kTestRunnerNamespace);
AddOption("--test-bail",
"abort test execution after first failure",
&EnvironmentOptions::test_runner_bail,
kDisallowedInEnvvar,
false,
OptionNamespaces::kTestRunnerNamespace);
AddOption("--test-concurrency",
"specify test runner concurrency",
&EnvironmentOptions::test_runner_concurrency,
Expand Down
1 change: 1 addition & 0 deletions src/node_options.h
Original file line number Diff line number Diff line change
Expand Up @@ -190,6 +190,7 @@ class EnvironmentOptions : public Options {
std::vector<std::string> optional_env_file;
bool has_env_file_string = false;
bool test_runner = false;
bool test_runner_bail = false;
uint64_t test_runner_concurrency = 0;
uint64_t test_runner_timeout = 0;
bool test_runner_coverage = false;
Expand Down
12 changes: 12 additions & 0 deletions test/fixtures/test-runner/bail/bail-test-1-pass.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
'use strict';
const { test } = require('node:test');
const { setTimeout } = require('timers/promises');

test('test 1 passes', async () => {
// This should pass
await setTimeout(500);
});

test('test 2 passes', () => {
// This should pass
});
11 changes: 11 additions & 0 deletions test/fixtures/test-runner/bail/bail-test-2-fail.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
'use strict';
const { test } = require('node:test');
const assert = require('assert');

test('failing test 1', () => {
assert.strictEqual(1, 2, 'This test should fail');
});

test('failing test 2', () => {
assert.strictEqual(3, 4, 'This test fails as well');
});
10 changes: 10 additions & 0 deletions test/fixtures/test-runner/bail/bail-test-3-pass.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
'use strict';
const { test } = require('node:test');

test('test 3 passes', () => {
// This should not run if bail happens in test 2
});

test('test 4 passes', () => {
// This should not run if bail happens in test 2
});
6 changes: 6 additions & 0 deletions test/fixtures/test-runner/bail/bail-test-4-pass.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
'use strict';
const { test } = require('node:test');

test('test 5 passes', () => {
// This should not run if bail happens earlier
});
13 changes: 13 additions & 0 deletions test/fixtures/test-runner/output/bail_concurrency_1.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
'use strict';
const fixtures = require('../../../common/fixtures');
const { spec } = require('node:test/reporters');
const { run } = require('node:test');

const files = [
fixtures.path('test-runner', 'bail', 'bail-test-1-pass.js'),
fixtures.path('test-runner', 'bail', 'bail-test-2-fail.js'),
fixtures.path('test-runner', 'bail', 'bail-test-3-pass.js'),
fixtures.path('test-runner', 'bail', 'bail-test-4-pass.js'),
];

run({ bail: true, concurrency: 1, files }).compose(spec).compose(process.stdout);
Loading