diff --git a/doc/api/cli.md b/doc/api/cli.md index 53b13748dafec3..9e45414379daeb 100644 --- a/doc/api/cli.md +++ b/doc/api/cli.md @@ -1839,6 +1839,15 @@ added: v20.10.0 The maximum number of test files that the test runner CLI will execute concurrently. The default value is `os.availableParallelism() - 1`. +### `--test-force-exit` + + + +Configures the test runner to exit the process once all known tests have +finished executing even if the event loop would otherwise remain active. + ### `--test-name-pattern` @@ -1165,6 +1170,9 @@ changes: **Default:** `false`. * `files`: {Array} An array containing the list of files to run. **Default** matching files from [test runner execution model][]. + * `forceExit`: {boolean} Configures the test runner to exit the process once + all known tests have finished executing even if the event loop would + otherwise remain active. **Default:** `false`. * `inspectPort` {number|Function} Sets inspector port of test child process. This can be a number, or a function that takes no arguments and returns a number. If a nullish value is provided, each process gets its own port, diff --git a/doc/node.1 b/doc/node.1 index 2966c14e1c3f5e..8dda6c13622171 100644 --- a/doc/node.1 +++ b/doc/node.1 @@ -425,6 +425,10 @@ Starts the Node.js command line test runner. The maximum number of test files that the test runner CLI will execute concurrently. . +.It Fl -test-force-exit +Configures the test runner to exit the process once all known tests have +finished executing even if the event loop would otherwise remain active. +. .It Fl -test-name-pattern A regular expression that configures the test runner to only execute tests whose name matches the provided pattern. diff --git a/lib/internal/test_runner/harness.js b/lib/internal/test_runner/harness.js index 2ca796dec34252..0db5ba4a60ff4a 100644 --- a/lib/internal/test_runner/harness.js +++ b/lib/internal/test_runner/harness.js @@ -207,6 +207,7 @@ function setup(root) { }, counters: null, shouldColorizeTestFiles: false, + teardown: exitHandler, }; root.harness.resetCounters(); root.startTime = hrtime(); diff --git a/lib/internal/test_runner/runner.js b/lib/internal/test_runner/runner.js index cbbea6084c5d4b..9e6bf4efb51455 100644 --- a/lib/internal/test_runner/runner.js +++ b/lib/internal/test_runner/runner.js @@ -156,8 +156,11 @@ function filterExecArgv(arg, i, arr) { !ArrayPrototypeSome(kFilterArgValues, (p) => arg === p || (i > 0 && arr[i - 1] === p) || StringPrototypeStartsWith(arg, `${p}=`)); } -function getRunArgs(path, { inspectPort, testNamePatterns, only }) { +function getRunArgs(path, { forceExit, inspectPort, testNamePatterns, only }) { const argv = ArrayPrototypeFilter(process.execArgv, filterExecArgv); + if (forceExit === true) { + ArrayPrototypePush(argv, '--test-force-exit'); + } if (isUsingInspector()) { ArrayPrototypePush(argv, `--inspect-port=${getInspectPort(inspectPort)}`); } @@ -490,7 +493,17 @@ function run(options) { options = kEmptyObject; } let { testNamePatterns, shard } = options; - const { concurrency, timeout, signal, files, inspectPort, watch, setup, only } = options; + const { + concurrency, + timeout, + signal, + files, + forceExit, + inspectPort, + watch, + setup, + only, + } = options; if (files != null) { validateArray(files, 'options.files'); @@ -498,6 +511,15 @@ function run(options) { if (watch != null) { validateBoolean(watch, 'options.watch'); } + if (forceExit != null) { + validateBoolean(forceExit, 'options.forceExit'); + + if (forceExit && watch) { + throw new ERR_INVALID_ARG_VALUE( + 'options.forceExit', watch, 'is not supported with watch mode', + ); + } + } if (only != null) { validateBoolean(only, 'options.only'); } @@ -553,7 +575,15 @@ function run(options) { let postRun = () => root.postRun(); let filesWatcher; - const opts = { __proto__: null, root, signal, inspectPort, testNamePatterns, only }; + const opts = { + __proto__: null, + root, + signal, + inspectPort, + testNamePatterns, + only, + forceExit, + }; if (watch) { filesWatcher = watchFiles(testFiles, opts); postRun = undefined; diff --git a/lib/internal/test_runner/test.js b/lib/internal/test_runner/test.js index 9edd0200a352b0..ceb68a0ce23e9b 100644 --- a/lib/internal/test_runner/test.js +++ b/lib/internal/test_runner/test.js @@ -78,7 +78,12 @@ const kHookNames = ObjectSeal(['before', 'after', 'beforeEach', 'afterEach']); const kUnwrapErrors = new SafeSet() .add(kTestCodeFailure).add(kHookFailure) .add('uncaughtException').add('unhandledRejection'); -const { sourceMaps, testNamePatterns, testOnlyFlag } = parseCommandLine(); +const { + forceExit, + sourceMaps, + testNamePatterns, + testOnlyFlag, +} = parseCommandLine(); let kResistStopPropagation; let findSourceMap; @@ -722,6 +727,16 @@ class Test extends AsyncResource { // This helps catch any asynchronous activity that occurs after the tests // have finished executing. this.postRun(); + } else if (forceExit) { + // This is the root test, and all known tests and hooks have finished + // executing. If the user wants to force exit the process regardless of + // any remaining ref'ed handles, then do that now. It is theoretically + // possible that a ref'ed handle could asynchronously create more tests, + // but the user opted into this behavior. + this.reporter.once('close', () => { + process.exit(); + }); + this.harness.teardown(); } } @@ -770,12 +785,11 @@ class Test extends AsyncResource { if (this.parent === this.root && this.root.activeSubtests === 0 && this.root.pendingSubtests.length === 0 && - this.root.readySubtests.size === 0 && - this.root.hooks.after.length > 0) { - // This is done so that any global after() hooks are run. At this point - // all of the tests have finished running. However, there might be - // ref'ed handles keeping the event loop alive. This gives the global - // after() hook a chance to clean them up. + this.root.readySubtests.size === 0) { + // At this point all of the tests have finished running. However, there + // might be ref'ed handles keeping the event loop alive. This gives the + // global after() hook a chance to clean them up. The user may also + // want to force the test runner to exit despite ref'ed handles. this.root.run(); } } else if (!this.reported) { diff --git a/lib/internal/test_runner/utils.js b/lib/internal/test_runner/utils.js index 583757d1d4524e..6306ddfa0689d3 100644 --- a/lib/internal/test_runner/utils.js +++ b/lib/internal/test_runner/utils.js @@ -199,6 +199,7 @@ function parseCommandLine() { const isTestRunner = getOptionValue('--test'); const coverage = getOptionValue('--experimental-test-coverage'); + const forceExit = getOptionValue('--test-force-exit'); const sourceMaps = getOptionValue('--enable-source-maps'); const isChildProcess = process.env.NODE_TEST_CONTEXT === 'child'; const isChildProcessV8 = process.env.NODE_TEST_CONTEXT === 'child-v8'; @@ -251,6 +252,7 @@ function parseCommandLine() { __proto__: null, isTestRunner, coverage, + forceExit, sourceMaps, testOnlyFlag, testNamePatterns, diff --git a/src/node_options.cc b/src/node_options.cc index 556b3c1592e056..937ce44696175d 100644 --- a/src/node_options.cc +++ b/src/node_options.cc @@ -179,6 +179,9 @@ void EnvironmentOptions::CheckOptions(std::vector* errors, } else if (force_repl) { errors->push_back("either --watch or --interactive " "can be used, not both"); + } else if (test_runner_force_exit) { + errors->push_back("either --watch or --test-force-exit " + "can be used, not both"); } else if (!test_runner && (argv->size() < 1 || (*argv)[1].empty())) { errors->push_back("--watch requires specifying a file"); } @@ -617,6 +620,9 @@ EnvironmentOptionsParser::EnvironmentOptionsParser() { AddOption("--test-concurrency", "specify test runner concurrency", &EnvironmentOptions::test_runner_concurrency); + AddOption("--test-force-exit", + "force test runner to exit upon completion", + &EnvironmentOptions::test_runner_force_exit); AddOption("--test-timeout", "specify test runner timeout", &EnvironmentOptions::test_runner_timeout); diff --git a/src/node_options.h b/src/node_options.h index 703e1d2b1bfc55..0a2f5512f689f8 100644 --- a/src/node_options.h +++ b/src/node_options.h @@ -166,6 +166,7 @@ class EnvironmentOptions : public Options { uint64_t test_runner_concurrency = 0; uint64_t test_runner_timeout = 0; bool test_runner_coverage = false; + bool test_runner_force_exit = false; std::vector test_name_pattern; std::vector test_reporter; std::vector test_reporter_destination; diff --git a/test/fixtures/test-runner/output/force_exit.js b/test/fixtures/test-runner/output/force_exit.js new file mode 100644 index 00000000000000..a629f972a2f0ef --- /dev/null +++ b/test/fixtures/test-runner/output/force_exit.js @@ -0,0 +1,27 @@ +// Flags: --test-force-exit --test-reporter=spec +'use strict'; +const { after, afterEach, before, beforeEach, test } = require('node:test'); + +before(() => { + console.log('BEFORE'); +}); + +beforeEach(() => { + console.log('BEFORE EACH'); +}); + +after(() => { + console.log('AFTER'); +}); + +afterEach(() => { + console.log('AFTER EACH'); +}); + +test('passes but oops', () => { + setTimeout(() => { + throw new Error('this should not have a chance to be thrown'); + }, 1000); +}); + +test('also passes'); diff --git a/test/fixtures/test-runner/output/force_exit.snapshot b/test/fixtures/test-runner/output/force_exit.snapshot new file mode 100644 index 00000000000000..b45901f80c5be8 --- /dev/null +++ b/test/fixtures/test-runner/output/force_exit.snapshot @@ -0,0 +1,16 @@ +BEFORE +BEFORE EACH +AFTER EACH +BEFORE EACH +AFTER EACH +AFTER +✔ passes but oops (*ms) +✔ also passes (*ms) +ℹ tests 2 +ℹ suites 0 +ℹ pass 2 +ℹ fail 0 +ℹ cancelled 0 +ℹ skipped 0 +ℹ todo 0 +ℹ duration_ms * diff --git a/test/fixtures/test-runner/throws_sync_and_async.js b/test/fixtures/test-runner/throws_sync_and_async.js new file mode 100644 index 00000000000000..50ed81b4acf511 --- /dev/null +++ b/test/fixtures/test-runner/throws_sync_and_async.js @@ -0,0 +1,10 @@ +'use strict'; +const { test } = require('node:test'); + +test('fails and schedules more work', () => { + setTimeout(() => { + throw new Error('this should not have a chance to be thrown'); + }, 1000); + + throw new Error('fails'); +}); diff --git a/test/parallel/test-runner-force-exit-failure.js b/test/parallel/test-runner-force-exit-failure.js new file mode 100644 index 00000000000000..1fff8f30d7e038 --- /dev/null +++ b/test/parallel/test-runner-force-exit-failure.js @@ -0,0 +1,15 @@ +'use strict'; +require('../common'); +const { match, doesNotMatch, strictEqual } = require('node:assert'); +const { spawnSync } = require('node:child_process'); +const fixtures = require('../common/fixtures'); +const fixture = fixtures.path('test-runner/throws_sync_and_async.js'); +const r = spawnSync(process.execPath, ['--test', '--test-force-exit', fixture]); + +strictEqual(r.status, 1); +strictEqual(r.signal, null); +strictEqual(r.stderr.toString(), ''); + +const stdout = r.stdout.toString(); +match(stdout, /error: 'fails'/); +doesNotMatch(stdout, /this should not have a chance to be thrown/); diff --git a/test/parallel/test-runner-output.mjs b/test/parallel/test-runner-output.mjs index 26763c6fc0f607..df1067ec04a4e0 100644 --- a/test/parallel/test-runner-output.mjs +++ b/test/parallel/test-runner-output.mjs @@ -105,6 +105,7 @@ const tests = [ { name: 'test-runner/output/global-hooks-with-no-tests.js' }, { name: 'test-runner/output/before-and-after-each-too-many-listeners.js' }, { name: 'test-runner/output/before-and-after-each-with-timeout-too-many-listeners.js' }, + { name: 'test-runner/output/force_exit.js', transform: specTransform }, { name: 'test-runner/output/global_after_should_fail_the_test.js' }, { name: 'test-runner/output/no_refs.js' }, { name: 'test-runner/output/no_tests.js' }, diff --git a/test/parallel/test-runner-run.mjs b/test/parallel/test-runner-run.mjs index 079fa57f25855f..d6295522f96d95 100644 --- a/test/parallel/test-runner-run.mjs +++ b/test/parallel/test-runner-run.mjs @@ -526,3 +526,21 @@ describe('require(\'node:test\').run', { concurrency: true }, () => { assert.match(stderr, /Warning: node:test run\(\) is being called recursively/); }); }); + +describe('forceExit', () => { + it('throws for non-boolean values', () => { + [Symbol(), {}, 0, 1, '1', Promise.resolve([])].forEach((forceExit) => { + assert.throws(() => run({ forceExit }), { + code: 'ERR_INVALID_ARG_TYPE', + message: /The "options\.forceExit" property must be of type boolean\./ + }); + }); + }); + + it('throws if enabled with watch mode', () => { + assert.throws(() => run({ forceExit: true, watch: true }), { + code: 'ERR_INVALID_ARG_VALUE', + message: /The property 'options\.forceExit' is not supported with watch mode\./ + }); + }); +});