From 56b749dc3cafa176c0553f369e6b1a031214b2b7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tiago=20N=C3=A1poli?= Date: Wed, 31 Jul 2019 13:57:37 -0300 Subject: [PATCH] Add `all` option (#353) Co-authored-by: ehmicky --- index.d.ts | 25 +++++++++++++--- index.js | 3 +- index.test-d.ts | 5 ++-- lib/stream.js | 15 ++-------- readme.md | 21 +++++++++++--- test/error.js | 4 +-- test/fixtures/max-buffer | 4 +-- test/stream.js | 63 ++++++++++++++++++++++++++++++++++------ 8 files changed, 104 insertions(+), 36 deletions(-) diff --git a/index.d.ts b/index.d.ts index 210299a245..54f2a1afdb 100644 --- a/index.d.ts +++ b/index.d.ts @@ -39,7 +39,7 @@ declare namespace execa { readonly localDir?: string; /** - Buffer the output from the spawned process. When buffering is disabled you must consume the output of the `stdout` and `stderr` streams because the promise will not be resolved/rejected until they have completed. + Buffer the output from the spawned process. When set to `false`, you must read the output of `stdout` and `stderr` (or `all` if the `all` option is `true`). Otherwise the returned promise will not be resolved/rejected. If the spawned process fails, `error.stdout`, `error.stderr`, and `error.all` will contain the buffered data. @@ -75,6 +75,13 @@ declare namespace execa { */ readonly reject?: boolean; + /** + Add an `.all` property on the promise and the resolved value. The property contains the output of the process with `stdout` and `stderr` interleaved. + + @default false + */ + readonly all?: boolean; + /** Strip the final [newline character](https://en.wikipedia.org/wiki/Newline) from the output. @@ -265,8 +272,12 @@ declare namespace execa { extends ExecaSyncReturnValue { /** The output of the process with `stdout` and `stderr` interleaved. + + This is `undefined` if either: + - the `all` option is `false` (default value) + - `execa.sync()` was used */ - all: StdoutErrorType; + all?: StdoutErrorType; /** Whether the process was canceled. @@ -287,8 +298,12 @@ declare namespace execa { extends ExecaSyncError { /** The output of the process with `stdout` and `stderr` interleaved. + + This is `undefined` if either: + - the `all` option is `false` (default value) + - `execa.sync()` was used */ - all: StdoutErrorType; + all?: StdoutErrorType; /** Whether the process was canceled. @@ -325,7 +340,9 @@ declare namespace execa { /** Stream combining/interleaving [`stdout`](https://nodejs.org/api/child_process.html#child_process_subprocess_stdout) and [`stderr`](https://nodejs.org/api/child_process.html#child_process_subprocess_stderr). - This is `undefined` when both `stdout` and `stderr` options are set to [`'pipe'`, `'ipc'`, `Stream` or `integer`](https://nodejs.org/dist/latest-v6.x/docs/api/child_process.html#child_process_options_stdio). + This is `undefined` if either: + - the `all` option is `false` (the default value) + - both `stdout` and `stderr` options are set to [`'inherit'`, `'ipc'`, `Stream` or `integer`](https://nodejs.org/dist/latest-v6.x/docs/api/child_process.html#child_process_options_stdio) */ all?: ReadableStream; } diff --git a/index.js b/index.js index 8b9df5fcc8..70466bb285 100644 --- a/index.js +++ b/index.js @@ -40,6 +40,7 @@ const handleArgs = (file, args, options = {}) => { encoding: 'utf8', reject: true, cleanup: true, + all: false, ...options, windowsHide: true }; @@ -150,7 +151,7 @@ const execa = (file, args, options) => { handleInput(spawned, parsed.options.input); - spawned.all = makeAllStream(spawned); + spawned.all = makeAllStream(spawned, parsed.options); return mergePromise(spawned, handlePromiseOnce); }; diff --git a/index.test-d.ts b/index.test-d.ts index 0b3347c7c9..f1261b34da 100644 --- a/index.test-d.ts +++ b/index.test-d.ts @@ -20,7 +20,7 @@ try { expectType(unicornsResult.exitCodeName); expectType(unicornsResult.stdout); expectType(unicornsResult.stderr); - expectType(unicornsResult.all); + expectType(unicornsResult.all); expectType(unicornsResult.failed); expectType(unicornsResult.timedOut); expectType(unicornsResult.isCanceled); @@ -34,7 +34,7 @@ try { expectType(execaError.exitCodeName); expectType(execaError.stdout); expectType(execaError.stderr); - expectType(execaError.all); + expectType(execaError.all); expectType(execaError.failed); expectType(execaError.timedOut); expectType(execaError.isCanceled); @@ -99,6 +99,7 @@ execa('unicorns', {stderr: 'inherit'}); execa('unicorns', {stderr: process.stderr}); execa('unicorns', {stderr: 1}); execa('unicorns', {stderr: undefined}); +execa('unicorns', {all: true}); execa('unicorns', {reject: false}); execa('unicorns', {stripFinalNewline: false}); execa('unicorns', {extendEnv: false}); diff --git a/lib/stream.js b/lib/stream.js index 827a73f9b6..2a87068274 100644 --- a/lib/stream.js +++ b/lib/stream.js @@ -19,8 +19,8 @@ const handleInput = (spawned, input) => { }; // `all` interleaves `stdout` and `stderr` -const makeAllStream = spawned => { - if (!spawned.stdout && !spawned.stderr) { +const makeAllStream = (spawned, {all}) => { + if (!all || (!spawned.stdout && !spawned.stderr)) { return; } @@ -53,19 +53,10 @@ const getBufferedData = async (stream, streamPromise) => { }; const getStreamPromise = (stream, {encoding, buffer, maxBuffer}) => { - if (!stream) { + if (!stream || !buffer) { return; } - if (!buffer) { - // TODO: Use `ret = util.promisify(stream.finished)(stream);` when targeting Node.js 10 - return new Promise((resolve, reject) => { - stream - .once('end', resolve) - .once('error', reject); - }); - } - if (encoding) { return getStream(stream, {encoding, maxBuffer}); } diff --git a/readme.md b/readme.md index 7f5bbf0721..83a55c0f5c 100644 --- a/readme.md +++ b/readme.md @@ -161,7 +161,9 @@ Type: `ReadableStream | undefined` Stream combining/interleaving [`stdout`](https://nodejs.org/api/child_process.html#child_process_subprocess_stdout) and [`stderr`](https://nodejs.org/api/child_process.html#child_process_subprocess_stderr). -This is `undefined` when both [`stdout`](#stdout-1) and [`stderr`](#stderr-1) options are set to [`'pipe'`, `'ipc'`, `Stream` or `integer`](https://nodejs.org/dist/latest-v6.x/docs/api/child_process.html#child_process_options_stdio). +This is `undefined` if either: + - the [`all` option](#all-2) is `false` (the default value) + - both [`stdout`](#stdout-1) and [`stderr`](#stderr-1) options are set to [`'inherit'`, `'ipc'`, `Stream` or `integer`](https://nodejs.org/dist/latest-v6.x/docs/api/child_process.html#child_process_options_stdio) ### execa.sync(file, [arguments], [options]) @@ -230,9 +232,13 @@ The output of the process on stderr. #### all -Type: `string | Buffer` +Type: `string | Buffer | undefined` + +The output of the process with `stdout` and `stderr` interleaved. -The output of the process on both stdout and stderr. `undefined` if `execa.sync()` was used. +This is `undefined` if either: + - the [`all` option](#all-2) is `false` (the default value) + - `execa.sync()` was used #### failed @@ -297,7 +303,7 @@ Preferred path to find locally installed binaries in (use with `preferLocal`). Type: `boolean`
Default: `true` -Buffer the output from the spawned process. When buffering is disabled you must consume the output of the `stdout` and `stderr` streams because the promise will not be resolved/rejected until they have completed. +Buffer the output from the spawned process. When set to `false`, you must read the output of [`stdout`](#stdout-1) and [`stderr`](#stderr-1) (or [`all`](#all) if the [`all`](#all-2) option is `true`). Otherwise the returned promise will not be resolved/rejected. If the spawned process fails, [`error.stdout`](#stdout), [`error.stderr`](#stderr), and [`error.all`](#all) will contain the buffered data. @@ -329,6 +335,13 @@ Default: `pipe` Same options as [`stdio`](https://nodejs.org/dist/latest-v6.x/docs/api/child_process.html#child_process_options_stdio). +#### all + +Type: `boolean`
+Default: `false` + +Add an `.all` property on the [promise](#all) and the [resolved value](#all-1). The property contains the output of the process with `stdout` and `stderr` interleaved. + #### reject Type: `boolean`
diff --git a/test/error.js b/test/error.js index cb2caee45d..a009b041fe 100644 --- a/test/error.js +++ b/test/error.js @@ -10,7 +10,7 @@ const TIMEOUT_REGEXP = /timed out after/; const getExitRegExp = exitMessage => new RegExp(`failed with exit code ${exitMessage}`); test('stdout/stderr/all available on errors', async t => { - const {stdout, stderr, all} = await t.throwsAsync(execa('exit', ['2']), {message: getExitRegExp('2')}); + const {stdout, stderr, all} = await t.throwsAsync(execa('exit', ['2'], {all: true}), {message: getExitRegExp('2')}); t.is(typeof stdout, 'string'); t.is(typeof stderr, 'string'); t.is(typeof all, 'string'); @@ -21,7 +21,7 @@ const WRONG_COMMAND = process.platform === 'win32' ? ''; test('stdout/stderr/all on process errors', async t => { - const {stdout, stderr, all} = await t.throwsAsync(execa('wrong command')); + const {stdout, stderr, all} = await t.throwsAsync(execa('wrong command', {all: true})); t.is(stdout, ''); t.is(stderr, WRONG_COMMAND); t.is(all, WRONG_COMMAND); diff --git a/test/fixtures/max-buffer b/test/fixtures/max-buffer index 7385a83083..d1cfea941c 100755 --- a/test/fixtures/max-buffer +++ b/test/fixtures/max-buffer @@ -1,6 +1,6 @@ #!/usr/bin/env node 'use strict'; -const output = process.argv[2]; -const bytes = Number(process.argv[3]); +const output = process.argv[2] || 'stdout'; +const bytes = Number(process.argv[3] || 1e7); process[output].write('.'.repeat(bytes - 1) + '\n'); diff --git a/test/stream.js b/test/stream.js index 6cbab5e2e8..fc388ad9f0 100644 --- a/test/stream.js +++ b/test/stream.js @@ -27,19 +27,24 @@ test('pass `stderr` to a file descriptor', async t => { }); test.serial('result.all shows both `stdout` and `stderr` intermixed', async t => { - const {all} = await execa('noop-132'); + const {all} = await execa('noop-132', {all: true}); t.is(all, '132'); }); +test('result.all is undefined unless opts.all is true', async t => { + const {all} = await execa('noop'); + t.is(all, undefined); +}); + test('stdout/stderr/all are undefined if ignored', async t => { - const {stdout, stderr, all} = await execa('noop', {stdio: 'ignore'}); + const {stdout, stderr, all} = await execa('noop', {stdio: 'ignore', all: true}); t.is(stdout, undefined); t.is(stderr, undefined); t.is(all, undefined); }); test('stdout/stderr/all are undefined if ignored in sync mode', t => { - const {stdout, stderr, all} = execa.sync('noop', {stdio: 'ignore'}); + const {stdout, stderr, all} = execa.sync('noop', {stdio: 'ignore', all: true}); t.is(stdout, undefined); t.is(stderr, undefined); t.is(all, undefined); @@ -98,14 +103,14 @@ test('helpful error trying to provide an input stream in sync mode', t => { test('maxBuffer affects stdout', async t => { await t.notThrowsAsync(execa('max-buffer', ['stdout', '10'], {maxBuffer: 10})); - const {stdout, all} = await t.throwsAsync(execa('max-buffer', ['stdout', '11'], {maxBuffer: 10}), /max-buffer stdout/); + const {stdout, all} = await t.throwsAsync(execa('max-buffer', ['stdout', '11'], {maxBuffer: 10, all: true}), /max-buffer stdout/); t.is(stdout, '.'.repeat(10)); t.is(all, '.'.repeat(10)); }); test('maxBuffer affects stderr', async t => { await t.notThrowsAsync(execa('max-buffer', ['stderr', '10'], {maxBuffer: 10})); - const {stderr, all} = await t.throwsAsync(execa('max-buffer', ['stderr', '11'], {maxBuffer: 10}), /max-buffer stderr/); + const {stderr, all} = await t.throwsAsync(execa('max-buffer', ['stderr', '11'], {maxBuffer: 10, all: true}), /max-buffer stderr/); t.is(stderr, '.'.repeat(10)); t.is(all, '.'.repeat(10)); }); @@ -114,8 +119,7 @@ test('do not buffer stdout when `buffer` set to `false`', async t => { const promise = execa('max-buffer', ['stdout', '10'], {buffer: false}); const [result, stdout] = await Promise.all([ promise, - getStream(promise.stdout), - getStream(promise.all) + getStream(promise.stdout) ]); t.is(result.stdout, undefined); @@ -126,8 +130,7 @@ test('do not buffer stderr when `buffer` set to `false`', async t => { const promise = execa('max-buffer', ['stderr', '10'], {buffer: false}); const [result, stderr] = await Promise.all([ promise, - getStream(promise.stderr), - getStream(promise.all) + getStream(promise.stderr) ]); t.is(result.stderr, undefined); @@ -139,3 +142,45 @@ test('do not buffer when streaming', async t => { const result = await getStream(stdout); t.is(result, '....................\n'); }); + +test('buffer: false > promise resolves', async t => { + await t.notThrowsAsync(execa('noop', {buffer: false})); +}); + +test('buffer: false > promise resolves when output is big but is not pipable', async t => { + await t.notThrowsAsync(execa('max-buffer', {buffer: false, stdout: 'ignore'})); +}); + +test('buffer: false > promise resolves when output is big and is read', async t => { + const cp = execa('max-buffer', {buffer: false}); + cp.stdout.resume(); + cp.stderr.resume(); + await t.notThrowsAsync(cp); +}); + +test('buffer: false > promise resolves when output is big and "all" is used and is read', async t => { + const cp = execa('max-buffer', {buffer: false, all: true}); + cp.all.resume(); + await t.notThrowsAsync(cp); +}); + +test('buffer: false > promise rejects when process returns non-zero', async t => { + const cp = execa('fail', {buffer: false}); + const {exitCode} = await t.throwsAsync(cp); + t.is(exitCode, 2); +}); + +const BUFFER_TIMEOUT = 1e3; + +test.serial('buffer: false > promise does not resolve when output is big and is not read', async t => { + const {timedOut} = await t.throwsAsync(execa('max-buffer', {buffer: false, timeout: BUFFER_TIMEOUT})); + t.true(timedOut); +}); + +test.serial('buffer: false > promise does not resolve when output is big and "all" is used but not read', async t => { + const cp = execa('max-buffer', {buffer: false, all: true, timeout: BUFFER_TIMEOUT}); + cp.stdout.resume(); + cp.stderr.resume(); + const {timedOut} = await t.throwsAsync(cp); + t.true(timedOut); +});