Skip to content

Make sure nested at_exit blocks don't break exit status

gitlab-qa-bot requested to merge github/fork/utahstreetlabs/master into master

Created by: travis

Per https://github.com/jnicklas/capybara/issues/178, it's possible to have at_exit handlers installed inside Runner's run method that execute after the 'exit' call in autorun. These at_exit handlers can actually change the exit status (in this case, manual testing reveals that capybara's call to @browser.quit in driver.rb seems to be the problem).

Avoid this issue entirely by ensuring our at_exit handler can't nest at_exit handlers.

Note that I think there's some confusion around this issue: the fundamental problem appears to be the call to @browser.exit in driver.rb of capybara, which happens inside an at_exit handler that is nested inside the rspec runner's (whew!). Somehow the exit status is being changed to 0 inside this call, resulting in the exit status of the whole (unix) command being 0 even when specs fail.

In the capybara discussion, it's believed that a ruby 1.9.3 patch fixes this issue, but from my reading of the patch that's not the case. The ruby patch takes the following commands:

at_exit { puts :outer0 } at_exit { puts :outer1_begin; at_exit { puts :inner1 }; puts :outer1_end } at_exit { puts :outer2_begin; at_exit { puts :inner2 }; puts :outer2_end } at_exit { puts :outer3 }

and changes the execution order from

outer3 outer2_begin outer2_end outer1_begin outer1_end outer0 inner1 inner2

to

outer3 outer2_begin outer2_end inner2 outer1_begin outer1_end inner1 outer0

Our issue, however, is a single nested stack, like, for instance:

at_exit { puts :outer1_begin; at_exit { puts :inner1 }; puts :outer1_end }

in the example above, this would mean a change like:

outer1_begin outer1_end outer0 # not actually in our example, here for clarity inner1 # this is inside a nested at_exit like the capybara code that changes exit status

to

outer1_begin outer1_end inner1 outer0

The problematic call in our case is "inner1" (where capybara's @browser.quit runs), whose position relative to outer1_{begin,end} (where our exit call happens) hasn't changed.

This patch (cribbed from the minitest patch linked in https://github.com/dchelimsky/rspec/issues/12) puts the exit call in its own at_exit block, which doesn't nest any other at_exits, ensuring it will be run last.

As a bonus, this will make this all work with ruby 1.9.2 and before.

I've discovered all of this while deep inside fixing our continous deployment, which was breaking as a result of this issue (ie, deploying on failed builds), so sincere apologies if I'm confused or incoherent. Thanks for great software!!

Merge request reports