Skip to content
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

test: pass env through to child processes #14845

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions test/common/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,12 @@ Platform normalizes the `dd` command

Check if there is more than 1gb of total memory.

### envPlus(additionalEnv)
* return [<Object>]

Returns `process.env` plus `additionalEnv`. Used to pass a temporarily modified
Copy link
Member

Choose a reason for hiding this comment

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

I think the word temporarily is misguiding here. The environment of the child process is modified for its entire lifetime, and the enviroment of the calling process is left unchanged.

environment to a child process.

### expectsError([fn, ]settings[, exact])
* `fn` [<Function>] a function that should throw.
* `settings` [<Object>]
Expand Down
4 changes: 4 additions & 0 deletions test/common/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -182,6 +182,10 @@ if (exports.isLinux) {
];
}

exports.envPlus = function(additionalEnv) {
return Object.assign({}, process.env, additionalEnv);
};

Object.defineProperty(exports, 'inFreeBSDJail', {
get: function() {
if (inFreeBSDJail !== null) return inFreeBSDJail;
Expand Down
7 changes: 4 additions & 3 deletions test/parallel/test-benchmark-crypto.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,10 @@ const argv = ['--set', 'algo=sha256',
'--set', 'v=crypto',
'--set', 'writes=1',
'crypto'];
const env = Object.assign({}, process.env,
{ NODEJS_BENCHMARK_ZERO_ALLOWED: 1 });
const child = fork(runjs, argv, { env });

const child = fork(runjs, argv, { env: common.envPlus({
NODEJS_BENCHMARK_ZERO_ALLOWED: 1 }) });

child.on('exit', (code, signal) => {
assert.strictEqual(code, 0);
assert.strictEqual(signal, null);
Expand Down
7 changes: 3 additions & 4 deletions test/parallel/test-benchmark-timers.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
'use strict';

require('../common');
const common = require('../common');

// Minimal test for timers benchmarks. This makes sure the benchmarks aren't
// horribly broken but nothing more than that.
Expand All @@ -15,10 +15,9 @@ const argv = ['--set', 'type=depth',
'--set', 'thousands=0.001',
'timers'];

const env = Object.assign({}, process.env,
{ NODEJS_BENCHMARK_ZERO_ALLOWED: 1 });
const child = fork(runjs, argv, { env: common.envPlus({
NODEJS_BENCHMARK_ZERO_ALLOWED: 1 }) });

const child = fork(runjs, argv, { env });
child.on('exit', (code, signal) => {
assert.strictEqual(code, 0);
assert.strictEqual(signal, null);
Expand Down
4 changes: 2 additions & 2 deletions test/parallel/test-child-process-env.js
Original file line number Diff line number Diff line change
Expand Up @@ -34,9 +34,9 @@ Object.setPrototypeOf(env, {

let child;
if (common.isWindows) {
child = spawn('cmd.exe', ['/c', 'set'], { env: env });
child = spawn('cmd.exe', ['/c', 'set'], common.envPlus({ env: env }));
} else {
child = spawn('/usr/bin/env', [], { env: env });
child = spawn('/usr/bin/env', [], common.envPlus({ env: env }));
}


Expand Down
2 changes: 1 addition & 1 deletion test/parallel/test-child-process-exec-env.js
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ function after(err, stdout, stderr) {
if (!common.isWindows) {
child = exec('/usr/bin/env', { env: { 'HELLO': 'WORLD' } }, after);
} else {
child = exec('set', { env: { 'HELLO': 'WORLD' } }, after);
child = exec('set', { env: common.envPlus({ 'HELLO': 'WORLD' }) }, after);
}

child.stdout.setEncoding('utf8');
Expand Down
2 changes: 1 addition & 1 deletion test/parallel/test-child-process-spawn-shell.js
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ command.on('close', common.mustCall((code, signal) => {

// Verify that the environment is properly inherited
const env = cp.spawn(`"${process.execPath}" -pe process.env.BAZ`, {
env: Object.assign({}, process.env, { BAZ: 'buzz' }),
env: common.envPlus({ BAZ: 'buzz' }),
encoding: 'utf8',
shell: true
});
Expand Down
4 changes: 2 additions & 2 deletions test/parallel/test-child-process-spawnsync-env.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
// USE OR OTHER DEALINGS IN THE SOFTWARE.

'use strict';
require('../common');
const common = require('../common');
const assert = require('assert');
const cp = require('child_process');

Expand All @@ -29,7 +29,7 @@ if (process.argv[2] === 'child') {
} else {
const expected = 'bar';
const child = cp.spawnSync(process.execPath, [__filename, 'child'], {
env: Object.assign(process.env, { foo: expected })
env: common.envPlus({ foo: expected })
});

assert.strictEqual(child.stdout.toString().trim(), expected);
Expand Down
2 changes: 1 addition & 1 deletion test/parallel/test-child-process-spawnsync-shell.js
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ assert.strictEqual(command.stdout.toString().trim(), 'bar');

// Verify that the environment is properly inherited
const env = cp.spawnSync(`"${process.execPath}" -pe process.env.BAZ`, {
env: Object.assign({}, process.env, { BAZ: 'buzz' }),
env: common.envPlus({ BAZ: 'buzz' }),
shell: true
});

Expand Down
4 changes: 2 additions & 2 deletions test/parallel/test-cli-node-options.js
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ disallow('--');
disallow('--no_warnings'); // Node options don't allow '_' instead of '-'.

function disallow(opt) {
const options = { env: { NODE_OPTIONS: opt } };
const options = { env: common.envPlus({ NODE_OPTIONS: opt }) };
exec(process.execPath, options, common.mustCall(function(err) {
const message = err.message.split(/\r?\n/)[1];
const expect = `${process.execPath}: ${opt} is not allowed in NODE_OPTIONS`;
Expand Down Expand Up @@ -71,7 +71,7 @@ function expect(opt, want) {
const printB = require.resolve('../fixtures/printB.js');
const argv = [printB];
const opts = {
env: { NODE_OPTIONS: opt },
env: common.envPlus({ NODE_OPTIONS: opt }),
maxBuffer: 1000000000,
};
exec(process.execPath, argv, opts, common.mustCall(function(err, stdout) {
Expand Down
21 changes: 6 additions & 15 deletions test/parallel/test-crypto-fips.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,15 +26,6 @@ function sharedOpenSSL() {
return process.config.variables.node_shared_openssl;
}

function addToEnv(newVar, value) {
const envCopy = {};
for (const e in process.env) {
envCopy[e] = process.env[e];
}
envCopy[newVar] = value;
return envCopy;
}

function testHelper(stream, args, expectedOutput, cmd, env) {
const fullArgs = args.concat(['-e', `console.log(${cmd})`]);
const child = spawnSync(process.execPath, fullArgs, {
Expand Down Expand Up @@ -72,7 +63,7 @@ testHelper(
[],
FIPS_DISABLED,
'require("crypto").fips',
addToEnv('OPENSSL_CONF', ''));
common.envPlus({ 'OPENSSL_CONF': '' }));
Copy link
Member

Choose a reason for hiding this comment

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

You could as well drop the quotes here and below.


// --enable-fips should turn FIPS mode on
testHelper(
Expand Down Expand Up @@ -117,23 +108,23 @@ if (!sharedOpenSSL()) {
[],
compiledWithFips() ? FIPS_ENABLED : FIPS_DISABLED,
'require("crypto").fips',
addToEnv('OPENSSL_CONF', CNF_FIPS_ON));
common.envPlus({ 'OPENSSL_CONF': CNF_FIPS_ON }));

// --openssl-config option should override OPENSSL_CONF
testHelper(
'stdout',
[`--openssl-config=${CNF_FIPS_ON}`],
compiledWithFips() ? FIPS_ENABLED : FIPS_DISABLED,
'require("crypto").fips',
addToEnv('OPENSSL_CONF', CNF_FIPS_OFF));
common.envPlus({ 'OPENSSL_CONF': CNF_FIPS_OFF }));
}

testHelper(
'stdout',
[`--openssl-config=${CNF_FIPS_OFF}`],
FIPS_DISABLED,
'require("crypto").fips',
addToEnv('OPENSSL_CONF', CNF_FIPS_ON));
common.envPlus({ 'OPENSSL_CONF': CNF_FIPS_ON }));

// --enable-fips should take precedence over OpenSSL config file
testHelper(
Expand All @@ -149,7 +140,7 @@ testHelper(
['--enable-fips'],
compiledWithFips() ? FIPS_ENABLED : OPTION_ERROR_STRING,
'require("crypto").fips',
addToEnv('OPENSSL_CONF', CNF_FIPS_OFF));
common.envPlus({ 'OPENSSL_CONF': CNF_FIPS_OFF }));

// --force-fips should take precedence over OpenSSL config file
testHelper(
Expand All @@ -165,7 +156,7 @@ testHelper(
['--force-fips'],
compiledWithFips() ? FIPS_ENABLED : OPTION_ERROR_STRING,
'require("crypto").fips',
addToEnv('OPENSSL_CONF', CNF_FIPS_OFF));
common.envPlus({ 'OPENSSL_CONF': CNF_FIPS_OFF }));

// setFipsCrypto should be able to turn FIPS mode on
testHelper(
Expand Down
2 changes: 1 addition & 1 deletion test/parallel/test-fs-readfile-error.js
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ const fixtures = require('../common/fixtures');
function test(env, cb) {
const filename = fixtures.path('test-fs-readfile-error.js');
const execPath = `"${process.execPath}" "${filename}"`;
const options = { env: Object.assign(process.env, env) };
const options = { env: common.envPlus(env) };
exec(execPath, options, common.mustCall((err, stdout, stderr) => {
assert(err);
assert.strictEqual(stdout, '');
Expand Down
5 changes: 2 additions & 3 deletions test/parallel/test-http-server-stale-close.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,8 @@
// USE OR OTHER DEALINGS IN THE SOFTWARE.

'use strict';
require('../common');
const common = require('../common');
const http = require('http');
const util = require('util');
const fork = require('child_process').fork;

if (process.env.NODE_TEST_FORK_PORT) {
Expand All @@ -45,7 +44,7 @@ if (process.env.NODE_TEST_FORK_PORT) {
});
server.listen(0, function() {
fork(__filename, {
env: util._extend(process.env, {
env: common.envPlus({
NODE_TEST_FORK_PORT: this.address().port
})
});
Expand Down
2 changes: 1 addition & 1 deletion test/parallel/test-icu-data-dir.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ const expected =
}

{
const env = { NODE_ICU_DATA: '/' };
const env = common.envPlus({ NODE_ICU_DATA: '/' });
const child = spawnSync(process.execPath, ['-e', '0'], { env });
assert(child.stderr.toString().includes(expected));
}
2 changes: 1 addition & 1 deletion test/parallel/test-inspector-open.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ const url = require('url');
if (process.env.BE_CHILD)
return beChild();

const child = fork(__filename, { env: { BE_CHILD: 1 } });
const child = fork(__filename, { env: common.envPlus({ BE_CHILD: 1 }) });

child.once('message', common.mustCall((msg) => {
assert.strictEqual(msg.cmd, 'started');
Expand Down
2 changes: 1 addition & 1 deletion test/parallel/test-module-loading-globalpaths.js
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ if (process.argv[2] === 'child') {
const testFixturesDir = path.join(common.fixturesDir,
path.basename(__filename, '.js'));

const env = Object.assign({}, process.env);
const env = common.envPlus({});
// Turn on module debug to aid diagnosing failures.
env['NODE_DEBUG'] = 'module';
// Unset NODE_PATH.
Expand Down
11 changes: 6 additions & 5 deletions test/parallel/test-npm-install.js
Original file line number Diff line number Diff line change
Expand Up @@ -39,11 +39,12 @@ const pkgPath = path.join(installDir, 'package.json');

fs.writeFileSync(pkgPath, pkgContent);

const env = Object.create(process.env);
env['PATH'] = path.dirname(process.execPath);
env['NPM_CONFIG_PREFIX'] = path.join(npmSandbox, 'npm-prefix');
env['NPM_CONFIG_TMP'] = path.join(npmSandbox, 'npm-tmp');
env['HOME'] = path.join(npmSandbox, 'home');
const env = common.envPlus({
PATH: path.dirname(process.execPath),
NPM_CONFIG_PREFIX: path.join(npmSandbox, 'npm-prefix'),
NPM_CONFIG_TMP: path.join(npmSandbox, 'npm-tmp'),
HOME: path.join(npmSandbox, 'home'),
});

const proc = spawn(process.execPath, args, {
cwd: installDir,
Expand Down
2 changes: 1 addition & 1 deletion test/parallel/test-pending-deprecation.js
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ switch (process.argv[2]) {

// Test the NODE_PENDING_DEPRECATION environment var.
fork(__filename, ['env'], {
env: { NODE_PENDING_DEPRECATION: 1 },
env: common.envPlus({ NODE_PENDING_DEPRECATION: 1 }),
silent: true
}).on('exit', common.mustCall((code) => {
assert.strictEqual(code, 0, message('NODE_PENDING_DEPRECATION'));
Expand Down
2 changes: 1 addition & 1 deletion test/parallel/test-process-redirect-warnings-env.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ common.refreshTmpDir();
const warnmod = require.resolve(`${common.fixturesDir}/warnings.js`);
const warnpath = path.join(common.tmpDir, 'warnings.txt');

fork(warnmod, { env: { NODE_REDIRECT_WARNINGS: warnpath } })
fork(warnmod, { env: common.envPlus({ NODE_REDIRECT_WARNINGS: warnpath }) })
.on('exit', common.mustCall(() => {
fs.readFile(warnpath, 'utf8', common.mustCall((err, data) => {
assert.ifError(err);
Expand Down
2 changes: 1 addition & 1 deletion test/parallel/test-repl-envvars.js
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ const tests = [
];

function run(test) {
const env = test.env;
const env = common.envPlus(test.env);
const expected = test.expected;
const opts = {
terminal: true,
Expand Down
3 changes: 1 addition & 2 deletions test/parallel/test-require-symlink.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ const assert = require('assert');
const path = require('path');
const fs = require('fs');
const { exec, spawn } = require('child_process');
const util = require('util');
const fixtures = require('../common/fixtures');

common.refreshTmpDir();
Expand Down Expand Up @@ -61,7 +60,7 @@ function test() {

// Also verify that symlinks works for setting preserve via env variables
const childEnv = spawn(node, [linkScript], {
env: util._extend(process.env, { NODE_PRESERVE_SYMLINKS: '1' })
env: common.envPlus({ NODE_PRESERVE_SYMLINKS: '1' })
});
childEnv.on('close', function(code, signal) {
assert.strictEqual(code, 0);
Expand Down
2 changes: 1 addition & 1 deletion test/parallel/test-stdin-script-child.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ const assert = require('assert');
const { spawn } = require('child_process');
for (const args of [[], ['-']]) {
const child = spawn(process.execPath, args, {
env: Object.assign(process.env, {
env: common.envPlus({
NODE_DEBUG: process.argv[2]
})
});
Expand Down
2 changes: 1 addition & 1 deletion test/parallel/test-tls-env-bad-extra-ca.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ const env = Object.assign({}, process.env, {
});

const opts = {
env: env,
env: common.envPlus(env),
silent: true,
};
let stderr = '';
Expand Down
2 changes: 1 addition & 1 deletion test/parallel/test-tls-env-extra-ca.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ const server = tls.createServer(options, common.mustCall(function(s) {
s.end('bye');
server.close();
})).listen(0, common.mustCall(function() {
const env = Object.assign({}, process.env, {
const env = common.envPlus({
CHILD: 'yes',
PORT: this.address().port,
NODE_EXTRA_CA_CERTS: fixtures.path('keys', 'ca1-cert.pem')
Expand Down
3 changes: 1 addition & 2 deletions test/sequential/test-benchmark-http.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,7 @@ const path = require('path');

const runjs = path.join(__dirname, '..', '..', 'benchmark', 'run.js');

const env = Object.assign({}, process.env,
{ NODEJS_BENCHMARK_ZERO_ALLOWED: 1 });
const env = common.envPlus({ NODEJS_BENCHMARK_ZERO_ALLOWED: 1 });

const child = fork(runjs, ['--set', 'benchmarker=test-double',
'--set', 'c=1',
Expand Down