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.

Sign up for GitHub

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

Jump to bottom

Handle scripts failing with exit signals (fixes: #3954) #3995

Merged
merged 3 commits into from
Aug 2, 2017
Merged

Handle scripts failing with exit signals (fixes: #3954) #3995

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
201ea6b
Handle scripts failing with signals (fixes:#3954)
onurtemizkan Jul 23, 2017
9643175
Review updates
onurtemizkan Jul 23, 2017
70e161c
Review updates round #2
onurtemizkan Aug 1, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 7 additions & 13 deletions __tests__/lifecycle-scripts.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -144,21 +144,15 @@ test('should inherit existing environment variables when setting via yarnrc', as
});

test('should not show any error messages when script ends successfully', async () => {
const stdout = await execCommand('test', 'script-success');

expect(stdout).toMatch(/Done in /);
await expect(execCommand('test', 'script-success')).resolves.toBeDefined();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can simply return these, making it a one-liner:

test('should not show any error messages when script ends successfully', async () =>
  expect(execCommand('test', 'script-success')).resolves.toBeDefined()
);

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

});

test('should show correct error message when the script ends with an exit code', async () => {
await execCommand('test', 'script-fail').catch(error => {
expect(error.message).toMatch(/Command failed with exit code /);
expect(error.code).not.toBe(0);
});
test('should throw error when the script ends with an exit code', async () => {
await expect(execCommand('test', 'script-fail')).rejects.toBeDefined();
Copy link
Contributor Author

@onurtemizkan onurtemizkan Aug 1, 2017 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@BYK this test (also the test below) should fail when the script unexpectedly succeeds. This looked a good way to message-independently test the error to me. What do you think?

});

test('should show correct error message when the script ends an exit signal', async () => {
await execCommand('test', 'script-segfault').catch(error => {
expect(error.message).toMatch(/Command failed with signal "SIGSEGV"/);
expect(error.code).not.toBe(0);
if (process.platform === 'darwin') {
test('should throw error when the script ends with an exit signal', async () => {
await expect(execCommand('test', 'script-segfault')).rejects.toBeDefined();
});
});
}
10 changes: 5 additions & 5 deletions src/util/execute-lifecycle-script.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -254,11 +254,11 @@ export async function execCommand(stage: string, config: Config, cmd: string, cw
return Promise.resolve();
} catch (err) {
if (err instanceof SpawnError) {
if (err.EXIT_SIGNAL) {
throw new MessageError(reporter.lang('commandFailedWithSignal', err.EXIT_SIGNAL));
} else {
throw new MessageError(reporter.lang('commandFailedWithCode', err.EXIT_CODE));
}
throw new MessageError(
err.EXIT_SIGNAL
? reporter.lang('commandFailedWithSignal', err.EXIT_SIGNAL)
: reporter.lang('commandFailedWithCode', err.EXIT_CODE),
);
} else {
throw err;
}
Expand Down