-
Notifications
You must be signed in to change notification settings - Fork 158
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
Ensure GitHub CI fails when tests are skipped due to a build failure #4368
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice! Thank you for fixing this
6dbc605
to
7910595
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hm, I expected the PR to successfully fail, but it succeeded! Maybe it's not rebased against master?
We should probably see it it fail with the current master, then merge it once Joosep's PR that contains the fix is in.
@teodanciu It did fail, before and after rebasing. The message (in the "Tests completed" job) was "Tests were skipped" and the exit code was 1. |
@lehins Do you want us to wait until |
7910595
to
8c21ff5
Compare
Merged this, I reckon it's better to have this check in sooner rather than later. |
Thank you! I agree. |
Description
When one of the matrix jobs for
build
fails, thetest
job is skipped and the logic incomplete
was not treating this as a failure. As a result, PRs were being merged that contained build failures and hadn't passed tests.Checklist
.cabal
andCHANGELOG.md
files according to theversioning process.
.cabal
files for all affected packages are updated. If you change the bounds in a cabal file, that package itself must have a version increase. (See RELEASING.md)CHANGELOG.md
for the affected packages. New section is never added with the code changes. (See RELEASING.md)fourmolu
(usescripts/fourmolize.sh
)scripts/cabal-format.sh
)hie.yaml
has been updated (usescripts/gen-hie.sh
)