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

ensure ci.sh exits with error when any test fails #426

Merged
merged 1 commit into from
Mar 16, 2015

Conversation

dicej
Copy link
Member

@dicej dicej commented Mar 16, 2015

As explained in
http://stackoverflow.com/questions/25794905/why-does-set-e-true-false-true-not-exit,
"set -e" will only cause the script to exit if the failing command is
"unhandled". The fancy || and && shortcuts we were using to run some
of the tests caused bash to think we were handling the errors, which
isn't what we wanted.

Fixes #416

As explained in
http://stackoverflow.com/questions/25794905/why-does-set-e-true-false-true-not-exit,
"set -e" will only cause the script to exit if the failing command is
"unhandled".  The fancy || and && shortcuts we were using to run some
of the tests caused bash to think we were handling the errors, which
isn't what we wanted.
@joshuawarner32
Copy link
Collaborator

I'd hazard a guess to say that the first and third hunks aren't strictly necessary. However, do show a good example, and having that precedent in there ought to discourage further similar mistakes.

FWIW, I wasn't seeing this locally because I wasn't actually making ci.sh fail how it was in #425.

Good find!

@joshuawarner32
Copy link
Collaborator

Checking the build output this time... And looks good!

joshuawarner32 added a commit that referenced this pull request Mar 16, 2015
ensure ci.sh exits with error when any test fails
@joshuawarner32 joshuawarner32 merged commit cacc099 into ReadyTalk:master Mar 16, 2015
@dicej dicej deleted the ci-error branch May 4, 2015 17:11
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.

ci.sh test failures don't cause Travis job to fail
2 participants