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

Avoid using Promise.prototype.finally in idlharness.js #12432

Merged
merged 1 commit into from Aug 13, 2018

Conversation

foolip
Copy link
Member

@foolip foolip commented Aug 13, 2018

In this context, using .then() achieves the same thing because of the
preceding .catch().

Fixes #12428.

In this context, using .then() achieves the same thing because of the
preceding .catch().

Fixes #12428.
@foolip
Copy link
Member Author

foolip commented Aug 13, 2018

I've verified that this fixes the setup error in #12428.

I've also checked that Promise.prototype.finally is in Edge Insider Preview, so that the root problem is going to be resolved. The only way to prevent this from happening in the first place would be to run stable versions of browsers on PRs and have some sort of smoke test for idlharness.js which can be assumed to pass 100%. Second best would be noticing the regression automatically once a new run happens in wpt.fyi. cc @lukebjerring.

@thejohnjansen FYI that this has happened, in case you're also noticing these failures on the imported tests.

@jgraham jgraham merged commit 8b3baca into master Aug 13, 2018
@foolip foolip deleted the idl_test-finally branch August 13, 2018 13:31
@foolip
Copy link
Member Author

foolip commented Aug 13, 2018

#6266 is about writing down the policy that I've implicitly followed here, and #6333 the PR to do so.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants