test_runner: support coverage with isolation:'none' via run() API#63079
Open
Han5991 wants to merge 4 commits intonodejs:mainfrom
Open
test_runner: support coverage with isolation:'none' via run() API#63079Han5991 wants to merge 4 commits intonodejs:mainfrom
Han5991 wants to merge 4 commits intonodejs:mainfrom
Conversation
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: nodejs#60023 Signed-off-by: sangwook <rewq5991@gmail.com>
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: nodejs#60023
Signed-off-by: sangwook <rewq5991@gmail.com>
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: nodejs#60023 Signed-off-by: sangwook <rewq5991@gmail.com>
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: nodejs#60023
Signed-off-by: sangwook <rewq5991@gmail.com>
Collaborator
|
Review requested:
|
nodejs-github-bot
added
c++
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #63079 +/- ##
==========================================
+ Coverage 89.65% 89.67% +0.02%
==========================================
Files 712 712
Lines 220498 220515 +17
Branches 42278 42286 +8
==========================================
+ Hits 197683 197745 +62
+ Misses 14663 14613 -50
- Partials 8152 8157 +5
🚀 New features to boost your workflow:
|
atlowChemi
approved these changes
May 3, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #60023.
run({ coverage: true, isolation: 'none' })returned an empty file list because V8 precise coverage is only enabled at bootstrap whenNODE_V8_COVERAGEor--experimental-test-coverageis set, neither of which the API path requires. The reporter also surfaced a related inconsistency: withisolation: 'process', test files (e.g.foo.test.mjs) appeared in the coverage report via the API path, while the CLI path excluded them by default.This PR fixes both halves:
run()is the entry point. A newprofiler.startCoverage()internal binding enables V8 precise coverage atrun()time. It is idempotent against the existing bootstrap path and a no-op when the inspector is unavailable (the parent of--test --test-isolation=process, where workers handle coverage themselves).coverage: trueis passed torun()withoutcoverageExcludeGlobs,[kDefaultPattern]is applied — matchingparseCommandLine's behavior. Two existing run() coverage tests that depended on the absent default are updated to opt out viacoverageExcludeGlobs: '!test/**'.Test plan
test/parallel/test-runner-coverage-isolation-none-api.mjspasses — covers isolation=none reports src files, default exclusion drops*.test.mjsin both isolation modes, and idempotency under--experimental-test-coverage.parallel/test-runner-*tests pass.tools/test.pysuite passes; unrelated FFI fixture-build setup and watch-mode flakiness verified independent of this change.Refs: #60023