test(coverage): instrument every test subprocess implicitly#9073
Closed
BridgeAR wants to merge 5 commits into
Closed
test(coverage): instrument every test subprocess implicitly#9073BridgeAR wants to merge 5 commits into
BridgeAR wants to merge 5 commits into
Conversation
Contributor
Overall package sizeSelf size: 6.37 MB Dependency sizes| name | version | self size | total size | |------|---------|-----------|------------| | import-in-the-middle | 3.2.0 | 104.26 kB | 843.44 kB | | opentracing | 0.14.7 | 194.81 kB | 194.81 kB | | dc-polyfill | 0.1.11 | 25.74 kB | 25.74 kB |🤖 This report was automatically generated by heaviest-objects-in-the-universe |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #9073 +/- ##
==========================================
+ Coverage 93.66% 93.79% +0.12%
==========================================
Files 889 889
Lines 50841 50841
Branches 11799 11799
==========================================
+ Hits 47621 47684 +63
+ Misses 3220 3157 -63 Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
BenchmarksBenchmark execution time: 2026-06-26 21:15:59 Comparing candidate commit 74f7568 in PR branch Found 0 performance improvements and 0 performance regressions! Performance is the same for 2250 metrics, 36 unstable metrics.
|
…tion
The coverage harness re-injects its NYC bootstrap into spawned children by
wrapping the public `child_process` methods. A caller that captured `fork` or
`spawn` before the patch installed, or one that overwrites `NODE_OPTIONS` to add
its own loader, slipped past those wrappers, so the child ran uninstrumented and
its coverage was dropped. Inject at `ChildProcess.prototype.spawn` instead, the
shared junction every async spawn passes through after the env is normalized into
`options.envPairs`, so no captured reference or env overwrite escapes it. The
public wrappers stay for the synchronous spawns, which do not pass through the
junction, and as the fallback for a future Node that reshapes the internal. The
envPairs merge matches `NODE_OPTIONS` case-insensitively so a Windows
`Node_Options` entry is rewritten in place rather than duplicated.
The wrappers go through `shimmer.wrap`, which copies the original's own properties
and symbols onto each wrapper. That keeps `util.promisify.custom` on `exec` and
`execFile`, the symbol `util.promisify` prefers over the callback form. Without it
a promisified call resolves the raw stdout string instead of `{ stdout, stderr }`
and uses the global Promise, which breaks the child_process plugin's Bluebird
tests.
BridgeAR
force-pushed
the
BridgeAR/2026-06-26-coverage-subprocess-chokepoint
branch
2 times, most recently
from
June 26, 2026 01:03
06a9cca to
7eedb9c
Compare
`test:plugins:ci` ran under plain `nyc`, which only instruments the main process and the sandboxes nyc spawn-wraps. Plugin tests that fork a sandbox after overwriting `NODE_OPTIONS` (every ESM fixture) left their children uninstrumented, so a branch reached only from a spawned child, like graphql's ESM `@graphql-tools/executor` wrap, never showed up in coverage even though a test exercised it. Route `test:plugins:ci` through the coverage harness and add an `--instrument-main` mode to `run-suite.js` that also instruments the mocha process (plugin unit suites run product code there, not only in sandboxes) and finalizes its coverage like a sandbox. Both streams merge into the same `coverage/node-<version>` report the upload step already reads, so no workflow changes are needed. child_process keeps the classic `nyc` flow instead: its tests spawn real OS subprocesses and assert spans under tight per-test timeouts, and nyc's runtime require-hook over the mocha process slows it enough to time them out. It has no sandbox specs, so the harness would add nothing there anyway.
BridgeAR
force-pushed
the
BridgeAR/2026-06-26-coverage-subprocess-chokepoint
branch
from
June 26, 2026 01:22
7eedb9c to
65bc8b8
Compare
3 tasks
BridgeAR
marked this pull request as ready for review
June 26, 2026 14:47
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0d532fc329
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
The `--instrument-main` flow finalizes the repo root as if it were a sandbox, and `finalizeSandbox` merges every JSON under `<repo>/.nyc_output/integration-tests`. That directory is persistent, not a throwaway sandbox temp dir, so a second `test:plugins:ci` run in the same checkout and Node version would fold the previous run's raw coverage into the new report. Clear it before the spawn, matching how `resetCollectorRoot` already wipes the collector dir.
BridgeAR
marked this pull request as draft
July 7, 2026 18:06
Member
Author
|
Superseded with the V8 coverage switch |
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
1 participant
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
The integration coverage harness is meant to instrument every Node descendant a
test spawns, but two gaps let subprocess coverage slip through silently:
child_processmethods. A caller that captured
fork/spawnbefore the patch installed, or thatoverwrote
NODE_OPTIONSto add its own loader (every ESM fixture), bypassed thewrappers and ran the child uninstrumented. Injection now happens at
ChildProcess.prototype.spawn— the internal point every async spawn funnelsthrough after the env is normalized into
options.envPairs— so nothing adeveloper writes can escape it. The public-method wrappers remain for the
synchronous spawns (which do not traverse the chokepoint) and as the fallback
when a future Node reshapes the internal.
test:plugins:ciran under plainnyc, which never instruments a forked sandboxwhose
NODE_OPTIONSwas replaced. A branch reached only from a spawned child —graphql's ESM
@graphql-tools/executorwrap is the case that surfaced this —never appeared in coverage even though a test exercised it.
test:plugins:cinowruns through the harness, with a new
--instrument-mainmode that alsoinstruments the mocha process (plugin unit suites run product code there) and
finalizes it like a sandbox. Both streams merge into the same
coverage/node-<version>report the upload step already consumes, so no workflowchanges are needed.
Why
The harness is implicit by design so a developer spawning a subprocess never has to
think about coverage. A "remember to call a helper at every spawn site" fix would be
forgotten the first time someone adds a site; the chokepoint enforces the invariant
structurally instead.
Test plan
coverage-child-process.spec.jspasses on Node 18/20/22/24/26 (validated locally).*:coveragerun shows a sandbox-only branch (e.g. graphql's ESM executor wrap) covered in the merged report.