From deb498807c014399d4e09d26f3e4ec367cc7977c Mon Sep 17 00:00:00 2001 From: ehmicky Date: Wed, 26 Jun 2019 07:50:25 -0700 Subject: [PATCH] Separate promise-related logic into their own files (#325) --- index.js | 50 ++------------------------------------------- lib/promise.js | 54 +++++++++++++++++++++++++++++++++++++++++++++++++ test/promise.js | 52 +++++++++++++++++++++++++++++++++++++++++++++++ test/test.js | 47 ------------------------------------------ 4 files changed, 108 insertions(+), 95 deletions(-) create mode 100644 lib/promise.js create mode 100644 test/promise.js diff --git a/index.js b/index.js index 2aa3e19d18..f9407ffea4 100644 --- a/index.js +++ b/index.js @@ -9,6 +9,7 @@ const makeError = require('./lib/error'); const normalizeStdio = require('./lib/stdio'); const {spawnedKill, spawnedCancel, setupTimeout, setExitHandler, cleanup} = require('./lib/kill'); const {handleInput, getSpawnedResult, makeAllStream, validateInputSync} = require('./lib/stream.js'); +const {mergePromise, getSpawnedPromise} = require('./lib/promise.js'); const DEFAULT_MAX_BUFFER = 1000 * 1000 * 100; @@ -78,53 +79,6 @@ const joinCommand = (file, args = []) => { return [file, ...args].join(' '); }; -const mergePromiseProperty = (spawned, getPromise, property) => { - Object.defineProperty(spawned, property, { - value(...args) { - return getPromise()[property](...args); - }, - writable: true, - enumerable: false, - configurable: true - }); -}; - -// The return value is a mixin of `childProcess` and `Promise` -const mergePromise = (spawned, getPromise) => { - mergePromiseProperty(spawned, getPromise, 'then'); - mergePromiseProperty(spawned, getPromise, 'catch'); - - // TODO: Remove the `if`-guard when targeting Node.js 10 - if (Promise.prototype.finally) { - mergePromiseProperty(spawned, getPromise, 'finally'); - } - - return spawned; -}; - -const handleSpawned = (spawned, context) => { - return new Promise((resolve, reject) => { - spawned.on('exit', (code, signal) => { - if (context.timedOut) { - reject(Object.assign(new Error('Timed out'), {code, signal})); - return; - } - - resolve({code, signal}); - }); - - spawned.on('error', error => { - reject(error); - }); - - if (spawned.stdin) { - spawned.stdin.on('error', error => { - reject(error); - }); - } - }); -}; - const execa = (file, args, options) => { const parsed = handleArgs(file, args, options); const command = joinCommand(file, args); @@ -157,7 +111,7 @@ const execa = (file, args, options) => { const removeExitHandler = setExitHandler(spawned, parsed.options); // TODO: Use native "finally" syntax when targeting Node.js 10 - const processDone = pFinally(handleSpawned(spawned, context), () => { + const processDone = pFinally(getSpawnedPromise(spawned, context), () => { cleanup(timeoutId, removeExitHandler); }); diff --git a/lib/promise.js b/lib/promise.js new file mode 100644 index 0000000000..74d2cf3004 --- /dev/null +++ b/lib/promise.js @@ -0,0 +1,54 @@ +'use strict'; +const mergePromiseProperty = (spawned, getPromise, property) => { + Object.defineProperty(spawned, property, { + value(...args) { + return getPromise()[property](...args); + }, + writable: true, + enumerable: false, + configurable: true + }); +}; + +// The return value is a mixin of `childProcess` and `Promise` +const mergePromise = (spawned, getPromise) => { + mergePromiseProperty(spawned, getPromise, 'then'); + mergePromiseProperty(spawned, getPromise, 'catch'); + + // TODO: Remove the `if`-guard when targeting Node.js 10 + if (Promise.prototype.finally) { + mergePromiseProperty(spawned, getPromise, 'finally'); + } + + return spawned; +}; + +// Use promises instead of `child_process` events +const getSpawnedPromise = (spawned, context) => { + return new Promise((resolve, reject) => { + spawned.on('exit', (code, signal) => { + if (context.timedOut) { + reject(Object.assign(new Error('Timed out'), {code, signal})); + return; + } + + resolve({code, signal}); + }); + + spawned.on('error', error => { + reject(error); + }); + + if (spawned.stdin) { + spawned.stdin.on('error', error => { + reject(error); + }); + } + }); +}; + +module.exports = { + mergePromise, + getSpawnedPromise +}; + diff --git a/test/promise.js b/test/promise.js new file mode 100644 index 0000000000..9f119e39b7 --- /dev/null +++ b/test/promise.js @@ -0,0 +1,52 @@ +import path from 'path'; +import test from 'ava'; +import execa from '..'; + +process.env.PATH = path.join(__dirname, 'fixtures') + path.delimiter + process.env.PATH; + +test('promise methods are not enumerable', t => { + const descriptors = Object.getOwnPropertyDescriptors(execa('noop')); + // eslint-disable-next-line promise/prefer-await-to-then + t.false(descriptors.then.enumerable); + t.false(descriptors.catch.enumerable); + // TOOD: Remove the `if`-guard when targeting Node.js 10 + if (Promise.prototype.finally) { + t.false(descriptors.finally.enumerable); + } +}); + +// TOOD: Remove the `if`-guard when targeting Node.js 10 +if (Promise.prototype.finally) { + test('finally function is executed on success', async t => { + let isCalled = false; + const {stdout} = await execa('noop', ['foo']).finally(() => { + isCalled = true; + }); + t.is(isCalled, true); + t.is(stdout, 'foo'); + }); + + test('finally function is executed on failure', async t => { + let isError = false; + const {stdout, stderr} = await t.throwsAsync(execa('exit', ['2']).finally(() => { + isError = true; + })); + t.is(isError, true); + t.is(typeof stdout, 'string'); + t.is(typeof stderr, 'string'); + }); + + test('throw in finally function bubbles up on success', async t => { + const {message} = await t.throwsAsync(execa('noop', ['foo']).finally(() => { + throw new Error('called'); + })); + t.is(message, 'called'); + }); + + test('throw in finally bubbles up on error', async t => { + const {message} = await t.throwsAsync(execa('exit', ['2']).finally(() => { + throw new Error('called'); + })); + t.is(message, 'called'); + }); +} diff --git a/test/test.js b/test/test.js index 70d1387352..3aa68b47c1 100644 --- a/test/test.js +++ b/test/test.js @@ -225,53 +225,6 @@ test('detach child process', async t => { process.kill(pid, 'SIGKILL'); }); -test('promise methods are not enumerable', t => { - const descriptors = Object.getOwnPropertyDescriptors(execa('noop')); - // eslint-disable-next-line promise/prefer-await-to-then - t.false(descriptors.then.enumerable); - t.false(descriptors.catch.enumerable); - // TOOD: Remove the `if`-guard when targeting Node.js 10 - if (Promise.prototype.finally) { - t.false(descriptors.finally.enumerable); - } -}); - -// TOOD: Remove the `if`-guard when targeting Node.js 10 -if (Promise.prototype.finally) { - test('finally function is executed on success', async t => { - let isCalled = false; - const {stdout} = await execa('noop', ['foo']).finally(() => { - isCalled = true; - }); - t.is(isCalled, true); - t.is(stdout, 'foo'); - }); - - test('finally function is executed on failure', async t => { - let isError = false; - const {stdout, stderr} = await t.throwsAsync(execa('exit', ['2']).finally(() => { - isError = true; - })); - t.is(isError, true); - t.is(typeof stdout, 'string'); - t.is(typeof stderr, 'string'); - }); - - test('throw in finally function bubbles up on success', async t => { - const {message} = await t.throwsAsync(execa('noop', ['foo']).finally(() => { - throw new Error('called'); - })); - t.is(message, 'called'); - }); - - test('throw in finally bubbles up on error', async t => { - const {message} = await t.throwsAsync(execa('exit', ['2']).finally(() => { - throw new Error('called'); - })); - t.is(message, 'called'); - }); -} - test('allow commands with spaces and no array arguments', async t => { const {stdout} = await execa('command with space'); t.is(stdout, '');