From c6d1c40e8766509958a93cd6362068b532e9b71e Mon Sep 17 00:00:00 2001 From: ehmicky Date: Tue, 7 May 2019 18:53:20 +0200 Subject: [PATCH 1/2] Fix test for options.cleanup --- fixtures/sub-process | 6 ++++-- fixtures/sub-process-false | 5 ----- package.json | 2 +- test.js | 44 +++++++++++++++----------------------- 4 files changed, 22 insertions(+), 35 deletions(-) delete mode 100755 fixtures/sub-process-false diff --git a/fixtures/sub-process b/fixtures/sub-process index fd0412c280..e7213780b5 100755 --- a/fixtures/sub-process +++ b/fixtures/sub-process @@ -1,5 +1,7 @@ #!/usr/bin/env node 'use strict'; -const m = require('../'); +const execa = require('..'); -console.log(m('forever').pid); +const cleanup = process.argv[2] === 'true'; +const subprocess = execa('node', ['./fixtures/forever'], {cleanup}); +process.send(subprocess.pid); diff --git a/fixtures/sub-process-false b/fixtures/sub-process-false deleted file mode 100755 index 9b40478529..0000000000 --- a/fixtures/sub-process-false +++ /dev/null @@ -1,5 +0,0 @@ -#!/usr/bin/env node -'use strict'; -const m = require('../'); - -console.log(m('forever', {cleanup: false}).pid); diff --git a/package.json b/package.json index fed7dec3f4..93c422e754 100644 --- a/package.json +++ b/package.json @@ -52,9 +52,9 @@ "ava": "^1.4.1", "cat-names": "^2.0.0", "coveralls": "^3.0.3", - "delay": "^4.1.0", "is-running": "^2.1.0", "nyc": "^14.1.0", + "p-event": "^4.1.0", "tempfile": "^3.0.0", "tsd": "^0.7.1", "xo": "^0.24.0" diff --git a/test.js b/test.js index 2cb3095348..84c1892e26 100644 --- a/test.js +++ b/test.js @@ -5,8 +5,8 @@ import childProcess from 'child_process'; import test from 'ava'; import getStream from 'get-stream'; import isRunning from 'is-running'; -import delay from 'delay'; import tempfile from 'tempfile'; +import pEvent from 'p-event'; import execa from '.'; process.env.PATH = path.join(__dirname, 'fixtures') + path.delimiter + process.env.PATH; @@ -486,38 +486,28 @@ test(command, ' foo bar', 'foo', 'bar'); test(command, ' baz quz', 'baz', 'quz'); test(command, ''); -async function spawnAndKill(t, signal, cleanup) { - const name = cleanup ? 'sub-process' : 'sub-process-false'; - const cp = execa(name); - let pid; +async function spawnAndKill(t, signal, cleanup, isKilled) { + const subprocess = execa('sub-process', [cleanup], {stdio: ['ignore', 'inherit', 'ignore', 'ipc']}); - cp.stdout.setEncoding('utf8'); - cp.stdout.on('data', chunk => { - pid = parseInt(chunk, 10); - t.is(typeof pid, 'number'); - - setTimeout(() => { - process.kill(cp.pid, signal); - }, 100); - }); + const pid = await pEvent(subprocess, 'message'); + t.true(Number.isInteger(pid)); + t.true(isRunning(pid)); - await t.throwsAsync(cp); + process.kill(subprocess.pid, signal); - // Give everybody some time to breath and kill things - await delay(200); + await t.throwsAsync(subprocess); - t.false(isRunning(cp.pid)); - t.is(isRunning(pid), !cleanup); + t.false(isRunning(subprocess.pid)); + t.is(isRunning(pid), !isKilled); } -test('cleanup - SIGINT', spawnAndKill, 'SIGINT', true); -test('cleanup - SIGKILL', spawnAndKill, 'SIGTERM', true); - -if (process.platform !== 'win32') { - // On Windows the subprocesses are actually always killed - test('cleanup false - SIGINT', spawnAndKill, 'SIGTERM', false); - test('cleanup false - SIGKILL', spawnAndKill, 'SIGKILL', false); -} +// On Windows subprocesses are always killed +const exitIfWindows = process.platform === 'win32'; +test('cleanup true - SIGTERM', spawnAndKill, 'SIGTERM', 'true', true); +test('cleanup false - SIGTERM', spawnAndKill, 'SIGTERM', 'false', exitIfWindows); +// SIGKILL cannot be handled, so `options.cleanup` does not work with it +test('cleanup true - SIGKILL', spawnAndKill, 'SIGKILL', 'true', exitIfWindows); +test('cleanup false - SIGKILL', spawnAndKill, 'SIGKILL', 'false', exitIfWindows); test('execa.shell() supports the `shell` option', async t => { const {stdout} = await execa.shell('node fixtures/noop foo', { From 3013d93a9470f9245447c2f95c784b01e8ee103b Mon Sep 17 00:00:00 2001 From: ehmicky Date: Tue, 7 May 2019 19:46:04 +0200 Subject: [PATCH 2/2] Improve comments --- test.js | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/test.js b/test.js index 84c1892e26..421817b189 100644 --- a/test.js +++ b/test.js @@ -487,7 +487,7 @@ test(command, ' baz quz', 'baz', 'quz'); test(command, ''); async function spawnAndKill(t, signal, cleanup, isKilled) { - const subprocess = execa('sub-process', [cleanup], {stdio: ['ignore', 'inherit', 'ignore', 'ipc']}); + const subprocess = execa('sub-process', [cleanup], {stdio: ['ignore', 'ignore', 'ignore', 'ipc']}); const pid = await pEvent(subprocess, 'message'); t.true(Number.isInteger(pid)); @@ -501,11 +501,15 @@ async function spawnAndKill(t, signal, cleanup, isKilled) { t.is(isRunning(pid), !isKilled); } -// On Windows subprocesses are always killed +// Without `options.cleanup`: +// - on Windows subprocesses are killed if `options.detached: false`, but not +// if `options.detached: true` +// - on Linux subprocesses are never killed regardless of `options.detached` +// With `options.cleanup`, subprocesses are always killed +// - `options.cleanup` with SIGKILL is a noop, since it cannot be handled const exitIfWindows = process.platform === 'win32'; test('cleanup true - SIGTERM', spawnAndKill, 'SIGTERM', 'true', true); test('cleanup false - SIGTERM', spawnAndKill, 'SIGTERM', 'false', exitIfWindows); -// SIGKILL cannot be handled, so `options.cleanup` does not work with it test('cleanup true - SIGKILL', spawnAndKill, 'SIGKILL', 'true', exitIfWindows); test('cleanup false - SIGKILL', spawnAndKill, 'SIGKILL', 'false', exitIfWindows);