Skip to content

Commit a08c687

Browse files
MoLowMoLow
committed
test_runner: propagate only to test ancestors
1 parent a88ec28 commit a08c687

File tree

8 files changed

+139
-115
lines changed

8 files changed

+139
-115
lines changed

‎lib/internal/test_runner/test.js

+29-13
Original file line numberDiff line numberDiff line change
@@ -230,9 +230,8 @@ 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;
234+
this.runOnlySubtests = testOnlyFlag;
236235
this.testNumber = 0;
237236
this.timeout = kDefaultTimeout;
238237
this.root = this;
@@ -249,9 +248,8 @@ class Test extends AsyncResource {
249248

250249
this.concurrency = parent.concurrency;
251250
this.nesting = nesting;
252-
this.only = only ?? !parent.runOnlySubtests;
253251
this.reporter = parent.reporter;
254-
this.runOnlySubtests = !this.only;
252+
this.runOnlySubtests = false;
255253
this.testNumber = parent.subtests.length + 1;
256254
this.timeout = parent.timeout;
257255
this.root = parent.root;
@@ -296,10 +294,6 @@ class Test extends AsyncResource {
296294
skip = 'test name does not match pattern';
297295
}
298296

299-
if (testOnlyFlag && !this.only) {
300-
skip = '\'only\' option not set';
301-
}
302-
303297
if (skip) {
304298
fn = noop;
305299
}
@@ -338,10 +332,11 @@ class Test extends AsyncResource {
338332
this.subtests = [];
339333
this.waitingOn = 0;
340334
this.finished = false;
335+
this.only = testOnlyFlag || parent?.runOnlySubtests ? only : undefined;
341336

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

@@ -355,6 +350,18 @@ class Test extends AsyncResource {
355350
file: loc[2],
356351
};
357352
}
353+
354+
if (this.only && parent !== null) {
355+
parent.markOnly();
356+
}
357+
}
358+
359+
markOnly() {
360+
if (this.runOnlySubtests) {
361+
return;
362+
}
363+
this.runOnlySubtests = true;
364+
this.parent?.markOnly();
358365
}
359366

360367
matchesTestNamePatterns() {
@@ -580,9 +587,18 @@ class Test extends AsyncResource {
580587
}
581588
}
582589

590+
get runOnlySibling() {
591+
return this.parent?.runOnlySubtests && !this.only && !this.runOnlySubtests;
592+
}
593+
583594
async run() {
584595
this.startTime = hrtime();
585596

597+
if (this.runOnlySibling || this.only === false) {
598+
this.fn = noop;
599+
this.skip('\'only\' option not set');
600+
}
601+
586602
if (this[kShouldAbort]()) {
587603
this.postRun();
588604
return;
@@ -893,7 +909,6 @@ class Suite extends Test {
893909
this.fn = options.fn || this.fn;
894910
this.skipped = false;
895911
}
896-
this.runOnlySubtests = testOnlyFlag;
897912

898913
try {
899914
const { ctx, args } = this.getRunArgs();
@@ -915,7 +930,7 @@ class Suite extends Test {
915930

916931
this.buildPhaseFinished = true;
917932
}
918-
this.fn = () => {};
933+
this.fn = noop;
919934
}
920935

921936
getRunArgs() {
@@ -932,6 +947,7 @@ class Suite extends Test {
932947

933948
async run() {
934949
const hookArgs = this.getRunArgs();
950+
this.runOnlySubtests ||= this.runOnlySibling;
935951

936952
let stopPromise;
937953
try {

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

+57-61
Original file line numberDiff line numberDiff line change
@@ -1,100 +1,96 @@
11
// Flags: --test-only
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
@@ -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)