Expand granular child_process parity coverage#6446
Conversation
📝 WalkthroughWalkthroughThe child_process parity suite adds fixtures for validation, lifecycle ordering, cwd and environment handling, stdio, synchronous inputs and options, IPC serialization, and advanced values. Documentation and Node 26 regression thresholds are updated for the expanded measurements. ChangesChild process parity suite
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (2)
test-parity/node-suite/child_process/async/stdio-flush.ts (1)
3-5: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueImprove type safety for
child.Consider using
ChildProcessWithoutNullStreamsfromnode:child_processfor better type checking on thechildargument.🛠️ Proposed refactor
-function collect( - child: any, -): Promise<{ code: number | null; stdout: string; stderr: string }> { +function collect( + child: import("node:child_process").ChildProcessWithoutNullStreams, +): Promise<{ code: number | null; stdout: string; stderr: string }> {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@test-parity/node-suite/child_process/async/stdio-flush.ts` around lines 3 - 5, Update the collect function’s child parameter to use ChildProcessWithoutNullStreams imported from node:child_process instead of any, preserving the existing return type and behavior.test-parity/node-suite/child_process/async/cwd-env.ts (1)
10-12: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueImprove type safety for
child.Consider using the
ChildProcesstype fromnode:child_processinstead ofanyfor better type checking.🛠️ Proposed refactor
-function close(child: any): Promise<number | null> { +function close(child: import("node:child_process").ChildProcess): Promise<number | null> { return new Promise((resolve) => child.on("close", resolve)); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@test-parity/node-suite/child_process/async/cwd-env.ts` around lines 10 - 12, Update the close function’s child parameter to use the ChildProcess type imported from node:child_process instead of any, preserving the existing Promise<number | null> behavior and close-event handling.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@test-parity/node-suite/child_process/async/cwd-env.ts`:
- Around line 10-12: Update the close function’s child parameter to use the
ChildProcess type imported from node:child_process instead of any, preserving
the existing Promise<number | null> behavior and close-event handling.
In `@test-parity/node-suite/child_process/async/stdio-flush.ts`:
- Around line 3-5: Update the collect function’s child parameter to use
ChildProcessWithoutNullStreams imported from node:child_process instead of any,
preserving the existing return type and behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 1cb76d34-33d4-40d1-867c-ef4e5a5d539c
📒 Files selected for processing (10)
test-parity/node-suite/child_process/README.mdtest-parity/node-suite/child_process/async/callback-ordering.tstest-parity/node-suite/child_process/async/cwd-env.tstest-parity/node-suite/child_process/async/event-ordering.tstest-parity/node-suite/child_process/async/stdio-flush.tstest-parity/node-suite/child_process/serialization/basic.tstest-parity/node-suite/child_process/sync/cwd-env-input.tstest-parity/node-suite/child_process/validation/null-bytes.tstest-parity/node-suite/child_process/validation/options.tstest-parity/node_suite_baseline.json
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@test-parity/node_suite_baseline.json`:
- Around line 7-10: Recompute the overall summary in the overall object from the
per-module totals: set pass to 2815 out of 2886 and update pct to the
corresponding floored percentage, replacing the stale 2824 and 97.9 values.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 4622f514-7dba-449f-bb0c-0685404dddb0
📒 Files selected for processing (21)
test-parity/node-suite/child_process/README.mdtest-parity/node-suite/child_process/async/cwd-env.tstest-parity/node-suite/child_process/async/exec-success.tstest-parity/node-suite/child_process/async/execfile-args.tstest-parity/node-suite/child_process/async/exit-codes.tstest-parity/node-suite/child_process/async/spawn-behavior.tstest-parity/node-suite/child_process/async/stdio-dataflow.tstest-parity/node-suite/child_process/async/stdio-flush.tstest-parity/node-suite/child_process/serialization/basic.tstest-parity/node-suite/child_process/sync/default-env.tstest-parity/node-suite/child_process/sync/encoding-aliases.tstest-parity/node-suite/child_process/sync/env-values.tstest-parity/node-suite/child_process/sync/error-shell.tstest-parity/node-suite/child_process/sync/execfile-args.tstest-parity/node-suite/child_process/sync/input-views.tstest-parity/node-suite/child_process/sync/maxbuffer-infinity.tstest-parity/node-suite/child_process/sync/optional-args-metadata.tstest-parity/node-suite/child_process/sync/options-immutable.tstest-parity/node-suite/child_process/sync/windows-hide.tstest-parity/node-suite/child_process/validation/options.tstest-parity/node_suite_baseline.json
🚧 Files skipped from review as they are similar to previous changes (1)
- test-parity/node-suite/child_process/README.md
Summary
node:child_processfixtures, expanding the suite from 26 to 53 casesCoverage added
ChildProcessconstructor; option types/ranges; fork overloads; stdio layouts; duplicate/sync IPC; kill signalswindowsHide; basic shell execution; Unicode and infinite maxBuffer; multi-chunk/streamed output above 64 KiBexec/execFilesuccess and promisified shapes; callback/stream lifecycle ordering; non-expiring timeout options; text/binary stdin; stream piping; fd3; bounded multi-chunk stdio flushing; WHATWG URL forkVerification
NPM_CONFIG_CACHE=/tmp/perry-npm-cache npx -y prettier@3.6.2 --write <changed files>— passedgit diff --check— passed./run_parity_tests.sh --suite node-suite --module child_processwith Node 26.5.0 — 42 pass, 10 output mismatches, 0 compile failures, 0 crashes, 1 pre-existing host Node failure, 53 fixtures (80.7% of executed cases)NODE_BIN="$(command -v node)" python3 scripts/node_suite_run.py "$PWD/target/release/perry" "$PWD" child_processwith Node 26.5.0 — 43/53 (81.1%),diff=10Diagnostic mismatches
Nine expanded fixtures expose stable differences:
ChildProcessinstances lack Node's prototype identity, initial state, and coded validation errorsexecFileinvokes its callback but returns noChildProcesshandle; promisifiedexec/execFileresults also omit.childcloseThe existing AbortSignal fixture remains the tenth mismatch and now also demonstrates Perry returning
truefromkill()after close where Node returnsfalse.Upstream scope and exclusions
Compared against Node.js
608112affae2cf44d2f8a0a6bfe7967193b459c8, Denof8a17c8171569fa2870d740030aaa59c91fdf9ee, and Bunc4fad462e7dc20e5e9780f848db42e1e2f52186d.Deferred to separate platform/slow assessments: fork/IPC races and handle transfer beyond deterministic URL/serialization probes; socket/server and cluster/dgram integration; exhaustive signal, UID/GID, detached and scheduler-sensitive behavior; Windows quoting/batch/overlapped stdio; custom-shell discovery; EMFILE/resource exhaustion; large IPC framing; host-object/circular serialization; and multi-megabyte stress.