test_runner: add env option to run function #61367
Conversation
|
Review requested:
|
nodejs-github-bot
added
needs-ci
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #61367 +/- ##
==========================================
+ Coverage 88.51% 88.53% +0.01%
==========================================
Files 704 704
Lines 208739 208803 +64
Branches 40274 40310 +36
==========================================
+ Hits 184770 184857 +87
+ Misses 15968 15925 -43
- Partials 8001 8021 +20
🚀 New features to boost your workflow:
|
Ethan-Arrowood
force-pushed
the
test-runner-env-option
branch
from
724b84e to
cd4d84e
Compare
Ethan-Arrowood
requested review from
avivkeller,
ljharb,
mcollina and
pmarchini
lib/internal/test_runner/runner.js
Outdated
| const args = getRunArgs(path, opts); | ||
| const stdio = ['pipe', 'pipe', 'pipe']; | ||
| const env = { __proto__: null, ...process.env, NODE_TEST_CONTEXT: 'child-v8' }; | ||
| const env = opts.env || { __proto__: null, ...process.env, NODE_TEST_CONTEXT: 'child-v8' }; |
There was a problem hiding this comment.
NODE_TEST_CONTEXT is used by the test runner to identify the runner subprocesses, and removing it may cause issues with the reporters.
It must be preserved even if the user passes opts.env.
I would suggest, in any case, adding a snapshot test when using this option to cover the E2E once the option has been provided!
There was a problem hiding this comment.
Okay, I'm going to switch this line to this: const env = { __proto__: null, NODE_TEST_CONTEXT: 'child-v8', ...(opts.env || process.env) }; so that its configured by default, but is overridable if someone wanted to. I believe currently there is a subtle bug here where even if you specified the NODE_TEST_CONTEXT variable in the main process, it'd be overridden to child-v8 no matter what.
There was a problem hiding this comment.
I don't fully understand the ask here:
I would suggest, in any case, adding a snapshot test when using this option to cover the E2E once the option has been provided!
Snapshot of what? The environment variables? Do you mean for me to just assert that something such as NODE_TEST_CONTEXT is retained when env option is specified, or something more in depth?
doc/api/test.md
Outdated
| * `functionCoverage` {number} Require a minimum percent of covered functions. If code | ||
| coverage does not reach the threshold specified, the process will exit with code `1`. | ||
| **Default:** `0`. | ||
| * `env` {Object} Specify environment variables to be passed along to the test process. This options is not compatible with `isolation='none'`. |
There was a problem hiding this comment.
I think it might be worth specifying here that these variables will override those from the main process
Support an `env` option that is passed to the underlying child_process. Fixes: nodejs#60709
Ethan-Arrowood
force-pushed
the
test-runner-env-option
branch
from
cd4d84e to
315e0bb
Compare
6 participants
Support an
envoption that is passed to theunderlying child_process.
Fixes: #60709