Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork 36.4k
test_runner: add env option to run function#61367
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Uh oh!
There was an error while loading. Please reload this page.
Changes from all commits
File filter
Filter by extension
Conversations
Uh oh!
There was an error while loading. Please reload this page.
Jump to
Uh oh!
There was an error while loading. Please reload this page.
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -403,7 +403,7 @@ function runTestFile(path, filesWatcher, opts) { | ||||||
| const subtest = opts.root.createSubtest(FileTest, testPath, testOpts, async (t) => { | ||||||
| const args = getRunArgs(path, opts); | ||||||
| const stdio = ['pipe', 'pipe', 'pipe']; | ||||||
| const env = { __proto__: null, ...process.env, NODE_TEST_CONTEXT: 'child-v8' }; | ||||||
| const env = { __proto__: null, NODE_TEST_CONTEXT: 'child-v8', ...(opts.env || process.env) }; | ||||||
Member There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
nitpic | ||||||
| if (watchMode) { | ||||||
| stdio.push('ipc'); | ||||||
| env.WATCH_REPORT_DEPENDENCIES = '1'; | ||||||
| @@ -610,6 +610,7 @@ function run(options = kEmptyObject) { | ||||||
| argv = [], | ||||||
| cwd = process.cwd(), | ||||||
| rerunFailuresFilePath, | ||||||
| env, | ||||||
| } = options; | ||||||
| if (files != null) { | ||||||
| @@ -718,6 +719,14 @@ function run(options = kEmptyObject) { | ||||||
| validatePath(globalSetupPath, 'options.globalSetupPath'); | ||||||
| } | ||||||
| if (env != null) { | ||||||
| validateObject(env); | ||||||
| if (isolation === 'none') { | ||||||
| throw new ERR_INVALID_ARG_VALUE('options.env', env, 'is not supported with isolation=\'none\''); | ||||||
| } | ||||||
| } | ||||||
| const rootTestOptions = { __proto__: null, concurrency, timeout, signal }; | ||||||
| const globalOptions = { | ||||||
| __proto__: null, | ||||||
| @@ -763,6 +772,7 @@ function run(options = kEmptyObject) { | ||||||
| argv, | ||||||
| execArgv, | ||||||
| rerunFailuresFilePath, | ||||||
| env, | ||||||
| }; | ||||||
| if (isolation === 'process') { | ||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,7 @@ | ||
| const { test } = require('node:test'); | ||
| test('process.env is correct', (t) => { | ||
| t.assert.strictEqual(process.env.ABC, undefined, 'main process env var should be undefined'); | ||
| t.assert.strictEqual(process.env.NODE_TEST_CONTEXT, 'child-v8', 'NODE_TEST_CONTEXT should be set by run()'); | ||
| t.assert.strictEqual(process.env.FOOBAR, 'FUZZBUZZ', 'specified env var should be defined'); | ||
| }); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -650,6 +650,29 @@ describe('require(\'node:test\').run', { concurrency: true }, () => { | ||
| }); | ||
| }); | ||
| describe('env', () => { | ||
| it('should allow env variables to be configured', async () => { | ||
| // Need to inherit some process.env variables so it runs reliably across different environments. | ||
| const env = { ...process.env, FOOBAR: 'FUZZBUZZ' }; | ||
| // Set a variable on main process env and test it does not exist within test env. | ||
| process.env.ABC = 'XYZ'; | ||
| const stream = run({ files: [join(testFixtures, 'process-env.js')], env }); | ||
| stream.on('test:fail', common.mustNotCall()); | ||
| stream.on('test:pass', common.mustCall(1)); | ||
| // eslint-disable-next-line no-unused-vars | ||
| for await (const _ of stream); | ||
| delete process.env.ABC; | ||
| }); | ||
Ethan-Arrowood marked this conversation as resolved.
Uh oh!There was an error while loading. Please reload this page. | ||
| it('should throw error when env is specified with isolation=none', async () => { | ||
| assert.throws(() => run({ env: { foo: 'bar' }, isolation: 'none' }), { | ||
| code: 'ERR_INVALID_ARG_VALUE', | ||
| message: /The property 'options\.env' is not supported with isolation='none'\. Received { foo: 'bar' }/ | ||
| }); | ||
| }); | ||
| }); | ||
| describe('forceExit', () => { | ||
| it('throws for non-boolean values', () => { | ||
| [Symbol(), {}, 0, 1, '1', Promise.resolve([])].forEach((forceExit) => { | ||
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
env vars are supposed to be inherited, what is the usecase here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
They still are by default; this feature is essentially just bubbling up the underlying
child_process.spawn()envoption.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you are using the
run()method multiple times to spawn multiple test processes. And want to be able to specify unique environment variables for each; there currently is no way to do so.Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Ethan-Arrowood The docs here are missing the default value.
I created a doc PR here:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch!