From 7fad771bbff88c2cf01d64517c164180e90c8bfb Mon Sep 17 00:00:00 2001 From: Pietro Marchini Date: Thu, 8 Aug 2024 07:50:32 +0200 Subject: [PATCH] test_runner: fix erroneous diagnostic warning when only: false Co-author: rstagi PR-URL: https://github.com/nodejs/node/pull/54116 Reviewed-By: Matteo Collina Reviewed-By: Colin Ihrig Reviewed-By: Moshe Atlow --- lib/internal/test_runner/test.js | 6 +- .../fixtures/test-runner/output/only_tests.js | 21 +++ .../test-runner/output/only_tests.snapshot | 45 ++++++- ...agnostic-warning-without-test-only-flag.js | 40 ++++++ ...ic-warning-without-test-only-flag.snapshot | 121 ++++++++++++++++++ test/parallel/test-runner-output.mjs | 1 + 6 files changed, 227 insertions(+), 7 deletions(-) create mode 100644 test/fixtures/test-runner/output/test-diagnostic-warning-without-test-only-flag.js create mode 100644 test/fixtures/test-runner/output/test-diagnostic-warning-without-test-only-flag.snapshot diff --git a/lib/internal/test_runner/test.js b/lib/internal/test_runner/test.js index bc29d9204552e1..00922c7b529272 100644 --- a/lib/internal/test_runner/test.js +++ b/lib/internal/test_runner/test.js @@ -409,9 +409,9 @@ class Test extends AsyncResource { this.concurrency = parent.concurrency; this.nesting = nesting; - this.only = only ?? !parent.runOnlySubtests; + this.only = only ?? (parent.only && !parent.runOnlySubtests); this.reporter = parent.reporter; - this.runOnlySubtests = !this.only; + this.runOnlySubtests = false; this.childNumber = parent.subtests.length + 1; this.timeout = parent.timeout; this.entryFile = parent.entryFile; @@ -501,7 +501,7 @@ class Test extends AsyncResource { this.waitingOn = 0; this.finished = false; - if (!testOnlyFlag && (only || this.runOnlySubtests)) { + if (!testOnlyFlag && (only || this.parent?.runOnlySubtests)) { const warning = "'only' and 'runOnly' require the --test-only command-line option."; this.diagnostic(warning); diff --git a/test/fixtures/test-runner/output/only_tests.js b/test/fixtures/test-runner/output/only_tests.js index e2f6975e5b8268..616a3351886362 100644 --- a/test/fixtures/test-runner/output/only_tests.js +++ b/test/fixtures/test-runner/output/only_tests.js @@ -119,3 +119,24 @@ describe('describe only = false, with nested only subtests', { only: false }, co test.only('nested test should run', common.mustNotCall()); })); })); + +test('nested tests with run only',{only: true}, common.mustCall(async (t) => { + // Within this test, all subtests are run by default. + await t.test('running subtest - 1'); + + // The test context can be updated to run subtests with the 'only' option. + await t.runOnly(true); + await t.test('this subtest is now skipped - 2', common.mustNotCall()); + await t.test('this subtest is run - 3', { only: true }, common.mustCall(async (t) => { + await t.test('this subtest is run - 4', common.mustCall()); + await t.test('this subtest is run - 5', { only: true }, common.mustCall()); + })); + + // Switch the context back to execute all tests. + await t.runOnly(false); + await t.test('this subtest is now run - 6'); + + // Explicitly do not run these tests. + await t.test('skipped subtest - 7', { only: false }, common.mustNotCall()); + await t.test('skipped subtest - 8', { skip: true }, common.mustNotCall()); +})) diff --git a/test/fixtures/test-runner/output/only_tests.snapshot b/test/fixtures/test-runner/output/only_tests.snapshot index aa71e4529f4b1c..25e2e9b65cdca2 100644 --- a/test/fixtures/test-runner/output/only_tests.snapshot +++ b/test/fixtures/test-runner/output/only_tests.snapshot @@ -155,12 +155,49 @@ ok 8 - describe only = true, with nested subtests duration_ms: * type: 'suite' ... -1..8 -# tests 21 +# Subtest: nested tests with run only + # Subtest: running subtest - 1 + ok 1 - running subtest - 1 + --- + duration_ms: * + ... + # Subtest: this subtest is run - 3 + # Subtest: this subtest is run - 4 + ok 1 - this subtest is run - 4 + --- + duration_ms: * + ... + # Subtest: this subtest is run - 5 + ok 2 - this subtest is run - 5 + --- + duration_ms: * + ... + 1..2 + ok 2 - this subtest is run - 3 + --- + duration_ms: * + ... + # Subtest: this subtest is now run - 6 + ok 3 - this subtest is now run - 6 + --- + duration_ms: * + ... + # Subtest: skipped subtest - 8 + ok 4 - skipped subtest - 8 # SKIP + --- + duration_ms: * + ... + 1..4 +ok 9 - nested tests with run only + --- + duration_ms: * + ... +1..9 +# tests 28 # suites 7 -# pass 18 +# pass 24 # fail 0 # cancelled 0 -# skipped 3 +# skipped 4 # todo 0 # duration_ms * diff --git a/test/fixtures/test-runner/output/test-diagnostic-warning-without-test-only-flag.js b/test/fixtures/test-runner/output/test-diagnostic-warning-without-test-only-flag.js new file mode 100644 index 00000000000000..cf6018deadbd2f --- /dev/null +++ b/test/fixtures/test-runner/output/test-diagnostic-warning-without-test-only-flag.js @@ -0,0 +1,40 @@ +'use strict'; +const { test, describe, it } = require('node:test'); + +describe('should NOT print --test-only diagnostic warning - describe-only-false', {only: false}, () => { + it('only false in describe'); + }); + + describe('should NOT print --test-only diagnostic warning - it-only-false', () => { + it('only false in the subtest', {only: false}); + }); + + describe('should NOT print --test-only diagnostic warning - no-only', () => { + it('no only'); + }); + + test('should NOT print --test-only diagnostic warning - test-only-false', {only: false}, async (t) => { + await t.test('only false in parent test'); + }); + + test('should NOT print --test-only diagnostic warning - t.test-only-false', async (t) => { + await t.test('only false in subtest', {only: false}); + }); + + test('should NOT print --test-only diagnostic warning - no-only', async (t) => { + await t.test('no only'); + }); + + test('should print --test-only diagnostic warning - test-only-true', {only: true}, async (t) => { + await t.test('only true in parent test'); + }) + + test('should print --test-only diagnostic warning - t.test-only-true', async (t) => { + await t.test('only true in subtest', {only: true}); + }); + + test('should print --test-only diagnostic warning - 2 levels of only', async (t) => { + await t.test('only true in parent test', {only: false}, async (t) => { + await t.test('only true in subtest', {only: true}); + }); + }) diff --git a/test/fixtures/test-runner/output/test-diagnostic-warning-without-test-only-flag.snapshot b/test/fixtures/test-runner/output/test-diagnostic-warning-without-test-only-flag.snapshot new file mode 100644 index 00000000000000..c64caa87a279e8 --- /dev/null +++ b/test/fixtures/test-runner/output/test-diagnostic-warning-without-test-only-flag.snapshot @@ -0,0 +1,121 @@ +TAP version 13 +# Subtest: should NOT print --test-only diagnostic warning - describe-only-false + # Subtest: only false in describe + ok 1 - only false in describe + --- + duration_ms: * + ... + 1..1 +ok 1 - should NOT print --test-only diagnostic warning - describe-only-false + --- + duration_ms: * + type: 'suite' + ... +# Subtest: should NOT print --test-only diagnostic warning - it-only-false + # Subtest: only false in the subtest + ok 1 - only false in the subtest + --- + duration_ms: * + ... + 1..1 +ok 2 - should NOT print --test-only diagnostic warning - it-only-false + --- + duration_ms: * + type: 'suite' + ... +# Subtest: should NOT print --test-only diagnostic warning - no-only + # Subtest: no only + ok 1 - no only + --- + duration_ms: * + ... + 1..1 +ok 3 - should NOT print --test-only diagnostic warning - no-only + --- + duration_ms: * + type: 'suite' + ... +# Subtest: should NOT print --test-only diagnostic warning - test-only-false + # Subtest: only false in parent test + ok 1 - only false in parent test + --- + duration_ms: * + ... + 1..1 +ok 4 - should NOT print --test-only diagnostic warning - test-only-false + --- + duration_ms: * + ... +# Subtest: should NOT print --test-only diagnostic warning - t.test-only-false + # Subtest: only false in subtest + ok 1 - only false in subtest + --- + duration_ms: * + ... + 1..1 +ok 5 - should NOT print --test-only diagnostic warning - t.test-only-false + --- + duration_ms: * + ... +# Subtest: should NOT print --test-only diagnostic warning - no-only + # Subtest: no only + ok 1 - no only + --- + duration_ms: * + ... + 1..1 +ok 6 - should NOT print --test-only diagnostic warning - no-only + --- + duration_ms: * + ... +# Subtest: should print --test-only diagnostic warning - test-only-true + # Subtest: only true in parent test + ok 1 - only true in parent test + --- + duration_ms: * + ... + 1..1 +ok 7 - should print --test-only diagnostic warning - test-only-true + --- + duration_ms: * + ... +# 'only' and 'runOnly' require the --test-only command-line option. +# Subtest: should print --test-only diagnostic warning - t.test-only-true + # Subtest: only true in subtest + ok 1 - only true in subtest + --- + duration_ms: * + ... + # 'only' and 'runOnly' require the --test-only command-line option. + 1..1 +ok 8 - should print --test-only diagnostic warning - t.test-only-true + --- + duration_ms: * + ... +# Subtest: should print --test-only diagnostic warning - 2 levels of only + # Subtest: only true in parent test + # Subtest: only true in subtest + ok 1 - only true in subtest + --- + duration_ms: * + ... + # 'only' and 'runOnly' require the --test-only command-line option. + 1..1 + ok 1 - only true in parent test + --- + duration_ms: * + ... + 1..1 +ok 9 - should print --test-only diagnostic warning - 2 levels of only + --- + duration_ms: * + ... +1..9 +# tests 16 +# suites 3 +# pass 16 +# fail 0 +# cancelled 0 +# skipped 0 +# todo 0 +# duration_ms * diff --git a/test/parallel/test-runner-output.mjs b/test/parallel/test-runner-output.mjs index da08cb1b146b3a..23a7cc711feaef 100644 --- a/test/parallel/test-runner-output.mjs +++ b/test/parallel/test-runner-output.mjs @@ -147,6 +147,7 @@ const tests = [ }, { name: 'test-runner/output/test-runner-plan.js' }, process.features.inspector ? { name: 'test-runner/output/coverage_failure.js' } : false, + { name: 'test-runner/output/test-diagnostic-warning-without-test-only-flag.js' }, ] .filter(Boolean) .map(({ name, tty, transform }) => ({