Skip to content

Commit 6d4f641

Browse files
MoLowMoLow
committed
test_runner: propogate only to test ancestors
1 parent f842b8b commit 6d4f641

10 files changed

+143
-120
lines changed

‎lib/internal/test_runner/test.js

+28-13
Original file line numberDiff line numberDiff line change
@@ -193,9 +193,7 @@ class Test extends AsyncResource {
193193
if (parent === null) {
194194
this.concurrency = 1;
195195
this.nesting = 0;
196-
this.only = testOnlyFlag;
197196
this.reporter = new TestsStream();
198-
this.runOnlySubtests = this.only;
199197
this.testNumber = 0;
200198
this.timeout = kDefaultTimeout;
201199
this.root = this;
@@ -212,9 +210,7 @@ class Test extends AsyncResource {
212210

213211
this.concurrency = parent.concurrency;
214212
this.nesting = nesting;
215-
this.only = only ?? !parent.runOnlySubtests;
216213
this.reporter = parent.reporter;
217-
this.runOnlySubtests = !this.only;
218214
this.testNumber = parent.subtests.length + 1;
219215
this.timeout = parent.timeout;
220216
this.root = parent.root;
@@ -259,10 +255,6 @@ class Test extends AsyncResource {
259255
skip = 'test name does not match pattern';
260256
}
261257

262-
if (testOnlyFlag && !this.only) {
263-
skip = '\'only\' option not set';
264-
}
265-
266258
if (skip) {
267259
fn = noop;
268260
}
@@ -301,12 +293,26 @@ class Test extends AsyncResource {
301293
this.subtests = [];
302294
this.waitingOn = 0;
303295
this.finished = false;
296+
this.only = testOnlyFlag ? only : undefined;
297+
this.runOnlySubtests = false;
304298

305-
if (!testOnlyFlag && (only || this.runOnlySubtests)) {
306-
const warning =
307-
"'only' and 'runOnly' require the --test-only command-line option.";
299+
300+
if (!testOnlyFlag && only && !parent.runOnlySubtests) {
301+
const warning = "'only' requires the --test-only command-line option.";
308302
this.diagnostic(warning);
309303
}
304+
305+
if (this.only && parent !== null) {
306+
parent.markOnly();
307+
}
308+
}
309+
310+
markOnly() {
311+
if (this.runOnlySubtests) {
312+
return;
313+
}
314+
this.runOnlySubtests = true;
315+
this.parent?.markOnly();
310316
}
311317

312318
matchesTestNamePatterns() {
@@ -539,9 +545,18 @@ class Test extends AsyncResource {
539545
}
540546
}
541547

548+
get runOnlySibling() {
549+
return this.parent?.runOnlySubtests && !this.only && !this.runOnlySubtests;
550+
}
551+
542552
async run() {
543553
this.startTime = hrtime();
544554

555+
if (this.runOnlySibling || this.only === false) {
556+
this.fn = noop;
557+
this.skip('\'only\' option not set');
558+
}
559+
545560
if (this[kShouldAbort]()) {
546561
this.postRun();
547562
return;
@@ -794,7 +809,6 @@ class Suite extends Test {
794809
this.fn = options.fn || this.fn;
795810
this.skipped = false;
796811
}
797-
this.runOnlySubtests = testOnlyFlag;
798812

799813
try {
800814
const { ctx, args } = this.getRunArgs();
@@ -815,7 +829,7 @@ class Suite extends Test {
815829
this.fail(new ERR_TEST_FAILURE(err, kTestCodeFailure));
816830
this.buildPhaseFinished = this.parent !== null;
817831
}
818-
this.fn = () => {};
832+
this.fn = noop;
819833
}
820834

821835
getRunArgs() {
@@ -825,6 +839,7 @@ class Suite extends Test {
825839

826840
async run() {
827841
const hookArgs = this.getRunArgs();
842+
this.runOnlySubtests ||= this.runOnlySibling;
828843

829844
try {
830845
await this.buildSuite;

‎lib/internal/test_runner/utils.js

+1-1
Original file line numberDiff line numberDiff line change
@@ -218,8 +218,8 @@ function parseCommandLine() {
218218
testOnlyFlag = false;
219219
testNamePatterns = null;
220220
} else {
221+
testOnlyFlag = getOptionValue('--test-only') || (!isChildProcess && !isChildProcessV8);
221222
const testNamePatternFlag = getOptionValue('--test-name-pattern');
222-
testOnlyFlag = getOptionValue('--test-only');
223223
testNamePatterns = testNamePatternFlag?.length > 0 ?
224224
ArrayPrototypeMap(
225225
testNamePatternFlag,

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

+1-1
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// Flags: --no-warnings --test-only --test-name-pattern=enabled
1+
// Flags: --no-warnings --test-name-pattern=enabled
22
'use strict';
33
const common = require('../../../common');
44
const { test } = require('node:test');

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

+58-62
Original file line numberDiff line numberDiff line change
@@ -1,100 +1,96 @@
1-
// Flags: --no-warnings --test-only
1+
// Flags: --no-warnings
22
'use strict';
3-
require('../../../common');
3+
const common = require('../../../common');
44
const { test, describe, it } = require('node:test');
55

66
// These tests should be skipped based on the 'only' option.
7-
test('only = undefined');
8-
test('only = undefined, skip = string', { skip: 'skip message' });
9-
test('only = undefined, skip = true', { skip: true });
10-
test('only = undefined, skip = false', { skip: false });
11-
test('only = false', { only: false });
12-
test('only = false, skip = string', { only: false, skip: 'skip message' });
13-
test('only = false, skip = true', { only: false, skip: true });
14-
test('only = false, skip = false', { only: false, skip: false });
7+
test('only = undefined', common.mustNotCall());
8+
test('only = undefined, skip = string', { skip: 'skip message' }, common.mustNotCall());
9+
test('only = undefined, skip = true', { skip: true }, common.mustNotCall());
10+
test('only = undefined, skip = false', { skip: false }, common.mustNotCall());
11+
test('only = false', { only: false }, common.mustNotCall());
12+
test('only = false, skip = string', { only: false, skip: 'skip message' }, common.mustNotCall());
13+
test('only = false, skip = true', { only: false, skip: true }, common.mustNotCall());
14+
test('only = false, skip = false', { only: false, skip: false }, common.mustNotCall());
1515

1616
// These tests should be skipped based on the 'skip' option.
17-
test('only = true, skip = string', { only: true, skip: 'skip message' });
18-
test('only = true, skip = true', { only: true, skip: true });
17+
test('only = true, skip = string', { only: true, skip: 'skip message' }, common.mustNotCall());
18+
test('only = true, skip = true', { only: true, skip: true }, common.mustNotCall());
1919

2020
// An 'only' test with subtests.
21-
test('only = true, with subtests', { only: true }, async (t) => {
21+
test('only = true, with subtests', { only: true }, common.mustCall(async (t) => {
2222
// These subtests should run.
23-
await t.test('running subtest 1');
24-
await t.test('running subtest 2');
23+
await t.test('running subtest 1', common.mustCall());
24+
await t.test('running subtest 2', common.mustCall());
2525

2626
// Switch the context to only execute 'only' tests.
2727
t.runOnly(true);
28-
await t.test('skipped subtest 1');
29-
await t.test('skipped subtest 2');
30-
await t.test('running subtest 3', { only: true });
28+
await t.test('skipped subtest 1', common.mustNotCall());
29+
await t.test('skipped subtest 2'), common.mustNotCall();
30+
await t.test('running subtest 3', { only: true }, common.mustCall());
3131

3232
// Switch the context back to execute all tests.
3333
t.runOnly(false);
34-
await t.test('running subtest 4', async (t) => {
34+
await t.test('running subtest 4', common.mustCall(async (t) => {
3535
// These subtests should run.
36-
await t.test('running sub-subtest 1');
37-
await t.test('running sub-subtest 2');
36+
await t.test('running sub-subtest 1', common.mustCall());
37+
await t.test('running sub-subtest 2', common.mustCall());
3838

3939
// Switch the context to only execute 'only' tests.
4040
t.runOnly(true);
41-
await t.test('skipped sub-subtest 1');
42-
await t.test('skipped sub-subtest 2');
43-
});
41+
await t.test('skipped sub-subtest 1', common.mustNotCall());
42+
await t.test('skipped sub-subtest 2', common.mustNotCall());
43+
}));
4444

4545
// Explicitly do not run these tests.
46-
await t.test('skipped subtest 3', { only: false });
47-
await t.test('skipped subtest 4', { skip: true });
48-
});
46+
await t.test('skipped subtest 3', { only: false }, common.mustNotCall());
47+
await t.test('skipped subtest 4', { skip: true }, common.mustNotCall());
48+
}));
4949

50-
describe.only('describe only = true, with subtests', () => {
51-
it.only('`it` subtest 1 should run', () => {});
50+
describe.only('describe only = true, with subtests', common.mustCall(() => {
51+
it.only('`it` subtest 1 should run', common.mustCall());
5252

53-
it('`it` subtest 2 should not run', async () => {});
54-
});
53+
it('`it` subtest 2 should not run', common.mustNotCall());
54+
}));
5555

56-
describe.only('describe only = true, with a mixture of subtests', () => {
57-
it.only('`it` subtest 1', () => {});
56+
describe.only('describe only = true, with a mixture of subtests', common.mustCall(() => {
57+
it.only('`it` subtest 1', common.mustCall());
5858

59-
it.only('`it` async subtest 1', async () => {});
59+
it.only('`it` async subtest 1', common.mustCall(async () => {}));
6060

61-
it('`it` subtest 2 only=true', { only: true });
61+
it('`it` subtest 2 only=true', { only: true }, common.mustCall());
6262

63-
it('`it` subtest 2 only=false', { only: false }, () => {
64-
throw new Error('This should not run');
65-
});
63+
it('`it` subtest 2 only=false', { only: false }, common.mustNotCall());
6664

67-
it.skip('`it` subtest 3 skip', () => {
68-
throw new Error('This should not run');
69-
});
65+
it.skip('`it` subtest 3 skip', common.mustNotCall());
7066

71-
it.todo('`it` subtest 4 todo', { only: false }, () => {
72-
throw new Error('This should not run');
73-
});
67+
it.todo('`it` subtest 4 todo', { only: false }, common.mustNotCall());
7468

75-
test.only('`test` subtest 1', () => {});
69+
test.only('`test` subtest 1', common.mustCall());
7670

77-
test.only('`test` async subtest 1', async () => {});
71+
test.only('`test` async subtest 1', common.mustCall(async () => {}));
7872

79-
test('`test` subtest 2 only=true', { only: true });
73+
test('`test` subtest 2 only=true', { only: true }, common.mustCall());
8074

81-
test('`test` subtest 2 only=false', { only: false }, () => {
82-
throw new Error('This should not run');
83-
});
75+
test('`test` subtest 2 only=false', { only: false }, common.mustNotCall());
8476

85-
test.skip('`test` subtest 3 skip', () => {
86-
throw new Error('This should not run');
87-
});
77+
test.skip('`test` subtest 3 skip', common.mustNotCall());
8878

89-
test.todo('`test` subtest 4 todo', { only: false }, () => {
90-
throw new Error('This should not run');
91-
});
92-
});
79+
test.todo('`test` subtest 4 todo', { only: false }, common.mustNotCall());
80+
}));
9381

94-
describe.only('describe only = true, with subtests', () => {
95-
test.only('subtest should run', () => {});
82+
describe.only('describe only = true, with subtests', common.mustCall(() => {
83+
test.only('subtest should run', common.mustCall());
9684

97-
test('async subtest should not run', async () => {});
85+
test('async subtest should not run', common.mustNotCall());
9886

99-
test('subtest should be skipped', { only: false }, () => {});
100-
});
87+
test('subtest should be skipped', { only: false }, common.mustNotCall());
88+
}));
89+
90+
describe('describe only = undefined, with subtests', common.mustCall(() => {
91+
test('async subtest should not run', common.mustNotCall());
92+
}));
93+
94+
describe('describe only = false, with subtests', { only: false }, common.mustCall(() => {
95+
test('async subtest should not run', common.mustNotCall());
96+
}));

‎test/fixtures/test-runner/output/only_tests.snapshot

+28-4
Original file line numberDiff line numberDiff line change
@@ -222,12 +222,36 @@ ok 14 - describe only = true, with subtests
222222
duration_ms: *
223223
type: 'suite'
224224
...
225-
1..14
226-
# tests 40
227-
# suites 3
225+
# Subtest: describe only = undefined, with subtests
226+
# Subtest: async subtest should not run
227+
ok 1 - async subtest should not run # SKIP 'only' option not set
228+
---
229+
duration_ms: *
230+
...
231+
1..1
232+
ok 15 - describe only = undefined, with subtests
233+
---
234+
duration_ms: *
235+
type: 'suite'
236+
...
237+
# Subtest: describe only = false, with subtests
238+
# Subtest: async subtest should not run
239+
ok 1 - async subtest should not run # SKIP 'only' option not set
240+
---
241+
duration_ms: *
242+
...
243+
1..1
244+
ok 16 - describe only = false, with subtests
245+
---
246+
duration_ms: *
247+
type: 'suite'
248+
...
249+
1..16
250+
# tests 42
251+
# suites 5
228252
# pass 15
229253
# fail 0
230254
# cancelled 0
231-
# skipped 25
255+
# skipped 27
232256
# todo 0
233257
# duration_ms *

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

+3-3
Original file line numberDiff line numberDiff line change
@@ -288,11 +288,11 @@ test('callback async throw after done', (t, done) => {
288288
done();
289289
});
290290

291-
test('only is set but not in only mode', { only: true }, async (t) => {
292-
// All of these subtests should run.
291+
test('runOnly is set', async (t) => {
292+
// Subtests should run only outside of a runOnly block, unless they have only: true.
293293
await t.test('running subtest 1');
294294
t.runOnly(true);
295-
await t.test('running subtest 2');
295+
await t.test('skipped subtest 2');
296296
await t.test('running subtest 3', { only: true });
297297
t.runOnly(false);
298298
await t.test('running subtest 4');

‎test/fixtures/test-runner/output/output.snapshot

+6-9
Original file line numberDiff line numberDiff line change
@@ -501,35 +501,32 @@ ok 52 - callback async throw after done
501501
---
502502
duration_ms: *
503503
...
504-
# Subtest: only is set but not in only mode
504+
# Subtest: runOnly is set
505505
# Subtest: running subtest 1
506506
ok 1 - running subtest 1
507507
---
508508
duration_ms: *
509509
...
510-
# Subtest: running subtest 2
511-
ok 2 - running subtest 2
510+
# Subtest: skipped subtest 2
511+
ok 2 - skipped subtest 2 # SKIP 'only' option not set
512512
---
513513
duration_ms: *
514514
...
515-
# 'only' and 'runOnly' require the --test-only command-line option.
516515
# Subtest: running subtest 3
517516
ok 3 - running subtest 3
518517
---
519518
duration_ms: *
520519
...
521-
# 'only' and 'runOnly' require the --test-only command-line option.
522520
# Subtest: running subtest 4
523521
ok 4 - running subtest 4
524522
---
525523
duration_ms: *
526524
...
527525
1..4
528-
ok 53 - only is set but not in only mode
526+
ok 53 - runOnly is set
529527
---
530528
duration_ms: *
531529
...
532-
# 'only' and 'runOnly' require the --test-only command-line option.
533530
# Subtest: custom inspect symbol fail
534531
not ok 54 - custom inspect symbol fail
535532
---
@@ -723,9 +720,9 @@ not ok 66 - invalid subtest fail
723720
# Warning: Test "callback async throw after done" generated asynchronous activity after the test ended. This activity created the error "Error: thrown from callback async throw after done" and would have caused the test to fail, but instead triggered an uncaughtException event.
724721
# tests 80
725722
# suites 0
726-
# pass 37
723+
# pass 36
727724
# fail 25
728725
# cancelled 3
729-
# skipped 10
726+
# skipped 11
730727
# todo 5
731728
# duration_ms *

0 commit comments

Comments
 (0)