Skip to content
This repository

RSpec fails to return a proper error code when a capybara test fails #410

Merged
merged 5 commits into from over 2 years ago

4 participants

Daniel Doubrovkine (dB.) @dblockdotorg David Chelimsky Ben Hoskings Edgars Beigarts
Daniel Doubrovkine (dB.) @dblockdotorg
dblock commented June 22, 2011

This is a workaround for jnicklas/capybara#178. Capybara has an at_exit call that messes up the status code returned from the process. This workaround ensures that at_exit from rspec is run last (ie. is setup first).

The same was applied in minitest seattlerb/minitest@979406d.

David Chelimsky dchelimsky closed this June 24, 2011
Daniel Doubrovkine (dB.) @dblockdotorg
dblock commented June 30, 2011

@dchelimsky: you closed this without a reason? this is still a problem that affects a ton of people, no?

David Chelimsky
Owner

Looks like I missed a step - sorry about that. This is a duplicate of #400.

Daniel Doubrovkine (dB.) @dblockdotorg
dblock commented June 30, 2011

But #400 has a messy fix that was rolled back because it breaks 1.8.7 support. This doesn't as far as I can think of, and works around the problem. No?

David Chelimsky dchelimsky reopened this June 30, 2011
David Chelimsky
Owner

I've reopened this to keep it on the radar, but I'm working on a different solution: eliminating the at_exit hook entirely. If that doesn't work out I'll revisit this.

David Chelimsky
Owner

I've been toying with removing the at_exit hook, and it is a bit problem-ridden. I think it's the right way to go in the long run, but I don't want that to hang up the next release.

In the mean time, I merged this and got a failing cuke: https://github.com/rspec/rspec-core/blob/master/features/command_line/ruby.feature. @dblock - can you look at getting that scenario to pass?

Daniel Doubrovkine (dB.) @dblockdotorg
dblock commented July 30, 2011

I was having all kinds of problems doing a repro, so I got a clean rspec-core from git master, running bundle install and rake. Rspec specs pass. Cucumber specs fail with the following.

    process still alive after 5 seconds (ChildProcess::TimeoutError)
    features/command_line/line_number_appended_to_path.feature:97:in `When I run `rspec example_spec.rb:7 --format doc`'

    process still alive after 5 seconds (ChildProcess::TimeoutError)
    features/command_line/line_number_option.feature:55:in `When I run `rspec example_spec.rb --line_number 5 --format doc`'

Many more like this... before I fix real errors, how do I fix this?

David Chelimsky
Owner

The failures you are seeing happen intermittently depending on the environment (they are both timeouts) - although I thought I bumped the time to 15 secs, but maybe not (if not, I should).

As for the failure:

$ bin/cucumber features/command_line/ruby.feature:10 --format pretty
Using the default profile...
Feature: run with ruby command

  You can use the `ruby` command to run specs. You just need to require
  `rspec/autorun`. 

  Generally speaking, you're better off using the `rspec` command, which
  requires `rspec/autorun` for you, but some tools only work with the `ruby`
  command.

  Scenario:                                                # features/command_line/ruby.feature:10
    Given a file named "example_spec.rb" with:             # aruba-0.4.2/lib/aruba/cucumber.rb:15
      """
      require 'rspec/autorun'

      describe 1 do
        it "is < 2" do
          1.should be < 2
        end
      end
      """
    When I run `ruby example_spec.rb`                      # aruba-0.4.2/lib/aruba/cucumber.rb:52
    Then the output should contain "1 example, 0 failures" # aruba-0.4.2/lib/aruba/cucumber.rb:78
      expected "No examples found.\n\n\nFinished in 0.00003 seconds\n0 examples, 0 failures\n" to include "1 example, 0 failures"
      Diff:
      @@ -1,2 +1,6 @@
      -1 example, 0 failures
      +No examples found.
      +
      +
      +Finished in 0.00003 seconds
      +0 examples, 0 failures
       (RSpec::Expectations::ExpectationNotMetError)
      features/command_line/ruby.feature:22:in `Then the output should contain "1 example, 0 failures"'

Failing Scenarios:
cucumber features/command_line/ruby.feature:10 # Scenario: 

1 scenario (1 failed)
3 steps (1 failed, 2 passed)
0m1.224s
Daniel Doubrovkine (dB.) @dblockdotorg
dblock commented July 31, 2011

Ok. Got a repro. But I don't think we can fix this.

Autorun used to run last by registering itself as an at_exit hook. So now that it doesn't, it no longer runs last. So specs that are describe-d after the inclusion don't exist yet. The only fix is to include autorun last or to go back to the at_exit hook that breaks the original problem.

Ideas?

Also, note to self, I should write a test that simulates the at_exit problem with capybara ...

Ben Hoskings

I just forked rspec-core (from v2.6.4) to merge this patch for our build server, it seems to do the trick:

http://github.com/conversation/rspec-core

Any chance of merging dblock's fixes while the proper at_exit fix is in progress?

David Chelimsky dchelimsky closed this in 03a5ca5 August 21, 2011
David Chelimsky
Owner

The problem was that the exit code was coming from Capy's at_exit hook, not that rspec's wasn't being run. The patch as/was solved that problem but broke other behavior: running specs with the rcov or ruby commands. I made a minor adjustment in dc290d3 that resolved that issue and now all specs and cukes are passing.

Thanks @dblock.

Daniel Doubrovkine (dB.) @dblockdotorg

Doesn't work :( If anything calls at_exit and sets the process exit code (which Capybara does), it overwrites RSpec's.
https://github.com/dblock/rspec-core/tree/at-exit-order has a cucumber test that reproduces it.
Still don't know how to fix it.

I did some cleanup on my fork, use at-exit-order branch that has the autorun workaround (but breaks autorun).

David Chelimsky dchelimsky referenced this pull request from a commit August 22, 2011
David Chelimsky Restore old at_exit hook. This leaves #410 open and unresolved.
Also tweak the 'upstream at_exit{exit 0}' cuke to fail correctly, but
leave it tagged @wip.
06991c4
David Chelimsky dchelimsky reopened this August 22, 2011
David Chelimsky
Owner

OK - I've reviewed jnicklas/capybara#178 and have a much better understanding of the issue now. I've restored the original at_exit hook, which passes all specs and cukes except for the new one associated with this issue, which I've also restored to something like @dblock's original version which exposes the issue.

I can't accept @dblock's patch as/is, as it breaks other functionality while resolving this issue. I've explored a number of other options and I'm stumped, so if anybody else can come up with a way to resolve this without breaking anything else, you'll make a bunch of people very happy.

Daniel Doubrovkine (dB.) @dblockdotorg

@dchelimsky: thank you, totally agree

I'd like to make a radical proposal, - to kill autorun. IMO, it's evil since it runs something automagically. I wonder who or why would use it? So I can't write ruby whatever_test.rb, but I can still do rspec whatever_test.rb which is how most people I know use rspec.

David Chelimsky
Owner

@dblock - not at all radical. That's what I was referring to in my 2nd comment in this thread. The trick is that there are a number of use cases that we need to support. For example, we can't have rspec running automatically just because it is required (it should only run when specs are loaded), and, as you suggest, it needs to run whether you type ruby some_spec.rb or rspec some_spec.rb. I've been playing around with this for a while and have yet to come up with a solution that solves for all the use cases. If you have some ideas about how to do it, I'm all ears/eyes.

David Chelimsky
Owner

@dblock - also - I'm pretty sure that this particular issue (an upstream at_exit hook that exits) will remain if we eliminate our own at_exit hook.

Daniel Doubrovkine (dB.) @dblockdotorg

@dchelimsky: half of the problems are because of the at_exit nesting (see #4400). So I would keep at_exit, just not nested. Now, why should spec autorun when I do ruby some_spec.rb?

David Chelimsky
Owner

@dblock - how else will the spec run?

Daniel Doubrovkine (dB.) @dblockdotorg

@dchelimsky: rspec some_spec.rb. That uses CommandLine to find the specs and then runs them. So you don't need autorun to do it on an at_exit. In contrast, when you do ruby spec.rb, it includes autorun first (you have to), then registers the spec and runs it on an at_exit. I'll try to find some time to R.I.P autorun out tomorrow - it will be clear what works and what is now gone.

David Chelimsky
Owner

@dblock - I look forward to seeing what you come up with. Keep in mind that we need to be able to run the same file with all of these:

rspec example_spec.rb
ruby example_spec.rb
rcov example_spec.rb

There are also other cases like rspec spec, which will run all the files in the spec directory that match a pattern. And a new feature that let's you type just rspec, which runs all the files in the default_path (defaults to 'spec', but is configurable) that match a pattern.

The other trick is that the specs should not be run by simply requiring 'rspec/core', as that would mean specs run when you run any rake task.

The code you'll need to work with is in lib/rspec/core/runner.rb and lib/rspec/core/configuration.rb.

Good luck!

David Chelimsky
Owner

@dblock - oh, and then there's the rake tasks :) See lib/rspec/core/rake_task.rb.

Edgars Beigarts

Yesterday I had the same issue and I fixed this at capybara side jnicklas/capybara#463 but then I found out this pull request here. Maybe this should be fixed at both sides?

Daniel Doubrovkine (dB.) @dblockdotorg

@ebeigarts: if your fix actually fixes the problem (looks like it does), then it should be merged to Capybara. We've been going back-and-forth here on trying to fix all scenarios at no avail so far.

David Chelimsky dchelimsky merged commit dc290d3 into from August 24, 2011
David Chelimsky dchelimsky closed this August 24, 2011
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
This page is out of date. Refresh to see the latest.
1  Changelog.md
Source Rendered
@@ -27,6 +27,7 @@ As of 2.7.0, you must explicity `require "rspec/autorun"` unless you use the
27 27
   * Fix --pattern option (wasn't being recognized) (David Chelimsky)
28 28
   * Only implicitly require "rspec/autorun" with the `rspec` command (David
29 29
     Chelimsky)
  30
+  * Ensure that rspec's at_exit defines the exit code (Daniel Doubrovkine)
30 31
 
31 32
 ### 2.6.4 / 2011-06-06
32 33
 
4  Gemfile
@@ -15,7 +15,7 @@ gem "rake", "0.9.2"
15 15
 gem "cucumber", "1.0.1"
16 16
 gem "aruba", "0.4.2"
17 17
 gem "ZenTest", "4.4.2"
18  
-gem "nokogiri", "1.4.4"
  18
+gem "nokogiri", "1.5.0"
19 19
 
20 20
 platforms :jruby do
21 21
   gem "jruby-openssl"
@@ -36,7 +36,7 @@ group :development do
36 36
   gem "spork", "0.9.0.rc9"
37 37
 
38 38
   platforms :mri_18 do
39  
-    gem "rcov", "0.9.9"
  39
+    gem "rcov", "0.9.10"
40 40
     gem 'ruby-debug'
41 41
   end
42 42
 
4  lib/rspec/core/runner.rb
@@ -8,7 +8,9 @@ class Runner
8 8
       def self.autorun
9 9
         return if autorun_disabled? || installed_at_exit? || running_in_drb?
10 10
         @installed_at_exit = true
11  
-        at_exit { exit(run(ARGV, $stderr, $stdout).to_i) }
  11
+        exit_code = nil
  12
+        at_exit { exit exit_code }
  13
+        at_exit { exit_code = run(ARGV, $stderr, $stdout).to_i }
12 14
       end
13 15
       AT_EXIT_HOOK_BACKTRACE_LINE = "#{__FILE__}:#{__LINE__ - 2}:in `autorun'"
14 16
 
3  spec/rspec/core/runner_spec.rb
@@ -7,7 +7,8 @@ module RSpec::Core
7 7
         RSpec::Core::Runner.stub(:installed_at_exit?).and_return(false)
8 8
         RSpec::Core::Runner.stub(:running_in_drb?).and_return(false)
9 9
         RSpec::Core::Runner.stub(:at_exit_hook_disabled?).and_return(false)
10  
-        RSpec::Core::Runner.should_receive(:at_exit)
  10
+        RSpec::Core::Runner.stub(:run).and_return(-1)
  11
+        RSpec::Core::Runner.should_receive(:at_exit).twice
11 12
         RSpec::Core::Runner.autorun
12 13
       end
13 14
 
2  spec/spec_helper.rb
@@ -15,7 +15,7 @@ def self.each_run
15 15
 end
16 16
 
17 17
 Spork.prefork do
18  
-  require 'rspec/core'
  18
+  require 'rspec/autorun'
19 19
   require 'autotest/rspec2'
20 20
 
21 21
   Dir['./spec/support/**/*.rb'].map {|f| require f}
Commit_comment_tip

Tip: You can add notes to lines in a file. Hover to the left of a line to make a note

Something went wrong with that request. Please try again.