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

Add devtools catch #1137

Merged
merged 1 commit into from Sep 30, 2020
Merged

Add devtools catch #1137

merged 1 commit into from Sep 30, 2020

Conversation

Aerijo
Copy link
Member

@Aerijo Aerijo commented Sep 30, 2020

Requirements

  • Filling out the template is required. Any pull request that does not include enough information to be reviewed in a timely manner may be closed at the maintainers' discretion.
  • All new code requires tests to ensure against regressions

Description of the Change

Reduces flaky test failures on the main Atom repo CI. The summary is that an uncaught exception is triggering dev tools, which breaks tests. See atom/atom#21335 for more.

Alternate Designs

Fixing the root of the problem. But that's a lot more difficult, seeing as the cause hasn't been completely determined yet. Investigations are welcome!

Benefits

Reduced false CI failures on main repo PRs

Possible Drawbacks

This is only masking the deeper issue, so isn't a fix and should not be treated as such.

Applicable Issues

None. It does not completely resolve any issues.


// The CI on Atom has been failing with this package. Experiments
// indicate the dev tools were being opened, which caused test
// failures. Dev tools are meant to be triggered on an uncaught
// exception.
//
// These failures are flaky though, so an exact cause
// has not yet been found. For now the following is added
// to reduce the number of flaky failures, which have been
// causing false failures on unrelated Atom PRs.
//
// See more in https://github.com/atom/atom/pull/21335
Copy link
Contributor

@sadick254 sadick254 Sep 30, 2020

Choose a reason for hiding this comment

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

Thanks for adding this context @Aerijo

@sadick254 sadick254 merged commit b8248b4 into atom:master Sep 30, 2020
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants