test_runner: fix erroneous diagnostic warning when only: false

Co-author: rstagi <[email protected]> PR-URL: #54116 Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Moshe Atlow <[email protected]>
nodejs · Aug 14, 2024 · 7fad771 · 7fad771
1 parent a394219
commit 7fad771
Show file tree

Hide file tree

Showing 6 changed files with 227 additions and 7 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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 }) => ({