Skip to content

Increase visibility of allowed failures #4584

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

Merged
merged 12 commits into from
Dec 7, 2020

Conversation

keewis
Copy link
Collaborator

@keewis keewis commented Nov 14, 2020

Right now, allowed failures are hard to see, so this tries to add warnings to increase the visibility without removing the allow_failure flag.

Copy link
Collaborator

@max-sixty max-sixty left a comment

Choose a reason for hiding this comment

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

👍

@keewis keewis marked this pull request as draft November 14, 2020 00:53
@keewis
Copy link
Collaborator Author

keewis commented Nov 14, 2020

It seems we can set the result to "succeeded with issues", which works pretty well in the pipeline overview. However, the only difference this makes on github is that it prints the result in the checks tab – no fancy icon, github seems to only know the success, failure, cancelled, and skipped results, so we still get a green check mark.

We also need to decide whether to set allow_failure: true for the upstream-dev CI. I'd vote for waiting until the summary bar at the bottom of a PR (which prints the number of failed and successful checks) also lists the number of partially successful checks.

Edit: before merging I still need to remove the intentionally failing test

@keewis keewis marked this pull request as ready for review November 14, 2020 15:23
Copy link
Collaborator

@mathause mathause left a comment

Choose a reason for hiding this comment

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

Thanks for figuring this out. Re the upstream-dev - I think if #4583 works as it should we could also allow failures for the upstream dev.

Co-authored-by: Mathias Hauser <[email protected]>
@keewis
Copy link
Collaborator Author

keewis commented Nov 18, 2020

since I removed the intentionally failing test, this is how a failed CI with allowed_failure: true looks like:

the pipelines overview

warning

checks tab (pydata.xarray)

overview_total

checks tab (pydata.xarray (Linux py38-flaky))overview_flaky

@keewis
Copy link
Collaborator Author

keewis commented Nov 18, 2020

if we agree on keeping the allow_failure flag on py38-upstream-dev this should be ready for merging.

@max-sixty
Copy link
Collaborator

Great! Definitely LGTM!

@keewis keewis changed the title WIP: Increase visibility of allowed failures Increase visibility of allowed failures Nov 19, 2020
@keewis keewis mentioned this pull request Nov 22, 2020
@keewis keewis force-pushed the increase-visibility-of-allowed-failures branch from cc0e74f to ebb8c40 Compare December 6, 2020 22:30
@keewis
Copy link
Collaborator Author

keewis commented Dec 6, 2020

should we merge this? I think we can decide which CI to keep (pipelines or github actions) in a different PR / issue

@keewis
Copy link
Collaborator Author

keewis commented Dec 7, 2020

we have two votes agreeing with me, so let's merge!

@keewis keewis merged commit c4f37b8 into pydata:master Dec 7, 2020
@keewis keewis deleted the increase-visibility-of-allowed-failures branch December 7, 2020 01:02
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.

3 participants