From 8569f4a4178d1a114a95aabcb27cb30cb265e621 Mon Sep 17 00:00:00 2001 From: Refael Ackermann Date: Sat, 25 Aug 2018 12:23:53 -0400 Subject: [PATCH] test: refacor spawn[Sync]Pwd * extract the gist into common.pwdCommand * Merge test-child-process-buffering.js into test-child-process-stdio.js PR-URL: https://github.com/nodejs/node/pull/22522 Reviewed-By: James M Snell Reviewed-By: Anna Henningsen --- test/common/README.md | 23 ++++--- test/common/index.js | 20 ++---- test/parallel/test-child-process-buffering.js | 51 --------------- .../parallel/test-child-process-custom-fds.js | 18 +++--- test/parallel/test-child-process-cwd.js | 55 +++++++--------- test/parallel/test-child-process-spawnsync.js | 27 ++++---- test/parallel/test-child-process-stdio.js | 62 ++++++++++++++----- 7 files changed, 108 insertions(+), 148 deletions(-) delete mode 100644 test/parallel/test-child-process-buffering.js diff --git a/test/common/README.md b/test/common/README.md index 36f3afb5c9a465..28d9bd04f0a8b6 100644 --- a/test/common/README.md +++ b/test/common/README.md @@ -317,6 +317,17 @@ A port number for tests to use if one is needed. Logs '1..0 # Skipped: ' + `msg` +### pwdCommand +* [<array>] First two argument for the `spawn`/`exec` functions. + +Platform normalized `pwd` command options. Usage example: +```js +const common = require('../common'); +const { spawn } = require('child_process'); + +spawn(...common.pwdCommand, { stdio: ['pipe'] }); +``` + ### rootDir * [<string>] @@ -350,18 +361,6 @@ was disabled at compile time. Skip the rest of the tests in the current file when the Node.js executable was compiled with a pointer size smaller than 64 bits. -### spawnPwd(options) -* `options` [<Object>] -* return [<Object>] - -Platform normalizes the `pwd` command. - -### spawnSyncPwd(options) -* `options` [<Object>] -* return [<Object>] - -Synchronous version of `spawnPwd`. - ## ArrayStream Module The `ArrayStream` module provides a simple `Stream` that pushes elements from diff --git a/test/common/index.js b/test/common/index.js index 18f102c5c3a637..6f37cb734a4240 100644 --- a/test/common/index.js +++ b/test/common/index.js @@ -26,7 +26,7 @@ const path = require('path'); const fs = require('fs'); const assert = require('assert'); const os = require('os'); -const { exec, execSync, spawn, spawnSync } = require('child_process'); +const { exec, execSync, spawnSync } = require('child_process'); const util = require('util'); const { fixturesDir } = require('./fixtures'); const tmpdir = require('./tmpdir'); @@ -268,22 +268,10 @@ exports.ddCommand = function(filename, kilobytes) { }; -exports.spawnPwd = function(options) { - if (exports.isWindows) { - return spawn('cmd.exe', ['/d', '/c', 'cd'], options); - } else { - return spawn('pwd', [], options); - } -}; - +exports.pwdCommand = exports.isWindows ? + ['cmd.exe', ['/d', '/c', 'cd']] : + ['pwd', []]; -exports.spawnSyncPwd = function(options) { - if (exports.isWindows) { - return spawnSync('cmd.exe', ['/d', '/c', 'cd'], options); - } else { - return spawnSync('pwd', [], options); - } -}; exports.platformTimeout = function(ms) { if (process.features.debug) diff --git a/test/parallel/test-child-process-buffering.js b/test/parallel/test-child-process-buffering.js deleted file mode 100644 index d37d255e18b007..00000000000000 --- a/test/parallel/test-child-process-buffering.js +++ /dev/null @@ -1,51 +0,0 @@ -// Copyright Joyent, Inc. and other Node contributors. -// -// Permission is hereby granted, free of charge, to any person obtaining a -// copy of this software and associated documentation files (the -// "Software"), to deal in the Software without restriction, including -// without limitation the rights to use, copy, modify, merge, publish, -// distribute, sublicense, and/or sell copies of the Software, and to permit -// persons to whom the Software is furnished to do so, subject to the -// following conditions: -// -// The above copyright notice and this permission notice shall be included -// in all copies or substantial portions of the Software. -// -// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS -// OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF -// MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN -// NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, -// DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR -// OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE -// USE OR OTHER DEALINGS IN THE SOFTWARE. - -'use strict'; -const common = require('../common'); -const assert = require('assert'); - -function pwd(callback) { - let output = ''; - const child = common.spawnPwd(); - - child.stdout.setEncoding('utf8'); - child.stdout.on('data', function(s) { - console.log(`stdout: ${JSON.stringify(s)}`); - output += s; - }); - - child.on('exit', common.mustCall(function(c) { - console.log(`exit: ${c}`); - assert.strictEqual(0, c); - })); - - child.on('close', common.mustCall(function() { - callback(output); - })); -} - - -pwd(function(result) { - console.dir(result); - assert.strictEqual(true, result.length > 1); - assert.strictEqual('\n', result[result.length - 1]); -}); diff --git a/test/parallel/test-child-process-custom-fds.js b/test/parallel/test-child-process-custom-fds.js index c3146564764295..1c89c207d37f99 100644 --- a/test/parallel/test-child-process-custom-fds.js +++ b/test/parallel/test-child-process-custom-fds.js @@ -2,19 +2,22 @@ 'use strict'; const common = require('../common'); const assert = require('assert'); +const { spawnSync } = require('child_process'); const internalCp = require('internal/child_process'); -const oldSpawnSync = internalCp.spawnSync; if (!common.isMainThread) common.skip('stdio is not associated with file descriptors in Workers'); +// This test uses the deprecated `customFds` option. We expect a deprecation +// warning, but only once (per node process). +const msg = 'child_process: options.customFds option is deprecated. ' + + 'Use options.stdio instead.'; +common.expectWarning('DeprecationWarning', msg, 'DEP0006'); + // Verify that customFds is used if stdio is not provided. { - const msg = 'child_process: options.customFds option is deprecated. ' + - 'Use options.stdio instead.'; - common.expectWarning('DeprecationWarning', msg, 'DEP0006'); - const customFds = [-1, process.stdout.fd, process.stderr.fd]; + const oldSpawnSync = internalCp.spawnSync; internalCp.spawnSync = common.mustCall(function(opts) { assert.deepStrictEqual(opts.options.customFds, customFds); assert.deepStrictEqual(opts.options.stdio, [ @@ -23,13 +26,14 @@ if (!common.isMainThread) { type: 'fd', fd: process.stderr.fd } ]); }); - common.spawnSyncPwd({ customFds }); + spawnSync(...common.pwdCommand, { customFds }); internalCp.spawnSync = oldSpawnSync; } // Verify that customFds is ignored when stdio is present. { const customFds = [0, 1, 2]; + const oldSpawnSync = internalCp.spawnSync; internalCp.spawnSync = common.mustCall(function(opts) { assert.deepStrictEqual(opts.options.customFds, customFds); assert.deepStrictEqual(opts.options.stdio, [ @@ -38,6 +42,6 @@ if (!common.isMainThread) { type: 'pipe', readable: false, writable: true } ]); }); - common.spawnSyncPwd({ customFds, stdio: 'pipe' }); + spawnSync(...common.pwdCommand, { customFds, stdio: 'pipe' }); internalCp.spawnSync = oldSpawnSync; } diff --git a/test/parallel/test-child-process-cwd.js b/test/parallel/test-child-process-cwd.js index e1dcf85b0937b2..fdca618a8bcc9c 100644 --- a/test/parallel/test-child-process-cwd.js +++ b/test/parallel/test-child-process-cwd.js @@ -22,47 +22,37 @@ 'use strict'; const common = require('../common'); const assert = require('assert'); - -let returns = 0; +const { spawn } = require('child_process'); /* Spawns 'pwd' with given options, then test - - whether the exit code equals forCode, - - optionally whether the stdout result matches forData - (after removing trailing whitespace) + - whether the exit code equals expectCode, + - optionally whether the trimmed stdout result matches expectData */ -function testCwd(options, forCode, forData) { - let data = ''; - - const child = common.spawnPwd(options); +function testCwd(options, expectCode = 0, expectData) { + const child = spawn(...common.pwdCommand, options); child.stdout.setEncoding('utf8'); + // No need to assert callback since `data` is asserted. + let data = ''; child.stdout.on('data', function(chunk) { data += chunk; }); + // Can't assert callback, as stayed in to API: + // _The 'exit' event may or may not fire after an error has occurred._ child.on('exit', function(code, signal) { - assert.strictEqual(forCode, code); - }); - - child.on('close', function() { - forData && assert.strictEqual(forData, data.replace(/[\s\r\n]+$/, '')); - returns--; + assert.strictEqual(expectCode, code); }); - returns++; + child.on('close', common.mustCall(function() { + expectData && assert.strictEqual(data.trim(), expectData); + })); return child; } -// Assume these exist, and 'pwd' gives us the right directory back -testCwd({ cwd: common.rootDir }, 0, common.rootDir); -if (common.isWindows) { - testCwd({ cwd: process.env.windir }, 0, process.env.windir); -} else { - testCwd({ cwd: '/dev' }, 0, '/dev'); -} // Assume does-not-exist doesn't exist, expect exitCode=-1 and errno=ENOENT { @@ -72,15 +62,12 @@ if (common.isWindows) { })); } -// Spawn() shouldn't try to chdir() so this should just work -testCwd(undefined, 0); -testCwd({}, 0); -testCwd({ cwd: '' }, 0); -testCwd({ cwd: undefined }, 0); -testCwd({ cwd: null }, 0); +// Assume these exist, and 'pwd' gives us the right directory back +testCwd({ cwd: common.rootDir }, 0, common.rootDir); +const shouldExistDir = common.isWindows ? process.env.windir : '/dev'; +testCwd({ cwd: shouldExistDir }, 0, shouldExistDir); -// Check whether all tests actually returned -assert.notStrictEqual(returns, 0); -process.on('exit', function() { - assert.strictEqual(returns, 0); -}); +// Spawn() shouldn't try to chdir() to invalid arg, so this should just work +testCwd({ cwd: '' }); +testCwd({ cwd: undefined }); +testCwd({ cwd: null }); diff --git a/test/parallel/test-child-process-spawnsync.js b/test/parallel/test-child-process-spawnsync.js index 6ff7b41edaba71..99ce75da12091a 100644 --- a/test/parallel/test-child-process-spawnsync.js +++ b/test/parallel/test-child-process-spawnsync.js @@ -22,10 +22,9 @@ 'use strict'; const common = require('../common'); const assert = require('assert'); +const { spawnSync } = require('child_process'); -const spawnSync = require('child_process').spawnSync; - -// Echo does different things on Windows and Unix, but in both cases, it does +// `sleep` does different things on Windows and Unix, but in both cases, it does // more-or-less nothing if there are no parameters const ret = spawnSync('sleep', ['0']); assert.strictEqual(ret.status, 0); @@ -42,21 +41,23 @@ assert.deepStrictEqual(ret_err.spawnargs, ['bar']); { // Test the cwd option const cwd = common.rootDir; - const response = common.spawnSyncPwd({ cwd }); + const response = spawnSync(...common.pwdCommand, { cwd }); assert.strictEqual(response.stdout.toString().trim(), cwd); } + { - // Test the encoding option - const noEncoding = common.spawnSyncPwd(); - const bufferEncoding = common.spawnSyncPwd({ encoding: 'buffer' }); - const utf8Encoding = common.spawnSyncPwd({ encoding: 'utf8' }); + // Assert Buffer is the default encoding + const retDefault = spawnSync(...common.pwdCommand); + const retBuffer = spawnSync(...common.pwdCommand, { encoding: 'buffer' }); + assert.deepStrictEqual(retDefault.output, retBuffer.output); - assert.deepStrictEqual(noEncoding.output, bufferEncoding.output); - assert.deepStrictEqual([ + const retUTF8 = spawnSync(...common.pwdCommand, { encoding: 'utf8' }); + const stringifiedDefault = [ null, - noEncoding.stdout.toString(), - noEncoding.stderr.toString() - ], utf8Encoding.output); + retDefault.stdout.toString(), + retDefault.stderr.toString() + ]; + assert.deepStrictEqual(retUTF8.output, stringifiedDefault); } diff --git a/test/parallel/test-child-process-stdio.js b/test/parallel/test-child-process-stdio.js index 8624a13e1beace..5ca3875f5600f6 100644 --- a/test/parallel/test-child-process-stdio.js +++ b/test/parallel/test-child-process-stdio.js @@ -22,24 +22,56 @@ 'use strict'; const common = require('../common'); const assert = require('assert'); -const spawnSync = require('child_process').spawnSync; +const { spawn } = require('child_process'); -let options = { stdio: ['pipe'] }; -let child = common.spawnPwd(options); +// Test stdio piping. +{ + const child = spawn(...common.pwdCommand, { stdio: ['pipe'] }); + assert.notStrictEqual(child.stdout, null); + assert.notStrictEqual(child.stderr, null); +} -assert.notStrictEqual(child.stdout, null); -assert.notStrictEqual(child.stderr, null); +// Test stdio ignoring. +{ + const child = spawn(...common.pwdCommand, { stdio: 'ignore' }); + assert.strictEqual(child.stdout, null); + assert.strictEqual(child.stderr, null); +} -options = { stdio: 'ignore' }; -child = common.spawnPwd(options); +// Asset options invariance. +{ + const options = { stdio: 'ignore' }; + spawn(...common.pwdCommand, options); + assert.deepStrictEqual(options, { stdio: 'ignore' }); +} -assert.strictEqual(child.stdout, null); -assert.strictEqual(child.stderr, null); +// Test stdout buffering. +{ + let output = ''; + const child = spawn(...common.pwdCommand); -options = { stdio: 'ignore' }; -child = spawnSync('cat', [], options); -assert.deepStrictEqual(options, { stdio: 'ignore' }); + child.stdout.setEncoding('utf8'); + child.stdout.on('data', function(s) { + output += s; + }); -common.expectsError(() => { - common.spawnPwd({ stdio: ['pipe', 'pipe', 'pipe', 'ipc', 'ipc'] }); -}, { code: 'ERR_IPC_ONE_PIPE', type: Error }); + child.on('exit', common.mustCall(function(code) { + assert.strictEqual(code, 0); + })); + + child.on('close', common.mustCall(function() { + assert.strictEqual(true, output.length > 1); + assert.strictEqual('\n', output[output.length - 1]); + })); +} + +// Assert only one IPC pipe allowed. +common.expectsError( + () => { + spawn( + ...common.pwdCommand, + { stdio: ['pipe', 'pipe', 'pipe', 'ipc', 'ipc'] } + ); + }, + { code: 'ERR_IPC_ONE_PIPE', type: Error } +);