From 50d08112c81dbbe77a86cf28f001fff002088a5d Mon Sep 17 00:00:00 2001 From: sangwook Date: Fri, 29 May 2026 20:30:41 +0900 Subject: [PATCH] test_runner: warn on stray .only filtering under isolation=none Under --test-isolation=none, .only filtering is always active even when --test-only was not passed, so a stray .only() silently filters out every other test with no CI-visible signal. Emit a one-time warning when this happens. Behavior (which tests run, exit code) is unchanged. --- lib/internal/test_runner/harness.js | 13 +++ lib/internal/test_runner/test.js | 16 ++++ .../stray-only/multiple-only.test.js | 10 +++ .../test-runner/stray-only/stray-only.test.js | 9 +++ .../test-runner-no-isolation-stray-only.js | 80 +++++++++++++++++++ 5 files changed, 128 insertions(+) create mode 100644 test/fixtures/test-runner/stray-only/multiple-only.test.js create mode 100644 test/fixtures/test-runner/stray-only/stray-only.test.js create mode 100644 test/parallel/test-runner-no-isolation-stray-only.js diff --git a/lib/internal/test_runner/harness.js b/lib/internal/test_runner/harness.js index 16f9392bf7763b..a7a9805603b2d1 100644 --- a/lib/internal/test_runner/harness.js +++ b/lib/internal/test_runner/harness.js @@ -47,6 +47,17 @@ function createTestTree(rootTestOptions, globalOptions) { globalOptions.testSkipPatterns; const isFilteringByOnly = (globalOptions.isolation === 'process' || process.env.NODE_TEST_CONTEXT) ? globalOptions.only : true; + // Under isolation='none' (without a child test context), isFilteringByOnly is + // forced to true regardless of --test-only. In that branch globalOptions.only + // still mirrors getOptionValue('--test-only'), so a falsy value means + // --test-only was not actually passed. When that happens, a stray .only() + // silently filters out every other test with no CI-visible signal. Track the + // condition so the first .only-driven filter can warn once. + // See https://github.com/nodejs/node/issues/60917. + const isStrayOnlyFiltering = isFilteringByOnly && + globalOptions.isolation === 'none' && + !process.env.NODE_TEST_CONTEXT && + !globalOptions.only; const isFilteringByTags = globalOptions.testTagFilters != null; const harness = { __proto__: null, @@ -78,6 +89,8 @@ function createTestTree(rootTestOptions, globalOptions) { isFilteringByName, isFilteringByOnly, isFilteringByTags, + isStrayOnlyFiltering, + strayOnlyWarningEmitted: false, async runBootstrap() { if (globalSetupExecuted) { return PromiseResolve(); diff --git a/lib/internal/test_runner/test.js b/lib/internal/test_runner/test.js index d19d6349f104bf..3fa43a0df352e3 100644 --- a/lib/internal/test_runner/test.js +++ b/lib/internal/test_runner/test.js @@ -657,6 +657,22 @@ class Test extends AsyncResource { if (isFilteringByOnly) { if (this.only) { + // Under isolation='none', isFilteringByOnly is forced on even without + // --test-only, so a stray .only() silently filters out every other + // test. Surface this once so it is not invisible in CI. + // See https://github.com/nodejs/node/issues/60917. + const { harness } = this.root; + if (harness.isStrayOnlyFiltering && !harness.strayOnlyWarningEmitted) { + harness.strayOnlyWarningEmitted = true; + process.emitWarning( + "A test using 'only' was found while running with " + + '--test-isolation=none. Because --test-only was not passed, ' + + "this 'only' is being honored and all other tests are being " + + 'filtered out. This behavior is not gated by --test-only under ' + + 'isolation=none. See https://github.com/nodejs/node/issues/60917.', + ); + } + // If filtering impacts the tests within a suite, then the suite only // runs those tests. If filtering does not impact the tests within a // suite, then all tests are run. diff --git a/test/fixtures/test-runner/stray-only/multiple-only.test.js b/test/fixtures/test-runner/stray-only/multiple-only.test.js new file mode 100644 index 00000000000000..f249b712fbdf87 --- /dev/null +++ b/test/fixtures/test-runner/stray-only/multiple-only.test.js @@ -0,0 +1,10 @@ +'use strict'; +const { test } = require('node:test'); + +// Multiple stray `.only` tests, to verify the stray-only warning is emitted at +// most once. See https://github.com/nodejs/node/issues/60917. +test('only test a', { only: true }, () => {}); + +test('only test b', { only: true }, () => {}); + +test('normal test', () => {}); diff --git a/test/fixtures/test-runner/stray-only/stray-only.test.js b/test/fixtures/test-runner/stray-only/stray-only.test.js new file mode 100644 index 00000000000000..d228f324b6e1d9 --- /dev/null +++ b/test/fixtures/test-runner/stray-only/stray-only.test.js @@ -0,0 +1,9 @@ +'use strict'; +const { test } = require('node:test'); + +// A stray `.only` test alongside a normal test. Under --test-isolation=none +// the `.only` is honored even without --test-only, silently filtering out the +// normal test. See https://github.com/nodejs/node/issues/60917. +test('only test', { only: true }, () => {}); + +test('normal test', () => {}); diff --git a/test/parallel/test-runner-no-isolation-stray-only.js b/test/parallel/test-runner-no-isolation-stray-only.js new file mode 100644 index 00000000000000..af4fd4885a88af --- /dev/null +++ b/test/parallel/test-runner-no-isolation-stray-only.js @@ -0,0 +1,80 @@ +'use strict'; +require('../common'); +const fixtures = require('../common/fixtures'); +const assert = require('node:assert'); +const { spawnSync } = require('node:child_process'); +const { test } = require('node:test'); + +const fixture = fixtures.path('test-runner', 'stray-only', 'stray-only.test.js'); + +// Matches the substring of the warning emitted by test.js when a stray +// `.only` filters tests under --test-isolation=none without --test-only. +// See https://github.com/nodejs/node/issues/60917. +const kStrayOnlyWarning = /A test using 'only' was found while running with --test-isolation=none/; + +test('warns about a stray .only under --test-isolation=none without --test-only', () => { + const args = [ + '--test', + '--test-reporter=tap', + '--test-isolation=none', + fixture, + ]; + const child = spawnSync(process.execPath, args); + const stdout = child.stdout.toString(); + const stderr = child.stderr.toString(); + + assert.strictEqual(child.status, 0); + assert.strictEqual(child.signal, null); + + // The diagnostic warning is emitted to stderr, not the TAP stream. + assert.match(stderr, kStrayOnlyWarning); + assert.doesNotMatch(stdout, kStrayOnlyWarning); + + // Behavior is unchanged: the `.only` test still filters out the normal test. + assert.match(stdout, /ok 1 - only test/); + assert.match(stdout, /# tests 1/); + assert.match(stdout, /# pass 1/); + assert.doesNotMatch(stdout, /normal test/); +}); + +test('does not warn when --test-only is passed under --test-isolation=none', () => { + const args = [ + '--test', + '--test-reporter=tap', + '--test-isolation=none', + '--test-only', + fixture, + ]; + const child = spawnSync(process.execPath, args); + const stdout = child.stdout.toString(); + const stderr = child.stderr.toString(); + + assert.strictEqual(child.status, 0); + assert.strictEqual(child.signal, null); + + // No stray-only warning when --test-only is explicitly requested. + assert.doesNotMatch(stderr, kStrayOnlyWarning); + + // Same filtering: only the `.only` test runs. + assert.match(stdout, /ok 1 - only test/); + assert.match(stdout, /# tests 1/); + assert.match(stdout, /# pass 1/); + assert.doesNotMatch(stdout, /normal test/); +}); + +test('emits the stray-only warning at most once with multiple .only tests', () => { + const multipleOnlyFixture = fixtures.path( + 'test-runner', 'stray-only', 'multiple-only.test.js', + ); + const args = [ + '--test', + '--test-reporter=tap', + '--test-isolation=none', + multipleOnlyFixture, + ]; + const child = spawnSync(process.execPath, args); + const stderr = child.stderr.toString(); + + const matches = stderr.match(new RegExp(kStrayOnlyWarning, 'g')) ?? []; + assert.strictEqual(matches.length, 1); +});