Skip to content

Proposal: avoid using #at_exit to run the suite where possible

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

Created by: benhoskings

TL;DR: This patch moves spec running out of ruby's #at_exit handler except when autorunning is explicitly enabled.

The specs are currently run like so:

  • rspec is required
  • the specs themselves are required
  • the ruby process exits, triggering #at_exit callbacks
  • this callback fires, which does the actual spec running.

The fact that the suite actually runs as the ruby process is exiting has caused some troubles for me recently.

  • A regular #exit call in unrelated code causes rspec to run the suite a second time.
  • Using #exit! to avoid that behaviour as libraries like test-queue do skips all the other #at_exit hooks, which skips writing coverage results, shutting down selenium browsers, and so on.

The run-during-exit behaviour has been present ever since rspec was forked from micronaut. David Chelimsky explained his rationale for it here -- it makes running via both ruby and rspec easy:

When running via ruby, autorun is required, because all that happens is that rspec and the spec mentioned are loaded; the run isn't explicitly triggered. A require spec/autorun would be required in spec_helper.rb in this case.

bundle exec ruby -Ispec spec/blah_spec.rb

But, in the common case that is used these days (often triggered via rake), there's no need for autorun, since we're running an rspec binary and we can trigger the run ourselves (as seen in my patch, in exe/rspec).

rspec spec/blah_spec.rb

Anecdotally, I don't think the ruby case is ever really used these days. In any case, it'd be simple to detect when rspec was invoked by ruby and log about it if autorun wasn't required explicitly. If there's interest I'll add this.

This #at_exit use and the associated workarounds have caused other problems in the past, like this one: https://github.com/rspec/rspec-core/pull/410

Anyhow, interested to hear everyone's thoughts on this.

Merge request reports