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
6 changes: 4 additions & 2 deletions lib/internal/test_runner/reporter/spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ const {
const assert = require('assert');
const Transform = require('internal/streams/transform');
const colors = require('internal/util/colors');
const { kSubtestsFailed } = require('internal/test_runner/test');
const { kSubtestsFailed, kBailedOut } = require('internal/test_runner/test');
const { getCoverageReport } = require('internal/test_runner/utils');
const { relative } = require('path');
const {
Expand Down Expand Up @@ -78,8 +78,10 @@ class SpecReporter extends Transform {
}
#handleEvent({ type, data }) {
switch (type) {
case 'test:bail':
return `${reporterColorMap['test:bail']}${reporterUnicodeSymbolMap[type]}Bailing out!${colors.white}\n`;
case 'test:fail':
if (data.details?.error?.failureType !== kSubtestsFailed) {
if (data.details?.error?.failureType !== kSubtestsFailed && data.details?.error?.failureType !== kBailedOut) {
ArrayPrototypePush(this.#failedTests, data);
}
return this.#handleTestReportEvent(type, data);
Expand Down
4 changes: 4 additions & 0 deletions lib/internal/test_runner/reporter/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ const reporterUnicodeSymbolMap = {
'test:coverage': '\u2139 ',
'arrow:right': '\u25B6 ',
'hyphen:minus': '\uFE63 ',
'test:bail': '\u26A0 ',
};

const reporterColorMap = {
Expand All @@ -37,6 +38,9 @@ const reporterColorMap = {
get 'test:diagnostic'() {
return colors.blue;
},
get 'test:bail'() {
return colors.yellow;
},
get 'info'() {
return colors.blue;
},
Expand Down
80 changes: 70 additions & 10 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 @@ -147,6 +149,7 @@ function getRunArgs(path, { forceExit,
testNamePatterns,
testSkipPatterns,
only,
bail,
argv: suppliedArgs,
execArgv,
rerunFailuresFilePath,
Expand Down Expand Up @@ -185,6 +188,9 @@ function getRunArgs(path, { forceExit,
if (only === true) {
ArrayPrototypePush(runArgs, '--test-only');
}
if (bail === true) {
ArrayPrototypePush(runArgs, '--test-bail');
}
if (timeout != null) {
ArrayPrototypePush(runArgs, `--test-timeout=${timeout}`);
}
Expand Down Expand Up @@ -271,9 +277,13 @@ class FileTest extends Test {
this.reporter[kEmitMessage](item.type, item.data);
}
#accumulateReportItem(item) {
if (item.type !== 'test:pass' && item.type !== 'test:fail') {
if (item.type !== 'test:pass' && item.type !== 'test:fail' && item.type !== 'test:bail') {
return;
}
// If a test failure occurred and bail is enabled, emit a bail event after reporting the failure
if (item.type === 'test:bail' && this.root.harness?.bail && !this.root.harness.bailedOut) {
this.root.harness.bailedOut = true;
}
this.#reportedChildren++;
if (item.data.nesting === 0 && item.type === 'test:fail') {
this.failedSubtests = true;
Expand Down Expand Up @@ -604,6 +614,7 @@ function run(options = kEmptyObject) {
} = options;
const {
concurrency,
bail,
timeout,
signal,
files,
Expand Down Expand Up @@ -747,7 +758,9 @@ function run(options = kEmptyObject) {
functionCoverage: functionCoverage,
cwd,
globalSetupPath,
bail,
};

const root = createTestTree(rootTestOptions, globalOptions);
let testFiles = files ?? createTestFileList(globPatterns, cwd);
const { isTestRunner } = globalOptions;
Expand All @@ -756,10 +769,18 @@ 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;

const opts = {
__proto__: null,
root,
Expand All @@ -770,6 +791,7 @@ function run(options = kEmptyObject) {
hasFiles: files != null,
globPatterns,
only,
bail,
forceExit,
cwd,
isolation,
Expand All @@ -792,15 +814,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 Down
16 changes: 13 additions & 3 deletions lib/internal/test_runner/test.js
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,7 @@ const { bigint: hrtime } = process.hrtime;
const kCallbackAndPromisePresent = 'callbackAndPromisePresent';
const kCancelledByParent = 'cancelledByParent';
const kAborted = 'testAborted';
const kBailedOut = 'bailedOut';
const kParentAlreadyFinished = 'parentAlreadyFinished';
const kSubtestsFailed = 'subtestsFailed';
const kTestCodeFailure = 'testCodeFailure';
Expand Down Expand Up @@ -523,7 +524,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 +541,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 +581,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 @@ -780,6 +781,10 @@ class Test extends AsyncResource {
*/
async processPendingSubtests() {
while (this.pendingSubtests.length > 0 && this.hasConcurrency()) {
if (this.root.harness?.bailedOut) {
queueMicrotask( () => this.postRun(new ERR_TEST_FAILURE("Test was aborted due to bailout", kBailedOut)));
break;
}
const deferred = ArrayPrototypeShift(this.pendingSubtests);
const test = deferred.test;
test.reporter.dequeue(test.nesting, test.loc, test.name, this.reportedType);
Expand Down Expand Up @@ -1382,6 +1387,10 @@ class Test extends AsyncResource {
this.reporter.ok(this.nesting, this.loc, this.testNumber, this.name, report.details, report.directive);
} else {
this.reporter.fail(this.nesting, this.loc, this.testNumber, this.name, report.details, report.directive);
if (this.root.harness?.bail && !this.root.harness.bailedOut) {
this.reporter.bail(this.nesting, this.loc, 'bailing out due to test failure');
this.root.harness.bailedOut = true;
}
}

for (let i = 0; i < this.diagnostics.length; i++) {
Expand Down Expand Up @@ -1558,6 +1567,7 @@ module.exports = {
kTestCodeFailure,
kTestTimeoutFailure,
kAborted,
kBailedOut,
kUnwrapErrors,
Suite,
Test,
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