Skip to content

Commit c41ebea

Browse files
committed
test_runner: support bail out at runner level
1 parent 720feff commit c41ebea

22 files changed

+578
-18
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: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,8 @@ class SpecReporter extends Transform {
7878
}
7979
#handleEvent({ type, data }) {
8080
switch (type) {
81+
case 'test:bail':
82+
return `${reporterColorMap['test:bail']}Bailing out! no new test files will be started!${colors.white}\n`;
8183
case 'test:fail':
8284
if (data.details?.error?.failureType !== kSubtestsFailed) {
8385
ArrayPrototypePush(this.#failedTests, data);

lib/internal/test_runner/reporter/utils.js

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,9 @@ const reporterColorMap = {
3737
get 'test:diagnostic'() {
3838
return colors.blue;
3939
},
40+
get 'test:bail'() {
41+
return colors.yellow;
42+
},
4043
get 'info'() {
4144
return colors.blue;
4245
},

lib/internal/test_runner/runner.js

Lines changed: 99 additions & 15 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,
@@ -254,6 +256,7 @@ class FileTest extends Test {
254256
if (item.data.details?.error) {
255257
item.data.details.error = deserializeError(item.data.details.error);
256258
}
259+
257260
if (item.type === 'test:pass' || item.type === 'test:fail') {
258261
item.data.testNumber = isTopLevel ? (this.root.harness.counters.topLevel + 1) : item.data.testNumber;
259262
countCompletedTest({
@@ -604,6 +607,7 @@ function run(options = kEmptyObject) {
604607
} = options;
605608
const {
606609
concurrency,
610+
bail,
607611
timeout,
608612
signal,
609613
files,
@@ -747,6 +751,7 @@ function run(options = kEmptyObject) {
747751
functionCoverage: functionCoverage,
748752
cwd,
749753
globalSetupPath,
754+
bail,
750755
};
751756
const root = createTestTree(rootTestOptions, globalOptions);
752757
let testFiles = files ?? createTestFileList(globPatterns, cwd);
@@ -756,10 +761,33 @@ function run(options = kEmptyObject) {
756761
testFiles = ArrayPrototypeFilter(testFiles, (_, index) => index % shard.total === shard.index - 1);
757762
}
758763

764+
if (bail) {
765+
validateBoolean(bail, 'options.bail');
766+
if (watch) {
767+
throw new ERR_INVALID_ARG_VALUE('options.bail', watch, 'bail not supported with watch mode');
768+
}
769+
}
770+
759771
let teardown;
760772
let postRun;
761773
let filesWatcher;
762774
let runFiles;
775+
776+
if (bail) {
777+
root.reporter.on('test:fail', (item) => {
778+
if (root.harness.bail && !root.harness.bailedOut) {
779+
root.harness.bailedOut = true;
780+
queueMicrotask(() => {
781+
root.reporter[kEmitMessage]('test:bail', {
782+
__proto__: null,
783+
file: item.name,
784+
test: item.data,
785+
});
786+
});
787+
}
788+
});
789+
}
790+
763791
const opts = {
764792
__proto__: null,
765793
root,
@@ -770,6 +798,7 @@ function run(options = kEmptyObject) {
770798
hasFiles: files != null,
771799
globPatterns,
772800
only,
801+
bail,
773802
forceExit,
774803
cwd,
775804
isolation,
@@ -792,15 +821,53 @@ function run(options = kEmptyObject) {
792821
teardown = () => root.harness.teardown();
793822
}
794823

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-
};
824+
if (bail) {
825+
runFiles = async () => {
826+
root.harness.bootstrapPromise = null;
827+
root.harness.buildPromise = null;
828+
829+
const running = new SafeSet();
830+
let index = 0;
831+
832+
const shouldBail = () => bail && root.harness.bailedOut;
833+
834+
const enqueueNext = () => {
835+
if (index < testFiles.length && !shouldBail()) {
836+
const path = testFiles[index++];
837+
const subtest = runTestFile(path, filesWatcher, opts);
838+
filesWatcher?.runningSubtests.set(path, subtest);
839+
running.add(subtest);
840+
SafePromisePrototypeFinally(subtest, () => running.delete(subtest));
841+
}
842+
};
843+
844+
// Fill initial pool up to root test concurrency
845+
// We use root test concurrency here because concurrency logic is handled at test level.
846+
while (running.size < root.concurrency && index < testFiles.length && !shouldBail()) {
847+
enqueueNext();
848+
}
849+
850+
// As each test completes, enqueue the next one
851+
while (running.size > 0) {
852+
await SafePromiseRace([...running]);
853+
854+
// Refill pool after completion(s)
855+
while (running.size < root.concurrency && index < testFiles.length && !shouldBail()) {
856+
enqueueNext();
857+
}
858+
}
859+
};
860+
} else {
861+
runFiles = () => {
862+
root.harness.bootstrapPromise = null;
863+
root.harness.buildPromise = null;
864+
return SafePromiseAllSettledReturnVoid(testFiles, (path) => {
865+
const subtest = runTestFile(path, filesWatcher, opts);
866+
filesWatcher?.runningSubtests.set(path, subtest);
867+
return subtest;
868+
});
869+
};
870+
}
804871
} else if (isolation === 'none') {
805872
if (watch) {
806873
const absoluteTestFiles = ArrayPrototypeMap(testFiles, (file) => (isAbsolute(file) ? file : resolve(cwd, file)));
@@ -813,17 +880,21 @@ function run(options = kEmptyObject) {
813880
return subtest;
814881
};
815882
} else {
883+
816884
runFiles = async () => {
817-
const { promise, resolve: finishBootstrap } = PromiseWithResolvers();
885+
// Ensure global bootstrap is completed before running files, then allow
886+
// subtests to start immediately so bail can stop further file imports.
887+
if (root.harness.bootstrapPromise) {
888+
await root.harness.bootstrapPromise;
889+
root.harness.bootstrapPromise = null;
890+
}
891+
root.harness.buildPromise = null;
818892

819893
await root.runInAsyncScope(async () => {
820894
const parentURL = pathToFileURL(cwd + sep).href;
821895
const cascadedLoader = esmLoader.getOrInitializeCascadedLoader();
822896
let topLevelTestCount = 0;
823-
824-
root.harness.bootstrapPromise = root.harness.bootstrapPromise ?
825-
SafePromiseAllReturnVoid([root.harness.bootstrapPromise, promise]) :
826-
promise;
897+
let failedCount = root.harness.counters.failed;
827898

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

843914
for (let i = 0; i < testFiles.length; ++i) {
915+
if (root.harness.bail && root.harness.bailedOut) {
916+
break;
917+
}
918+
844919
const testFile = testFiles[i];
920+
const testFilePath = resolve(cwd, testFile);
845921
const fileURL = pathToFileURL(resolve(cwd, testFile));
846922
const parent = i === 0 ? undefined : parentURL;
847923
let threw = false;
@@ -872,18 +948,26 @@ function run(options = kEmptyObject) {
872948
}
873949

874950
topLevelTestCount = root.subtests.length;
951+
952+
// Ensure any subtest chains spawned by this file are finished.
953+
if (root.subtestsPromise?.promise) {
954+
await root.subtestsPromise.promise;
955+
}
875956
}
876957
});
877958

878959
debug('beginning test execution');
879960
root.entryFile = null;
880-
finishBootstrap();
961+
if (root.harness.bail && root.harness.bailedOut) {
962+
return;
963+
}
881964
return root.processPendingSubtests();
882965
};
883966
}
884967
}
885968

886969
const runChain = async () => {
970+
887971
if (root.harness?.bootstrapPromise) {
888972
await root.harness.bootstrapPromise;
889973
}

lib/internal/test_runner/test.js

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -523,7 +523,7 @@ class Test extends AsyncResource {
523523
this.root = this;
524524
this.harness = options.harness;
525525
this.config = this.harness.config;
526-
this.concurrency = 1;
526+
this.concurrency = 1; // <-- is this needed?
527527
this.nesting = 0;
528528
this.only = this.config.only;
529529
this.reporter = new TestsStream();
@@ -540,7 +540,7 @@ class Test extends AsyncResource {
540540
this.root = parent.root;
541541
this.harness = null;
542542
this.config = config;
543-
this.concurrency = parent.concurrency;
543+
this.concurrency = parent.concurrency; // <-- is this needed?
544544
this.nesting = nesting;
545545
this.only = only;
546546
this.reporter = parent.reporter;
@@ -580,7 +580,7 @@ class Test extends AsyncResource {
580580
}
581581
}
582582

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

956+
// Immediate bail signalling for isolation='none' path: mark harness so the
957+
// runner can stop loading additional files as soon as a top-level test
958+
// fails. This is a no-op when bail is disabled.
959+
if (this.harness?.bail) {
960+
this.harness.bailedOut = true;
961+
}
962+
956963
this.error = err;
957964
}
958965

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)