Separate promise-related logic into their own files (#325)

sindresorhus · Jun 26, 2019 · deb4988 · deb4988
1 parent cba87e1
commit deb4988
Show file tree

Hide file tree

Showing 4 changed files with 108 additions and 95 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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, '');