Skip to content

Commit 1cff51c

Browse files
test_runner: add option to forbid exclusive tests
Add --test-forbid-only and the forbidOnly run() option to detect accidentally committed exclusive tests. Reject only declarations and runOnly(true) in both isolation modes, and record a run-level failure so TODO and expected failures cannot absorb the error. Document the runtime-only detection and add regression coverage for filtering, option inheritance, both isolation modes, and disabled guards. Refs: #60917 Assisted-by: a coding agent Signed-off-by: João Victor <joao.oliveira@softtor.com.br>
1 parent cd18965 commit 1cff51c

9 files changed

Lines changed: 227 additions & 0 deletions

File tree

‎doc/api/cli.md‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3222,6 +3222,22 @@ added: v22.8.0
32223222
Require a minimum percent of covered lines. If code coverage does not reach
32233223
the threshold specified, the process will exit with code `1`.
32243224

3225+
### `--test-forbid-only`
3226+
3227+
<!-- YAML
3228+
added: REPLACEME
3229+
-->
3230+
3231+
Fails the test run if a test or suite is declared with `only: true`, including
3232+
the `test.only()`, `it.only()`, `suite.only()`, and `describe.only()` shorthands,
3233+
or if `testContext.runOnly(true)` is called. This is useful in CI to prevent
3234+
accidentally committing exclusive tests that exclude the rest of a test suite.
3235+
3236+
This option applies with either test isolation mode and takes precedence over
3237+
`--test-only`. Name filtering, TODO markers, and expected failures do not suppress
3238+
detected violations. Only declarations evaluated during the run are checked;
3239+
code inside a skipped test callback is not evaluated.
3240+
32253241
### `--test-force-exit`
32263242

32273243
<!-- YAML
@@ -4339,6 +4355,7 @@ one is included in the list below.
43394355
* `--test-coverage-include-all`
43404356
* `--test-coverage-include`
43414357
* `--test-coverage-lines`
4358+
* `--test-forbid-only`
43424359
* `--test-global-setup`
43434360
* `--test-isolation`
43444361
* `--test-name-pattern`

‎doc/api/test.md‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -353,6 +353,14 @@ it.todo('should do the thing', { expectFailure: true }, () => {
353353

354354
## `only` tests
355355

356+
To prevent exclusive tests from accidentally excluding tests in CI, use
357+
[`--test-forbid-only`][] or the `forbidOnly` option of [`run()`][]. When enabled,
358+
declaring a test or suite with `only: true` or calling `testContext.runOnly(true)`
359+
fails the run. This includes the `.only()` shorthands and applies with either
360+
isolation mode, even when `--test-only` is also enabled. Name filtering, TODO
361+
markers, and expected failures cannot suppress detected violations. Declarations
362+
inside callbacks that are never executed are not checked.
363+
356364
If Node.js is started with the [`--test-only`][] command-line option, or test
357365
isolation is disabled, it is possible to skip all tests except for a selected
358366
subset by passing the `only` option to the tests that should run. When a test
@@ -1760,6 +1768,9 @@ changes:
17601768
* `forceExit` {boolean} Configures the test runner to exit the process once
17611769
all known tests have finished executing even if the event loop would
17621770
otherwise remain active. **Default:** `false`.
1771+
* `forbidOnly` {boolean} Fail the run when a test or suite is declared with
1772+
`only: true` or `testContext.runOnly(true)` is called. See [`only` tests](#only-tests).
1773+
**Default:** the value of [`--test-forbid-only`][].
17631774
* `globPatterns` {Array} An array containing the list of glob patterns to
17641775
match test files. This option cannot be used together with `files`.
17651776
**Default:** Same as [running tests from the command line][].
@@ -5105,6 +5116,7 @@ test.describe('my suite', (suite) => {
51055116
[`--test-concurrency`]: cli.md#--test-concurrency
51065117
[`--test-coverage-exclude`]: cli.md#--test-coverage-exclude
51075118
[`--test-coverage-include`]: cli.md#--test-coverage-include
5119+
[`--test-forbid-only`]: cli.md#--test-forbid-only
51085120
[`--test-name-pattern`]: cli.md#--test-name-pattern
51095121
[`--test-only`]: cli.md#--test-only
51105122
[`--test-reporter-destination`]: cli.md#--test-reporter-destination

‎doc/node.1‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1609,6 +1609,16 @@ filtering as the rest of the report.
16091609
Require a minimum percent of covered lines. If code coverage does not reach
16101610
the threshold specified, the process will exit with code \fB1\fR.
16111611
.
1612+
.It Fl -test-forbid-only
1613+
Fails the test run if a test or suite is declared with \fBonly: true\fR, including
1614+
the \fBtest.only()\fR, \fBit.only()\fR, \fBsuite.only()\fR, and \fBdescribe.only()\fR shorthands,
1615+
or if \fBtestContext.runOnly(true)\fR is called. This is useful in CI to prevent
1616+
accidentally committing exclusive tests that exclude the rest of a test suite.
1617+
This option applies with either test isolation mode and takes precedence over
1618+
\fB--test-only\fR. Name filtering, TODO markers, and expected failures do not suppress
1619+
detected violations. Only declarations evaluated during the run are checked;
1620+
code inside a skipped test callback is not evaluated.
1621+
.
16121622
.It Fl -test-force-exit
16131623
Configures the test runner to exit the process once all known tests have
16141624
finished executing even if the event loop would otherwise remain active.
@@ -2413,6 +2423,8 @@ one is included in the list below.
24132423
.It
24142424
\fB--test-coverage-lines\fR
24152425
.It
2426+
\fB--test-forbid-only\fR
2427+
.It
24162428
\fB--test-global-setup\fR
24172429
.It
24182430
\fB--test-isolation\fR

‎lib/internal/test_runner/runner.js‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -206,6 +206,7 @@ function getRunArgs(path, { forceExit,
206206
testSkipPatterns,
207207
testTagFilterExpressions,
208208
only,
209+
forbidOnly,
209210
hasFiles,
210211
testFiles,
211212
globPatterns,
@@ -252,6 +253,9 @@ function getRunArgs(path, { forceExit,
252253
if (only === true) {
253254
ArrayPrototypePush(runArgs, '--test-only');
254255
}
256+
if (forbidOnly != null && forbidOnly !== getOptionValue('--test-forbid-only')) {
257+
ArrayPrototypePush(runArgs, forbidOnly ? '--test-forbid-only' : '--no-test-forbid-only');
258+
}
255259
if (timeout != null) {
256260
ArrayPrototypePush(runArgs, `--test-timeout=${timeout}`);
257261
}
@@ -788,6 +792,7 @@ function run(options = kEmptyObject) {
788792
globalSetupPath,
789793
only,
790794
globPatterns,
795+
forbidOnly,
791796
coverage = false,
792797
coverageIncludeAll = false,
793798
lineCoverage = 0,
@@ -820,6 +825,9 @@ function run(options = kEmptyObject) {
820825
if (only != null) {
821826
validateBoolean(only, 'options.only');
822827
}
828+
if (forbidOnly != null) {
829+
validateBoolean(forbidOnly, 'options.forbidOnly');
830+
}
823831
if (globPatterns != null) {
824832
validateArray(globPatterns, 'options.globPatterns');
825833
}
@@ -1020,6 +1028,10 @@ function run(options = kEmptyObject) {
10201028
testTagFilters,
10211029
};
10221030

1031+
if (forbidOnly != null) {
1032+
globalOptions.forbidOnly = forbidOnly;
1033+
}
1034+
10231035
if (isolation === 'none') {
10241036
if (testNamePatterns != null) {
10251037
globalOptions.testNamePatterns = testNamePatterns;
@@ -1066,6 +1078,7 @@ function run(options = kEmptyObject) {
10661078
globPatterns,
10671079
only,
10681080
forceExit,
1081+
forbidOnly,
10691082
cwd,
10701083
isolation,
10711084
argv,

‎lib/internal/test_runner/test.js‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -370,6 +370,9 @@ class TestContext {
370370
}
371371

372372
runOnly(value) {
373+
if (value && this.#test.config.forbidOnly) {
374+
failForbiddenOnly(this.#test.root, 'runOnly() is not allowed when forbidOnly is enabled');
375+
}
373376
this.#test.runOnlySubtests = !!value;
374377
}
375378

@@ -568,6 +571,14 @@ function parseExpectFailure(expectFailure) {
568571
return { __proto__: null, label: undefined, match: expectFailure };
569572
}
570573

574+
function failForbiddenOnly(root, message) {
575+
// Filtering, TODO, and expected failures must not hide forbidden exclusivity.
576+
root.harness.success = false;
577+
root.diagnostic(message);
578+
process.exitCode = kGenericUserError;
579+
throw new ERR_TEST_FAILURE(message, kTestCodeFailure);
580+
}
581+
571582
class Test extends AsyncResource {
572583
reportedType = 'test';
573584
abortController;
@@ -634,6 +645,10 @@ class Test extends AsyncResource {
634645
parent.nesting + 1;
635646
const { config, isFilteringByName, isFilteringByOnly, isFilteringByTags } = parent.root.harness;
636647

648+
if (only && config.forbidOnly) {
649+
failForbiddenOnly(parent.root, 'The only option is not allowed when forbidOnly is enabled');
650+
}
651+
637652
this.root = parent.root;
638653
this.harness = null;
639654
this.config = config;

‎lib/internal/test_runner/utils.js‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -268,6 +268,7 @@ function parseCommandLine() {
268268
let destinations;
269269
let isolation;
270270
let only = getOptionValue('--test-only');
271+
const forbidOnly = getOptionValue('--test-forbid-only');
271272
let reporters;
272273
let shard;
273274
let testNamePatterns = mapPatternFlagToRegExArray('--test-name-pattern');
@@ -422,6 +423,7 @@ function parseCommandLine() {
422423
functionCoverage,
423424
lineCoverage,
424425
only,
426+
forbidOnly,
425427
reporters,
426428
setup,
427429
globalSetupPath,

‎src/node_options.cc‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1266,6 +1266,12 @@ EnvironmentOptionsParser::EnvironmentOptionsParser() {
12661266
kAllowedInEnvvar,
12671267
false,
12681268
OptionNamespaces::kTestRunnerNamespace);
1269+
AddOption("--test-forbid-only",
1270+
"fail tests that use the 'only' option",
1271+
BOOL_FIELD(test_forbid_only),
1272+
kAllowedInEnvvar,
1273+
false,
1274+
OptionNamespaces::kTestRunnerNamespace);
12691275
AddOption("--test-shard",
12701276
"run test at specific shard",
12711277
&EnvironmentOptions::test_shard,

‎src/node_options.h‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -265,6 +265,7 @@ class EnvironmentOptions : public Options {
265265
DEFINE_BOOL_FIELD(test_randomize) = false;
266266
DEFINE_BOOL_FIELD(has_test_random_seed) = false;
267267
DEFINE_BOOL_FIELD(test_only) = false;
268+
DEFINE_BOOL_FIELD(test_forbid_only) = false;
268269
DEFINE_BOOL_FIELD(test_udp_no_try_send) = false;
269270
DEFINE_BOOL_FIELD(coverage_include_all) = false;
270271
DEFINE_BOOL_FIELD(throw_deprecation) = false;
Lines changed: 149 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,149 @@
1+
'use strict';
2+
const common = require('../common');
3+
const assert = require('node:assert');
4+
const { writeFileSync } = require('node:fs');
5+
const { test, run } = require('node:test');
6+
const tmpdir = require('../common/tmpdir');
7+
8+
tmpdir.refresh();
9+
const fixture = tmpdir.resolve('only.test.js');
10+
writeFileSync(fixture, `
11+
const { test } = require('node:test');
12+
test.only('focused test', () => {});
13+
test('ordinary test', () => {});
14+
`);
15+
const env = { ...process.env, NODE_OPTIONS: '', FORCE_COLOR: '0' };
16+
delete env.NODE_TEST_CONTEXT;
17+
18+
function createFixture(name, source) {
19+
const file = tmpdir.resolve(`${name}.js`);
20+
writeFileSync(file, `
21+
const { test, it, suite, describe } = require('node:test');
22+
${source}
23+
`);
24+
return file;
25+
}
26+
27+
function assertForbidden({ code, signal, stdout, stderr }) {
28+
assert.strictEqual(signal, null);
29+
assert.strictEqual(code, 1, stdout + stderr);
30+
assert.match(stdout + stderr, /(?:only|runOnly).*not allowed/);
31+
assert.doesNotMatch(stdout, /BODY_EXECUTED/);
32+
}
33+
34+
const cases = {
35+
'test-only': "test.only('focused test', () => { console.log('BODY_EXECUTED'); });",
36+
'it-only': "it.only('focused test', () => { console.log('BODY_EXECUTED'); });",
37+
'suite-only': "suite.only('focused suite', () => { console.log('BODY_EXECUTED'); });",
38+
'describe-only': "describe.only('focused suite', () => { console.log('BODY_EXECUTED'); });",
39+
'only-option': "test('focused test', { only: true }, () => { console.log('BODY_EXECUTED'); });",
40+
'suite-option': "suite('focused suite', { only: true }, () => { console.log('BODY_EXECUTED'); });",
41+
'subtest-option': "test('parent', async (t) => { await t.test('child', { only: true }, () => {}); });",
42+
'run-only': "test('parent', (t) => { t.runOnly(true); console.log('BODY_EXECUTED'); });",
43+
'skipped-only': "test('focused test', { only: true, skip: true }, () => {});",
44+
'todo-only': "test('focused test', { only: true, todo: true }, () => {});",
45+
'expected-failure-only': "test('focused test', { only: true, expectFailure: true }, () => {});",
46+
'todo-parent': "test.todo('parent', () => { test.only('child', () => {}); });",
47+
'expected-failure-parent': "test('parent', { expectFailure: true }, (t) => { t.runOnly(true); });",
48+
};
49+
const caseFiles = Object.entries(cases).map(([name, source]) => [name, createFixture(name, source)]);
50+
51+
for (const isolation of ['process', 'none']) {
52+
test(`run() rejects only with isolation=${isolation}`, async () => {
53+
const runner = tmpdir.resolve(`runner-${isolation}.js`);
54+
writeFileSync(runner, `
55+
const { run } = require('node:test');
56+
const { tap } = require('node:test/reporters');
57+
run({
58+
files: [${JSON.stringify(fixture)}],
59+
isolation: ${JSON.stringify(isolation)},
60+
forbidOnly: true,
61+
}).on('test:fail', () => { process.exitCode = 1; }).compose(tap).pipe(process.stdout);
62+
`);
63+
const { code, signal, stdout, stderr } = await common.spawnPromisified(process.execPath, [runner], { env });
64+
assertForbidden({ code, signal, stdout, stderr });
65+
});
66+
67+
for (const [name, file] of caseFiles) {
68+
test(`CLI rejects ${name} with isolation=${isolation}`, async () => {
69+
assertForbidden(await common.spawnPromisified(process.execPath, [
70+
'--test', `--test-isolation=${isolation}`, '--test-reporter=tap', '--test-forbid-only', file,
71+
], { env }));
72+
});
73+
}
74+
75+
for (const args of [['--test-only'], ['--test-name-pattern=ordinary'], ['--test-skip-pattern=focused']]) {
76+
test(`filter ${args[0]} cannot hide only with isolation=${isolation}`, async () => {
77+
assertForbidden(await common.spawnPromisified(process.execPath, [
78+
'--test', `--test-isolation=${isolation}`, '--test-reporter=tap', '--test-forbid-only', ...args, fixture,
79+
], { env }));
80+
});
81+
}
82+
83+
test(`NODE_OPTIONS enables forbidOnly with isolation=${isolation}`, async () => {
84+
assertForbidden(await common.spawnPromisified(process.execPath, [
85+
'--test', `--test-isolation=${isolation}`, '--test-reporter=tap', fixture,
86+
], { env: { ...env, NODE_OPTIONS: '--test-forbid-only' } }));
87+
});
88+
89+
test(`filtered suites cannot hide only with isolation=${isolation}`, async () => {
90+
const file = createFixture(`filtered-${isolation}`, `
91+
suite('filtered', () => { it.only('child', () => {}); });
92+
test('ordinary', () => {});
93+
`);
94+
assertForbidden(await common.spawnPromisified(process.execPath, [
95+
'--test', `--test-isolation=${isolation}`, '--test-reporter=tap',
96+
'--test-forbid-only', '--test-name-pattern=ordinary', file,
97+
], { env }));
98+
});
99+
100+
test(`run() can override the CLI with forbidOnly=false and isolation=${isolation}`, async () => {
101+
const runner = createFixture(`override-${isolation}`, `
102+
const { run } = require('node:test');
103+
const { tap } = require('node:test/reporters');
104+
run({ files: [${JSON.stringify(fixture)}], isolation: ${JSON.stringify(isolation)}, forbidOnly: false })
105+
.on('test:fail', () => { process.exitCode = 1; }).compose(tap).pipe(process.stdout);
106+
`);
107+
const { code, signal, stdout, stderr } = await common.spawnPromisified(process.execPath, [
108+
runner,
109+
], { env: { ...env, NODE_OPTIONS: '--test-forbid-only' } });
110+
assert.strictEqual(signal, null);
111+
assert.strictEqual(code, 0, stdout + stderr);
112+
assert.match(stdout, /ok 1 - focused test/);
113+
});
114+
115+
test(`ordinary tests still run with isolation=${isolation}`, async () => {
116+
const file = createFixture(`ordinary-${isolation}`, `
117+
test('ordinary', () => {});
118+
test('explicit false', { only: false }, () => {});
119+
test('runOnly false', (t) => { t.runOnly(false); });
120+
`);
121+
const { code, signal, stdout, stderr } = await common.spawnPromisified(process.execPath, [
122+
'--test', `--test-isolation=${isolation}`, '--test-reporter=tap', '--test-forbid-only', file,
123+
], { env });
124+
assert.strictEqual(signal, null);
125+
assert.strictEqual(code, 0, stdout + stderr);
126+
// Explicit only:false is filtered out in the existing none-isolation mode.
127+
assert.match(stdout, isolation === 'none' ? /# pass 2/ : /# pass 3/);
128+
});
129+
130+
for (const flags of [[], ['--test-forbid-only', '--no-test-forbid-only']]) {
131+
test(`only still filters when disabled (${flags}) with isolation=${isolation}`, async () => {
132+
const { code, signal, stdout, stderr } = await common.spawnPromisified(process.execPath, [
133+
'--test', `--test-isolation=${isolation}`, '--test-reporter=tap', '--test-only', ...flags, fixture,
134+
], { env });
135+
assert.strictEqual(signal, null);
136+
assert.strictEqual(code, 0, stdout + stderr);
137+
assert.match(stdout, /# pass 1/);
138+
assert.doesNotMatch(stdout, /ordinary test/);
139+
});
140+
}
141+
}
142+
143+
test('run() validates forbidOnly', () => {
144+
for (const forbidOnly of [0, 1, '', 'true', {}, []]) {
145+
assert.throws(() => run({ forbidOnly }), {
146+
code: 'ERR_INVALID_ARG_TYPE',
147+
});
148+
}
149+
});

0 commit comments

Comments
 (0)