Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make error messages more consistent #230

Merged
merged 3 commits into from May 10, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
5 changes: 0 additions & 5 deletions fixtures/error-message.js

This file was deleted.

11 changes: 6 additions & 5 deletions index.js
Expand Up @@ -177,14 +177,15 @@ function makeError(result, options) {

const [exitCodeName, exitCode] = getCode(result, code);

if (!(error instanceof Error)) {
const message = [joinedCommand, stderr, stdout].filter(Boolean).join('\n');
const prefix = getErrorPrefix({timedOut, timeout, signal, exitCodeName, exitCode, isCanceled});
const message = `Command ${prefix}: ${joinedCommand}`;

if (error instanceof Error) {
error.message = `${message}${error.message}`;
} else {
error = new Error(message);
}

const prefix = getErrorPrefix({timedOut, timeout, signal, exitCodeName, exitCode, isCanceled});
error.message = `Command ${prefix}: ${error.message}`;

error.code = exitCode || exitCodeName;
error.exitCode = exitCode;
error.exitCodeName = exitCodeName;
Expand Down
43 changes: 4 additions & 39 deletions test.js
Expand Up @@ -12,8 +12,6 @@ import execa 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/;
const TIMEOUT_REGEXP = /timed out after/;

const getExitRegExp = exitMessage => new RegExp(`failed with exit code ${exitMessage}`);
Expand Down Expand Up @@ -63,32 +61,6 @@ test('stdout/stderr/all available on errors', async t => {
t.is(typeof error.all, 'string');
});

test('include stdout and stderr in errors for improved debugging', async t => {
await t.throwsAsync(execa('fixtures/error-message.js'), {message: STDERR_STDOUT_REGEXP, code: 1});
});

test('do not include in errors when `stdio` is set to `inherit`', async t => {
await t.throwsAsync(execa('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 => {
await t.throwsAsync(execa('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 => {
await t.throwsAsync(execa('fixtures/error-message.js', {stdio: [undefined, 'inherit', 'inherit']}), {message: NO_NEWLINES_REGEXP});
});

test('do not include `stdout` in errors when set to `inherit`', async t => {
const error = await t.throwsAsync(execa('fixtures/error-message.js', {stdout: 'inherit'}), {message: /stderr/});
t.notRegex(error.message, /stdout/);
});

test('do not include `stderr` in errors when set to `inherit`', async t => {
const error = await t.throwsAsync(execa('fixtures/error-message.js', {stderr: 'inherit'}), {message: /stdout/});
t.notRegex(error.message, /stderr/);
});

test('pass `stdout` to a file descriptor', async t => {
const file = tempfile('.txt');
await execa('fixtures/noop', ['foo bar'], {stdout: fs.openSync(file, 'w')});
Expand Down Expand Up @@ -145,19 +117,12 @@ test('execa.sync()', t => {
test('execa.sync() throws error if written to stderr', t => {
t.throws(() => {
execa.sync('foo');
}, process.platform === 'win32' ? /'foo' is not recognized as an internal or external command/ : /spawnSync foo ENOENT/);
});

test('execa.sync() includes stdout and stderr in errors for improved debugging', t => {
t.throws(() => {
execa.sync('node', ['fixtures/error-message.js']);
}, {message: STDERR_STDOUT_REGEXP, code: 1});
}, process.platform === 'win32' ? /failed with exit code 1/ : /spawnSync foo ENOENT/);
});

test('skip throwing when using reject option in execa.sync()', t => {
const error = execa.sync('node', ['fixtures/error-message.js'], {reject: false});
t.is(typeof error.stdout, 'string');
t.is(typeof error.stderr, 'string');
const error = execa.sync('noop-err', ['foo'], {reject: false});
t.is(error.stderr, 'foo');
});

test('stripEof option (legacy)', async t => {
Expand All @@ -172,7 +137,7 @@ test('stripFinalNewline option', async t => {

test('preferLocal option', async t => {
await execa('ava', ['--version'], {env: {PATH: ''}});
const errorRegExp = process.platform === 'win32' ? /not recognized/ : /spawn ava ENOENT/;
const errorRegExp = process.platform === 'win32' ? /failed with exit code 1/ : /spawn ava ENOENT/;
await t.throwsAsync(execa('ava', ['--version'], {preferLocal: false, env: {PATH: ''}}), errorRegExp);
});

Expand Down