Skip to content

Commit c818aab

Browse files
MoLowMoLow
committed
test_runner: propagate only to test ancestors
1 parent 544b845 commit c818aab

10 files changed

+142
-120
lines changed

‎lib/internal/test_runner/test.js

+28-13
Original file line numberDiff line numberDiff line change
@@ -230,9 +230,7 @@ class Test extends AsyncResource {
230230
if (parent === null) {
231231
this.concurrency = 1;
232232
this.nesting = 0;
233-
this.only = testOnlyFlag;
234233
this.reporter = new TestsStream();
235-
this.runOnlySubtests = this.only;
236234
this.testNumber = 0;
237235
this.timeout = kDefaultTimeout;
238236
this.root = this;
@@ -249,9 +247,7 @@ class Test extends AsyncResource {
249247

250248
this.concurrency = parent.concurrency;
251249
this.nesting = nesting;
252-
this.only = only ?? !parent.runOnlySubtests;
253250
this.reporter = parent.reporter;
254-
this.runOnlySubtests = !this.only;
255251
this.testNumber = parent.subtests.length + 1;
256252
this.timeout = parent.timeout;
257253
this.root = parent.root;
@@ -296,10 +292,6 @@ class Test extends AsyncResource {
296292
skip = 'test name does not match pattern';
297293
}
298294

299-
if (testOnlyFlag && !this.only) {
300-
skip = '\'only\' option not set';
301-
}
302-
303295
if (skip) {
304296
fn = noop;
305297
}
@@ -338,10 +330,12 @@ class Test extends AsyncResource {
338330
this.subtests = [];
339331
this.waitingOn = 0;
340332
this.finished = false;
333+
this.only = testOnlyFlag ? only : undefined;
334+
this.runOnlySubtests = false;
341335

342-
if (!testOnlyFlag && (only || this.runOnlySubtests)) {
343-
const warning =
344-
"'only' and 'runOnly' require the --test-only command-line option.";
336+
337+
if (!testOnlyFlag && only && !parent.runOnlySubtests) {
338+
const warning = "'only' requires the --test-only command-line option.";
345339
this.diagnostic(warning);
346340
}
347341

@@ -355,6 +349,18 @@ class Test extends AsyncResource {
355349
file: loc[2],
356350
};
357351
}
352+
353+
if (this.only && parent !== null) {
354+
parent.markOnly();
355+
}
356+
}
357+
358+
markOnly() {
359+
if (this.runOnlySubtests) {
360+
return;
361+
}
362+
this.runOnlySubtests = true;
363+
this.parent?.markOnly();
358364
}
359365

360366
matchesTestNamePatterns() {
@@ -587,9 +593,18 @@ class Test extends AsyncResource {
587593
}
588594
}
589595

596+
get runOnlySibling() {
597+
return this.parent?.runOnlySubtests && !this.only && !this.runOnlySubtests;
598+
}
599+
590600
async run() {
591601
this.startTime = hrtime();
592602

603+
if (this.runOnlySibling || this.only === false) {
604+
this.fn = noop;
605+
this.skip('\'only\' option not set');
606+
}
607+
593608
if (this[kShouldAbort]()) {
594609
this.postRun();
595610
return;
@@ -900,7 +915,6 @@ class Suite extends Test {
900915
this.fn = options.fn || this.fn;
901916
this.skipped = false;
902917
}
903-
this.runOnlySubtests = testOnlyFlag;
904918

905919
try {
906920
const { ctx, args } = this.getRunArgs();
@@ -921,7 +935,7 @@ class Suite extends Test {
921935
this.fail(new ERR_TEST_FAILURE(err, kTestCodeFailure));
922936
this.buildPhaseFinished = this.parent !== null;
923937
}
924-
this.fn = () => {};
938+
this.fn = noop;
925939
}
926940

927941
getRunArgs() {
@@ -931,6 +945,7 @@ class Suite extends Test {
931945

932946
async run() {
933947
const hookArgs = this.getRunArgs();
948+
this.runOnlySubtests ||= this.runOnlySibling;
934949

935950
let stopPromise;
936951
try {

‎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: --test-only --test-name-pattern=enabled
1+
// Flags: --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

+57-62
Original file line numberDiff line numberDiff line change
@@ -1,100 +1,95 @@
1-
// Flags: --test-only
21
'use strict';
3-
require('../../../common');
2+
const common = require('../../../common');
43
const { test, describe, it } = require('node:test');
54

65
// 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 });
6+
test('only = undefined', common.mustNotCall());
7+
test('only = undefined, skip = string', { skip: 'skip message' }, common.mustNotCall());
8+
test('only = undefined, skip = true', { skip: true }, common.mustNotCall());
9+
test('only = undefined, skip = false', { skip: false }, common.mustNotCall());
10+
test('only = false', { only: false }, common.mustNotCall());
11+
test('only = false, skip = string', { only: false, skip: 'skip message' }, common.mustNotCall());
12+
test('only = false, skip = true', { only: false, skip: true }, common.mustNotCall());
13+
test('only = false, skip = false', { only: false, skip: false }, common.mustNotCall());
1514

1615
// 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 });
16+
test('only = true, skip = string', { only: true, skip: 'skip message' }, common.mustNotCall());
17+
test('only = true, skip = true', { only: true, skip: true }, common.mustNotCall());
1918

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

2625
// Switch the context to only execute 'only' tests.
2726
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 });
27+
await t.test('skipped subtest 1', common.mustNotCall());
28+
await t.test('skipped subtest 2'), common.mustNotCall();
29+
await t.test('running subtest 3', { only: true }, common.mustCall());
3130

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

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

4544
// 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-
});
45+
await t.test('skipped subtest 3', { only: false }, common.mustNotCall());
46+
await t.test('skipped subtest 4', { skip: true }, common.mustNotCall());
47+
}));
4948

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

‎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
@@ -274,11 +274,11 @@ test('callback async throw after done', (t, done) => {
274274
done();
275275
});
276276

277-
test('only is set but not in only mode', { only: true }, async (t) => {
278-
// All of these subtests should run.
277+
test('runOnly is set', async (t) => {
278+
// Subtests should run only outside of a runOnly block, unless they have only: true.
279279
await t.test('running subtest 1');
280280
t.runOnly(true);
281-
await t.test('running subtest 2');
281+
await t.test('skipped subtest 2');
282282
await t.test('running subtest 3', { only: true });
283283
t.runOnly(false);
284284
await t.test('running subtest 4');

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

+6-9
Original file line numberDiff line numberDiff line change
@@ -499,35 +499,32 @@ ok 48 - callback async throw after done
499499
---
500500
duration_ms: *
501501
...
502-
# Subtest: only is set but not in only mode
502+
# Subtest: runOnly is set
503503
# Subtest: running subtest 1
504504
ok 1 - running subtest 1
505505
---
506506
duration_ms: *
507507
...
508-
# Subtest: running subtest 2
509-
ok 2 - running subtest 2
508+
# Subtest: skipped subtest 2
509+
ok 2 - skipped subtest 2 # SKIP 'only' option not set
510510
---
511511
duration_ms: *
512512
...
513-
# 'only' and 'runOnly' require the --test-only command-line option.
514513
# Subtest: running subtest 3
515514
ok 3 - running subtest 3
516515
---
517516
duration_ms: *
518517
...
519-
# 'only' and 'runOnly' require the --test-only command-line option.
520518
# Subtest: running subtest 4
521519
ok 4 - running subtest 4
522520
---
523521
duration_ms: *
524522
...
525523
1..4
526-
ok 49 - only is set but not in only mode
524+
ok 49 - runOnly is set
527525
---
528526
duration_ms: *
529527
...
530-
# 'only' and 'runOnly' require the --test-only command-line option.
531528
# Subtest: custom inspect symbol fail
532529
not ok 50 - custom inspect symbol fail
533530
---
@@ -733,9 +730,9 @@ not ok 62 - invalid subtest fail
733730
# 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.
734731
# tests 76
735732
# suites 0
736-
# pass 35
733+
# pass 34
737734
# fail 25
738735
# cancelled 3
739-
# skipped 9
736+
# skipped 10
740737
# todo 4
741738
# duration_ms *

0 commit comments

Comments
 (0)