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

tests(legacy-javascript): exit code 1 on failure #10946

Merged
merged 1 commit into from
Jun 17, 2020

Conversation

connorjclark
Copy link
Collaborator

context: #10937 (comment)

Copy link
Collaborator

@patrickhulce patrickhulce left a comment

Choose a reason for hiding this comment

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

LGTM though ironically will be blocked by #10937 because we're now catching the failure :)

Copy link
Member

@brendankenny brendankenny left a comment

Choose a reason for hiding this comment

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

process.on('unhandledRejection', err => {
  throw err;
});

also works for this, depending on how you're feeling on any particular day. I don't understand why node hasn't turned on --unhandled-rejections=strict by default despite warning about doing it for years now.

@brendankenny
Copy link
Member

I don't understand why node hasn't turned on --unhandled-rejections=strict by default despite warning about doing it for years now.

guh, the answer is that there are a bunch of people who think that rejected promises shouldn't be treated as exceptions, which seems reasonable unless you consider a rejected promise is all you get when an exception is thrown inside of async functions, which are an increasingly large part of the population of functions in most codebases these days...

nodejs/node#33021

@connorjclark connorjclark reopened this Jun 17, 2020
@devtools-bot devtools-bot merged commit 525c954 into master Jun 17, 2020
@devtools-bot devtools-bot deleted the fail-leg-test-script branch June 17, 2020 02:07
gMakunde pushed a commit to gMakunde/lighthouse that referenced this pull request Jul 6, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants