From 6d4f6411d631f031002968e8cac78bf27a297dbb Mon Sep 17 00:00:00 2001 From: Moshe Atlow Date: Wed, 26 Jul 2023 12:51:22 +0300 Subject: [PATCH] test_runner: propogate only to test ancestors --- lib/internal/test_runner/test.js | 41 ++++-- lib/internal/test_runner/utils.js | 2 +- .../output/name_pattern_with_only.js | 2 +- .../fixtures/test-runner/output/only_tests.js | 120 +++++++++--------- .../test-runner/output/only_tests.snapshot | 32 ++++- test/fixtures/test-runner/output/output.js | 6 +- .../test-runner/output/output.snapshot | 15 +-- .../test-runner/output/output_cli.snapshot | 17 +-- .../test-runner/output/spec_reporter.snapshot | 13 +- .../output/spec_reporter_cli.snapshot | 15 +-- 10 files changed, 143 insertions(+), 120 deletions(-) diff --git a/lib/internal/test_runner/test.js b/lib/internal/test_runner/test.js index 972cb25fa2709c..e12a76a69e4e8e 100644 --- a/lib/internal/test_runner/test.js +++ b/lib/internal/test_runner/test.js @@ -193,9 +193,7 @@ class Test extends AsyncResource { if (parent === null) { this.concurrency = 1; this.nesting = 0; - this.only = testOnlyFlag; this.reporter = new TestsStream(); - this.runOnlySubtests = this.only; this.testNumber = 0; this.timeout = kDefaultTimeout; this.root = this; @@ -212,9 +210,7 @@ class Test extends AsyncResource { this.concurrency = parent.concurrency; this.nesting = nesting; - this.only = only ?? !parent.runOnlySubtests; this.reporter = parent.reporter; - this.runOnlySubtests = !this.only; this.testNumber = parent.subtests.length + 1; this.timeout = parent.timeout; this.root = parent.root; @@ -259,10 +255,6 @@ class Test extends AsyncResource { skip = 'test name does not match pattern'; } - if (testOnlyFlag && !this.only) { - skip = '\'only\' option not set'; - } - if (skip) { fn = noop; } @@ -301,12 +293,26 @@ class Test extends AsyncResource { this.subtests = []; this.waitingOn = 0; this.finished = false; + this.only = testOnlyFlag ? only : undefined; + this.runOnlySubtests = false; - if (!testOnlyFlag && (only || this.runOnlySubtests)) { - const warning = - "'only' and 'runOnly' require the --test-only command-line option."; + + if (!testOnlyFlag && only && !parent.runOnlySubtests) { + const warning = "'only' requires the --test-only command-line option."; this.diagnostic(warning); } + + if (this.only && parent !== null) { + parent.markOnly(); + } + } + + markOnly() { + if (this.runOnlySubtests) { + return; + } + this.runOnlySubtests = true; + this.parent?.markOnly(); } matchesTestNamePatterns() { @@ -539,9 +545,18 @@ class Test extends AsyncResource { } } + get runOnlySibling() { + return this.parent?.runOnlySubtests && !this.only && !this.runOnlySubtests; + } + async run() { this.startTime = hrtime(); + if (this.runOnlySibling || this.only === false) { + this.fn = noop; + this.skip('\'only\' option not set'); + } + if (this[kShouldAbort]()) { this.postRun(); return; @@ -794,7 +809,6 @@ class Suite extends Test { this.fn = options.fn || this.fn; this.skipped = false; } - this.runOnlySubtests = testOnlyFlag; try { const { ctx, args } = this.getRunArgs(); @@ -815,7 +829,7 @@ class Suite extends Test { this.fail(new ERR_TEST_FAILURE(err, kTestCodeFailure)); this.buildPhaseFinished = this.parent !== null; } - this.fn = () => {}; + this.fn = noop; } getRunArgs() { @@ -825,6 +839,7 @@ class Suite extends Test { async run() { const hookArgs = this.getRunArgs(); + this.runOnlySubtests ||= this.runOnlySibling; try { await this.buildSuite; diff --git a/lib/internal/test_runner/utils.js b/lib/internal/test_runner/utils.js index 69b59b25410ff6..76943437fa4a31 100644 --- a/lib/internal/test_runner/utils.js +++ b/lib/internal/test_runner/utils.js @@ -218,8 +218,8 @@ function parseCommandLine() { testOnlyFlag = false; testNamePatterns = null; } else { + testOnlyFlag = getOptionValue('--test-only') || (!isChildProcess && !isChildProcessV8); const testNamePatternFlag = getOptionValue('--test-name-pattern'); - testOnlyFlag = getOptionValue('--test-only'); testNamePatterns = testNamePatternFlag?.length > 0 ? ArrayPrototypeMap( testNamePatternFlag, diff --git a/test/fixtures/test-runner/output/name_pattern_with_only.js b/test/fixtures/test-runner/output/name_pattern_with_only.js index a3e2f1be2ad42d..a76c850bb2eb20 100644 --- a/test/fixtures/test-runner/output/name_pattern_with_only.js +++ b/test/fixtures/test-runner/output/name_pattern_with_only.js @@ -1,4 +1,4 @@ -// Flags: --no-warnings --test-only --test-name-pattern=enabled +// Flags: --no-warnings --test-name-pattern=enabled 'use strict'; const common = require('../../../common'); const { test } = require('node:test'); diff --git a/test/fixtures/test-runner/output/only_tests.js b/test/fixtures/test-runner/output/only_tests.js index 5ac4a90c2cf264..7124622827caaa 100644 --- a/test/fixtures/test-runner/output/only_tests.js +++ b/test/fixtures/test-runner/output/only_tests.js @@ -1,100 +1,96 @@ -// Flags: --no-warnings --test-only +// Flags: --no-warnings 'use strict'; -require('../../../common'); +const common = require('../../../common'); const { test, describe, it } = require('node:test'); // These tests should be skipped based on the 'only' option. -test('only = undefined'); -test('only = undefined, skip = string', { skip: 'skip message' }); -test('only = undefined, skip = true', { skip: true }); -test('only = undefined, skip = false', { skip: false }); -test('only = false', { only: false }); -test('only = false, skip = string', { only: false, skip: 'skip message' }); -test('only = false, skip = true', { only: false, skip: true }); -test('only = false, skip = false', { only: false, skip: false }); +test('only = undefined', common.mustNotCall()); +test('only = undefined, skip = string', { skip: 'skip message' }, common.mustNotCall()); +test('only = undefined, skip = true', { skip: true }, common.mustNotCall()); +test('only = undefined, skip = false', { skip: false }, common.mustNotCall()); +test('only = false', { only: false }, common.mustNotCall()); +test('only = false, skip = string', { only: false, skip: 'skip message' }, common.mustNotCall()); +test('only = false, skip = true', { only: false, skip: true }, common.mustNotCall()); +test('only = false, skip = false', { only: false, skip: false }, common.mustNotCall()); // These tests should be skipped based on the 'skip' option. -test('only = true, skip = string', { only: true, skip: 'skip message' }); -test('only = true, skip = true', { only: true, skip: true }); +test('only = true, skip = string', { only: true, skip: 'skip message' }, common.mustNotCall()); +test('only = true, skip = true', { only: true, skip: true }, common.mustNotCall()); // An 'only' test with subtests. -test('only = true, with subtests', { only: true }, async (t) => { +test('only = true, with subtests', { only: true }, common.mustCall(async (t) => { // These subtests should run. - await t.test('running subtest 1'); - await t.test('running subtest 2'); + await t.test('running subtest 1', common.mustCall()); + await t.test('running subtest 2', common.mustCall()); // Switch the context to only execute 'only' tests. t.runOnly(true); - await t.test('skipped subtest 1'); - await t.test('skipped subtest 2'); - await t.test('running subtest 3', { only: true }); + await t.test('skipped subtest 1', common.mustNotCall()); + await t.test('skipped subtest 2'), common.mustNotCall(); + await t.test('running subtest 3', { only: true }, common.mustCall()); // Switch the context back to execute all tests. t.runOnly(false); - await t.test('running subtest 4', async (t) => { + await t.test('running subtest 4', common.mustCall(async (t) => { // These subtests should run. - await t.test('running sub-subtest 1'); - await t.test('running sub-subtest 2'); + await t.test('running sub-subtest 1', common.mustCall()); + await t.test('running sub-subtest 2', common.mustCall()); // Switch the context to only execute 'only' tests. t.runOnly(true); - await t.test('skipped sub-subtest 1'); - await t.test('skipped sub-subtest 2'); - }); + await t.test('skipped sub-subtest 1', common.mustNotCall()); + await t.test('skipped sub-subtest 2', common.mustNotCall()); + })); // Explicitly do not run these tests. - await t.test('skipped subtest 3', { only: false }); - await t.test('skipped subtest 4', { skip: true }); -}); + await t.test('skipped subtest 3', { only: false }, common.mustNotCall()); + await t.test('skipped subtest 4', { skip: true }, common.mustNotCall()); +})); -describe.only('describe only = true, with subtests', () => { - it.only('`it` subtest 1 should run', () => {}); +describe.only('describe only = true, with subtests', common.mustCall(() => { + it.only('`it` subtest 1 should run', common.mustCall()); - it('`it` subtest 2 should not run', async () => {}); -}); + it('`it` subtest 2 should not run', common.mustNotCall()); +})); -describe.only('describe only = true, with a mixture of subtests', () => { - it.only('`it` subtest 1', () => {}); +describe.only('describe only = true, with a mixture of subtests', common.mustCall(() => { + it.only('`it` subtest 1', common.mustCall()); - it.only('`it` async subtest 1', async () => {}); + it.only('`it` async subtest 1', common.mustCall(async () => {})); - it('`it` subtest 2 only=true', { only: true }); + it('`it` subtest 2 only=true', { only: true }, common.mustCall()); - it('`it` subtest 2 only=false', { only: false }, () => { - throw new Error('This should not run'); - }); + it('`it` subtest 2 only=false', { only: false }, common.mustNotCall()); - it.skip('`it` subtest 3 skip', () => { - throw new Error('This should not run'); - }); + it.skip('`it` subtest 3 skip', common.mustNotCall()); - it.todo('`it` subtest 4 todo', { only: false }, () => { - throw new Error('This should not run'); - }); + it.todo('`it` subtest 4 todo', { only: false }, common.mustNotCall()); - test.only('`test` subtest 1', () => {}); + test.only('`test` subtest 1', common.mustCall()); - test.only('`test` async subtest 1', async () => {}); + test.only('`test` async subtest 1', common.mustCall(async () => {})); - test('`test` subtest 2 only=true', { only: true }); + test('`test` subtest 2 only=true', { only: true }, common.mustCall()); - test('`test` subtest 2 only=false', { only: false }, () => { - throw new Error('This should not run'); - }); + test('`test` subtest 2 only=false', { only: false }, common.mustNotCall()); - test.skip('`test` subtest 3 skip', () => { - throw new Error('This should not run'); - }); + test.skip('`test` subtest 3 skip', common.mustNotCall()); - test.todo('`test` subtest 4 todo', { only: false }, () => { - throw new Error('This should not run'); - }); -}); + test.todo('`test` subtest 4 todo', { only: false }, common.mustNotCall()); +})); -describe.only('describe only = true, with subtests', () => { - test.only('subtest should run', () => {}); +describe.only('describe only = true, with subtests', common.mustCall(() => { + test.only('subtest should run', common.mustCall()); - test('async subtest should not run', async () => {}); + test('async subtest should not run', common.mustNotCall()); - test('subtest should be skipped', { only: false }, () => {}); -}); + test('subtest should be skipped', { only: false }, common.mustNotCall()); +})); + +describe('describe only = undefined, with subtests', common.mustCall(() => { + test('async subtest should not run', common.mustNotCall()); +})); + +describe('describe only = false, with subtests', { only: false }, common.mustCall(() => { + test('async subtest should not run', common.mustNotCall()); +})); diff --git a/test/fixtures/test-runner/output/only_tests.snapshot b/test/fixtures/test-runner/output/only_tests.snapshot index ded19f3bec4c6a..de6251f29d9ef2 100644 --- a/test/fixtures/test-runner/output/only_tests.snapshot +++ b/test/fixtures/test-runner/output/only_tests.snapshot @@ -222,12 +222,36 @@ ok 14 - describe only = true, with subtests duration_ms: * type: 'suite' ... -1..14 -# tests 40 -# suites 3 +# Subtest: describe only = undefined, with subtests + # Subtest: async subtest should not run + ok 1 - async subtest should not run # SKIP 'only' option not set + --- + duration_ms: * + ... + 1..1 +ok 15 - describe only = undefined, with subtests + --- + duration_ms: * + type: 'suite' + ... +# Subtest: describe only = false, with subtests + # Subtest: async subtest should not run + ok 1 - async subtest should not run # SKIP 'only' option not set + --- + duration_ms: * + ... + 1..1 +ok 16 - describe only = false, with subtests + --- + duration_ms: * + type: 'suite' + ... +1..16 +# tests 42 +# suites 5 # pass 15 # fail 0 # cancelled 0 -# skipped 25 +# skipped 27 # todo 0 # duration_ms * diff --git a/test/fixtures/test-runner/output/output.js b/test/fixtures/test-runner/output/output.js index 47d99d1c8d4984..8aeb187629ed9d 100644 --- a/test/fixtures/test-runner/output/output.js +++ b/test/fixtures/test-runner/output/output.js @@ -288,11 +288,11 @@ test('callback async throw after done', (t, done) => { done(); }); -test('only is set but not in only mode', { only: true }, async (t) => { - // All of these subtests should run. +test('runOnly is set', async (t) => { + // Subtests should run only outside of a runOnly block, unless they have only: true. await t.test('running subtest 1'); t.runOnly(true); - await t.test('running subtest 2'); + await t.test('skipped subtest 2'); await t.test('running subtest 3', { only: true }); t.runOnly(false); await t.test('running subtest 4'); diff --git a/test/fixtures/test-runner/output/output.snapshot b/test/fixtures/test-runner/output/output.snapshot index 87ecbbe562f0a0..3db1538db2aa98 100644 --- a/test/fixtures/test-runner/output/output.snapshot +++ b/test/fixtures/test-runner/output/output.snapshot @@ -501,35 +501,32 @@ ok 52 - callback async throw after done --- duration_ms: * ... -# Subtest: only is set but not in only mode +# Subtest: runOnly is set # Subtest: running subtest 1 ok 1 - running subtest 1 --- duration_ms: * ... - # Subtest: running subtest 2 - ok 2 - running subtest 2 + # Subtest: skipped subtest 2 + ok 2 - skipped subtest 2 # SKIP 'only' option not set --- duration_ms: * ... - # 'only' and 'runOnly' require the --test-only command-line option. # Subtest: running subtest 3 ok 3 - running subtest 3 --- duration_ms: * ... - # 'only' and 'runOnly' require the --test-only command-line option. # Subtest: running subtest 4 ok 4 - running subtest 4 --- duration_ms: * ... 1..4 -ok 53 - only is set but not in only mode +ok 53 - runOnly is set --- duration_ms: * ... -# 'only' and 'runOnly' require the --test-only command-line option. # Subtest: custom inspect symbol fail not ok 54 - custom inspect symbol fail --- @@ -723,9 +720,9 @@ not ok 66 - invalid subtest fail # 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. # tests 80 # suites 0 -# pass 37 +# pass 36 # fail 25 # cancelled 3 -# skipped 10 +# skipped 11 # todo 5 # duration_ms * diff --git a/test/fixtures/test-runner/output/output_cli.snapshot b/test/fixtures/test-runner/output/output_cli.snapshot index 6e9f2bb35c0feb..c0c26fcb11453f 100644 --- a/test/fixtures/test-runner/output/output_cli.snapshot +++ b/test/fixtures/test-runner/output/output_cli.snapshot @@ -501,35 +501,32 @@ ok 52 - callback async throw after done --- duration_ms: * ... -# Subtest: only is set but not in only mode +# Subtest: runOnly is set # Subtest: running subtest 1 ok 1 - running subtest 1 --- duration_ms: * ... - # Subtest: running subtest 2 - ok 2 - running subtest 2 + # Subtest: skipped subtest 2 + ok 2 - skipped subtest 2 # SKIP 'only' option not set --- duration_ms: * ... - # 'only' and 'runOnly' require the --test-only command-line option. # Subtest: running subtest 3 - ok 3 - running subtest 3 + ok 3 - running subtest 3 # SKIP 'only' option not set --- duration_ms: * ... - # 'only' and 'runOnly' require the --test-only command-line option. # Subtest: running subtest 4 ok 4 - running subtest 4 --- duration_ms: * ... 1..4 -ok 53 - only is set but not in only mode +ok 53 - runOnly is set --- duration_ms: * ... -# 'only' and 'runOnly' require the --test-only command-line option. # Subtest: custom inspect symbol fail not ok 54 - custom inspect symbol fail --- @@ -728,9 +725,9 @@ ok 67 - last test 1..67 # tests 81 # suites 0 -# pass 38 +# pass 36 # fail 25 # cancelled 3 -# skipped 10 +# skipped 12 # todo 5 # duration_ms * diff --git a/test/fixtures/test-runner/output/spec_reporter.snapshot b/test/fixtures/test-runner/output/spec_reporter.snapshot index 48780eae1575d1..fb79887ce929a4 100644 --- a/test/fixtures/test-runner/output/spec_reporter.snapshot +++ b/test/fixtures/test-runner/output/spec_reporter.snapshot @@ -220,16 +220,13 @@ * callback async throw after done (*ms) - only is set but not in only mode + runOnly is set running subtest 1 (*ms) - running subtest 2 (*ms) - 'only' and 'runOnly' require the --test-only command-line option. + skipped subtest 2 (*ms) # SKIP running subtest 3 (*ms) - 'only' and 'runOnly' require the --test-only command-line option. running subtest 4 (*ms) - only is set but not in only mode (*ms) + runOnly is set (*ms) - 'only' and 'runOnly' require the --test-only command-line option. custom inspect symbol fail (*ms) customized @@ -323,10 +320,10 @@ 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. tests 80 suites 0 - pass 37 + pass 36 fail 25 cancelled 3 - skipped 10 + skipped 11 todo 5 duration_ms * diff --git a/test/fixtures/test-runner/output/spec_reporter_cli.snapshot b/test/fixtures/test-runner/output/spec_reporter_cli.snapshot index aad41f0547ae44..7f72f03cc8f4bc 100644 --- a/test/fixtures/test-runner/output/spec_reporter_cli.snapshot +++ b/test/fixtures/test-runner/output/spec_reporter_cli.snapshot @@ -220,16 +220,13 @@ * callback async throw after done (*ms) - only is set but not in only mode + runOnly is set running subtest 1 (*ms) - running subtest 2 (*ms) - 'only' and 'runOnly' require the --test-only command-line option. - running subtest 3 (*ms) - 'only' and 'runOnly' require the --test-only command-line option. + skipped subtest 2 (*ms) # SKIP + running subtest 3 (*ms) # SKIP running subtest 4 (*ms) - only is set but not in only mode (*ms) + runOnly is set (*ms) - 'only' and 'runOnly' require the --test-only command-line option. custom inspect symbol fail (*ms) customized @@ -323,10 +320,10 @@ 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. tests 80 suites 0 - pass 37 + pass 35 fail 25 cancelled 3 - skipped 10 + skipped 12 todo 5 duration_ms *