From c16cc46689aec09869a2b0cbba62c11ff5d7de3d Mon Sep 17 00:00:00 2001 From: Tommaso Sotte Date: Thu, 17 Jan 2019 23:43:46 +0100 Subject: [PATCH 1/6] When killing the child process, kill all the descendents as well This is an attempt to fix the problem described by the issue #96. If a child process is killed, the descendents of that child process won't be killed as well. This happens on Linux but not on Windows [^1]. The adopted solution is the "PID range hack" [^2] that uses the `detached` mode for spawning a process and then kills that child process by killing the PID group, using `process.kill(-pid)`, effectively killing all the descendents. *Implementation* - added an internal option `killByPid` as a remained for the spawned child process that it will be `detached` and to kill it by PID - expanded and moved to a separate function the routine to kill the spawned process to `killSpawned` - the `ChildProcess#kill` method of the spawned child process will be replaced by the `killSpawned` routine, to kill by pid if necessary - the `killSpawned` routine signals that the child process has been killed, if it has been killed by the pid I checked and all the tests pass. This implementation also considers the issue #115 and shouldn't interfere with the detached/cleanup fix. [^1]: https://nodejs.org/api/child_process.html#child_process_subprocess_kill_signal [^2]: https://azimi.me/2014/12/31/kill-child_process-node-js.html --- index.js | 41 ++++++++++++++++++++++++++++++++++------- 1 file changed, 34 insertions(+), 7 deletions(-) diff --git a/index.js b/index.js index 0610a581cc..66bed0391d 100644 --- a/index.js +++ b/index.js @@ -11,6 +11,8 @@ const onExit = require('signal-exit'); const errname = require('./lib/errname'); const stdio = require('./lib/stdio'); +const isWin = process.platform === 'win32'; + const TEN_MEGABYTES = 1000 * 1000 * 10; function handleArgs(command, args, options) { @@ -50,7 +52,9 @@ function handleArgs(command, args, options) { encoding: 'utf8', reject: true, cleanup: true - }, parsed.options, {windowsHide: true}); + }, parsed.options, { + windowsHide: true + }); // TODO: Remove in the next major release if (options.stripEof === false) { @@ -68,7 +72,14 @@ function handleArgs(command, args, options) { options.cleanup = false; } - if (process.platform === 'win32' && path.basename(parsed.command) === 'cmd.exe') { + if (!isWin) { + // #96 + // Windows automatically kills every descendents of the child process + // On Linux and macOS we need to detach the process and kill by pid range + options.detached = true; + } + + if (isWin && path.basename(parsed.command) === 'cmd.exe') { // #116 parsed.args.unshift('/q'); } @@ -212,11 +223,24 @@ module.exports = (command, args, options) => { return Promise.reject(error); } + let killedByPid = false; + + spawned._kill = spawned.kill; + + const killSpawned = signal => { + if (process.platform === 'win32') { + spawned._kill(signal); + } else { + // Kills the spawned process and its descendents using the pid range hack + // Source: https://azimi.me/2014/12/31/kill-child_process-node-js.html + process.kill(-spawned.pid, signal); + killedByPid = true; + } + }; + let removeExitHandler; if (parsed.options.cleanup) { - removeExitHandler = onExit(() => { - spawned.kill(); - }); + removeExitHandler = onExit(killSpawned); } let timeoutId = null; @@ -237,7 +261,7 @@ module.exports = (command, args, options) => { timeoutId = setTimeout(() => { timeoutId = null; timedOut = true; - spawned.kill(parsed.options.killSignal); + killSpawned(parsed.options.killSignal); }, parsed.options.timeout); } @@ -289,7 +313,7 @@ module.exports = (command, args, options) => { // TODO: missing some timeout logic for killed // https://github.com/nodejs/node/blob/master/lib/child_process.js#L203 // error.killed = spawned.killed || killed; - error.killed = error.killed || spawned.killed; + error.killed = error.killed || spawned.killed || killedByPid; if (!parsed.options.reject) { return error; @@ -312,6 +336,9 @@ module.exports = (command, args, options) => { crossSpawn._enoent.hookChildProcess(spawned, parsed.parsed); + // Enhance the `ChildProcess#kill` to ensure killing the process and its descendents + spawned.kill = killSpawned; + handleInput(spawned, parsed.options.input); spawned.then = (onfulfilled, onrejected) => handlePromise().then(onfulfilled, onrejected); From a82d9da2b02b85283daf51f4e35f70d4bae5b31a Mon Sep 17 00:00:00 2001 From: Tommaso Sotte Date: Sun, 10 Mar 2019 18:28:35 +0100 Subject: [PATCH 2/6] Merge upstream/master --- .travis.yml | 1 - fixtures/noop-132 | 9 ++ index.js | 223 ++++++++++++++++++++++---------------- lib/errname.js | 2 +- package.json | 15 +-- readme.md | 37 +++++-- test.js | 271 +++++++++++++++++++++++++++------------------- test/errname.js | 4 +- 8 files changed, 336 insertions(+), 226 deletions(-) create mode 100755 fixtures/noop-132 diff --git a/.travis.yml b/.travis.yml index 0e8f272892..f1d659d0a5 100644 --- a/.travis.yml +++ b/.travis.yml @@ -5,6 +5,5 @@ language: node_js node_js: - '10' - '8' - - '6' after_success: - './node_modules/.bin/nyc report --reporter=text-lcov | ./node_modules/.bin/coveralls' diff --git a/fixtures/noop-132 b/fixtures/noop-132 new file mode 100755 index 0000000000..277e817db6 --- /dev/null +++ b/fixtures/noop-132 @@ -0,0 +1,9 @@ +#!/usr/bin/env node +'use strict'; + +process.stdout.write('1'); +process.stderr.write('3'); + +setTimeout(() => { + process.stdout.write('2'); +}, 1000); diff --git a/index.js b/index.js index 66bed0391d..507a5e8ee0 100644 --- a/index.js +++ b/index.js @@ -1,11 +1,13 @@ 'use strict'; const path = require('path'); +const os = require('os'); const childProcess = require('child_process'); const crossSpawn = require('cross-spawn'); const stripFinalNewline = require('strip-final-newline'); const npmRunPath = require('npm-run-path'); const isStream = require('is-stream'); const _getStream = require('get-stream'); +const mergeStream = require('merge-stream'); const pFinally = require('p-finally'); const onExit = require('signal-exit'); const errname = require('./lib/errname'); @@ -16,45 +18,37 @@ const isWin = process.platform === 'win32'; const TEN_MEGABYTES = 1000 * 1000 * 10; function handleArgs(command, args, options) { - let parsed; + const parsed = crossSpawn._parse(command, args, options); + command = parsed.command; + args = parsed.args; + options = parsed.options; - options = Object.assign({ - extendEnv: true, - env: {} - }, options); - - if (options.extendEnv) { - options.env = Object.assign({}, process.env, options.env); - } - - if (options.__winShell === true) { - delete options.__winShell; - parsed = { - command, - args, - options, - file: command, - original: { - command, - args - } - }; - } else { - parsed = crossSpawn._parse(command, args, options); - } - - options = Object.assign({ + options = { maxBuffer: TEN_MEGABYTES, buffer: true, stripFinalNewline: true, preferLocal: true, - localDir: parsed.options.cwd || process.cwd(), + localDir: options.cwd || process.cwd(), encoding: 'utf8', reject: true, - cleanup: true - }, parsed.options, { + cleanup: true, + ...options, windowsHide: true - }); + }; + + if (options.extendEnv !== false) { + options.env = { + ...process.env, + ...options.env + }; + } + + if (options.preferLocal) { + options.env = npmRunPath.env({ + ...options, + cwd: options.localDir + }); + } // TODO: Remove in the next major release if (options.stripEof === false) { @@ -63,33 +57,17 @@ function handleArgs(command, args, options) { options.stdio = stdio(options); - if (options.preferLocal) { - options.env = npmRunPath.env(Object.assign({}, options, {cwd: options.localDir})); - } - if (options.detached) { // #115 options.cleanup = false; } - if (!isWin) { - // #96 - // Windows automatically kills every descendents of the child process - // On Linux and macOS we need to detach the process and kill by pid range - options.detached = true; - } - - if (isWin && path.basename(parsed.command) === 'cmd.exe') { + if (process.platform === 'win32' && path.basename(command) === 'cmd.exe') { // #116 - parsed.args.unshift('/q'); + args.unshift('/q'); } - return { - command: parsed.command, - args: parsed.args, - options, - parsed - }; + return {command, args, options, parsed}; } function handleInput(spawned, input) { @@ -113,24 +91,25 @@ function handleOutput(options, value) { } function handleShell(fn, command, options) { - let file = '/bin/sh'; - let args = ['-c', command]; + return fn(command, {...options, shell: true}); +} + +function makeAllStream(spawned) { + if (!spawned.stdout && !spawned.stderr) { + return null; + } - options = Object.assign({}, options); + const mixed = mergeStream(); - if (process.platform === 'win32') { - options.__winShell = true; - file = process.env.comspec || 'cmd.exe'; - args = ['/s', '/c', `"${command}"`]; - options.windowsVerbatimArguments = true; + if (spawned.stdout) { + mixed.add(spawned.stdout); } - if (options.shell) { - file = options.shell; - delete options.shell; + if (spawned.stderr) { + mixed.add(spawned.stderr); } - return fn(file, args, options); + return mixed; } function getStream(process, stream, {encoding, buffer, maxBuffer}) { @@ -164,43 +143,73 @@ function getStream(process, stream, {encoding, buffer, maxBuffer}) { } function makeError(result, options) { - const {stdout, stderr} = result; - + const {stdout, stderr, code, signal} = result; let {error} = result; - const {code, signal} = result; - - const {parsed, joinedCommand} = options; - const timedOut = options.timedOut || false; + const {joinedCommand, timedOut, parsed: {options: {timeout}}} = options; - if (!error) { - let output = ''; - - if (Array.isArray(parsed.options.stdio)) { - if (parsed.options.stdio[2] !== 'inherit') { - output += output.length > 0 ? stderr : `\n${stderr}`; - } + const [exitCodeName, exitCode] = getCode(result, code); - if (parsed.options.stdio[1] !== 'inherit') { - output += `\n${stdout}`; - } - } else if (parsed.options.stdio !== 'inherit') { - output = `\n${stderr}${stdout}`; - } - - error = new Error(`Command failed: ${joinedCommand}${output}`); - error.code = code < 0 ? errname(code) : code; + if (!(error instanceof Error)) { + const message = [joinedCommand, stderr, stdout].filter(Boolean).join('\n'); + error = new Error(message); } + const prefix = getErrorPrefix({timedOut, timeout, signal, exitCodeName, exitCode}); + error.message = `Command ${prefix}: ${error.message}`; + + error.code = exitCode || exitCodeName; + error.exitCode = exitCode; + error.exitCodeName = exitCodeName; error.stdout = stdout; error.stderr = stderr; error.failed = true; error.signal = signal || null; error.cmd = joinedCommand; - error.timedOut = timedOut; + error.timedOut = Boolean(timedOut); + + if ('all' in result) { + error.all = result.all; + } return error; } +function getCode({error = {}}, code) { + if (error.code) { + return [error.code, os.constants.errno[error.code]]; + } + + if (Number.isInteger(code)) { + return [errname(-Math.abs(code)), Math.abs(code)]; + } + + return []; +} + +function getErrorPrefix({timedOut, timeout, signal, exitCodeName, exitCode}) { + if (timedOut) { + return `timed out after ${timeout} milliseconds`; + } + + if (signal) { + return `was killed with ${signal}`; + } + + if (exitCodeName !== undefined && exitCode !== undefined) { + return `failed with exit code ${exitCode} (${exitCodeName})`; + } + + if (exitCodeName !== undefined) { + return `failed with exit code ${exitCodeName}`; + } + + if (exitCode !== undefined) { + return `failed with exit code ${exitCode}`; + } + + return 'failed'; +} + function joinCommand(command, args) { let joinedCommand = command; @@ -292,16 +301,23 @@ module.exports = (command, args, options) => { if (spawned.stderr) { spawned.stderr.destroy(); } + + if (spawned.all) { + spawned.all.destroy(); + } } + // TODO: Use native "finally" syntax when targeting Node.js 10 const handlePromise = () => pFinally(Promise.all([ processDone, getStream(spawned, 'stdout', {encoding, buffer, maxBuffer}), - getStream(spawned, 'stderr', {encoding, buffer, maxBuffer}) - ]).then(arr => { - const result = arr[0]; - result.stdout = arr[1]; - result.stderr = arr[2]; + getStream(spawned, 'stderr', {encoding, buffer, maxBuffer}), + getStream(spawned, 'all', {encoding, buffer, maxBuffer: maxBuffer * 2}) + ]).then(results => { // eslint-disable-line promise/prefer-await-to-then + const result = results[0]; + result.stdout = results[1]; + result.stderr = results[2]; + result.all = results[3]; if (result.error || result.code !== 0 || result.signal !== null) { const error = makeError(result, { @@ -325,7 +341,10 @@ module.exports = (command, args, options) => { return { stdout: handleOutput(parsed.options, result.stdout), stderr: handleOutput(parsed.options, result.stderr), + all: handleOutput(parsed.options, result.all), code: 0, + exitCode: 0, + exitCodeName: 'SUCCESS', failed: false, killed: false, signal: null, @@ -341,17 +360,31 @@ module.exports = (command, args, options) => { handleInput(spawned, parsed.options.input); - spawned.then = (onfulfilled, onrejected) => handlePromise().then(onfulfilled, onrejected); - spawned.catch = onrejected => handlePromise().catch(onrejected); + spawned.all = makeAllStream(spawned); + + // eslint-disable-next-line promise/prefer-await-to-then + spawned.then = (onFulfilled, onRejected) => handlePromise().then(onFulfilled, onRejected); + spawned.catch = onRejected => handlePromise().catch(onRejected); + + // TOOD: Remove the `if`-guard when targeting Node.js 10 + if (Promise.prototype.finally) { + spawned.finally = onFinally => handlePromise().finally(onFinally); + } return spawned; }; // TODO: set `stderr: 'ignore'` when that option is implemented -module.exports.stdout = (...args) => module.exports(...args).then(x => x.stdout); +module.exports.stdout = async (...args) => { + const {stdout} = await module.exports(...args); + return stdout; +}; // TODO: set `stdout: 'ignore'` when that option is implemented -module.exports.stderr = (...args) => module.exports(...args).then(x => x.stderr); +module.exports.stderr = async (...args) => { + const {stderr} = await module.exports(...args); + return stderr; +}; module.exports.shell = (command, options) => handleShell(module.exports, command, options); @@ -383,6 +416,8 @@ module.exports.sync = (command, args, options) => { stdout: handleOutput(parsed.options, result.stdout), stderr: handleOutput(parsed.options, result.stderr), code: 0, + exitCode: 0, + exitCodeName: 'SUCCESS', failed: false, signal: null, cmd: joinedCommand, diff --git a/lib/errname.js b/lib/errname.js index 562284abf3..8f9e1fb1b5 100644 --- a/lib/errname.js +++ b/lib/errname.js @@ -9,7 +9,7 @@ if (typeof util.getSystemErrorName === 'function') { module.exports = util.getSystemErrorName; } else { try { - uv = process.binding('uv'); + uv = process.binding('uv'); // eslint-disable-line node/no-deprecated-api if (typeof uv.errname !== 'function') { throw new TypeError('uv.errname is not a function'); diff --git a/package.json b/package.json index 4336d549a8..d4355ee0dc 100644 --- a/package.json +++ b/package.json @@ -10,7 +10,7 @@ "url": "sindresorhus.com" }, "engines": { - "node": ">=6" + "node": ">=8" }, "scripts": { "test": "xo && nyc ava" @@ -40,20 +40,21 @@ "cross-spawn": "^6.0.0", "get-stream": "^4.0.0", "is-stream": "^1.1.0", + "merge-stream": "1.0.1", "npm-run-path": "^2.0.0", "p-finally": "^1.0.0", "signal-exit": "^3.0.0", "strip-final-newline": "^2.0.0" }, "devDependencies": { - "ava": "^0.25.0", - "cat-names": "^1.0.2", - "coveralls": "^3.0.1", - "delay": "^3.0.0", + "ava": "^1.3.1", + "cat-names": "^2.0.0", + "coveralls": "^3.0.3", + "delay": "^4.1.0", "is-running": "^2.0.0", - "nyc": "^13.0.1", + "nyc": "^13.3.0", "tempfile": "^2.0.0", - "xo": "^0.23.0" + "xo": "^0.24.0" }, "nyc": { "exclude": [ diff --git a/readme.md b/readme.md index b6921faaaf..c6d9d83ca6 100644 --- a/readme.md +++ b/readme.md @@ -12,6 +12,7 @@ - Higher max buffer. 10 MB instead of 200 KB. - [Executes locally installed binaries by name.](#preferlocal) - [Cleans up spawned processes when the parent process dies.](#cleanup) +- [Adds an `.all` property](#execafile-arguments-options) with interleaved output from `stdout` and `stderr`, similar to what the terminal sees. [*(Async only)*](#execasyncfile-arguments-options) ## Install @@ -59,14 +60,18 @@ const execa = require('execa'); console.log(error); /* { - message: 'Command failed: /bin/sh -c exit 3' - killed: false, + message: 'Command failed with exit code 3 (ESRCH): exit 3', code: 3, - signal: null, - cmd: '/bin/sh -c exit 3', + exitCode: 3, + exitCodeName: 'ESRCH', stdout: '', stderr: '', - timedOut: false + all: '', + failed: true, + signal: null, + cmd: 'exit 3', + timedOut: false, + killed: false } */ } @@ -79,12 +84,15 @@ try { console.log(error); /* { - message: 'Command failed: /bin/sh -c exit 3' + message: 'Command failed with exit code 3 (ESRCH): exit 3', code: 3, - signal: null, - cmd: '/bin/sh -c exit 3', + exitCode: 3, + exitCodeName: 'ESRCH', stdout: '', stderr: '', + failed: true, + signal: null, + cmd: 'exit 3', timedOut: false } */ @@ -100,7 +108,11 @@ Execute a file. Think of this as a mix of `child_process.execFile` and `child_process.spawn`. -Returns a [`child_process` instance](https://nodejs.org/api/child_process.html#child_process_class_childprocess), which is enhanced to also be a `Promise` for a result `Object` with `stdout` and `stderr` properties. +Returns a [`child_process` instance](https://nodejs.org/api/child_process.html#child_process_class_childprocess) which is enhanced to be a `Promise`. + +It exposes an additional `.all` stream, with `stdout` and `stderr` interleaved. + +The promise result is an `Object` with `stdout`, `stderr` and `all` properties. ### execa.stdout(file, [arguments], [options]) @@ -124,6 +136,8 @@ Execute a file synchronously. Returns the same result object as [`child_process.spawnSync`](https://nodejs.org/api/child_process.html#child_process_child_process_spawnsync_command_args_options). +It does not have the `.all` property that `execa()` has because the [underlying synchronous implementation](https://nodejs.org/api/child_process.html#child_process_child_process_execfilesync_file_args_options) only returns `stdout` and `stderr` at the end of the execution, so they cannot be interleaved. + This method throws an `Error` if the command fails. ### execa.shellSync(file, [options]) @@ -322,6 +336,7 @@ getStream(stream).then(value => { ``` -## License +## Maintainers -MIT © [Sindre Sorhus](https://sindresorhus.com) +- [Sindre Sorhus](https://github.com/sindresorhus) +- [@ehmicky](https://github.com/ehmicky) diff --git a/test.js b/test.js index c9e56f30dc..9c043a4729 100644 --- a/test.js +++ b/test.js @@ -7,149 +7,146 @@ import getStream from 'get-stream'; import isRunning from 'is-running'; import delay from 'delay'; import tempfile from 'tempfile'; -import m from '.'; +import execa from '.'; process.env.PATH = path.join(__dirname, 'fixtures') + path.delimiter + process.env.PATH; process.env.FOO = 'foo'; +const NO_NEWLINES_REGEXP = /^[^\n]*$/; +const STDERR_STDOUT_REGEXP = /stderr[^]*stdout/; +const TIMEOUT_REGEXP = /timed out after/; + +const getExitRegExp = exitMessage => new RegExp(`failed with exit code ${exitMessage}`); + test('execa()', async t => { - const {stdout} = await m('noop', ['foo']); + const {stdout} = await execa('noop', ['foo']); t.is(stdout, 'foo'); }); if (process.platform === 'win32') { test('execa() - cmd file', async t => { - const {stdout} = await m('hello.cmd'); - + const {stdout} = await execa('hello.cmd'); t.is(stdout, 'Hello World'); }); } test('buffer', async t => { - const {stdout} = await m('noop', ['foo'], {encoding: null}); + const {stdout} = await execa('noop', ['foo'], {encoding: null}); t.true(Buffer.isBuffer(stdout)); t.is(stdout.toString(), 'foo'); }); test('execa.stdout()', async t => { - const stdout = await m.stdout('noop', ['foo']); + const stdout = await execa.stdout('noop', ['foo']); t.is(stdout, 'foo'); }); test('execa.stderr()', async t => { - const stderr = await m.stderr('noop-err', ['foo']); + const stderr = await execa.stderr('noop-err', ['foo']); t.is(stderr, 'foo'); }); -test('stdout/stderr available on errors', async t => { - const err = await t.throws(m('exit', ['2'])); +test.serial('result.all shows both `stdout` and `stderr` intermixed', async t => { + const result = await execa('noop-132'); + t.is(result.all, '132'); +}); + +test('stdout/stderr/all available on errors', async t => { + const err = await t.throwsAsync(execa('exit', ['2']), {message: getExitRegExp('2')}); t.is(typeof err.stdout, 'string'); t.is(typeof err.stderr, 'string'); + t.is(typeof err.all, 'string'); }); test('include stdout and stderr in errors for improved debugging', async t => { - const err = await t.throws(m('fixtures/error-message.js')); - t.regex(err.message, /stdout/); - t.regex(err.message, /stderr/); - t.is(err.code, 1); + await t.throwsAsync(execa('fixtures/error-message.js'), {message: STDERR_STDOUT_REGEXP, code: 1}); }); test('do not include in errors when `stdio` is set to `inherit`', async t => { - const err = await t.throws(m('fixtures/error-message.js', {stdio: 'inherit'})); - t.notRegex(err.message, /\n/); + await t.throwsAsync(execa('fixtures/error-message.js', {stdio: 'inherit'}), {message: NO_NEWLINES_REGEXP}); }); test('do not include `stderr` and `stdout` in errors when set to `inherit`', async t => { - const err = await t.throws(m('fixtures/error-message.js', {stdout: 'inherit', stderr: 'inherit'})); - t.notRegex(err.message, /\n/); + await t.throwsAsync(execa('fixtures/error-message.js', {stdout: 'inherit', stderr: 'inherit'}), {message: NO_NEWLINES_REGEXP}); }); test('do not include `stderr` and `stdout` in errors when `stdio` is set to `inherit`', async t => { - const err = await t.throws(m('fixtures/error-message.js', {stdio: [null, 'inherit', 'inherit']})); - t.notRegex(err.message, /\n/); + await t.throwsAsync(execa('fixtures/error-message.js', {stdio: [null, 'inherit', 'inherit']}), {message: NO_NEWLINES_REGEXP}); }); test('do not include `stdout` in errors when set to `inherit`', async t => { - const err = await t.throws(m('fixtures/error-message.js', {stdout: 'inherit'})); + const err = await t.throwsAsync(execa('fixtures/error-message.js', {stdout: 'inherit'}), {message: /stderr/}); t.notRegex(err.message, /stdout/); - t.regex(err.message, /stderr/); }); test('do not include `stderr` in errors when set to `inherit`', async t => { - const err = await t.throws(m('fixtures/error-message.js', {stderr: 'inherit'})); - t.regex(err.message, /stdout/); + const err = await t.throwsAsync(execa('fixtures/error-message.js', {stderr: 'inherit'}), {message: /stdout/}); t.notRegex(err.message, /stderr/); }); test('pass `stdout` to a file descriptor', async t => { const file = tempfile('.txt'); - await m('fixtures/noop', ['foo bar'], {stdout: fs.openSync(file, 'w')}); + await execa('fixtures/noop', ['foo bar'], {stdout: fs.openSync(file, 'w')}); t.is(fs.readFileSync(file, 'utf8'), 'foo bar\n'); }); test('pass `stderr` to a file descriptor', async t => { const file = tempfile('.txt'); - await m('fixtures/noop-err', ['foo bar'], {stderr: fs.openSync(file, 'w')}); + await execa('fixtures/noop-err', ['foo bar'], {stderr: fs.openSync(file, 'w')}); t.is(fs.readFileSync(file, 'utf8'), 'foo bar\n'); }); test('execa.shell()', async t => { - const {stdout} = await m.shell('node fixtures/noop foo'); + const {stdout} = await execa.shell('node fixtures/noop foo'); t.is(stdout, 'foo'); }); test('execa.sync()', t => { - const {stdout} = m.sync('noop', ['foo']); + const {stdout} = execa.sync('noop', ['foo']); t.is(stdout, 'foo'); }); test('execa.sync() throws error if written to stderr', t => { - t.throws(() => m.sync('foo'), process.platform === 'win32' ? /'foo' is not recognized as an internal or external command/ : 'spawnSync foo ENOENT'); + t.throws(() => execa.sync('foo'), process.platform === 'win32' ? /'foo' is not recognized as an internal or external command/ : /spawnSync foo ENOENT/); }); test('execa.sync() includes stdout and stderr in errors for improved debugging', t => { - const err = t.throws(() => m.sync('node', ['fixtures/error-message.js'])); - t.regex(err.message, /stdout/); - t.regex(err.message, /stderr/); - t.is(err.code, 1); + t.throws(() => execa.sync('node', ['fixtures/error-message.js']), {message: STDERR_STDOUT_REGEXP, code: 1}); }); test('skip throwing when using reject option in execa.sync()', t => { - const err = m.sync('node', ['fixtures/error-message.js'], {reject: false}); + const err = execa.sync('node', ['fixtures/error-message.js'], {reject: false}); t.is(typeof err.stdout, 'string'); t.is(typeof err.stderr, 'string'); }); test('execa.shellSync()', t => { - const {stdout} = m.shellSync('node fixtures/noop foo'); + const {stdout} = execa.shellSync('node fixtures/noop foo'); t.is(stdout, 'foo'); }); test('execa.shellSync() includes stdout and stderr in errors for improved debugging', t => { - const err = t.throws(() => m.shellSync('node fixtures/error-message.js')); - t.regex(err.message, /stdout/); - t.regex(err.message, /stderr/); - t.is(err.code, 1); + t.throws(() => execa.shellSync('node fixtures/error-message.js'), {message: STDERR_STDOUT_REGEXP, code: 1}); }); test('skip throwing when using reject option in execa.shellSync()', t => { - const err = m.shellSync('node fixtures/error-message.js', {reject: false}); + const err = execa.shellSync('node fixtures/error-message.js', {reject: false}); t.is(typeof err.stdout, 'string'); t.is(typeof err.stderr, 'string'); }); test('stripEof option (legacy)', async t => { - const {stdout} = await m('noop', ['foo'], {stripEof: false}); + const {stdout} = await execa('noop', ['foo'], {stripEof: false}); t.is(stdout, 'foo\n'); }); test('stripFinalNewline option', async t => { - const {stdout} = await m('noop', ['foo'], {stripFinalNewline: false}); + const {stdout} = await execa('noop', ['foo'], {stripFinalNewline: false}); t.is(stdout, 'foo\n'); }); test.serial('preferLocal option', async t => { - t.true((await m('cat-names')).stdout.length > 2); + t.true((await execa('cat-names')).stdout.length > 2); if (process.platform === 'win32') { // TODO: figure out how to make the below not hang on Windows @@ -159,7 +156,7 @@ test.serial('preferLocal option', async t => { // Account for npm adding local binaries to the PATH const _path = process.env.PATH; process.env.PATH = ''; - await t.throws(m('cat-names', {preferLocal: false}), /spawn .* ENOENT/); + await t.throwsAsync(execa('cat-names', {preferLocal: false}), /spawn .* ENOENT/); process.env.PATH = _path; }); @@ -167,20 +164,20 @@ test.serial('localDir option', async t => { const cwd = 'fixtures/local-dir'; const bin = path.resolve(cwd, 'node_modules/.bin/self-path'); - await m('npm', ['install', '--no-package-lock'], {cwd}); + await execa('npm', ['install', '--no-package-lock'], {cwd}); - const {stdout} = await m(bin, {localDir: cwd}); + const {stdout} = await execa(bin, {localDir: cwd}); t.is(path.relative(cwd, stdout), path.normalize('node_modules/self-path')); }); test('input option can be a String', async t => { - const {stdout} = await m('stdin', {input: 'foobar'}); + const {stdout} = await execa('stdin', {input: 'foobar'}); t.is(stdout, 'foobar'); }); test('input option can be a Buffer', async t => { - const {stdout} = await m('stdin', {input: 'testing12'}); + const {stdout} = await execa('stdin', {input: 'testing12'}); t.is(stdout, 'testing12'); }); @@ -188,28 +185,28 @@ test('input can be a Stream', async t => { const s = new stream.PassThrough(); s.write('howdy'); s.end(); - const {stdout} = await m('stdin', {input: s}); + const {stdout} = await execa('stdin', {input: s}); t.is(stdout, 'howdy'); }); test('you can write to child.stdin', async t => { - const child = m('stdin'); + const child = execa('stdin'); child.stdin.end('unicorns'); t.is((await child).stdout, 'unicorns'); }); test('input option can be a String - sync', t => { - const {stdout} = m.sync('stdin', {input: 'foobar'}); + const {stdout} = execa.sync('stdin', {input: 'foobar'}); t.is(stdout, 'foobar'); }); test('input option can be a Buffer - sync', t => { - const {stdout} = m.sync('stdin', {input: Buffer.from('testing12', 'utf8')}); + const {stdout} = execa.sync('stdin', {input: Buffer.from('testing12', 'utf8')}); t.is(stdout, 'testing12'); }); test('opts.stdout:ignore - stdout will not collect data', async t => { - const {stdout} = await m('stdin', { + const {stdout} = await execa('stdin', { input: 'hello', stdio: [null, 'ignore', null] }); @@ -218,32 +215,33 @@ test('opts.stdout:ignore - stdout will not collect data', async t => { test('helpful error trying to provide an input stream in sync mode', t => { t.throws( - () => m.sync('stdin', {input: new stream.PassThrough()}), + () => execa.sync('stdin', {input: new stream.PassThrough()}), /The `input` option cannot be a stream in sync mode/ ); }); test('execa() returns a promise with kill() and pid', t => { - const promise = m('noop', ['foo']); + const promise = execa('noop', ['foo']); t.is(typeof promise.kill, 'function'); t.is(typeof promise.pid, 'number'); }); test('maxBuffer affects stdout', async t => { - await t.throws(m('max-buffer', ['stdout', '11'], {maxBuffer: 10}), /stdout maxBuffer exceeded/); - await t.notThrows(m('max-buffer', ['stdout', '10'], {maxBuffer: 10})); + await t.throwsAsync(execa('max-buffer', ['stdout', '11'], {maxBuffer: 10}), /stdout maxBuffer exceeded/); + await t.notThrowsAsync(execa('max-buffer', ['stdout', '10'], {maxBuffer: 10})); }); test('maxBuffer affects stderr', async t => { - await t.throws(m('max-buffer', ['stderr', '13'], {maxBuffer: 12}), /stderr maxBuffer exceeded/); - await t.notThrows(m('max-buffer', ['stderr', '12'], {maxBuffer: 12})); + await t.throwsAsync(execa('max-buffer', ['stderr', '13'], {maxBuffer: 12}), /stderr maxBuffer exceeded/); + await t.notThrowsAsync(execa('max-buffer', ['stderr', '12'], {maxBuffer: 12})); }); test('do not buffer stdout when `buffer` set to `false`', async t => { - const promise = m('max-buffer', ['stdout', '10'], {buffer: false}); + const promise = execa('max-buffer', ['stdout', '10'], {buffer: false}); const [result, stdout] = await Promise.all([ promise, - getStream(promise.stdout) + getStream(promise.stdout), + getStream(promise.all) ]); t.is(result.stdout, undefined); @@ -251,10 +249,11 @@ test('do not buffer stdout when `buffer` set to `false`', async t => { }); test('do not buffer stderr when `buffer` set to `false`', async t => { - const promise = m('max-buffer', ['stderr', '10'], {buffer: false}); + const promise = execa('max-buffer', ['stderr', '10'], {buffer: false}); const [result, stderr] = await Promise.all([ promise, - getStream(promise.stderr) + getStream(promise.stderr), + getStream(promise.all) ]); t.is(result.stderr, undefined); @@ -262,53 +261,66 @@ test('do not buffer stderr when `buffer` set to `false`', async t => { }); test('skip throwing when using reject option', async t => { - const error = await m('exit', ['2'], {reject: false}); + const error = await execa('exit', ['2'], {reject: false}); t.is(typeof error.stdout, 'string'); t.is(typeof error.stderr, 'string'); }); +test('allow unknown exit code', async t => { + const {exitCode, exitCodeName} = await t.throwsAsync(execa('exit', ['255']), {message: /exit code 255 \(Unknown system error -255\)/}); + t.is(exitCode, 255); + t.is(exitCodeName, 'Unknown system error -255'); +}); + test('execa() returns code and failed properties', async t => { - const {code, failed} = await m('noop', ['foo']); - const error = await t.throws(m('exit', ['2'])); + const {code, exitCode, exitCodeName, failed} = await execa('noop', ['foo']); t.is(code, 0); + t.is(exitCode, 0); + t.is(exitCodeName, 'SUCCESS'); t.false(failed); - t.is(error.code, 2); + + const error = await t.throwsAsync(execa('exit', ['2']), {code: 2, message: getExitRegExp('2')}); + t.is(error.exitCode, 2); + const expectedName = process.platform === 'win32' ? 'Unknown system error -2' : 'ENOENT'; + t.is(error.exitCodeName, expectedName); t.true(error.failed); }); test('use relative path with \'..\' chars', async t => { const pathViaParentDir = path.join('..', path.basename(__dirname), 'fixtures', 'noop'); - const {stdout} = await m(pathViaParentDir, ['foo']); + const {stdout} = await execa(pathViaParentDir, ['foo']); t.is(stdout, 'foo'); }); if (process.platform !== 'win32') { test('execa() rejects if running non-executable', async t => { - const cp = m('non-executable'); - await t.throws(cp); + const cp = execa('non-executable'); + await t.throwsAsync(cp); }); } test('error.killed is true if process was killed directly', async t => { - const cp = m('forever'); + const cp = execa('forever'); setTimeout(() => { cp.kill(); }, 100); - const error = await t.throws(cp); + const error = await t.throwsAsync(cp, {message: /was killed with SIGTERM/}); t.true(error.killed); }); // TODO: Should this really be the case, or should we improve on child_process? test('error.killed is false if process was killed indirectly', async t => { - const cp = m('forever'); + const cp = execa('forever'); setTimeout(() => { process.kill(cp.pid, 'SIGINT'); }, 100); - const error = await t.throws(cp); + // `process.kill()` is emulated by Node.js on Windows + const message = process.platform === 'win32' ? /failed with exit code 1/ : /was killed with SIGINT/; + const error = await t.throwsAsync(cp, {message}); t.false(error.killed); }); @@ -328,45 +340,45 @@ if (process.platform === 'darwin') { if (process.platform !== 'win32') { test('error.signal is SIGINT', async t => { - const cp = m('forever'); + const cp = execa('forever'); setTimeout(() => { process.kill(cp.pid, 'SIGINT'); }, 100); - const error = await t.throws(cp); + const error = await t.throwsAsync(cp, {message: /was killed with SIGINT/}); t.is(error.signal, 'SIGINT'); }); test('error.signal is SIGTERM', async t => { - const cp = m('forever'); + const cp = execa('forever'); setTimeout(() => { process.kill(cp.pid, 'SIGTERM'); }, 100); - const error = await t.throws(cp); + const error = await t.throwsAsync(cp, {message: /was killed with SIGTERM/}); t.is(error.signal, 'SIGTERM'); }); test('custom error.signal', async t => { - const error = await t.throws(m('delay', ['3000', '0'], {killSignal: 'SIGHUP', timeout: 1500})); + const error = await t.throwsAsync(execa('delay', ['3000', '0'], {killSignal: 'SIGHUP', timeout: 1500, message: TIMEOUT_REGEXP})); t.is(error.signal, 'SIGHUP'); }); } test('result.signal is null for successful execution', async t => { - t.is((await m('noop')).signal, null); + t.is((await execa('noop')).signal, null); }); test('result.signal is null if process failed, but was not killed', async t => { - const error = await t.throws(m('exit', [2])); + const error = await t.throwsAsync(execa('exit', [2]), {message: getExitRegExp('2')}); t.is(error.signal, null); }); async function code(t, num) { - const error = await t.throws(m('exit', [`${num}`])); - t.is(error.code, num); + const error = await t.throwsAsync(execa('exit', [`${num}`]), {code: num, message: getExitRegExp(num)}); + t.is(error.exitCode, num); } test('error.code is 2', code, 2); @@ -374,44 +386,41 @@ test('error.code is 3', code, 3); test('error.code is 4', code, 4); test('timeout will kill the process early', async t => { - const error = await t.throws(m('delay', ['60000', '0'], {timeout: 1500})); + const error = await t.throwsAsync(execa('delay', ['60000', '0'], {timeout: 1500, message: TIMEOUT_REGEXP})); t.true(error.timedOut); - t.not(error.code, 22); + t.not(error.exitCode, 22); }); test('timeout will not kill the process early', async t => { - const error = await t.throws(m('delay', ['3000', '22'], {timeout: 30000})); - + const error = await t.throwsAsync(execa('delay', ['3000', '22'], {timeout: 30000}), {code: 22, message: getExitRegExp('22')}); t.false(error.timedOut); - t.is(error.code, 22); }); test('timedOut will be false if no timeout was set and zero exit code', async t => { - const result = await m('delay', ['1000', '0']); + const result = await execa('delay', ['1000', '0']); t.false(result.timedOut); }); test('timedOut will be false if no timeout was set and non-zero exit code', async t => { - const error = await t.throws(m('delay', ['1000', '3'])); + const error = await t.throwsAsync(execa('delay', ['1000', '3']), {message: getExitRegExp('3')}); t.false(error.timedOut); }); async function errorMessage(t, expected, ...args) { - const error = await t.throws(m('exit', args)); - t.regex(error.message, expected); + await t.throwsAsync(execa('exit', args), {message: expected}); } errorMessage.title = (message, expected) => `error.message matches: ${expected}`; -test(errorMessage, /Command failed: exit 2 foo bar/, 2, 'foo', 'bar'); -test(errorMessage, /Command failed: exit 3 baz quz/, 3, 'baz', 'quz'); +test(errorMessage, /Command failed with exit code 2.*: exit 2 foo bar/, 2, 'foo', 'bar'); +test(errorMessage, /Command failed with exit code 3.*: exit 3 baz quz/, 3, 'baz', 'quz'); async function cmd(t, expected, ...args) { - const error = await t.throws(m('fail', args)); + const error = await t.throwsAsync(execa('fail', args)); t.is(error.cmd, `fail${expected}`); - const result = await m('noop', args); + const result = await execa('noop', args); t.is(result.cmd, `noop${expected}`); } @@ -423,7 +432,7 @@ test(cmd, ''); async function spawnAndKill(t, signal, cleanup) { const name = cleanup ? 'sub-process' : 'sub-process-false'; - const cp = m(name); + const cp = execa(name); let pid; cp.stdout.setEncoding('utf8'); @@ -436,7 +445,7 @@ async function spawnAndKill(t, signal, cleanup) { }, 100); }); - await t.throws(cp); + await t.throwsAsync(cp); // Give everybody some time to breath and kill things await delay(200); @@ -454,10 +463,8 @@ if (process.platform !== 'win32') { test('cleanup false - SIGKILL', spawnAndKill, 'SIGKILL', false); } -// See: https://github.com/sindresorhus/execa/issues/56 -const onlyWinFailing = test[process.platform === 'win32' ? 'failing' : 'serial']; -onlyWinFailing('execa.shell() supports the `shell` option', async t => { - const {stdout} = await m.shell('noop foo', { +test('execa.shell() supports the `shell` option', async t => { + const {stdout} = await execa.shell('node fixtures/noop foo', { shell: process.platform === 'win32' ? 'cmd.exe' : '/bin/bash' }); t.is(stdout, 'foo'); @@ -468,16 +475,16 @@ if (process.platform !== 'win32') { // Try-catch here is necessary, because this test is not 100% accurate // Sometimes process can manage to accept input before exiting try { - await m(`fast-exit-${process.platform}`, [], {input: 'data'}); + await execa(`fast-exit-${process.platform}`, [], {input: 'data'}); t.pass(); } catch (error) { - t.is(error.code, 'EPIPE'); + t.is(error.exitCode, 32); } }); } test('use environment variables by default', async t => { - const result = await m.stdout('environment'); + const result = await execa.stdout('environment'); t.deepEqual(result.split('\n'), [ 'foo', @@ -486,7 +493,7 @@ test('use environment variables by default', async t => { }); test('extend environment variables by default', async t => { - const result = await m.stdout('environment', [], {env: {BAR: 'bar'}}); + const result = await execa.stdout('environment', [], {env: {BAR: 'bar'}}); t.deepEqual(result.split('\n'), [ 'foo', @@ -494,8 +501,8 @@ test('extend environment variables by default', async t => { ]); }); -test('do not extend environment with `extendEnv` option', async t => { - const result = await m.stdout('environment', [], {env: {BAR: 'bar', PATH: process.env.PATH}, extendEnv: false}); +test('do not extend environment with `extendEnv: false`', async t => { + const result = await execa.stdout('environment', [], {env: {BAR: 'bar', PATH: process.env.PATH}, extendEnv: false}); t.deepEqual(result.split('\n'), [ 'undefined', @@ -503,8 +510,16 @@ test('do not extend environment with `extendEnv` option', async t => { ]); }); +test('use extend environment with `extendEnv: true` and `shell: true`', async t => { + process.env.TEST = 'test'; + const command = process.platform === 'win32' ? 'echo %TEST%' : 'echo $TEST'; + const stdout = await execa.stdout(command, {shell: true, env: {}, extendEnv: true}); + t.is(stdout, 'test'); + delete process.env.TEST; +}); + test('do not buffer when streaming', async t => { - const result = await getStream(m('max-buffer', ['stdout', '21'], {maxBuffer: 10}).stdout); + const result = await getStream(execa('max-buffer', ['stdout', '21'], {maxBuffer: 10}).stdout); t.is(result, '....................\n'); }); @@ -512,7 +527,7 @@ test('do not buffer when streaming', async t => { test('detach child process', async t => { const file = tempfile('.txt'); - await m('detach', [file]); + await execa('detach', [file]); await delay(5000); @@ -524,7 +539,7 @@ test('removes exit handler on exit', async t => { // FIXME: This relies on `signal-exit` internals const ee = process.__signal_exit_emitter__; - const child = m('noop'); + const child = execa('noop'); const listener = ee.listeners('exit').pop(); await new Promise((resolve, reject) => { @@ -535,3 +550,39 @@ test('removes exit handler on exit', async t => { const included = ee.listeners('exit').includes(listener); t.false(included); }); + +// TOOD: Remove the `if`-guard when targeting Node.js 10 +if (Promise.prototype.finally) { + test('finally function is executed on success', async t => { + let called = false; + const {stdout} = await execa('noop', ['foo']).finally(() => { + called = true; + }); + t.is(called, true); + t.is(stdout, 'foo'); + }); + + test('finally function is executed on failure', async t => { + let called = false; + const err = await t.throwsAsync(execa('exit', ['2']).finally(() => { + called = true; + })); + t.is(called, true); + t.is(typeof err.stdout, 'string'); + t.is(typeof err.stderr, 'string'); + }); + + test('throw in finally function bubbles up on success', async t => { + const result = await t.throwsAsync(execa('noop', ['foo']).finally(() => { + throw new Error('called'); + })); + t.is(result.message, 'called'); + }); + + test('throw in finally bubbles up on error', async t => { + const result = await t.throwsAsync(execa('exit', ['2']).finally(() => { + throw new Error('called'); + })); + t.is(result.message, 'called'); + }); +} diff --git a/test/errname.js b/test/errname.js index 97dafb3d8e..d47720d992 100644 --- a/test/errname.js +++ b/test/errname.js @@ -1,7 +1,7 @@ import test from 'ava'; import errname from '../lib/errname'; -const isWin = process.platform === 'win32'; +const isWindows = process.platform === 'win32'; // Simulates failure to capture `process.binding('uv');` const fallback = code => errname.__test__(null, code); @@ -22,5 +22,5 @@ function makeTests(name, m, expected) { const unknown = 'Unknown system error -2'; -makeTests('native', errname, isWin ? unknown : 'ENOENT'); +makeTests('native', errname, isWindows ? unknown : 'ENOENT'); makeTests('fallback', fallback, unknown); From 6aa52546ce00d91774e25f3546a96a6846a1c136 Mon Sep 17 00:00:00 2001 From: Tommaso Sotte Date: Thu, 21 Mar 2019 19:53:40 +0100 Subject: [PATCH 3/6] Cleaned code and minimized changes --- index.js | 39 ++++++++++++++++++--------------------- 1 file changed, 18 insertions(+), 21 deletions(-) diff --git a/index.js b/index.js index 2d18702213..cbec10ce6d 100644 --- a/index.js +++ b/index.js @@ -13,8 +13,6 @@ const onExit = require('signal-exit'); const errname = require('./lib/errname'); const stdio = require('./lib/stdio'); -const isWin = process.platform === 'win32'; - const TEN_MEGABYTES = 1000 * 1000 * 10; function handleArgs(command, args, options) { @@ -239,24 +237,11 @@ const execa = (command, args, options) => { return Promise.reject(error); } - let killedByPid = false; - - spawned._kill = spawned.kill; - - const killSpawned = signal => { - if (process.platform === 'win32') { - spawned._kill(signal); - } else { - // Kills the spawned process and its descendents using the pid range hack - // Source: https://azimi.me/2014/12/31/kill-child_process-node-js.html - process.kill(-spawned.pid, signal); - killedByPid = true; - } - }; - let removeExitHandler; if (parsed.options.cleanup) { - removeExitHandler = onExit(killSpawned); + removeExitHandler = onExit(() => { + spawned.kill(); + }); } let timeoutId; @@ -278,7 +263,7 @@ const execa = (command, args, options) => { timeoutId = setTimeout(() => { timeoutId = undefined; timedOut = true; - killSpawned(parsed.options.killSignal); + spawned.kill(parsed.options.killSignal); }, parsed.options.timeout); } @@ -338,7 +323,7 @@ const execa = (command, args, options) => { // TODO: missing some timeout logic for killed // https://github.com/nodejs/node/blob/master/lib/child_process.js#L203 // error.killed = spawned.killed || killed; - error.killed = error.killed || spawned.killed || killedByPid; + error.killed = error.killed || spawned.killed; if (!parsed.options.reject) { return error; @@ -364,8 +349,20 @@ const execa = (command, args, options) => { crossSpawn._enoent.hookChildProcess(spawned, parsed.parsed); + spawned._kill = spawned.kill; + + const killSpawnedAndDescendants = signal => { + if (process.platform === 'win32') { + spawned._kill(signal); + } else { + spawned._kill(signal); + // TODO kill with pidtree + // do not kill this process via pidtree, exclude this PID? + } + }; + // Enhance the `ChildProcess#kill` to ensure killing the process and its descendents - spawned.kill = killSpawned; + spawned.kill = killSpawnedAndDescendants; handleInput(spawned, parsed.options.input); From c73e7545f2cf9a3200fb7a177df23e5bc31e0756 Mon Sep 17 00:00:00 2001 From: Tommaso Sotte Date: Thu, 21 Mar 2019 21:03:57 +0100 Subject: [PATCH 4/6] Refactored enhanced kill more consistently --- index.js | 26 +++++++++++--------------- 1 file changed, 11 insertions(+), 15 deletions(-) diff --git a/index.js b/index.js index cbec10ce6d..e02e851417 100644 --- a/index.js +++ b/index.js @@ -349,21 +349,6 @@ const execa = (command, args, options) => { crossSpawn._enoent.hookChildProcess(spawned, parsed.parsed); - spawned._kill = spawned.kill; - - const killSpawnedAndDescendants = signal => { - if (process.platform === 'win32') { - spawned._kill(signal); - } else { - spawned._kill(signal); - // TODO kill with pidtree - // do not kill this process via pidtree, exclude this PID? - } - }; - - // Enhance the `ChildProcess#kill` to ensure killing the process and its descendents - spawned.kill = killSpawnedAndDescendants; - handleInput(spawned, parsed.options.input); spawned.all = makeAllStream(spawned); @@ -381,6 +366,17 @@ const execa = (command, args, options) => { } }; + spawned._kill = spawned.kill; + spawned.kill = signal => { + if (process.platform === 'win32') { + return spawned._kill(signal); + } else { + return spawned._kill(signal); + // TODO kill with pidtree + // do not kill this process via pidtree, exclude this PID? + } + }; + // TOOD: Remove the `if`-guard when targeting Node.js 10 if (Promise.prototype.finally) { spawned.finally = onFinally => handlePromise().finally(onFinally); From d116af8f24240d11c956078c10dc5f953800af99 Mon Sep 17 00:00:00 2001 From: Tommaso Sotte Date: Thu, 21 Mar 2019 21:08:15 +0100 Subject: [PATCH 5/6] Added test, should fail on linux but pass on windows --- fixtures/descendant | 5 +++++ test.js | 23 +++++++++++++++++++++++ 2 files changed, 28 insertions(+) create mode 100755 fixtures/descendant diff --git a/fixtures/descendant b/fixtures/descendant new file mode 100755 index 0000000000..0e08c6a280 --- /dev/null +++ b/fixtures/descendant @@ -0,0 +1,5 @@ +#!/usr/bin/env node +'use strict'; +const { spawn } = require('child_process'); + +console.log(spawn('forever').pid); diff --git a/test.js b/test.js index 6dfb7430b1..3d7b30d63e 100644 --- a/test.js +++ b/test.js @@ -463,6 +463,29 @@ if (process.platform !== 'win32') { test('cleanup false - SIGKILL', spawnAndKill, 'SIGKILL', false); } +test('when killing the child process, kill all its descendants as well', async t => { + const cp = execa('descendant'); + let descendantPid; + + cp.stdout.setEncoding('utf8'); + cp.stdout.on('data', chunk => { + descendantPid = parseInt(chunk, 10); + t.is(typeof descendantPid, 'number'); + + setTimeout(() => { + cp.kill(); + }, 100); + }); + + await t.throwsAsync(cp); + + // Give everybody some time to breath and kill things + // await delay(200); + + t.false(isRunning(cp.pid)); + t.false(isRunning(descendantPid)); +}); + test('execa.shell() supports the `shell` option', async t => { const {stdout} = await execa.shell('node fixtures/noop foo', { shell: process.platform === 'win32' ? 'cmd.exe' : '/bin/bash' From 278fa7debcd53c64ca14b9e5dc2fcb531277934a Mon Sep 17 00:00:00 2001 From: Tommaso Sotte Date: Thu, 21 Mar 2019 21:11:36 +0100 Subject: [PATCH 6/6] Fixed code styling --- index.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/index.js b/index.js index e02e851417..51123af8ec 100644 --- a/index.js +++ b/index.js @@ -370,11 +370,11 @@ const execa = (command, args, options) => { spawned.kill = signal => { if (process.platform === 'win32') { return spawned._kill(signal); - } else { - return spawned._kill(signal); - // TODO kill with pidtree - // do not kill this process via pidtree, exclude this PID? } + + return spawned._kill(signal); + // TODO kill with pidtree + // do not kill this process via pidtree, exclude this PID? }; // TOOD: Remove the `if`-guard when targeting Node.js 10