Skip to content

Commit 427661d

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

File tree

14 files changed

+339
-12
lines changed

14 files changed

+339
-12
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/runner.js

Lines changed: 74 additions & 9 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,17 @@ class FileTest extends Test {
254256
if (item.data.details?.error) {
255257
item.data.details.error = deserializeError(item.data.details.error);
256258
}
259+
260+
if (item.type === 'test:fail' && this.root.harness.bail && !this.root.harness.bailedOut) {
261+
// Trigger bail if enabled and this is the first failure
262+
this.root.harness.bailedOut = true;
263+
this.reporter[kEmitMessage]('test:bail', {
264+
__proto__: null,
265+
file: this.name,
266+
test: item.data,
267+
});
268+
}
269+
257270
if (item.type === 'test:pass' || item.type === 'test:fail') {
258271
item.data.testNumber = isTopLevel ? (this.root.harness.counters.topLevel + 1) : item.data.testNumber;
259272
countCompletedTest({
@@ -604,6 +617,7 @@ function run(options = kEmptyObject) {
604617
} = options;
605618
const {
606619
concurrency,
620+
bail,
607621
timeout,
608622
signal,
609623
files,
@@ -747,6 +761,7 @@ function run(options = kEmptyObject) {
747761
functionCoverage: functionCoverage,
748762
cwd,
749763
globalSetupPath,
764+
bail,
750765
};
751766
const root = createTestTree(rootTestOptions, globalOptions);
752767
let testFiles = files ?? createTestFileList(globPatterns, cwd);
@@ -756,6 +771,16 @@ function run(options = kEmptyObject) {
756771
testFiles = ArrayPrototypeFilter(testFiles, (_, index) => index % shard.total === shard.index - 1);
757772
}
758773

774+
if (bail) {
775+
validateBoolean(bail, 'options.bail');
776+
if (watch) {
777+
throw new ERR_INVALID_ARG_VALUE('options.bail', watch, 'bail not supported with watch mode');
778+
}
779+
if (isolation === 'none') {
780+
throw new ERR_INVALID_ARG_VALUE('options.bail', isolation, 'bail not supported with \'none\' isolation');
781+
}
782+
}
783+
759784
let teardown;
760785
let postRun;
761786
let filesWatcher;
@@ -770,6 +795,7 @@ function run(options = kEmptyObject) {
770795
hasFiles: files != null,
771796
globPatterns,
772797
only,
798+
bail,
773799
forceExit,
774800
cwd,
775801
isolation,
@@ -792,15 +818,54 @@ function run(options = kEmptyObject) {
792818
teardown = () => root.harness.teardown();
793819
}
794820

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

lib/internal/test_runner/test.js

Lines changed: 3 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;

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+
});
Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
'use strict';
2+
const { test } = require('node:test');
3+
const assert = require('assert');
4+
5+
test('failing test 1', () => {
6+
assert.strictEqual(1, 2, 'This test should fail');
7+
});
8+
9+
test('failing test 2', () => {
10+
assert.strictEqual(3, 4, 'This test should also fail but might not run');
11+
});
Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
'use strict';
2+
const { test } = require('node:test');
3+
4+
test('test 3 passes', () => {
5+
// This should not run if bail happens in test 2
6+
});
7+
8+
test('test 4 passes', () => {
9+
// This should not run if bail happens in test 2
10+
});

0 commit comments

Comments
 (0)