Skip to content

Commit d867666

Browse files
committed
test_runner: support runner bailout at files level
1 parent 427661d commit d867666

13 files changed

+274
-30
lines changed

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: 64 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -257,16 +257,6 @@ class FileTest extends Test {
257257
item.data.details.error = deserializeError(item.data.details.error);
258258
}
259259

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-
270260
if (item.type === 'test:pass' || item.type === 'test:fail') {
271261
item.data.testNumber = isTopLevel ? (this.root.harness.counters.topLevel + 1) : item.data.testNumber;
272262
countCompletedTest({
@@ -776,15 +766,28 @@ function run(options = kEmptyObject) {
776766
if (watch) {
777767
throw new ERR_INVALID_ARG_VALUE('options.bail', watch, 'bail not supported with watch mode');
778768
}
779-
if (isolation === 'none') {
780-
throw new ERR_INVALID_ARG_VALUE('options.bail', isolation, 'bail not supported with \'none\' isolation');
781-
}
782769
}
783770

784771
let teardown;
785772
let postRun;
786773
let filesWatcher;
787774
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+
788791
const opts = {
789792
__proto__: null,
790793
root,
@@ -854,7 +857,6 @@ function run(options = kEmptyObject) {
854857
}
855858
}
856859
};
857-
858860
} else {
859861
runFiles = () => {
860862
root.harness.bootstrapPromise = null;
@@ -878,17 +880,21 @@ function run(options = kEmptyObject) {
878880
return subtest;
879881
};
880882
} else {
883+
881884
runFiles = async () => {
882-
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;
883892

884893
await root.runInAsyncScope(async () => {
885894
const parentURL = pathToFileURL(cwd + sep).href;
886895
const cascadedLoader = esmLoader.getOrInitializeCascadedLoader();
887896
let topLevelTestCount = 0;
888-
889-
root.harness.bootstrapPromise = root.harness.bootstrapPromise ?
890-
SafePromiseAllReturnVoid([root.harness.bootstrapPromise, promise]) :
891-
promise;
897+
let failedCount = root.harness.counters.failed;
892898

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

908914
for (let i = 0; i < testFiles.length; ++i) {
915+
if (root.harness.bail && root.harness.bailedOut) {
916+
break;
917+
}
918+
909919
const testFile = testFiles[i];
920+
const testFilePath = resolve(cwd, testFile);
910921
const fileURL = pathToFileURL(resolve(cwd, testFile));
911922
const parent = i === 0 ? undefined : parentURL;
912923
let threw = false;
@@ -933,22 +944,54 @@ function run(options = kEmptyObject) {
933944
if (threw) {
934945
subtest.fail(importError);
935946
}
936-
startSubtestAfterBootstrap(subtest);
947+
startSubtestAfterBootstrap(subtest);
937948
}
938949

939950
topLevelTestCount = root.subtests.length;
951+
952+
// Run pending subtests created so far; if bail triggers, stop loading more files.
953+
await root.processPendingSubtests();
954+
// Allow reporter events (including bail) to flush before continuing.
955+
await new Promise((resolve) => setImmediate(resolve));
956+
// Ensure any subtest chains spawned by this file are finished.
957+
if (root.subtestsPromise?.promise) {
958+
await root.subtestsPromise.promise;
959+
}
960+
961+
if (root.harness.bail) {
962+
if (root.harness.counters.failed > failedCount) {
963+
root.harness.bailedOut = true;
964+
} else {
965+
const fileFailed = ArrayPrototypeSome(
966+
root.subtests,
967+
(t) => t.loc?.file === testFilePath && t.error != null,
968+
);
969+
if (fileFailed) {
970+
root.harness.bailedOut = true;
971+
}
972+
}
973+
}
974+
975+
if (root.harness.bail && root.harness.bailedOut) {
976+
break;
977+
}
978+
979+
failedCount = root.harness.counters.failed;
940980
}
941981
});
942982

943983
debug('beginning test execution');
944984
root.entryFile = null;
945-
finishBootstrap();
985+
if (root.harness.bail && root.harness.bailedOut) {
986+
return;
987+
}
946988
return root.processPendingSubtests();
947989
};
948990
}
949991
}
950992

951993
const runChain = async () => {
994+
952995
if (root.harness?.bootstrapPromise) {
953996
await root.harness.bootstrapPromise;
954997
}

lib/internal/test_runner/test.js

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -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

test/fixtures/test-runner/bail/bail-test-2-fail.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,5 +7,5 @@ test('failing test 1', () => {
77
});
88

99
test('failing test 2', () => {
10-
assert.strictEqual(3, 4, 'This test should also fail but might not run');
10+
assert.strictEqual(3, 4, 'This test fails as well');
1111
});
Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
'use strict';
2+
const fixtures = require('../../../common/fixtures');
3+
const { spec } = require('node:test/reporters');
4+
const { run } = require('node:test');
5+
6+
const files = [
7+
fixtures.path('test-runner', 'bail', 'bail-test-1-pass.js'),
8+
fixtures.path('test-runner', 'bail', 'bail-test-2-fail.js'),
9+
fixtures.path('test-runner', 'bail', 'bail-test-3-pass.js'),
10+
fixtures.path('test-runner', 'bail', 'bail-test-4-pass.js'),
11+
];
12+
13+
run({ bail: true, concurrency: 1, files }).compose(spec).compose(process.stdout);
Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,55 @@
1+
✔ test 1 passes (*ms)
2+
✔ test 2 passes (*ms)
3+
✖ failing test 1 (*ms)
4+
Bailing out! no new test files will be started!
5+
✖ failing test 2 (*ms)
6+
ℹ tests 4
7+
ℹ suites 0
8+
ℹ pass 2
9+
ℹ fail 2
10+
ℹ cancelled 0
11+
ℹ skipped 0
12+
ℹ todo 0
13+
ℹ duration_ms *
14+
15+
✖ failing tests:
16+
17+
*
18+
✖ failing test 1 (*ms)
19+
AssertionError [ERR_ASSERTION]: This test should fail
20+
21+
1 !== 2
22+
23+
*
24+
*
25+
*
26+
*
27+
* {
28+
generatedMessage: false,
29+
code: 'ERR_ASSERTION',
30+
actual: 1,
31+
expected: 2,
32+
operator: 'strictEqual',
33+
diff: 'simple'
34+
}
35+
36+
*
37+
✖ failing test 2 (*ms)
38+
AssertionError [ERR_ASSERTION]: This test fails as well
39+
40+
3 !== 4
41+
42+
*
43+
*
44+
*
45+
*
46+
*
47+
*
48+
* {
49+
generatedMessage: false,
50+
code: 'ERR_ASSERTION',
51+
actual: 3,
52+
expected: 4,
53+
operator: 'strictEqual',
54+
diff: 'simple'
55+
}
Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
'use strict';
2+
const fixtures = require('../../../common/fixtures');
3+
const { spec } = require('node:test/reporters');
4+
const { run } = require('node:test');
5+
6+
const files = [
7+
fixtures.path('test-runner', 'bail', 'bail-test-1-pass.js'),
8+
fixtures.path('test-runner', 'bail', 'bail-test-2-fail.js'),
9+
fixtures.path('test-runner', 'bail', 'bail-test-3-pass.js'),
10+
fixtures.path('test-runner', 'bail', 'bail-test-4-pass.js'),
11+
];
12+
13+
run({ bail: true, concurrency: 1, isolation: 'none', files }).compose(spec).compose(process.stdout);
Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,60 @@
1+
✔ test 1 passes (*ms)
2+
✔ test 2 passes (*ms)
3+
✖ failing test 1 (*ms)
4+
Bailing out! no new test files will be started!
5+
✖ failing test 2 (*ms)
6+
ℹ tests 4
7+
ℹ suites 0
8+
ℹ pass 2
9+
ℹ fail 2
10+
ℹ cancelled 0
11+
ℹ skipped 0
12+
ℹ todo 0
13+
ℹ duration_ms *
14+
15+
✖ failing tests:
16+
17+
*
18+
✖ failing test 1 (*ms)
19+
AssertionError [ERR_ASSERTION]: This test should fail
20+
21+
1 !== 2
22+
23+
*
24+
*
25+
*
26+
*
27+
*
28+
*
29+
*
30+
*
31+
*
32+
* {
33+
generatedMessage: false,
34+
code: 'ERR_ASSERTION',
35+
actual: 1,
36+
expected: 2,
37+
operator: 'strictEqual',
38+
diff: 'simple'
39+
}
40+
41+
*
42+
✖ failing test 2 (*ms)
43+
AssertionError [ERR_ASSERTION]: This test fails as well
44+
45+
3 !== 4
46+
47+
*
48+
*
49+
*
50+
*
51+
*
52+
*
53+
* {
54+
generatedMessage: false,
55+
code: 'ERR_ASSERTION',
56+
actual: 3,
57+
expected: 4,
58+
operator: 'strictEqual',
59+
diff: 'simple'
60+
}
Lines changed: 36 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,44 @@
1-
import '../common/index.mjs';
1+
import * as common from '../common/index.mjs';
22
import assert from 'node:assert';
3-
import { run } from 'node:test';
3+
import { test, run } from 'node:test';
44
import * as fixtures from '../common/fixtures.mjs';
55

66
const files = [
77
fixtures.path('test-runner', 'bail', 'bail-test-1-pass.js'),
8+
fixtures.path('test-runner', 'bail', 'bail-test-2-fail.js'),
9+
fixtures.path('test-runner', 'bail', 'bail-test-3-pass.js'),
10+
fixtures.path('test-runner', 'bail', 'bail-test-4-pass.js'),
811
];
912

10-
assert.throws(() => {
11-
run({ bail: true, isolation: 'none', files });
12-
}, {
13-
code: 'ERR_INVALID_ARG_VALUE',
14-
message: /bail not supported with 'none' isolation/,
13+
async function runScenario() {
14+
const stream = run({ bail: true, isolation: 'process', concurrency: 1, files });
15+
16+
let testCount = 0;
17+
let failCount = 0;
18+
let bailCount = 0;
19+
20+
stream.on('test:bail', common.mustCall(() => {
21+
bailCount++;
22+
}));
23+
24+
stream.on('test:pass', () => {
25+
testCount++;
26+
});
27+
28+
stream.on('test:fail', () => {
29+
failCount++;
30+
});
31+
32+
// eslint-disable-next-line no-unused-vars, no-empty
33+
for await (const _ of stream) {}
34+
35+
return { testCount, failCount, bailCount };
36+
}
37+
38+
test('it should bail in isolation process mode', async () => {
39+
const { testCount, failCount, bailCount } = await runScenario();
40+
41+
assert.strictEqual(failCount, 2);
42+
assert.strictEqual(testCount, 2);
43+
assert.strictEqual(bailCount, 1);
1544
});

0 commit comments

Comments
 (0)