Skip to content

test_runner: capture process state in entry point instead of runner - #64446

Open
krsnaSuraj wants to merge 1 commit into
nodejs:mainfrom
krsnaSuraj:fix/53867-capture-process-state-in-entry-point
Open

test_runner: capture process state in entry point instead of runner#64446
krsnaSuraj wants to merge 1 commit into
nodejs:mainfrom
krsnaSuraj:fix/53867-capture-process-state-in-entry-point

Conversation

@krsnaSuraj

@krsnaSuraj krsnaSuraj commented Jul 12, 2026

Copy link
Copy Markdown

This is a fix for #53867.

The run() function in the test runner directly accesses process.execArgv to find V8-only flags like --expose-gc that need propagation to child processes. This is problematic because run() is part of the public API exposed to end users via node:test and should not depend on ambient process state.

This PR moves those reads to the CLI entry point (test_runner.js) and passes them as explicit options to run():

  • lib/internal/main/test_runner.js: capture process.env and process.cwd() and pass them to run() as options
  • lib/internal/test_runner/runner.js (getRunArgs): use the execArgv parameter when non-empty, falling back to process.execArgv when execArgv is empty for backward compatibility
  • lib/internal/test_runner/runner.js (runTestFile): use opts.env ?? {} instead of opts.env || process.env so the env precedence is driven by the options object being set
  • lib/internal/test_runner/runner.js (run): default env to process.env and check 'env' in options for validation

This ensures that:

  1. The CLI entry point captures the relevant process state once
  2. run() can be called with explicit options by end users
  3. Existing behavior is preserved -- when run() is called without execArgv, it still falls back to process.execArgv for V8 flag detection

Copilot AI review requested due to automatic review settings July 12, 2026 06:55

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@nodejs-github-bot nodejs-github-bot added needs-ci PRs that need a full CI run. test_runner Issues and PRs related to the test runner subsystem. labels Jul 12, 2026
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/test_runner

@krsnaSuraj
krsnaSuraj force-pushed the fix/53867-capture-process-state-in-entry-point branch 2 times, most recently from b533d0d to 6b2cb24 Compare July 12, 2026 07:13
@zmzlois

zmzlois commented Aug 9, 2026

Copy link
Copy Markdown

hi @krsnaSuraj mind if fix the merge conflicts ?

Move process.* reads (process.argv, process.env, process.cwd) from
runner.js to the CLI entry point (main/test_runner.js), so that runner.js
can be loaded without side effects in snapshot environments.

The execArgv case is handled via a fallback in getRunArgs(): when no
execArgv is passed in options, process.execArgv is still read directly
to propagate V8-only flags to child processes.

PR-URL: https://github.com/nodejs/node/pull/XXXXX
Refs: nodejs#56131
@krsnaSuraj
krsnaSuraj force-pushed the fix/53867-capture-process-state-in-entry-point branch from 6b2cb24 to b4afa69 Compare August 9, 2026 16:29
@krsnaSuraj

Copy link
Copy Markdown
Author

Thanks @zmzlois for the ping! πŸ™

I've resolved the merge conflicts by rebasing onto the latest main:

  • Rebased the PR branch onto upstream/main (single clean commit: b4afa693e6)
  • Resolved the conflict in lib/internal/test_runner/runner.js β€” kept the PR's options-driven env validation ('env' in options) while adopting upstream's validateObject(env, 'options.env') signature and error label
  • Verified: git diff upstream/main...HEAD shows only the two intended files (8 insertions, 5 deletions)
  • PR is now MERGEABLE (no conflicts)

Happy to adjust anything the reviewers flag β€” thanks for the review! πŸ™Œ

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs-ci PRs that need a full CI run. test_runner Issues and PRs related to the test runner subsystem.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants