[RSpec][2.14rc1] Inconsistent results

[conradwt]

I'm getting a failing spec when using the new expect syntax:

require 'rspec/autorun'

class Teacher

  def give_detention
    call_parent( :warning )
  end

  def call_parent( message )
  end

end

describe Teacher do

  it 'old style mock' do
    teacher = Teacher.new
    Teacher.any_instance.should_receive(:call_parent).with( :warning )

    teacher.give_detention
  end

  it 'new style mock' do
    teacher = Teacher.new

    expect( Teacher.any_instance ).to receive(:call_parent).with( :warning )
    teacher.give_detention
  end

end

$ rspec teacher_spec.rb 
  1) Teacher new style mock
     Failure/Error: expect( Teacher.any_instance ).to receive(:call_parent).with( :warning )
       (#<RSpec::Mocks::AnyInstance::Recorder:0x007f96dabe4588>).call_parent(:warning)
           expected: 1 time with arguments: (:warning)
           received: 0 times with arguments: (:warning)
     # ./teacher_spec.rb:26:in `block (2 levels) in <top (required)>'

Finished in 0.00417 seconds
2 examples, 1 failure

Failed examples:

rspec ./teacher_spec.rb:23 # Teacher new style mock

Here's the version information:

$ rspec -v 2.14.0.rc1

[myronmarston]

For any_instance, you need to use expect_any_instance_of(Teacher).to receive(:call_parent).with(:warning):

http://rubydoc.info/github/rspec/rspec-mocks/RSpec/Mocks/Syntax:expect_any_instance_of

expect(Teacher.any_instance).to receive is setting an expectation on the object returned by Teacher.any_instance that it will receive call_parent.

FWIW, when you configure things to only use the new syntax Class#any_instance is removed and not even available, so it helps avoid this confusions.

That said, I think we're missing a cuke for this.

Anyone want to take a stab at adding a cuke (which will be pushed to relish) for expect_any_instance_of and allow_any_instance_of?

/cc @samphippen @JonRowe @soulcutter @alindeman

[conradwt]

@myronmarston Yes, this was definitely missing from documentation and things are working. On a side note, I simply prefer 'stub' over 'allow' because it simply fits better within our known vocabulary. Chapter 23 or 'xUnit Test Patterns : Refactoring Test Code' goes into very good detail on the different test double patterns.

[myronmarston]

@conradwt: we had a long discussion on the naming for the new syntax in #153. You can read the whole thread and the reasons it was chosen there.

[conradwt]

@myronmarston Do you think it would be a great time to fix the API drift with this new allow/expect syntax? This appears to affect 'allow' more than 'expect' if one is doing TDD. In short, I would like the following to blow up when a method isn't defined on a class or an instance of the class. For example,

some_double_object = double()
allow( some_double_object ).to receive(:this_method_does_not_exist)  # should generate an exception
allow( real_object ).to receive(:this_method_does_not_exist)  # should generate an exception

It should generate message similar to the following:

teacher = Teacher.new
teacher.this_method_does_not_exist # does generate a NoMethodError

[JonRowe]

@conradwt Personally I believe if you're allowing that object to receive a message then it should work no matter what... If you're doing outside in development then having an error raised defeats the point. In particular:

allow( some_double_object ).to receive(:this_method_does_not_exist)

Should work, as it's operating on a test double... I'd be more ok with:

allow( real_object ).to receive(:this_method_does_not_exist)  # should generate an exception

but then you have api drift between the two things in RSpec and the former is more important behaviour IMO. We could make such a change in 3 though...

[conradwt]

@JonRowe I would like to catch this stuff at the level for which I'm writing specs. I have been burned on many occasions where I renamed a method within one spec using BDD which caused false positives in other specs. Also, I believe Sandi Metz briefly talked about this topic at Railsconf 2013 and mentioned that MiniTest provides this functionality out of the box. Agreed, RSpec 3 would be a good place to introduce such a change.

[JonRowe]

To be clear...

allow( double("some double") ).to receive(:this_method_does_not_exist)

should always work. As it's one of the clear points of using a test double in the first place.

[conradwt]

@JonRowe thanks for clarification.

[myronmarston]

I've already got plans to make some changes here for RSpec 3 -- in fact, it's one of the main features I've been thinking about. Stay tuned for more details :).

[myronmarston]

I think all that's left for this are the docs I mentioned above. I opened #312 to track that work.