Skip to content

Commit 40d5ccd

Browse files
committed
test_runner: support runner bailout at files level
1 parent 720feff commit 40d5ccd

22 files changed

+555
-15
lines changed

lib/internal/test_runner/harness.js

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,8 @@ function createTestTree(rootTestOptions, globalOptions) {
5252
buildSuites: [],
5353
isWaitingForBuildPhase: false,
5454
watching: false,
55+
bail: globalOptions.bail,
56+
bailedOut: false,
5557
config: globalOptions,
5658
coverage: null,
5759
resetCounters() {

lib/internal/test_runner/reporter/spec.js

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ const {
1111
const assert = require('assert');
1212
const Transform = require('internal/streams/transform');
1313
const colors = require('internal/util/colors');
14-
const { kSubtestsFailed } = require('internal/test_runner/test');
14+
const { kSubtestsFailed, kBailedOut } = require('internal/test_runner/test');
1515
const { getCoverageReport } = require('internal/test_runner/utils');
1616
const { relative } = require('path');
1717
const {
@@ -78,8 +78,10 @@ class SpecReporter extends Transform {
7878
}
7979
#handleEvent({ type, data }) {
8080
switch (type) {
81+
case 'test:bail':
82+
return `${reporterColorMap['test:bail']}${reporterUnicodeSymbolMap[type]}Bailing out!${colors.white}\n`;
8183
case 'test:fail':
82-
if (data.details?.error?.failureType !== kSubtestsFailed) {
84+
if (data.details?.error?.failureType !== kSubtestsFailed && data.details?.error?.failureType !== kBailedOut) {
8385
ArrayPrototypePush(this.#failedTests, data);
8486
}
8587
return this.#handleTestReportEvent(type, data);

lib/internal/test_runner/reporter/utils.js

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ const reporterUnicodeSymbolMap = {
2424
'test:coverage': '\u2139 ',
2525
'arrow:right': '\u25B6 ',
2626
'hyphen:minus': '\uFE63 ',
27+
'test:bail': '\u26A0 ',
2728
};
2829

2930
const reporterColorMap = {
@@ -37,6 +38,9 @@ const reporterColorMap = {
3738
get 'test:diagnostic'() {
3839
return colors.blue;
3940
},
41+
get 'test:bail'() {
42+
return colors.yellow;
43+
},
4044
get 'info'() {
4145
return colors.blue;
4246
},

lib/internal/test_runner/runner.js

Lines changed: 70 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,8 @@ const {
2222
SafePromiseAll,
2323
SafePromiseAllReturnVoid,
2424
SafePromiseAllSettledReturnVoid,
25+
SafePromisePrototypeFinally,
26+
SafePromiseRace,
2527
SafeSet,
2628
StringPrototypeIndexOf,
2729
StringPrototypeSlice,
@@ -147,6 +149,7 @@ function getRunArgs(path, { forceExit,
147149
testNamePatterns,
148150
testSkipPatterns,
149151
only,
152+
bail,
150153
argv: suppliedArgs,
151154
execArgv,
152155
rerunFailuresFilePath,
@@ -185,6 +188,9 @@ function getRunArgs(path, { forceExit,
185188
if (only === true) {
186189
ArrayPrototypePush(runArgs, '--test-only');
187190
}
191+
if (bail === true) {
192+
ArrayPrototypePush(runArgs, '--test-bail');
193+
}
188194
if (timeout != null) {
189195
ArrayPrototypePush(runArgs, `--test-timeout=${timeout}`);
190196
}
@@ -271,9 +277,13 @@ class FileTest extends Test {
271277
this.reporter[kEmitMessage](item.type, item.data);
272278
}
273279
#accumulateReportItem(item) {
274-
if (item.type !== 'test:pass' && item.type !== 'test:fail') {
280+
if (item.type !== 'test:pass' && item.type !== 'test:fail' && item.type !== 'test:bail') {
275281
return;
276282
}
283+
// If a test failure occurred and bail is enabled, emit a bail event after reporting the failure
284+
if (item.type === 'test:bail' && this.root.harness?.bail && !this.root.harness.bailedOut) {
285+
this.root.harness.bailedOut = true;
286+
}
277287
this.#reportedChildren++;
278288
if (item.data.nesting === 0 && item.type === 'test:fail') {
279289
this.failedSubtests = true;
@@ -604,6 +614,7 @@ function run(options = kEmptyObject) {
604614
} = options;
605615
const {
606616
concurrency,
617+
bail,
607618
timeout,
608619
signal,
609620
files,
@@ -747,7 +758,9 @@ function run(options = kEmptyObject) {
747758
functionCoverage: functionCoverage,
748759
cwd,
749760
globalSetupPath,
761+
bail,
750762
};
763+
751764
const root = createTestTree(rootTestOptions, globalOptions);
752765
let testFiles = files ?? createTestFileList(globPatterns, cwd);
753766
const { isTestRunner } = globalOptions;
@@ -756,10 +769,18 @@ function run(options = kEmptyObject) {
756769
testFiles = ArrayPrototypeFilter(testFiles, (_, index) => index % shard.total === shard.index - 1);
757770
}
758771

772+
if (bail) {
773+
validateBoolean(bail, 'options.bail');
774+
if (watch) {
775+
throw new ERR_INVALID_ARG_VALUE('options.bail', watch, 'bail not supported with watch mode');
776+
}
777+
}
778+
759779
let teardown;
760780
let postRun;
761781
let filesWatcher;
762782
let runFiles;
783+
763784
const opts = {
764785
__proto__: null,
765786
root,
@@ -770,6 +791,7 @@ function run(options = kEmptyObject) {
770791
hasFiles: files != null,
771792
globPatterns,
772793
only,
794+
bail,
773795
forceExit,
774796
cwd,
775797
isolation,
@@ -792,15 +814,53 @@ function run(options = kEmptyObject) {
792814
teardown = () => root.harness.teardown();
793815
}
794816

795-
runFiles = () => {
796-
root.harness.bootstrapPromise = null;
797-
root.harness.buildPromise = null;
798-
return SafePromiseAllSettledReturnVoid(testFiles, (path) => {
799-
const subtest = runTestFile(path, filesWatcher, opts);
800-
filesWatcher?.runningSubtests.set(path, subtest);
801-
return subtest;
802-
});
803-
};
817+
if (bail) {
818+
runFiles = async () => {
819+
root.harness.bootstrapPromise = null;
820+
root.harness.buildPromise = null;
821+
822+
const running = new SafeSet();
823+
let index = 0;
824+
825+
const shouldBail = () => bail && root.harness.bailedOut;
826+
827+
const enqueueNext = () => {
828+
if (index < testFiles.length && !shouldBail()) {
829+
const path = testFiles[index++];
830+
const subtest = runTestFile(path, filesWatcher, opts);
831+
filesWatcher?.runningSubtests.set(path, subtest);
832+
running.add(subtest);
833+
SafePromisePrototypeFinally(subtest, () => running.delete(subtest));
834+
}
835+
};
836+
837+
// Fill initial pool up to root test concurrency
838+
// We use root test concurrency here because concurrency logic is handled at test level.
839+
while (running.size < root.concurrency && index < testFiles.length && !shouldBail()) {
840+
enqueueNext();
841+
}
842+
843+
// As each test completes, enqueue the next one
844+
while (running.size > 0) {
845+
await SafePromiseRace([...running]);
846+
847+
// Refill pool after completion(s)
848+
while (running.size < root.concurrency && index < testFiles.length && !shouldBail()) {
849+
enqueueNext();
850+
}
851+
}
852+
};
853+
} else {
854+
runFiles = () => {
855+
root.harness.bootstrapPromise = null;
856+
root.harness.buildPromise = null;
857+
return SafePromiseAllSettledReturnVoid(testFiles, (path) => {
858+
const subtest = runTestFile(path, filesWatcher, opts);
859+
filesWatcher?.runningSubtests.set(path, subtest);
860+
return subtest;
861+
});
862+
};
863+
}
804864
} else if (isolation === 'none') {
805865
if (watch) {
806866
const absoluteTestFiles = ArrayPrototypeMap(testFiles, (file) => (isAbsolute(file) ? file : resolve(cwd, file)));

lib/internal/test_runner/test.js

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,7 @@ const { bigint: hrtime } = process.hrtime;
7878
const kCallbackAndPromisePresent = 'callbackAndPromisePresent';
7979
const kCancelledByParent = 'cancelledByParent';
8080
const kAborted = 'testAborted';
81+
const kBailedOut = 'bailedOut';
8182
const kParentAlreadyFinished = 'parentAlreadyFinished';
8283
const kSubtestsFailed = 'subtestsFailed';
8384
const kTestCodeFailure = 'testCodeFailure';
@@ -523,7 +524,7 @@ class Test extends AsyncResource {
523524
this.root = this;
524525
this.harness = options.harness;
525526
this.config = this.harness.config;
526-
this.concurrency = 1;
527+
this.concurrency = 1; // <-- is this needed?
527528
this.nesting = 0;
528529
this.only = this.config.only;
529530
this.reporter = new TestsStream();
@@ -540,7 +541,7 @@ class Test extends AsyncResource {
540541
this.root = parent.root;
541542
this.harness = null;
542543
this.config = config;
543-
this.concurrency = parent.concurrency;
544+
this.concurrency = parent.concurrency; // <-- is this needed?
544545
this.nesting = nesting;
545546
this.only = only;
546547
this.reporter = parent.reporter;
@@ -580,7 +581,7 @@ class Test extends AsyncResource {
580581
}
581582
}
582583

583-
switch (typeof concurrency) {
584+
switch (typeof concurrency) { // <-- here we are overriding this.concurrency with the value from options!
584585
case 'number':
585586
validateUint32(concurrency, 'options.concurrency', true);
586587
this.concurrency = concurrency;
@@ -780,6 +781,10 @@ class Test extends AsyncResource {
780781
*/
781782
async processPendingSubtests() {
782783
while (this.pendingSubtests.length > 0 && this.hasConcurrency()) {
784+
if (this.root.harness?.bailedOut) {
785+
queueMicrotask( () => this.postRun(new ERR_TEST_FAILURE("Test was aborted due to bailout", kBailedOut)));
786+
break;
787+
}
783788
const deferred = ArrayPrototypeShift(this.pendingSubtests);
784789
const test = deferred.test;
785790
test.reporter.dequeue(test.nesting, test.loc, test.name, this.reportedType);
@@ -1382,6 +1387,10 @@ class Test extends AsyncResource {
13821387
this.reporter.ok(this.nesting, this.loc, this.testNumber, this.name, report.details, report.directive);
13831388
} else {
13841389
this.reporter.fail(this.nesting, this.loc, this.testNumber, this.name, report.details, report.directive);
1390+
if (this.root.harness?.bail && !this.root.harness.bailedOut) {
1391+
this.reporter.bail(this.nesting, this.loc, 'bailing out due to test failure');
1392+
this.root.harness.bailedOut = true;
1393+
}
13851394
}
13861395

13871396
for (let i = 0; i < this.diagnostics.length; i++) {
@@ -1558,6 +1567,7 @@ module.exports = {
15581567
kTestCodeFailure,
15591568
kTestTimeoutFailure,
15601569
kAborted,
1570+
kBailedOut,
15611571
kUnwrapErrors,
15621572
Suite,
15631573
Test,

lib/internal/test_runner/tests_stream.js

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -149,6 +149,15 @@ class TestsStream extends Readable {
149149
});
150150
}
151151

152+
bail(nesting, loc, test) {
153+
this[kEmitMessage]('test:bail', {
154+
__proto__: null,
155+
nesting,
156+
test,
157+
...loc,
158+
});
159+
}
160+
152161
end() {
153162
this.#tryPush(null);
154163
}

lib/internal/test_runner/utils.js

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -211,6 +211,7 @@ function parseCommandLine() {
211211
}
212212

213213
const isTestRunner = getOptionValue('--test');
214+
const bail = getOptionValue('--test-bail');
214215
const coverage = getOptionValue('--experimental-test-coverage');
215216
const forceExit = getOptionValue('--test-force-exit');
216217
const sourceMaps = getOptionValue('--enable-source-maps');
@@ -341,6 +342,7 @@ function parseCommandLine() {
341342
globalTestOptions = {
342343
__proto__: null,
343344
isTestRunner,
345+
bail,
344346
concurrency,
345347
coverage,
346348
coverageExcludeGlobs,

src/node_options.cc

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -861,6 +861,12 @@ EnvironmentOptionsParser::EnvironmentOptionsParser() {
861861
kDisallowedInEnvvar,
862862
false,
863863
OptionNamespaces::kTestRunnerNamespace);
864+
AddOption("--test-bail",
865+
"abort test execution after first failure",
866+
&EnvironmentOptions::test_runner_bail,
867+
kDisallowedInEnvvar,
868+
false,
869+
OptionNamespaces::kTestRunnerNamespace);
864870
AddOption("--test-concurrency",
865871
"specify test runner concurrency",
866872
&EnvironmentOptions::test_runner_concurrency,

src/node_options.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -190,6 +190,7 @@ class EnvironmentOptions : public Options {
190190
std::vector<std::string> optional_env_file;
191191
bool has_env_file_string = false;
192192
bool test_runner = false;
193+
bool test_runner_bail = false;
193194
uint64_t test_runner_concurrency = 0;
194195
uint64_t test_runner_timeout = 0;
195196
bool test_runner_coverage = false;
Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
'use strict';
2+
const { test } = require('node:test');
3+
const { setTimeout } = require('timers/promises');
4+
5+
test('test 1 passes', async () => {
6+
// This should pass
7+
await setTimeout(500);
8+
});
9+
10+
test('test 2 passes', () => {
11+
// This should pass
12+
});

0 commit comments

Comments
 (0)