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

Actually fail the build if rake test fails #433

Merged
merged 1 commit into from
Jun 29, 2017
Merged

Conversation

mvz
Copy link
Contributor

@mvz mvz commented Jun 9, 2017

Summary

Fixes script/test so its exit code is that of rake rather than of echo.

Motivation and Context

The Travis builds seem green, but if you look at the logs there are a lot of failed scenarios. It seems a lot of things are broken and actually failing the build is a first step to fixing them.

@junaruga
Copy link
Contributor

junaruga commented Jun 9, 2017

Looks good for me. We have to look the fact :)
The error causes from Rubocop test.

@junaruga
Copy link
Contributor

junaruga commented Jun 9, 2017

Adding set -e in the script/test to prevent unintended success.

Or without adding set -e, show exit status expressly like this.

diff --git a/script/test b/script/test
index bdad103..b85fcaa 100755
--- a/script/test
+++ b/script/test
@@ -2,9 +2,12 @@
 
 : ${RUN_IN_DOCKER:='0'}
 
+STATUS=0
 if [ "$RUN_IN_DOCKER" == '1' ]; then
   bundle exec rake "docker:run[$*]"
+  STATUS="${?}"
 else
   bundle exec rake test $*
-  echo bundle exec rake test $*
+  STATUS="${?}"
 fi
+exit "${STATUS}"

By the way, as the command line tool to do lint a bash shell script code such as Rubocop for Ruby.
I can recommend bashate and shellcheck

https://github.com/openstack-dev/bashate
https://github.com/koalaman/shellcheck

@mvz
Copy link
Contributor Author

mvz commented Jun 9, 2017

The error causes from Rubocop test.

The cukes are also failing. I am preparing another PR to fix those.

@mvz mvz mentioned this pull request Jun 9, 2017
@mvz
Copy link
Contributor Author

mvz commented Jun 9, 2017

Something weird is going on in licence_finder 😕

@maxmeyer maxmeyer merged commit 4ae4079 into master Jun 29, 2017
@maxmeyer
Copy link
Member

Thanks.

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