Skip to content

Commit 850ce7b

Browse files
committed
test_runner: propagate only to test ancestors
1 parent cdbbcdd commit 850ce7b

10 files changed

Lines changed: 142 additions & 120 deletions

File tree

lib/internal/test_runner/test.js

Lines changed: 28 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -222,9 +222,7 @@ class Test extends AsyncResource {
222222
if (parent === null) {
223223
this.concurrency = 1;
224224
this.nesting = 0;
225-
this.only = testOnlyFlag;
226225
this.reporter = new TestsStream();
227-
this.runOnlySubtests = this.only;
228226
this.testNumber = 0;
229227
this.timeout = kDefaultTimeout;
230228
this.root = this;
@@ -241,9 +239,7 @@ class Test extends AsyncResource {
241239

242240
this.concurrency = parent.concurrency;
243241
this.nesting = nesting;
244-
this.only = only ?? !parent.runOnlySubtests;
245242
this.reporter = parent.reporter;
246-
this.runOnlySubtests = !this.only;
247243
this.testNumber = parent.subtests.length + 1;
248244
this.timeout = parent.timeout;
249245
this.root = parent.root;
@@ -288,10 +284,6 @@ class Test extends AsyncResource {
288284
skip = 'test name does not match pattern';
289285
}
290286

291-
if (testOnlyFlag && !this.only) {
292-
skip = '\'only\' option not set';
293-
}
294-
295287
if (skip) {
296288
fn = noop;
297289
}
@@ -330,12 +322,26 @@ class Test extends AsyncResource {
330322
this.subtests = [];
331323
this.waitingOn = 0;
332324
this.finished = false;
325+
this.only = testOnlyFlag ? only : undefined;
326+
this.runOnlySubtests = false;
333327

334-
if (!testOnlyFlag && (only || this.runOnlySubtests)) {
335-
const warning =
336-
"'only' and 'runOnly' require the --test-only command-line option.";
328+
329+
if (!testOnlyFlag && only && !parent.runOnlySubtests) {
330+
const warning = "'only' requires the --test-only command-line option.";
337331
this.diagnostic(warning);
338332
}
333+
334+
if (this.only && parent !== null) {
335+
parent.markOnly();
336+
}
337+
}
338+
339+
markOnly() {
340+
if (this.runOnlySubtests) {
341+
return;
342+
}
343+
this.runOnlySubtests = true;
344+
this.parent?.markOnly();
339345
}
340346

341347
matchesTestNamePatterns() {
@@ -568,9 +574,18 @@ class Test extends AsyncResource {
568574
}
569575
}
570576

577+
get runOnlySibling() {
578+
return this.parent?.runOnlySubtests && !this.only && !this.runOnlySubtests;
579+
}
580+
571581
async run() {
572582
this.startTime = hrtime();
573583

584+
if (this.runOnlySibling || this.only === false) {
585+
this.fn = noop;
586+
this.skip('\'only\' option not set');
587+
}
588+
574589
if (this[kShouldAbort]()) {
575590
this.postRun();
576591
return;
@@ -843,7 +858,6 @@ class Suite extends Test {
843858
this.fn = options.fn || this.fn;
844859
this.skipped = false;
845860
}
846-
this.runOnlySubtests = testOnlyFlag;
847861

848862
try {
849863
const { ctx, args } = this.getRunArgs();
@@ -864,7 +878,7 @@ class Suite extends Test {
864878
this.fail(new ERR_TEST_FAILURE(err, kTestCodeFailure));
865879
this.buildPhaseFinished = this.parent !== null;
866880
}
867-
this.fn = () => {};
881+
this.fn = noop;
868882
}
869883

870884
getRunArgs() {
@@ -874,6 +888,7 @@ class Suite extends Test {
874888

875889
async run() {
876890
const hookArgs = this.getRunArgs();
891+
this.runOnlySubtests ||= this.runOnlySibling;
877892

878893
let stopPromise;
879894
try {

lib/internal/test_runner/utils.js

Lines changed: 1 addition & 1 deletion
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

Lines changed: 1 addition & 1 deletion
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');
Lines changed: 57 additions & 62 deletions
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

Lines changed: 28 additions & 4 deletions
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

Lines changed: 3 additions & 3 deletions
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

Lines changed: 6 additions & 9 deletions
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)