Skip to content

Commit e054dc8

Browse files
cjihrigmarco-ippolito
cjihrig
authored and
marco-ippolito
committed
test_runner: support forced exit
This commit updates the test runner to allow a forced exit once all known tests have finished running. Fixes: #49925 PR-URL: #52038 Reviewed-By: Moshe Atlow <moshe@atlow.co.il> Reviewed-By: Chemi Atlow <chemi@atlow.co.il> Reviewed-By: Raz Luvaton <rluvaton@gmail.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
1 parent 1b949b8 commit e054dc8

File tree

15 files changed

+173
-11
lines changed

15 files changed

+173
-11
lines changed

‎doc/api/cli.md‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1824,6 +1824,15 @@ added: v20.10.0
18241824
The maximum number of test files that the test runner CLI will execute
18251825
concurrently. The default value is `os.availableParallelism() - 1`.
18261826

1827+
### `--test-force-exit`
1828+
1829+
<!-- YAML
1830+
added: REPLACEME
1831+
-->
1832+
1833+
Configures the test runner to exit the process once all known tests have
1834+
finished executing even if the event loop would otherwise remain active.
1835+
18271836
### `--test-name-pattern`
18281837

18291838
<!-- YAML

‎doc/api/test.md‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1123,7 +1123,12 @@ added:
11231123
- v18.9.0
11241124
- v16.19.0
11251125
changes:
1126-
- version: v20.1.0
1126+
- version: REPLACEME
1127+
pr-url: https://github.com/nodejs/node/pull/52038
1128+
description: Added the `forceExit` option.
1129+
- version:
1130+
- v20.1.0
1131+
- v18.17.0
11271132
pr-url: https://github.com/nodejs/node/pull/47628
11281133
description: Add a testNamePatterns option.
11291134
-->
@@ -1139,6 +1144,9 @@ changes:
11391144
**Default:** `false`.
11401145
* `files`: {Array} An array containing the list of files to run.
11411146
**Default** matching files from [test runner execution model][].
1147+
* `forceExit`: {boolean} Configures the test runner to exit the process once
1148+
all known tests have finished executing even if the event loop would
1149+
otherwise remain active. **Default:** `false`.
11421150
* `inspectPort` {number|Function} Sets inspector port of test child process.
11431151
This can be a number, or a function that takes no arguments and returns a
11441152
number. If a nullish value is provided, each process gets its own port,

‎doc/node.1‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -425,6 +425,10 @@ Starts the Node.js command line test runner.
425425
The maximum number of test files that the test runner CLI will execute
426426
concurrently.
427427
.
428+
.It Fl -test-force-exit
429+
Configures the test runner to exit the process once all known tests have
430+
finished executing even if the event loop would otherwise remain active.
431+
.
428432
.It Fl -test-name-pattern
429433
A regular expression that configures the test runner to only execute tests
430434
whose name matches the provided pattern.

‎lib/internal/test_runner/harness.js‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -195,6 +195,7 @@ function setup(root) {
195195
suites: 0,
196196
},
197197
shouldColorizeTestFiles: false,
198+
teardown: exitHandler,
198199
};
199200
root.startTime = hrtime();
200201
return root;

‎lib/internal/test_runner/runner.js‎

Lines changed: 33 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -155,8 +155,11 @@ function filterExecArgv(arg, i, arr) {
155155
!ArrayPrototypeSome(kFilterArgValues, (p) => arg === p || (i > 0 && arr[i - 1] === p) || StringPrototypeStartsWith(arg, `${p}=`));
156156
}
157157

158-
function getRunArgs(path, { inspectPort, testNamePatterns, only }) {
158+
function getRunArgs(path, { forceExit, inspectPort, testNamePatterns, only }) {
159159
const argv = ArrayPrototypeFilter(process.execArgv, filterExecArgv);
160+
if (forceExit === true) {
161+
ArrayPrototypePush(argv, '--test-force-exit');
162+
}
160163
if (isUsingInspector()) {
161164
ArrayPrototypePush(argv, `--inspect-port=${getInspectPort(inspectPort)}`);
162165
}
@@ -484,14 +487,33 @@ function run(options) {
484487
options = kEmptyObject;
485488
}
486489
let { testNamePatterns, shard } = options;
487-
const { concurrency, timeout, signal, files, inspectPort, watch, setup, only } = options;
490+
const {
491+
concurrency,
492+
timeout,
493+
signal,
494+
files,
495+
forceExit,
496+
inspectPort,
497+
watch,
498+
setup,
499+
only,
500+
} = options;
488501

489502
if (files != null) {
490503
validateArray(files, 'options.files');
491504
}
492505
if (watch != null) {
493506
validateBoolean(watch, 'options.watch');
494507
}
508+
if (forceExit != null) {
509+
validateBoolean(forceExit, 'options.forceExit');
510+
511+
if (forceExit && watch) {
512+
throw new ERR_INVALID_ARG_VALUE(
513+
'options.forceExit', watch, 'is not supported with watch mode',
514+
);
515+
}
516+
}
495517
if (only != null) {
496518
validateBoolean(only, 'options.only');
497519
}
@@ -545,7 +567,15 @@ function run(options) {
545567

546568
let postRun = () => root.postRun();
547569
let filesWatcher;
548-
const opts = { __proto__: null, root, signal, inspectPort, testNamePatterns, only };
570+
const opts = {
571+
__proto__: null,
572+
root,
573+
signal,
574+
inspectPort,
575+
testNamePatterns,
576+
only,
577+
forceExit,
578+
};
549579
if (watch) {
550580
filesWatcher = watchFiles(testFiles, opts);
551581
postRun = undefined;

‎lib/internal/test_runner/test.js‎

Lines changed: 21 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,12 @@ const kHookNames = ObjectSeal(['before', 'after', 'beforeEach', 'afterEach']);
7575
const kUnwrapErrors = new SafeSet()
7676
.add(kTestCodeFailure).add(kHookFailure)
7777
.add('uncaughtException').add('unhandledRejection');
78-
const { sourceMaps, testNamePatterns, testOnlyFlag } = parseCommandLine();
78+
const {
79+
forceExit,
80+
sourceMaps,
81+
testNamePatterns,
82+
testOnlyFlag,
83+
} = parseCommandLine();
7984
let kResistStopPropagation;
8085
let findSourceMap;
8186

@@ -720,6 +725,16 @@ class Test extends AsyncResource {
720725
// This helps catch any asynchronous activity that occurs after the tests
721726
// have finished executing.
722727
this.postRun();
728+
} else if (forceExit) {
729+
// This is the root test, and all known tests and hooks have finished
730+
// executing. If the user wants to force exit the process regardless of
731+
// any remaining ref'ed handles, then do that now. It is theoretically
732+
// possible that a ref'ed handle could asynchronously create more tests,
733+
// but the user opted into this behavior.
734+
this.reporter.once('close', () => {
735+
process.exit();
736+
});
737+
this.harness.teardown();
723738
}
724739
}
725740

@@ -770,12 +785,11 @@ class Test extends AsyncResource {
770785
if (this.parent === this.root &&
771786
this.root.activeSubtests === 0 &&
772787
this.root.pendingSubtests.length === 0 &&
773-
this.root.readySubtests.size === 0 &&
774-
this.root.hooks.after.length > 0) {
775-
// This is done so that any global after() hooks are run. At this point
776-
// all of the tests have finished running. However, there might be
777-
// ref'ed handles keeping the event loop alive. This gives the global
778-
// after() hook a chance to clean them up.
788+
this.root.readySubtests.size === 0) {
789+
// At this point all of the tests have finished running. However, there
790+
// might be ref'ed handles keeping the event loop alive. This gives the
791+
// global after() hook a chance to clean them up. The user may also
792+
// want to force the test runner to exit despite ref'ed handles.
779793
this.root.run();
780794
}
781795
} else if (!this.reported) {

‎lib/internal/test_runner/utils.js‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -199,6 +199,7 @@ function parseCommandLine() {
199199

200200
const isTestRunner = getOptionValue('--test');
201201
const coverage = getOptionValue('--experimental-test-coverage');
202+
const forceExit = getOptionValue('--test-force-exit');
202203
const sourceMaps = getOptionValue('--enable-source-maps');
203204
const isChildProcess = process.env.NODE_TEST_CONTEXT === 'child';
204205
const isChildProcessV8 = process.env.NODE_TEST_CONTEXT === 'child-v8';
@@ -251,6 +252,7 @@ function parseCommandLine() {
251252
__proto__: null,
252253
isTestRunner,
253254
coverage,
255+
forceExit,
254256
sourceMaps,
255257
testOnlyFlag,
256258
testNamePatterns,

‎src/node_options.cc‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -179,6 +179,9 @@ void EnvironmentOptions::CheckOptions(std::vector<std::string>* errors,
179179
} else if (force_repl) {
180180
errors->push_back("either --watch or --interactive "
181181
"can be used, not both");
182+
} else if (test_runner_force_exit) {
183+
errors->push_back("either --watch or --test-force-exit "
184+
"can be used, not both");
182185
} else if (!test_runner && (argv->size() < 1 || (*argv)[1].empty())) {
183186
errors->push_back("--watch requires specifying a file");
184187
}
@@ -611,6 +614,9 @@ EnvironmentOptionsParser::EnvironmentOptionsParser() {
611614
AddOption("--test-concurrency",
612615
"specify test runner concurrency",
613616
&EnvironmentOptions::test_runner_concurrency);
617+
AddOption("--test-force-exit",
618+
"force test runner to exit upon completion",
619+
&EnvironmentOptions::test_runner_force_exit);
614620
AddOption("--test-timeout",
615621
"specify test runner timeout",
616622
&EnvironmentOptions::test_runner_timeout);

‎src/node_options.h‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -165,6 +165,7 @@ class EnvironmentOptions : public Options {
165165
uint64_t test_runner_concurrency = 0;
166166
uint64_t test_runner_timeout = 0;
167167
bool test_runner_coverage = false;
168+
bool test_runner_force_exit = false;
168169
std::vector<std::string> test_name_pattern;
169170
std::vector<std::string> test_reporter;
170171
std::vector<std::string> test_reporter_destination;

‎test/fixtures/test-runner/output/force_exit.js‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
// Flags: --test-force-exit --test-reporter=spec
2+
'use strict';
3+
const { after, afterEach, before, beforeEach, test } = require('node:test');
4+
5+
before(() => {
6+
console.log('BEFORE');
7+
});
8+
9+
beforeEach(() => {
10+
console.log('BEFORE EACH');
11+
});
12+
13+
after(() => {
14+
console.log('AFTER');
15+
});
16+
17+
afterEach(() => {
18+
console.log('AFTER EACH');
19+
});
20+
21+
test('passes but oops', () => {
22+
setTimeout(() => {
23+
throw new Error('this should not have a chance to be thrown');
24+
}, 1000);
25+
});
26+
27+
test('also passes');

0 commit comments

Comments
 (0)