Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

errors in before/after(:all) are not rescued and treated as failures #21

Closed
dchelimsky opened this Issue · 36 comments

7 participants

@dchelimsky
Owner

No description provided.

@dchelimsky
Owner

This applies to after(:all) as well - maybe these shouldn't be treated the same as failures, but should have their own messaging other than bubbling up an error? Or maybe bubbling up an error in before/after(:all) is perfectly fine feedback.

@dchelimsky
Owner

Giving this some more thought, I think an error in before/after all is perfectly fine feedback. If anybody is paying attention and thinks otherwise, make your case a comment in this issue, please.

@hjdivad

david, to answer your questions from the issue logged in rspec-rails[1].

What version of rspec are you using? We're already up to beta.14 and you're
citing beta.8. I'm definitely not seeing the error being swallowed. What I see
is that the error bubbles up and execution stops entirely:

$ bundle exec rspec --version
2.0.0.beta.12
$ bundle show rspec
/home/davidjh/.rvm/gems/ree-1.8.7-2010.01/gems/rspec-2.0.0.beta.12

I don't see the error being swallowed when running spec (or rspec) directly
either: truncating that part of the output when I opened the issue was sloppy on
my part.

However, the stack trace can be swallowed (and I think I have seen it swallowed
by autotest) because it prints to STDERR but the other output prints to STDOUT.
The problem is that the STDOUT output that lists the numbers of examples and
failures is exactly the same as in success, and indeed is even green when rspec
is run with -c.

One alternative would be to capture and report it in the same way as
before(:each), but in before(:each) an error halts execution of the example -
if it's in a before(:all), should we report all the examples in that group as
failing? Or just bail out of the group on the first failure?

Thoughts?

Treating all examples in the group as failing seems like the right thing to do.
Personally I think not being able to set up an example counts as a failure, but
I can understand a view that says it's a different kind of error condition.

BTW I updated to 2.0.0.beta.15 and get the same behaviour (which is of course,
not surprising).

$ bundle exec rspec spec/models/zombie_spec.rb

Finished in 0.00036 seconds
1 example, 0 failures
/home/davidjh/.rvm/gems/ree-1.8.7-2010.01/gems/rspec-core-2.0.0.beta.15/lib/rspec/core/backward_compatibility.rb:26:in `const_missing': uninitialized constant UndefinedConstant (NameError)
    from /home/davidjh/.rvm/gems/ree-1.8.7-2010.01/gems/rspec-expectations-2.0.0.beta.15/lib/rspec/expectations/backward_compatibility.rb:6:in `const_missing'
    from /home/davidjh/devel/.sandbox/rspec-issue-before-all-badness/spec/models/zombie_spec.rb:6
    from /home/davidjh/.rvm/gems/ree-1.8.7-2010.01/gems/rspec-core-2.0.0.beta.15/lib/rspec/core/hooks.rb:60:in `instance_eval'
    from /home/davidjh/.rvm/gems/ree-1.8.7-2010.01/gems/rspec-core-2.0.0.beta.15/lib/rspec/core/hooks.rb:60:in `run_hook!'
    from /home/davidjh/.rvm/gems/ree-1.8.7-2010.01/gems/rspec-core-2.0.0.beta.15/lib/rspec/core/example_group.rb:148:in `eval_before_alls'
    from /home/davidjh/.rvm/gems/ree-1.8.7-2010.01/gems/rspec-core-2.0.0.beta.15/lib/rspec/core/example_group.rb:173:in `run'
    from /home/davidjh/.rvm/gems/ree-1.8.7-2010.01/gems/rspec-core-2.0.0.beta.15/lib/rspec/core/command_line.rb:41:in `run_examples'
    from /home/davidjh/.rvm/gems/ree-1.8.7-2010.01/gems/rspec-core-2.0.0.beta.15/lib/rspec/core/command_line.rb:41:in `each'
    from /home/davidjh/.rvm/gems/ree-1.8.7-2010.01/gems/rspec-core-2.0.0.beta.15/lib/rspec/core/command_line.rb:41:in `inject'
    from /home/davidjh/.rvm/gems/ree-1.8.7-2010.01/gems/rspec-core-2.0.0.beta.15/lib/rspec/core/command_line.rb:41:in `run_examples'
    from /home/davidjh/.rvm/gems/ree-1.8.7-2010.01/gems/rspec-core-2.0.0.beta.15/lib/rspec/core/command_line.rb:24:in `run'
    from /home/davidjh/.rvm/gems/ree-1.8.7-2010.01/gems/rspec-core-2.0.0.beta.15/lib/rspec/core/formatters/base_formatter.rb:37:in `report'
    from /home/davidjh/.rvm/gems/ree-1.8.7-2010.01/gems/rspec-core-2.0.0.beta.15/lib/rspec/core/formatters/base_formatter.rb:152:in `sync_output'
    from /home/davidjh/.rvm/gems/ree-1.8.7-2010.01/gems/rspec-core-2.0.0.beta.15/lib/rspec/core/formatters/base_formatter.rb:34:in `report'
    from /home/davidjh/.rvm/gems/ree-1.8.7-2010.01/gems/rspec-core-2.0.0.beta.15/lib/rspec/core/command_line.rb:21:in `run'
    from /home/davidjh/.rvm/gems/ree-1.8.7-2010.01/gems/rspec-core-2.0.0.beta.15/lib/rspec/core/runner.rb:46:in `run_in_process'
    from /home/davidjh/.rvm/gems/ree-1.8.7-2010.01/gems/rspec-core-2.0.0.beta.15/lib/rspec/core/runner.rb:37:in `run'
    from /home/davidjh/.rvm/gems/ree-1.8.7-2010.01/gems/rspec-core-2.0.0.beta.15/lib/rspec/core/runner.rb:22
    from /home/davidjh/.rvm/gems/ree-1.8.7-2010.01/bin/rspec:19

[1] http://github.com/rspec/rspec-rails/issues/issue/110

@hjdivad

I am either not able to figure out how, or simply don't have permission to,
reopen this issue, but in case it's not clear from my previous comment, I
consider this still an issue, at least in version 2.0.0.beta.15.

Again (sorry for being repetitive) the stack trace is printed to stderr, this is
fine. But what is printed to stdout is in green and strongly implies that
the examples pass, when they in fact do not. Worse, if stderr is swallowed for
whatever reason, the output will appear exactly as if the examples all pass.

On a (semi-)related note, thanks for rspec. It is much appreciated :-)

@markiz

I confirm on this one.
I think considering whole group a failure is a good idea.

@ashmoran

Does this relate to the fact that if you use this in an example group:

before(:each) { pending }

You get this error?

 uncaught throw :pending_declared_in_example

This is really inconvenient, as I do this all the time to selectively put examples in the background to deal with later :-(

@dchelimsky
Owner

@ashleymoran - unrelated.

FYI - to make a group pending, just say so at the group level:

describe "something", :pending => true do

That makes all of the examples in the group pending.

HTH,
David

@markiz

off-topic: wow, another trick I never knew.

@ashmoran

Thanks - I didn't know that. Just checked and it works fine.

Is pending in before blocks deprecated, or is that still a bug? The error message is not very helpful, and looks like it's unanticipated behaviour.

@ashmoran

@markiz - I like these! Using focus: true with the appropriate RSpec config is really useful too, in case you've not seen it (on David's blog).

@dchelimsky
Owner

@ashleymoran - deprecated suggests it was supported before :) Please open a separate issue and we can discuss there.

@ashmoran

Ha :) I've opened #77 to discuss pending in before.

@rdp

is this bug the same as:

describe 'it' do
  
  it 'should' do
  end
  after(:all) do
    raise 'bad'
  end
end

=>

Finished in 0.019 seconds
1 example, 0 failures
D:/installs/ruby-1.9.2-rc1-i386-mingw32/ruby-1.9.2-rc1-i386-mingw32/lib/ruby/gems/1.9.1/gems/rspec-core-2.0.0.beta.17/lib/rspec/core/hooks.rb:42:in `rescue in run_in': undefined method
`set_exception' for nil:NilClass (NoMethodError)
        from D:/installs/ruby-1.9.2-rc1-i386-mingw32/ruby-1.9.2-rc1-i386-mingw32/lib/ruby/gems/1.9.1/gems/rspec-core-2.0.0.beta.17/lib/rspec/core/hooks.rb:35:in `run_in'
@inspire22

I've been slamming my head against this as well.

When using the textmate bundle everything shows in html mode. And of course the formatter doesn't work with real exceptions, so I'm left seeing useless message:

Exception encountered: # backtrace: /Users/kevin/.rvm/gems/ree-1.8.7-2010.02/gems/dm-validations-1.0.0/lib/dm-validations.rb:141:in name' /Users/kevin/.rvm/gems/ree-1.8.7-2010.02/gems/dm-validations-1.0.0/lib/dm-validations.rb:141:insend' /Users/kevin/.rvm/gems/ree-1.8.7-

The exception message doesn't escape the < and > so its not even shown and the backtrace with no newlines is painful.

It'd be nice if it was at least rendered nicely as a part of the formatter.

Alternatively, perhaps there should be an implicit first test that any before(:all) blocks don't raise an exception.

@ashmoran

I think this is the same issue (or please correct me - if let is a different beast I'll move this). Anything in a class definition causes RSpec to blow up spectacularly:

module DomainLib
  describe Representation do
    let(:representation_class) {
      Class.new do
        extend Representation
        properties :a, :b, :c
      end
    }

    let(:representation) {
      representation_class.new(a: 1, b: 2)
    }

    it "exposes the property attributes" do
      representation.a.should eq 1
      representation.b.should eq 2
      representation.c.should be_nil
    end
  end  
end

You don't get a runner output, just the following:

/Users/ashleymoran/.rvm/rubies/ruby-1.9.2-rc2/bin/ruby -S rspec /Users/ashleymoran/Dropbox/PatchSpace/Development/work-source/twisty_lists/twisty_lists/spec/domain/checklist_template_spec.rb
/Users/ashleymoran/Dropbox/PatchSpace/Development/work-source/twisty_lists/twisty_lists/app/domain/checklist_template.rb:10:in `<class:Representation>': undefined method `properties' for ChecklistTemplate::Representation:Class (NoMethodError)
    from /Users/ashleymoran/Dropbox/PatchSpace/Development/work-source/twisty_lists/twisty_lists/app/domain/checklist_template.rb:8:in `<class:ChecklistTemplate>
@dchelimsky
Owner

@ashleymoran - seems unrelated. Separate issue please, and please include the def for Representation.

@ashmoran

I haven't written that yet :) Will file when I've got the code working...

@dchelimsky
Owner

@ashleymoran - aha! So your issue is that the error is not being captured, yes? If so, then it's the same issue :)

@ashmoran

Yep - that's the one! Did I just save myself a ticket? =)

@dchelimsky
Owner

FYI - I'm keen to get this resolved before a release candidate of rspec-2. I've just been very busy wrapping up The RSpec Book and this issue relates to some deeper architectural questions so I don't want to just hack together a fix.

@dchelimsky
Owner

@ashleymoran - your self and everyone else's selves.

@ashmoran

Cool cool. I agree, if it's a fundamental design issue, it's definitely one that needs resolving properly before an RC. I always assume (being used to RSpec 1.3) that if the output doesn't get formatted that I've made a syntax, so it puts my brain in the wrong type of bug-hunting mode. Lemme know if you want me test anything.

@spicycode

I've pushed out an approach to fixing this in the issue_21 branch

@dchelimsky
Owner

@spicycode - that catches the error and allows after(:all)s to get run, but the examples are reported as passing.

@dchelimsky
Owner

I added a @wip feature for this to the issue_21 branch: 87ed4b3

WDYT?

@spicycode

I'll review after lunch, and get back to you

@inspire22

whats @wip?

@markiz

Random speculation: work in progress?

@dchelimsky
Owner

It actually stands for "work in process," which has a different meaning, but most folks think of it as "work in progress."

@spicycode

fail_fast is in place, once more round the world we go

@dchelimsky
Owner

That looks good - but there's one small matter remaining :) Here's the output from the feature that's passing now:

$ bundle exec rspec before_all_spec.rb -cfd

an error in before(:all)
  fails this example (FAILED - 1)
  fails this example, too (FAILED - 2)
after all ran

Failures:
  1) an error in before(:all) fails this example
     Failure/Error: raise "oops"
     oops
     # ./before_all_spec.rb:3:in `block (2 levels) in '

  2) an error in before(:all) fails this example, too
     Failure/Error: Unable to find matching line from backtrace
     oops
     # ./before_all_spec.rb:3:in `block (2 levels) in '

Finished in 0.00034 seconds
2 examples, 2 failures

Note the details of the 1st failure capture the error:

Failure/Error: raise "oops"

While the details of the 2nd failure don't:

Failure/Error: Unable to find matching line from backtrace

I'm heading out right now but if you don't address this before I get back I'll add that to the feature.

@dchelimsky
Owner

Got the backtrace line worked out. Here are the relevant commits closing this issue:

b25fea3
d6a08ca
cc5e20c
5423a1c

@rdp
rdp commented

thanks for doing that.

@spicycode

Cool

This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.