diff --git a/lib/internal/test_runner/harness.js b/lib/internal/test_runner/harness.js index 6b3b13b2c88d65..ecdbd1d41608c6 100644 --- a/lib/internal/test_runner/harness.js +++ b/lib/internal/test_runner/harness.js @@ -52,6 +52,8 @@ function createTestTree(rootTestOptions, globalOptions) { buildSuites: [], isWaitingForBuildPhase: false, watching: false, + bail: globalOptions.bail, + bailedOut: false, config: globalOptions, coverage: null, resetCounters() { diff --git a/lib/internal/test_runner/reporter/spec.js b/lib/internal/test_runner/reporter/spec.js index 14c447f316492f..af0c915fd00bec 100644 --- a/lib/internal/test_runner/reporter/spec.js +++ b/lib/internal/test_runner/reporter/spec.js @@ -78,6 +78,8 @@ class SpecReporter extends Transform { } #handleEvent({ type, data }) { switch (type) { + case 'test:bail': + return `${reporterColorMap['test:bail']}Bailing out! no new test files will be started!${colors.white}\n`; case 'test:fail': if (data.details?.error?.failureType !== kSubtestsFailed) { ArrayPrototypePush(this.#failedTests, data); diff --git a/lib/internal/test_runner/reporter/utils.js b/lib/internal/test_runner/reporter/utils.js index 26e4a2d1a5c36c..13af3da8abf8ba 100644 --- a/lib/internal/test_runner/reporter/utils.js +++ b/lib/internal/test_runner/reporter/utils.js @@ -37,6 +37,9 @@ const reporterColorMap = { get 'test:diagnostic'() { return colors.blue; }, + get 'test:bail'() { + return colors.yellow; + }, get 'info'() { return colors.blue; }, diff --git a/lib/internal/test_runner/runner.js b/lib/internal/test_runner/runner.js index a9b3cfb93c8abb..9af339b7a2e711 100644 --- a/lib/internal/test_runner/runner.js +++ b/lib/internal/test_runner/runner.js @@ -22,6 +22,8 @@ const { SafePromiseAll, SafePromiseAllReturnVoid, SafePromiseAllSettledReturnVoid, + SafePromisePrototypeFinally, + SafePromiseRace, SafeSet, StringPrototypeIndexOf, StringPrototypeSlice, @@ -254,6 +256,7 @@ class FileTest extends Test { if (item.data.details?.error) { item.data.details.error = deserializeError(item.data.details.error); } + if (item.type === 'test:pass' || item.type === 'test:fail') { item.data.testNumber = isTopLevel ? (this.root.harness.counters.topLevel + 1) : item.data.testNumber; countCompletedTest({ @@ -604,6 +607,7 @@ function run(options = kEmptyObject) { } = options; const { concurrency, + bail, timeout, signal, files, @@ -747,6 +751,7 @@ function run(options = kEmptyObject) { functionCoverage: functionCoverage, cwd, globalSetupPath, + bail, }; const root = createTestTree(rootTestOptions, globalOptions); let testFiles = files ?? createTestFileList(globPatterns, cwd); @@ -756,6 +761,16 @@ 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'); + } + if (isolation === 'none') { + throw new ERR_INVALID_ARG_VALUE('options.bail', isolation, 'bail not supported with \'none\' isolation'); + } + } + let teardown; let postRun; let filesWatcher; @@ -770,6 +785,7 @@ function run(options = kEmptyObject) { hasFiles: files != null, globPatterns, only, + bail, forceExit, cwd, isolation, @@ -792,15 +808,66 @@ 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(); + } + } + }; + + root.reporter.on('test:fail', (item) => { + if (root.harness.bail && !root.harness.bailedOut) { + process.nextTick(() => { + root.harness.bailedOut = true; + root.reporter[kEmitMessage]('test:bail', { + __proto__: null, + file: item.name, + test: item.data, + }); + }); + } }); - }; + } 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))); diff --git a/lib/internal/test_runner/test.js b/lib/internal/test_runner/test.js index e426438faba75f..decd369caae547 100644 --- a/lib/internal/test_runner/test.js +++ b/lib/internal/test_runner/test.js @@ -523,7 +523,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(); @@ -540,7 +540,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; @@ -580,7 +580,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; diff --git a/lib/internal/test_runner/tests_stream.js b/lib/internal/test_runner/tests_stream.js index 7b64487696f53f..cc33cb4afd7c79 100644 --- a/lib/internal/test_runner/tests_stream.js +++ b/lib/internal/test_runner/tests_stream.js @@ -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); } diff --git a/lib/internal/test_runner/utils.js b/lib/internal/test_runner/utils.js index 5b53342933cdcb..58d584f7d905e1 100644 --- a/lib/internal/test_runner/utils.js +++ b/lib/internal/test_runner/utils.js @@ -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'); @@ -341,6 +342,7 @@ function parseCommandLine() { globalTestOptions = { __proto__: null, isTestRunner, + bail, concurrency, coverage, coverageExcludeGlobs, diff --git a/src/node_options.cc b/src/node_options.cc index 54c6c17c3d007d..e5e823ab21e1d9 100644 --- a/src/node_options.cc +++ b/src/node_options.cc @@ -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, diff --git a/src/node_options.h b/src/node_options.h index 25bd23c72f9e3b..cd0b0089ea15b0 100644 --- a/src/node_options.h +++ b/src/node_options.h @@ -190,6 +190,7 @@ class EnvironmentOptions : public Options { std::vector 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; diff --git a/test/fixtures/test-runner/bail/bail-test-1-pass.js b/test/fixtures/test-runner/bail/bail-test-1-pass.js new file mode 100644 index 00000000000000..f359d8045e8fed --- /dev/null +++ b/test/fixtures/test-runner/bail/bail-test-1-pass.js @@ -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 +}); diff --git a/test/fixtures/test-runner/bail/bail-test-2-fail.js b/test/fixtures/test-runner/bail/bail-test-2-fail.js new file mode 100644 index 00000000000000..ca18420309df5e --- /dev/null +++ b/test/fixtures/test-runner/bail/bail-test-2-fail.js @@ -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'); +}); diff --git a/test/fixtures/test-runner/bail/bail-test-3-pass.js b/test/fixtures/test-runner/bail/bail-test-3-pass.js new file mode 100644 index 00000000000000..b1abf5cb975e85 --- /dev/null +++ b/test/fixtures/test-runner/bail/bail-test-3-pass.js @@ -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 +}); diff --git a/test/fixtures/test-runner/bail/bail-test-4-pass.js b/test/fixtures/test-runner/bail/bail-test-4-pass.js new file mode 100644 index 00000000000000..b5a3cbe3fa492b --- /dev/null +++ b/test/fixtures/test-runner/bail/bail-test-4-pass.js @@ -0,0 +1,6 @@ +'use strict'; +const { test } = require('node:test'); + +test('test 5 passes', () => { + // This should not run if bail happens earlier +}); diff --git a/test/fixtures/test-runner/output/bail_concurrency_1.js b/test/fixtures/test-runner/output/bail_concurrency_1.js new file mode 100644 index 00000000000000..277abf6f8cef32 --- /dev/null +++ b/test/fixtures/test-runner/output/bail_concurrency_1.js @@ -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); diff --git a/test/fixtures/test-runner/output/bail_concurrency_1.snapshot b/test/fixtures/test-runner/output/bail_concurrency_1.snapshot new file mode 100644 index 00000000000000..319c5397794afa --- /dev/null +++ b/test/fixtures/test-runner/output/bail_concurrency_1.snapshot @@ -0,0 +1,55 @@ +✔ test 1 passes (*ms) +✔ test 2 passes (*ms) +✖ failing test 1 (*ms) +Bailing out! no new test files will be started! +✖ failing test 2 (*ms) +ℹ tests 4 +ℹ suites 0 +ℹ pass 2 +ℹ fail 2 +ℹ cancelled 0 +ℹ skipped 0 +ℹ todo 0 +ℹ duration_ms * + +✖ failing tests: + +* +✖ failing test 1 (*ms) + AssertionError [ERR_ASSERTION]: This test should fail + + 1 !== 2 + + * + * + * + * + * { + generatedMessage: false, + code: 'ERR_ASSERTION', + actual: 1, + expected: 2, + operator: 'strictEqual', + diff: 'simple' + } + +* +✖ failing test 2 (*ms) + AssertionError [ERR_ASSERTION]: This test fails as well + + 3 !== 4 + + * + * + * + * + * + * + * { + generatedMessage: false, + code: 'ERR_ASSERTION', + actual: 3, + expected: 4, + operator: 'strictEqual', + diff: 'simple' + } diff --git a/test/parallel/test-runner-bail.js b/test/parallel/test-runner-bail.js new file mode 100644 index 00000000000000..c9e86e2ae23939 --- /dev/null +++ b/test/parallel/test-runner-bail.js @@ -0,0 +1,114 @@ +'use strict'; +require('../common'); +const { test } = require('node:test'); +const assert = require('node:assert'); +const { spawn } = require('node:child_process'); +const { once } = require('node:events'); +const fixtures = require('../common/fixtures'); + +const runTest = async (args, cwd = process.cwd()) => { + const child = spawn(process.execPath, args, { + cwd, + stdio: 'pipe', + env: { ...process.env }, + }); + + const stdout = []; + const stderr = []; + + child.stdout.on('data', (chunk) => stdout.push(chunk)); + child.stderr.on('data', (chunk) => stderr.push(chunk)); + + const [code] = await once(child, 'exit'); + + return { + code, + stdout: Buffer.concat(stdout).toString('utf8'), + stderr: Buffer.concat(stderr).toString('utf8'), + }; +}; + +test('bail stops test execution on first failure with isolation=process', async () => { + const fixturesDir = fixtures.path('test-runner', 'bail'); + const result = await runTest([ + '--test-bail', + '--test-concurrency=1', // <-- fixed concurrency to ensure predictable order of test execution + '--test', + 'bail-test-1-pass.js', + 'bail-test-2-fail.js', + 'bail-test-3-pass.js', + 'bail-test-4-pass.js', + ], fixturesDir); + + // Should exit with non-zero code due to failure + assert.notStrictEqual(result.code, 0); + + // bail-test-1-pass.js should run (2 tests) + assert.match(result.stdout, /test 1 passes/); + assert.match(result.stdout, /test 2 passes/); + + // bail-test-2-fail.js should run and fail + assert.match(result.stdout, /failing test 1/); + + // Files after failure should NOT run + assert.doesNotMatch(result.stdout, /test 3 passes/); + assert.doesNotMatch(result.stdout, /test 4 passes/); + assert.doesNotMatch(result.stdout, /test 5 passes/); +}); + +test.todo('bail stops test execution on first failure with isolation=none'); + +test('bail is not supported with isolation=none', async () => { + const fixturesDir = fixtures.path('test-runner', 'bail'); + const result = await runTest([ + '--test', + '--test-bail', + '--test-isolation=none', + 'bail-test-1-pass.js', + ], fixturesDir); + + // Should exit with error + assert.notStrictEqual(result.code, 0); + + // Should contain error message about bail+isolation incompatibility + assert.match(result.stderr, /bail not supported with 'none' isolation/); +}); + +test('bail throws error when combined with watch mode', async () => { + const fixturesDir = fixtures.path('test-runner', 'bail'); + const result = await runTest([ + '--test', + '--test-bail', + '--watch', + 'bail-test-1-pass.js', + ], fixturesDir); + + // Should exit with error + assert.notStrictEqual(result.code, 0); + + // Should contain error message about bail+watch incompatibility + assert.match(result.stderr, /not supported with watch mode/); +}); + +test('without bail, all tests run even after failure', async () => { + const fixturesDir = fixtures.path('test-runner', 'bail'); + const result = await runTest([ + '--test', + 'bail-test-1-pass.js', + 'bail-test-2-fail.js', + 'bail-test-3-pass.js', + ], fixturesDir); + + // Should exit with non-zero code due to failure + assert.notStrictEqual(result.code, 0); + + // All tests should run + assert.match(result.stdout, /test 1 passes/); + assert.match(result.stdout, /test 2 passes/); + assert.match(result.stdout, /failing test 1/); + assert.match(result.stdout, /test 3 passes/); + assert.match(result.stdout, /test 4 passes/); + + // Should NOT contain bail event + assert.doesNotMatch(result.stdout, /test:bail/); +}); diff --git a/test/parallel/test-runner-run-bail-isolation-none.mjs b/test/parallel/test-runner-run-bail-isolation-none.mjs new file mode 100644 index 00000000000000..7627dc30884c6c --- /dev/null +++ b/test/parallel/test-runner-run-bail-isolation-none.mjs @@ -0,0 +1,15 @@ +import '../common/index.mjs'; +import assert from 'node:assert'; +import { run } from 'node:test'; +import * as fixtures from '../common/fixtures.mjs'; + +const files = [ + fixtures.path('test-runner', 'bail', 'bail-test-1-pass.js'), +]; + +assert.throws(() => { + run({ bail: true, isolation: 'none', files }); +}, { + code: 'ERR_INVALID_ARG_VALUE', + message: /bail not supported with 'none' isolation/, +}); diff --git a/test/parallel/test-runner-run-bail.mjs b/test/parallel/test-runner-run-bail.mjs new file mode 100644 index 00000000000000..093a5bf2e12a73 --- /dev/null +++ b/test/parallel/test-runner-run-bail.mjs @@ -0,0 +1,74 @@ +import * as common from '../common/index.mjs'; +import assert from 'node:assert'; +import { test, run } from 'node:test'; +import * as fixtures from '../common/fixtures.mjs'; + +test('bail option validation', async () => { + // Test that bail rejects watch mode + assert.throws(() => { + run({ bail: true, watch: true, files: [] }); + }, { + code: 'ERR_INVALID_ARG_VALUE', + message: /not supported with watch mode/, + }); +}); + +test('bail stops test execution on first failure', async () => { + 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'), + ]; + + const stream = run({ bail: true, concurrency: 1, files }); + + let testCount = 0; + let failCount = 0; + + stream.on('test:bail', common.mustCall((data) => { + assert.ok(data.file); + assert.ok(data.test); + })); + + stream.on('test:pass', () => { + testCount++; + }); + + stream.on('test:fail', () => { + failCount++; + }); + + // eslint-disable-next-line no-unused-vars, no-empty + for await (const event of stream) {} + + assert.strictEqual(failCount, 2); + assert.strictEqual(testCount, 2); +}); + +test('without bail, all tests run even after failure', async () => { + 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'), + ]; + + const stream = run({ bail: false, files }); + + let bailEventReceived = false; + let testCount = 0; + + stream.on('test:bail', () => { + bailEventReceived = true; + }); + + stream.on('test:pass', () => { + testCount++; + }); + + // eslint-disable-next-line no-unused-vars, no-empty + for await (const _ of stream) {} + + assert.strictEqual(bailEventReceived, false); + assert.strictEqual(testCount, 4); +}); diff --git a/test/test-runner/test-output-bail-spec-reporter.mjs b/test/test-runner/test-output-bail-spec-reporter.mjs new file mode 100644 index 00000000000000..97b4240b1691d7 --- /dev/null +++ b/test/test-runner/test-output-bail-spec-reporter.mjs @@ -0,0 +1,10 @@ +// Test that the output of test-runner/output/bail_concurrency_1.js matches test-runner/output/bail_concurrency_1.snapshot +import '../common/index.mjs'; +import * as fixtures from '../common/fixtures.mjs'; +import { spawnAndAssert, specTransform, ensureCwdIsProjectRoot } from '../common/assertSnapshot.js'; + +ensureCwdIsProjectRoot(); +await spawnAndAssert( + fixtures.path('test-runner/output/bail_concurrency_1.js'), + specTransform, +);