Feature Idea: Fail examples when no expectation was set

[sens3]

I'm wondering if it would make sense to fail an example if no expectations were set during it's execution.

Basically I'd want to be notified (one way or the other) if I do something like this

it "shoud calculate a tuples sum" do
  tuple =  Tuple.new(1,2)
  tuple.sum == nil
end

it "should process the payment" do
end

Both examples will "pass". All green. Programmer is happy (at least for a while).

Another useful addition might be to fail examples if a message expectation is the last expression in the example, and thus cannot be met:

it "saves the record" do
  post :create
  @user.should_receive(:save)
end

after do
  # nothing here
end

(I hope I haven't forgotten any corner cases where the expectation could still be met)

One can argue that programmers should be smart enough not to write tests like this. On the other hand, having a passing example should guarantee that something was in fact passed.

Any thoughts?

Simon

[fj]

I disagree that the test should fail unilaterally. What about pending tests, for example? By definition, we don't want them to fail (otherwise, what is the point of marking them pending?).

[cgriego]

I could get behind this request. @fj pending is treated as a special-cased failure so it wouldn't be disturbed by this change.

[fj]

@cgriego: If that's true, then I'd +1 the change. (Added bonus: I've long favored assertion coverage over code coverage and this could get folks thinking about how to pull that off in a more general way.)

[sens3]

Just to clarify: Pending will be treated as-is. (as @cgriego said)
It's up for discussion if "empty" examples should fail or be treated more like a pending example.. I lean towards failing them.

[cgriego]

@sens3 Do you know how the Test::Unit integration works? I mean if a tes suite uses assertions either because they prefer it or from legacy suites, could this feature still work? Even if Test::Unit can be handled special-case, custom assertions and other expectation frameworks could be a blocker.

[myronmarston]

I sometimes purposely write examples that do not set any expectations. Here are a couple cases where I do that:

• When I'm using mocks, I may just set up a mock expectation (should_receive) and then call a method.
• When I'm testing that no error is raised, I prefer to not set an expectation rather than using (expect { ... }.not_to raise_error). If an error was raised, the expectation would swallow it and print out its own backtrace rather than the backtrace I care about.

Also, as a rule, I always make my examples fail before they pass, so this has never been a pain point for me. That said, I know others don't do that and would find this useful, so if we supported this, I think it would have to be a config option you can turn on.

I'm also a bit concerned that this feature requires the interaction of rspec-expectations and rspec-core, and we've worked hard to decouple these so that other assertion/expectation frameworks can be used as first class citizens with rspec-core.

Another useful addition might be to fail examples if a message expectation is the last expression in the example, and thus cannot be met

The example will already fail, since the expected message will not be received. Why bother adding a bunch of complexity for this special case? Plus, the implementation for this would be really horrendous; you'd have to either look at the source code and parse it in order to figure out if any code came after the message expectation, or use set_trace_func which is known to be very, very slow.

[patmaddox]

I love this idea. The first one, anyway. The second one already fails, so I don't care about that.

[dchelimsky]

As @myronmarston suggests, this would require binding rspec-expectations and rspec-mocks with rspec-core. Plus it would be surprising behavior. Plus even if we added it, it wouldn't with other mock frameworks and expectations frameworks.

I'm closing this now, but welcome more discussion if anybody has other suggestions as to how to solve the problem.

[dchelimsky]

BTW - I do think it's an interesting idea. Just not interesting enough to add the complexity and coupling necessary to make it happen. Simple ideas welcome.

[alindeman]

Yah .. I think this is one of those inherent reasons you should write tests first. Getting a test to fail for the right reason is key.

[sens3]

Just as a final comment since I wasn't around to participate in the discussion yesterday.

I agree that the second example is unnecessary, I added it to the feature request last minute and didn't really think it through.

I also understand that we wouldn't want to add a additional link between rspec-core and rspec-expectations / rspec-mocks.
But there are existing adapters between the three that we could just re-use.

Given it would be the MockFramework's responsibility to set if it's a message expectation has been set or not.
On rspec-core's side we could just add to the MockFrameworkAdapters:
https://github.com/rspec/rspec-core/blob/master/lib/rspec/core/mocking/with_rspec.rb

module RSpec
  module Core
    module MockFrameworkAdapter
        def expectation_set?
            RSpec::Mocks::BaseExpectation.expectation_set?     
        end  
        # ....      
    end
  end
end

Something similar could be done for the ExpectationFrameworkAdapter
https://github.com/rspec/rspec-core/blob/master/lib/rspec/core/expecting/with_rspec.rb

Then in https://github.com/rspec/rspec-core/blob/master/lib/rspec/core/example.rb we could do something along the lines of:

unless RSpec.configuration.mock_framework.expectation_set? or 
             RSpec.configuration.expectation_frameworks.any?(&:expectation_set?)
     # fail example
end

I hope this makes sense.
I just started diving into rspec internals a few days ago. Any feedback (even/especially if it doesn't make any sense) is greatly appreciated.

Thanks!!
Simon

[myronmarston]

@sens3: That's definitely a start. There's a lot more that would have to be done to get this to work, though. You don't really care if the expectation frameworks say they have any expectations set; what you really care about is that there was an expectation set for this example. You'd have to add multiple methods to the expectation framework adapters (to notify it that an example is started and finished so it reset its expectation tracking)...and suddenly this starts to get quite complicated.

You might consider taking a stab at implementing this as an rspec extension gem, and if you can come up with simple enough implementation, we can consider merging your code in at some future point.

[sens3]

@myronmarston Agreed there is a lot more to be done. I was just trying to get the basic idea across. Good to hear that the idea itself is worth pursuing. I'll try to find an easy/clean solution...

Thanks for your feedback!

[myronmarston]

FWIW, here's a way to get this:

RSpec.configure do |config|
  config.after(:each) do
    result = self.example.metadata[:execution_result]
    has_mock_expectations = RSpec::Mocks.space.instance_eval{receivers}.empty?
    if !result[:exception] && !result[:pending_message] && !RSpec::Matchers.last_should && hasnt_mock_expectations
      $stderr.puts "[WARN] No expectations found in example at #{self.example.location}: Maybe you forgot to write `should` in the example?"
    end
  end
end

(taken from this blog post).