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

Don't exit on successful RSpec and Cucumber runs #25

Merged
merged 1 commit into from
Oct 19, 2015

Conversation

sentience
Copy link
Contributor

In our CI setup, we run a single Rake task that invokes multiple test suites, including RSpec and Cucumber. We wish to use Knapsack to parallelise the RSpec and Cucumber suites; however, both RSpecRunner and CucumberRunner end with a call to exit, which exits Rake whether or not the suite ran successfully. This prevented any subsequent Rake tasks from running.

Knapsack’s runners should coexist with other Rake tasks, including any that may depend on the Knapsack tasks running first.

This change only exits from Knapsack when the child process exited with a non-zero status.

Knapsack runners should coexist with other Rake tasks. Exiting even when the
tests ran successfully was preventing any subsequent Rake tasks from running.
@ArturT
Copy link
Member

ArturT commented Oct 19, 2015

@ciembor Will this change break your rerun failing specs solution? http://blog.lunarlogic.io/2015/randomly-failing-cucumber-scenarios-in-continuous-integration-system/

Related old PR #17

@sentience Thanks for PR. Is that mean when the first test suite failed then chain is stopped. The second test suite won't be executed?
Maybe we should introduce configuration option to enable/disable exit instead of doing it only when non zero exist status.
Your current solution is the default behaviour how regular rake task should work?

@sentience
Copy link
Contributor Author

@ArturT Currently if you run a set of Rake tasks and the first task fails, the task will normally exit with a non-zero status code. Rake doesn’t catch that; the exit actually exits the Rake process, and the non-zero code indicates to the shell that it failed.

Calling exit inside a Rake task with a status code of 0 is very weird, because it forces the Rake process to exit immediately, but with a status code that tells the shell it was successful.

My fix makes knapsack’s runners behave the way Rake tasks are expected to behave. If there’s a use case where it’s desirable for a task to force Rake to exit with a successful status code, I’d be surprised, but of course very interested! :)

ArturT added a commit to KnapsackPro/knapsack_pro-ruby that referenced this pull request Oct 19, 2015
ArturT added a commit that referenced this pull request Oct 19, 2015
Don't exit on successful RSpec and Cucumber runs
@ArturT ArturT merged commit db0a8be into KnapsackPro:master Oct 19, 2015
@ArturT
Copy link
Member

ArturT commented Oct 19, 2015

@sentience Thanks for clarification and PR! I'm going to release new version of the gem today.

@@ -15,7 +15,7 @@ def self.run(args)
cmd = %Q[bundle exec rspec #{args} --default-path #{allocator.test_dir} -- #{allocator.stringify_node_tests}]

system(cmd)
exit($?.exitstatus)
exit($?.exitstatus) unless $?.exitstatus == 0
Copy link
Contributor

Choose a reason for hiding this comment

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

or $?.success? or $?.exitstatus.zero? :tmyk:

@sentience
Copy link
Contributor Author

$?.success? would indeed be cleaner. Happy with that if you want to change it.

@ArturT
Copy link
Member

ArturT commented Oct 23, 2015

I will do change today so no need for such tiny PR :)

ArturT added a commit that referenced this pull request Oct 23, 2015
ArturT added a commit to KnapsackPro/knapsack_pro-ruby that referenced this pull request Oct 23, 2015
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