Repository: sindresorhus/execa (js)

Issue:
Make the child process closer to a real Promise
Execa doesn't implement `.finally()` on the child process. This is making it difficult to come up with a correct Flow type definition for it – see discussion [on this PR](https://github.com/flow-typed/flow-typed/pull/2585).

Would you object to adding a `.finally()` method to execa's spawned child process, to make it fully implement `Promise`?

The `.finally()` method is at Stage 4 and is in the latest [draft spec](https://tc39.github.io/ecma262/#sec-promise.prototype.finally).

Pull request description:
Make the Promise interface complete by adding a `.finally()` method
Fixes #147

Patch:
diff --git a/index.js b/index.js
index 0610a58..a64dce3 100644
--- a/index.js
+++ b/index.js
@@ -316,6 +316,10 @@ module.exports = (command, args, options) => {
 
 	spawned.then = (onfulfilled, onrejected) => handlePromise().then(onfulfilled, onrejected);
 	spawned.catch = onrejected => handlePromise().catch(onrejected);
+	// eslint-disable-next-line no-use-extend-native/no-use-extend-native
+	if (Promise.prototype.finally) {
+		spawned.finally = onfinally => handlePromise().finally(onfinally);
+	}
 
 	return spawned;
 };
diff --git a/package.json b/package.json
index 4336d54..29a6dd2 100644
--- a/package.json
+++ b/package.json
@@ -46,7 +46,7 @@
 		"strip-final-newline": "^2.0.0"
 	},
 	"devDependencies": {
-		"ava": "^0.25.0",
+		"ava": "^1.2.1",
 		"cat-names": "^1.0.2",
 		"coveralls": "^3.0.1",
 		"delay": "^3.0.0",


Test patch:
diff --git a/test.js b/test.js
index c9e56f3..1c11ada 100644
--- a/test.js
+++ b/test.js
@@ -12,6 +12,9 @@ import m 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/;
+
 test('execa()', async t => {
 	const {stdout} = await m('noop', ['foo']);
 	t.is(stdout, 'foo');
@@ -42,42 +45,34 @@ test('execa.stderr()', async t => {
 });
 
 test('stdout/stderr available on errors', async t => {
-	const err = await t.throws(m('exit', ['2']));
+	const err = await t.throwsAsync(m('exit', ['2']));
 	t.is(typeof err.stdout, 'string');
 	t.is(typeof err.stderr, '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(m('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(m('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(m('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(m('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(m('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(m('fixtures/error-message.js', {stderr: 'inherit'}), {message: /stdout/});
 	t.notRegex(err.message, /stderr/);
 });
 
@@ -108,10 +103,7 @@ test('execa.sync() throws error if written to stderr', t => {
 });
 
 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(() => m.sync('node', ['fixtures/error-message.js']), {message: STDERR_STDOUT_REGEXP, code: 1});
 });
 
 test('skip throwing when using reject option in execa.sync()', t => {
@@ -126,10 +118,7 @@ test('execa.shellSync()', t => {
 });
 
 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(() => m.shellSync('node fixtures/error-message.js'), {message: STDERR_STDOUT_REGEXP, code: 1});
 });
 
 test('skip throwing when using reject option in execa.shellSync()', t => {
@@ -159,7 +148,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(m('cat-names', {preferLocal: false}), /spawn .* ENOENT/);
 	process.env.PATH = _path;
 });
 
@@ -230,13 +219,13 @@ test('execa() returns a promise with kill() and pid', t => {
 });
 
 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(m('max-buffer', ['stdout', '11'], {maxBuffer: 10}), /stdout maxBuffer exceeded/);
+	await t.notThrowsAsync(m('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(m('max-buffer', ['stderr', '13'], {maxBuffer: 12}), /stderr maxBuffer exceeded/);
+	await t.notThrowsAsync(m('max-buffer', ['stderr', '12'], {maxBuffer: 12}));
 });
 
 test('do not buffer stdout when `buffer` set to `false`', async t => {
@@ -269,10 +258,9 @@ test('skip throwing when using reject option', async t => {
 
 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 error = await t.throwsAsync(m('exit', ['2']), {code: 2});
 	t.is(code, 0);
 	t.false(failed);
-	t.is(error.code, 2);
 	t.true(error.failed);
 });
 
@@ -285,7 +273,7 @@ test('use relative path with \'..\' chars', async t => {
 if (process.platform !== 'win32') {
 	test('execa() rejects if running non-executable', async t => {
 		const cp = m('non-executable');
-		await t.throws(cp);
+		await t.throwsAsync(cp);
 	});
 }
 
@@ -296,7 +284,7 @@ test('error.killed is true if process was killed directly', async t => {
 		cp.kill();
 	}, 100);
 
-	const error = await t.throws(cp);
+	const error = await t.throwsAsync(cp);
 	t.true(error.killed);
 });
 
@@ -308,7 +296,7 @@ test('error.killed is false if process was killed indirectly', async t => {
 		process.kill(cp.pid, 'SIGINT');
 	}, 100);
 
-	const error = await t.throws(cp);
+	const error = await t.throwsAsync(cp);
 	t.false(error.killed);
 });
 
@@ -334,7 +322,7 @@ if (process.platform !== 'win32') {
 			process.kill(cp.pid, 'SIGINT');
 		}, 100);
 
-		const error = await t.throws(cp);
+		const error = await t.throwsAsync(cp);
 		t.is(error.signal, 'SIGINT');
 	});
 
@@ -345,12 +333,12 @@ if (process.platform !== 'win32') {
 			process.kill(cp.pid, 'SIGTERM');
 		}, 100);
 
-		const error = await t.throws(cp);
+		const error = await t.throwsAsync(cp);
 		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(m('delay', ['3000', '0'], {killSignal: 'SIGHUP', timeout: 1500}));
 		t.is(error.signal, 'SIGHUP');
 	});
 }
@@ -360,13 +348,12 @@ test('result.signal is null for successful execution', async t => {
 });
 
 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(m('exit', [2]));
 	t.is(error.signal, null);
 });
 
 async function code(t, num) {
-	const error = await t.throws(m('exit', [`${num}`]));
-	t.is(error.code, num);
+	await t.throwsAsync(m('exit', [`${num}`]), {code: num});
 }
 
 test('error.code is 2', code, 2);
@@ -374,17 +361,15 @@ 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(m('delay', ['60000', '0'], {timeout: 1500}));
 
 	t.true(error.timedOut);
 	t.not(error.code, 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(m('delay', ['3000', '22'], {timeout: 30000}), {code: 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 => {
@@ -393,13 +378,12 @@ test('timedOut will be false if no timeout was set and zero exit code', async t
 });
 
 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(m('delay', ['1000', '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(m('exit', args), {message: expected});
 }
 
 errorMessage.title = (message, expected) => `error.message matches: ${expected}`;
@@ -408,7 +392,7 @@ test(errorMessage, /Command failed: exit 2 foo bar/, 2, 'foo', 'bar');
 test(errorMessage, /Command failed: 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(m('fail', args));
 	t.is(error.cmd, `fail${expected}`);
 
 	const result = await m('noop', args);
@@ -436,7 +420,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);
@@ -535,3 +519,39 @@ test('removes exit handler on exit', async t => {
 	const included = ee.listeners('exit').includes(listener);
 	t.false(included);
 });
+
+// eslint-disable-next-line no-use-extend-native/no-use-extend-native
+if (Promise.prototype.finally) {
+	test('finally function is executed on success', async t => {
+		let called = false;
+		const {stdout} = await m('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.throws(m('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.throws(m('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.throws(m('exit', ['2']).finally(() => {
+			throw new Error('called');
+		}));
+		t.is(result.message, 'called');
+	});
+}