From dd98e0e52ec8c62810dfd47793579716703ba726 Mon Sep 17 00:00:00 2001 From: sangwook Date: Sat, 2 May 2026 15:45:11 +0900 Subject: [PATCH 1/4] inspector: expose precise coverage start to JS runtime Add a `startCoverage` method on the `profiler` internal binding so that V8 precise coverage can be enabled after bootstrap. The method is idempotent against the existing bootstrap path (which creates a V8CoverageConnection when NODE_V8_COVERAGE or --experimental-test-coverage is set) and a no-op when the inspector is unavailable, e.g. in the parent process of `--test --test-isolation=process` where workers handle coverage and Environment::should_create_inspector() returns false. Refs: https://github.com/nodejs/node/issues/60023 Signed-off-by: sangwook --- src/inspector_profiler.cc | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/src/inspector_profiler.cc b/src/inspector_profiler.cc index 28653a3939daef..559b4fd27d56ab 100644 --- a/src/inspector_profiler.cc +++ b/src/inspector_profiler.cc @@ -548,6 +548,30 @@ static void SetSourceMapCacheGetter(const FunctionCallbackInfo& args) { env->set_source_map_cache_getter(args[0].As()); } +static void StartCoverage(const FunctionCallbackInfo& args) { + Environment* env = Environment::GetCurrent(args); + + Debug(env, + DebugCategory::INSPECTOR_PROFILER, + "StartCoverage, connection %s nullptr\n", + env->coverage_connection() == nullptr ? "==" : "!="); + + if (env->coverage_connection() != nullptr) { + return; + } + + // The parent of `--test --test-isolation=process` intentionally has no + // inspector (see Environment::should_create_inspector); workers handle + // coverage themselves. Without an inspector, V8CoverageConnection would + // get a null session and crash on the first DispatchMessage. + if (!env->should_create_inspector()) { + return; + } + + env->set_coverage_connection(std::make_unique(env)); + env->coverage_connection()->Start(); +} + static void TakeCoverage(const FunctionCallbackInfo& args) { Environment* env = Environment::GetCurrent(args); V8CoverageConnection* connection = env->coverage_connection(); @@ -601,6 +625,7 @@ static void Initialize(Local target, SetMethod(context, target, "setCoverageDirectory", SetCoverageDirectory); SetMethod( context, target, "setSourceMapCacheGetter", SetSourceMapCacheGetter); + SetMethod(context, target, "startCoverage", StartCoverage); SetMethod(context, target, "takeCoverage", TakeCoverage); SetMethod(context, target, "stopCoverage", StopCoverage); SetMethod(context, target, "endCoverage", EndCoverage); @@ -609,6 +634,7 @@ static void Initialize(Local target, void RegisterExternalReferences(ExternalReferenceRegistry* registry) { registry->Register(SetCoverageDirectory); registry->Register(SetSourceMapCacheGetter); + registry->Register(StartCoverage); registry->Register(TakeCoverage); registry->Register(StopCoverage); registry->Register(EndCoverage); From e989cf2f54c99d96ec21ad6592eee2b890e8fbdd Mon Sep 17 00:00:00 2001 From: sangwook Date: Sat, 2 May 2026 15:45:16 +0900 Subject: [PATCH 2/4] test_runner: enable coverage on run() with isolation: 'none' run({ coverage: true, isolation: 'none' }) previously returned an empty file list because V8 precise coverage is only started at bootstrap when NODE_V8_COVERAGE or --experimental-test-coverage is set, neither of which the API path requires. Call the new profiler.startCoverage() binding from setupCoverage() so the parent isolate is instrumented when the run() API is the entry point. Fixes: https://github.com/nodejs/node/issues/60023 Signed-off-by: sangwook --- lib/internal/test_runner/coverage.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/lib/internal/test_runner/coverage.js b/lib/internal/test_runner/coverage.js index 8fa9c872568d1e..577ce15f147d03 100644 --- a/lib/internal/test_runner/coverage.js +++ b/lib/internal/test_runner/coverage.js @@ -533,6 +533,8 @@ function setupCoverage(options) { return null; } + internalBinding('profiler').startCoverage(); + // Ensure that NODE_V8_COVERAGE is set so that coverage can propagate to // child processes. process.env.NODE_V8_COVERAGE = coverageDirectory; From 80a2a460bc980de4ab871c39903c7f578811f8a2 Mon Sep 17 00:00:00 2001 From: sangwook Date: Sat, 2 May 2026 15:45:24 +0900 Subject: [PATCH 3/4] test_runner: apply default test-file exclusion via run() API The CLI path defaults coverageExcludeGlobs to [kDefaultPattern] when --experimental-test-coverage is set, dropping test files from the coverage report. The run() API skipped this default, so callers got test files mixed into their coverage data. Apply the same default when run() is invoked with coverage: true and no explicit coverageExcludeGlobs, so both entry points behave consistently. Update the existing run() coverage tests that depended on the absent default to opt out via coverageExcludeGlobs: '!test/**'. Refs: https://github.com/nodejs/node/issues/60023 Signed-off-by: sangwook --- lib/internal/test_runner/runner.js | 2 ++ test/parallel/test-runner-run-coverage.mjs | 10 +++++++++- 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/lib/internal/test_runner/runner.js b/lib/internal/test_runner/runner.js index 2033ac16e8ea49..ec0677831c373a 100644 --- a/lib/internal/test_runner/runner.js +++ b/lib/internal/test_runner/runner.js @@ -805,6 +805,8 @@ function run(options = kEmptyObject) { coverageExcludeGlobs = [coverageExcludeGlobs]; } validateStringArray(coverageExcludeGlobs, 'options.coverageExcludeGlobs'); + } else if (coverage) { + coverageExcludeGlobs = [kDefaultPattern]; } if (coverageIncludeGlobs != null) { if (!ArrayIsArray(coverageIncludeGlobs)) { diff --git a/test/parallel/test-runner-run-coverage.mjs b/test/parallel/test-runner-run-coverage.mjs index 15fcfef5238843..89a9da2a179e44 100644 --- a/test/parallel/test-runner-run-coverage.mjs +++ b/test/parallel/test-runner-run-coverage.mjs @@ -123,6 +123,7 @@ describe('require(\'node:test\').run coverage settings', { concurrency: true }, const stream = run({ files, coverage: true, + coverageExcludeGlobs: '!test/**', coverageIncludeGlobs: ['test/fixtures/test-runner/coverage.js', 'test/*/v8-coverage/throw.js'], }); stream.on('test:fail', common.mustNotCall()); @@ -157,7 +158,14 @@ describe('require(\'node:test\').run coverage settings', { concurrency: true }, const thresholdErrors = []; const originalExitCode = process.exitCode; assert.notStrictEqual(originalExitCode, 1); - const stream = run({ files, coverage: true, lineCoverage: 99, branchCoverage: 99, functionCoverage: 99 }); + const stream = run({ + files, + coverage: true, + coverageExcludeGlobs: '!test/**', + lineCoverage: 99, + branchCoverage: 99, + functionCoverage: 99, + }); stream.on('test:fail', common.mustNotCall()); stream.on('test:pass', common.mustCall(1)); stream.on('test:diagnostic', ({ message }) => { From 711ac87c52cb3ed576180193d3084c4cc240c5df Mon Sep 17 00:00:00 2001 From: sangwook Date: Sat, 2 May 2026 15:45:35 +0900 Subject: [PATCH 4/4] test: add coverage tests for run() with isolation: 'none' Verify that: - run({ coverage: true, isolation: 'none' }) reports src files, - default test-file exclusion drops *.test.mjs in both isolation modes, - the path is idempotent when --experimental-test-coverage is set on the same process. Refs: https://github.com/nodejs/node/issues/60023 Signed-off-by: sangwook --- .../coverage-isolation-none/src/foo.mjs | 11 +++ .../tests/foo.test.mjs | 11 +++ ...est-runner-coverage-isolation-none-api.mjs | 92 +++++++++++++++++++ 3 files changed, 114 insertions(+) create mode 100644 test/fixtures/test-runner/coverage-isolation-none/src/foo.mjs create mode 100644 test/fixtures/test-runner/coverage-isolation-none/tests/foo.test.mjs create mode 100644 test/parallel/test-runner-coverage-isolation-none-api.mjs diff --git a/test/fixtures/test-runner/coverage-isolation-none/src/foo.mjs b/test/fixtures/test-runner/coverage-isolation-none/src/foo.mjs new file mode 100644 index 00000000000000..f6a50e85a9d412 --- /dev/null +++ b/test/fixtures/test-runner/coverage-isolation-none/src/foo.mjs @@ -0,0 +1,11 @@ +export function add(a, b) { + return a + b; +} + +export function sub(a, b) { + return a - b; +} + +export function unused() { + return 'unused'; +} diff --git a/test/fixtures/test-runner/coverage-isolation-none/tests/foo.test.mjs b/test/fixtures/test-runner/coverage-isolation-none/tests/foo.test.mjs new file mode 100644 index 00000000000000..efbccddc87412b --- /dev/null +++ b/test/fixtures/test-runner/coverage-isolation-none/tests/foo.test.mjs @@ -0,0 +1,11 @@ +import test from 'node:test'; +import assert from 'node:assert'; +import { add, sub } from '../src/foo.mjs'; + +test('add', () => { + assert.strictEqual(add(2, 3), 5); +}); + +test('sub', () => { + assert.strictEqual(sub(5, 3), 2); +}); diff --git a/test/parallel/test-runner-coverage-isolation-none-api.mjs b/test/parallel/test-runner-coverage-isolation-none-api.mjs new file mode 100644 index 00000000000000..08417e5dc3a1b6 --- /dev/null +++ b/test/parallel/test-runner-coverage-isolation-none-api.mjs @@ -0,0 +1,92 @@ +import * as common from '../common/index.mjs'; +import { before, describe, it, run } from 'node:test'; +import assert from 'node:assert'; +import { spawnSync } from 'node:child_process'; +import { cp, writeFile } from 'node:fs/promises'; +import { join, sep } from 'node:path'; +import tmpdir from '../common/tmpdir.js'; +import fixtures from '../common/fixtures.js'; + +const skipIfNoInspector = { + skip: !process.features.inspector ? 'inspector disabled' : false, +}; + +tmpdir.refresh(); + +async function setupFixtures() { + const fixtureDir = fixtures.path('test-runner', 'coverage-isolation-none'); + await cp(fixtureDir, tmpdir.path, { recursive: true }); +} + +describe('run() coverage with isolation: none', skipIfNoInspector, () => { + before(async () => { + await setupFixtures(); + }); + + for (const isolation of ['none', 'process']) { + it(`reports src coverage and excludes test files by default (isolation=${isolation})`, async () => { + const stream = run({ + files: [join(tmpdir.path, 'tests', 'foo.test.mjs')], + coverage: true, + isolation, + cwd: tmpdir.path, + }); + stream.on('test:fail', common.mustNotCall()); + + let summary; + stream.on('test:coverage', common.mustCall(({ summary: s }) => { + summary = s; + })); + // eslint-disable-next-line no-unused-vars + for await (const _ of stream); + + assert.ok(summary, 'test:coverage event must fire'); + const paths = summary.files.map((f) => f.path); + assert.ok( + paths.length > 0, + `coverage files must be reported (isolation=${isolation}); got ${JSON.stringify(paths)}`, + ); + assert.ok( + paths.some((p) => p.endsWith(`src${sep}foo.mjs`)), + `expected src/foo.mjs to be present (isolation=${isolation}); got ${JSON.stringify(paths)}`, + ); + assert.ok( + paths.every((p) => !p.endsWith('foo.test.mjs')), + `expected foo.test.mjs to be excluded by default (isolation=${isolation}); got ${JSON.stringify(paths)}`, + ); + }); + } + + it('is idempotent when --experimental-test-coverage is also passed', async () => { + const runnerPath = join(tmpdir.path, 'runner.mjs'); + await writeFile(runnerPath, `\ +import { run } from 'node:test'; +import { join } from 'node:path'; + +const stream = run({ + files: [join(import.meta.dirname, 'tests', 'foo.test.mjs')], + coverage: true, + isolation: 'none', + cwd: import.meta.dirname, +}); +stream.on('test:fail', () => process.exit(10)); +let summary; +stream.on('test:coverage', (event) => { summary = event.summary; }); +for await (const _ of stream); +if (!summary || summary.files.length === 0) process.exit(11); +const hasSrc = summary.files.some((f) => f.path.endsWith('foo.mjs') && !f.path.endsWith('foo.test.mjs')); +const hasTest = summary.files.some((f) => f.path.endsWith('foo.test.mjs')); +if (!hasSrc) process.exit(12); +if (hasTest) process.exit(13); +`); + const result = spawnSync(process.execPath, [ + '--experimental-test-coverage', + runnerPath, + ], { cwd: tmpdir.path }); + assert.strictEqual( + result.status, + 0, + `exited with ${result.status}\nstderr: ${result.stderr}\nstdout: ${result.stdout}`, + ); + }); +});