From ead0c229cec71d2dca34f7cdd239511a759036fe Mon Sep 17 00:00:00 2001 From: Gibson Fahnestock Date: Tue, 15 Aug 2017 19:14:54 +0100 Subject: [PATCH] test: add common.envPlus() Add a helper function to provide an easy way to modify the environment passed to child processes. Fixes: https://github.com/nodejs/node/issues/14823 --- test/common/README.md | 6 ++++++ test/common/index.js | 4 ++++ test/parallel/test-benchmark-crypto.js | 7 ++++--- test/parallel/test-benchmark-timers.js | 7 +++---- test/parallel/test-child-process-env.js | 4 ++-- test/parallel/test-child-process-exec-env.js | 2 +- .../test-child-process-spawn-shell.js | 2 +- .../test-child-process-spawnsync-env.js | 4 ++-- .../test-child-process-spawnsync-shell.js | 2 +- test/parallel/test-cli-node-options.js | 4 ++-- test/parallel/test-crypto-fips.js | 21 ++++++------------- test/parallel/test-fs-readfile-error.js | 2 +- test/parallel/test-http-server-stale-close.js | 5 ++--- test/parallel/test-icu-data-dir.js | 2 +- test/parallel/test-inspector-open.js | 2 +- .../test-module-loading-globalpaths.js | 2 +- test/parallel/test-npm-install.js | 11 +++++----- test/parallel/test-pending-deprecation.js | 2 +- .../test-process-redirect-warnings-env.js | 2 +- test/parallel/test-repl-envvars.js | 2 +- test/parallel/test-require-symlink.js | 3 +-- test/parallel/test-stdin-script-child.js | 2 +- test/parallel/test-tls-env-bad-extra-ca.js | 2 +- test/parallel/test-tls-env-extra-ca.js | 2 +- test/sequential/test-benchmark-http.js | 3 +-- 25 files changed, 52 insertions(+), 53 deletions(-) diff --git a/test/common/README.md b/test/common/README.md index b8d9af2fcf70f5..ac1877d908a86c 100644 --- a/test/common/README.md +++ b/test/common/README.md @@ -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 +environment to a child process. + ### expectsError([fn, ]settings[, exact]) * `fn` [<Function>] a function that should throw. * `settings` [<Object>] diff --git a/test/common/index.js b/test/common/index.js index 8175474818b9dc..8d01c6c8984aec 100644 --- a/test/common/index.js +++ b/test/common/index.js @@ -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; diff --git a/test/parallel/test-benchmark-crypto.js b/test/parallel/test-benchmark-crypto.js index 5750a4e2afdadf..d02af4b2e9ac17 100644 --- a/test/parallel/test-benchmark-crypto.js +++ b/test/parallel/test-benchmark-crypto.js @@ -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); diff --git a/test/parallel/test-benchmark-timers.js b/test/parallel/test-benchmark-timers.js index 991ffda7186e72..12eedf027fca8a 100644 --- a/test/parallel/test-benchmark-timers.js +++ b/test/parallel/test-benchmark-timers.js @@ -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. @@ -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); diff --git a/test/parallel/test-child-process-env.js b/test/parallel/test-child-process-env.js index 65e2bb3f163a6a..c797233bf5d0ec 100644 --- a/test/parallel/test-child-process-env.js +++ b/test/parallel/test-child-process-env.js @@ -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 })); } diff --git a/test/parallel/test-child-process-exec-env.js b/test/parallel/test-child-process-exec-env.js index 3b1ef741e6c9b2..f13af9e1f75587 100644 --- a/test/parallel/test-child-process-exec-env.js +++ b/test/parallel/test-child-process-exec-env.js @@ -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'); diff --git a/test/parallel/test-child-process-spawn-shell.js b/test/parallel/test-child-process-spawn-shell.js index e9a753e91a4e48..244f61807cedcf 100644 --- a/test/parallel/test-child-process-spawn-shell.js +++ b/test/parallel/test-child-process-spawn-shell.js @@ -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 }); diff --git a/test/parallel/test-child-process-spawnsync-env.js b/test/parallel/test-child-process-spawnsync-env.js index c8e11b5067c7d1..4e531e57b620a1 100644 --- a/test/parallel/test-child-process-spawnsync-env.js +++ b/test/parallel/test-child-process-spawnsync-env.js @@ -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'); @@ -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); diff --git a/test/parallel/test-child-process-spawnsync-shell.js b/test/parallel/test-child-process-spawnsync-shell.js index f0838ae116e3be..34d9c9a7d2cc39 100644 --- a/test/parallel/test-child-process-spawnsync-shell.js +++ b/test/parallel/test-child-process-spawnsync-shell.js @@ -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 }); diff --git a/test/parallel/test-cli-node-options.js b/test/parallel/test-cli-node-options.js index eb31e280c3d47b..bd84ddf855fbdd 100644 --- a/test/parallel/test-cli-node-options.js +++ b/test/parallel/test-cli-node-options.js @@ -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`; @@ -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) { diff --git a/test/parallel/test-crypto-fips.js b/test/parallel/test-crypto-fips.js index d72918756a200b..6abe3c75cdb98c 100644 --- a/test/parallel/test-crypto-fips.js +++ b/test/parallel/test-crypto-fips.js @@ -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, { @@ -72,7 +63,7 @@ testHelper( [], FIPS_DISABLED, 'require("crypto").fips', - addToEnv('OPENSSL_CONF', '')); + common.envPlus({ 'OPENSSL_CONF': '' })); // --enable-fips should turn FIPS mode on testHelper( @@ -117,7 +108,7 @@ 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( @@ -125,7 +116,7 @@ if (!sharedOpenSSL()) { [`--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( @@ -133,7 +124,7 @@ testHelper( [`--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( @@ -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( @@ -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( diff --git a/test/parallel/test-fs-readfile-error.js b/test/parallel/test-fs-readfile-error.js index aa6e6bdecb0b08..9afa9ecbdc7354 100644 --- a/test/parallel/test-fs-readfile-error.js +++ b/test/parallel/test-fs-readfile-error.js @@ -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, ''); diff --git a/test/parallel/test-http-server-stale-close.js b/test/parallel/test-http-server-stale-close.js index 1f3c4595d4d251..be572dedeaf0ca 100644 --- a/test/parallel/test-http-server-stale-close.js +++ b/test/parallel/test-http-server-stale-close.js @@ -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) { @@ -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 }) }); diff --git a/test/parallel/test-icu-data-dir.js b/test/parallel/test-icu-data-dir.js index 1a97699ca51691..2b09e80a9f9fff 100644 --- a/test/parallel/test-icu-data-dir.js +++ b/test/parallel/test-icu-data-dir.js @@ -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)); } diff --git a/test/parallel/test-inspector-open.js b/test/parallel/test-inspector-open.js index e01d332e0348ba..6db24f0687e449 100644 --- a/test/parallel/test-inspector-open.js +++ b/test/parallel/test-inspector-open.js @@ -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'); diff --git a/test/parallel/test-module-loading-globalpaths.js b/test/parallel/test-module-loading-globalpaths.js index c12228ece763bf..8998b74953b74a 100644 --- a/test/parallel/test-module-loading-globalpaths.js +++ b/test/parallel/test-module-loading-globalpaths.js @@ -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. diff --git a/test/parallel/test-npm-install.js b/test/parallel/test-npm-install.js index 1faa698b15fc22..adb63e981949aa 100644 --- a/test/parallel/test-npm-install.js +++ b/test/parallel/test-npm-install.js @@ -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, diff --git a/test/parallel/test-pending-deprecation.js b/test/parallel/test-pending-deprecation.js index fedc7c96b83e6b..cb7d3d15754e1c 100644 --- a/test/parallel/test-pending-deprecation.js +++ b/test/parallel/test-pending-deprecation.js @@ -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')); diff --git a/test/parallel/test-process-redirect-warnings-env.js b/test/parallel/test-process-redirect-warnings-env.js index 6878024e7955b7..35f0ec0b251f93 100644 --- a/test/parallel/test-process-redirect-warnings-env.js +++ b/test/parallel/test-process-redirect-warnings-env.js @@ -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); diff --git a/test/parallel/test-repl-envvars.js b/test/parallel/test-repl-envvars.js index c4efd184c4c91c..5545d1b4ea0ea5 100644 --- a/test/parallel/test-repl-envvars.js +++ b/test/parallel/test-repl-envvars.js @@ -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, diff --git a/test/parallel/test-require-symlink.js b/test/parallel/test-require-symlink.js index 8f15845947ca67..7c00072ae1101c 100644 --- a/test/parallel/test-require-symlink.js +++ b/test/parallel/test-require-symlink.js @@ -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(); @@ -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); diff --git a/test/parallel/test-stdin-script-child.js b/test/parallel/test-stdin-script-child.js index 6e37fad9414de4..706d3d850c0e8e 100644 --- a/test/parallel/test-stdin-script-child.js +++ b/test/parallel/test-stdin-script-child.js @@ -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] }) }); diff --git a/test/parallel/test-tls-env-bad-extra-ca.js b/test/parallel/test-tls-env-bad-extra-ca.js index 5c6e47d52a654a..8806adce6ac989 100644 --- a/test/parallel/test-tls-env-bad-extra-ca.js +++ b/test/parallel/test-tls-env-bad-extra-ca.js @@ -21,7 +21,7 @@ const env = Object.assign({}, process.env, { }); const opts = { - env: env, + env: common.envPlus(env), silent: true, }; let stderr = ''; diff --git a/test/parallel/test-tls-env-extra-ca.js b/test/parallel/test-tls-env-extra-ca.js index 80d9cc9ec3ecef..91d9b97402d662 100644 --- a/test/parallel/test-tls-env-extra-ca.js +++ b/test/parallel/test-tls-env-extra-ca.js @@ -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') diff --git a/test/sequential/test-benchmark-http.js b/test/sequential/test-benchmark-http.js index b8d47fb8980db1..42e9307c599dfc 100644 --- a/test/sequential/test-benchmark-http.js +++ b/test/sequential/test-benchmark-http.js @@ -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',