[rspec-devel] [ rspec-Bugs-12547 ] validates_each issue in rails with mocking and stubbing

Thu Aug 23 11:25:54 EDT 2007

Bugs item #12547, was opened at 2007-07-24 15:17
You can respond by visiting: 
http://rubyforge.org/tracker/?func=detail&atid=3149&aid=12547&group_id=797

Category: rails plugin
Group: None
>Status: Closed
>Resolution: Rejected
Priority: 3
Submitted By: Daniel Neighman (hassox)
Assigned to: Nobody (None)
Summary: validates_each issue in rails with mocking and stubbing

Initial Comment:
I've been trying some code that checks for a validates_each call in an AR model.

The spec:
http://pastie.caboo.se/81744

This produces the following error:

does not detect that two responses are equal

Mock 'Book_1027' expected :store_with_privacy? with (#<Clip:0x1a9139c @name="Clip_1025">) but received it with (#<Clip:0x1a9139c @name="Clip_1025">)

and

stub overwrites mock

Mock 'Book_1026' expected :store_with_privacy? with (#<Clip:0x1a91b1c @name="Clip_1025">) once, but received it 0 times

The Model is:
http://pastie.caboo.se/81745

----------------------------------------------------------------------

>Comment By: David Chelimsky (dchelimsky)
Date: 2007-08-23 15:25

Message:
OK - this has been staring me in the face for a few days but I only just say it through the trees.

The problem is not that the mock is lying, or that the comparison is failing. The mock is telling the truth - the object being passed to can_store? is a Rails association proxy, not the mock @clip that you are telling it to expect.

To get your spec to pass, you need to ensure that @clipping.clip returns the mock @clip:

  @clipping.stub!(:clip).and_return(@clip)

That way, when this gets executed:

  validates_each :clip do |model, attr, value|
    if !model.book.nil? && !model.clip.nil?
      if !model.book.store_with_privacy?( model.clip )
        model.errors.add(:clip, "You can't add this item to this book")
      end
    end
  end

model.clip => your mock (instead of the association proxy, which is lying through its teeth about its identity)

Cheers,
David

----------------------------------------------------------------------

Comment By: Daniel Neighman (hassox)
Date: 2007-08-22 23:49

Message:
David,

I have been thinking about this a bit more, and I think
checking both ways could be a bad move in 99.9% of cases.

I am thinking that if this happens will the error messages
be around the wrong way as well?

----------------------------------------------------------------------

Comment By: Daniel Neighman (hassox)
Date: 2007-08-22 14:18

Message:
Since this directly involves the rails magic that is
association_proxy, I don't see how it can be gotten around
without checking both ways.

That is unless you wanted to check to make sure it was an
association_proxy first then swap the comparison.  But that
seems the worse course.

I would grudgingly have to say you need to check both ways.
 As weird as it seems to me, it seems it is necessary in
this situation.

My 0.02

----------------------------------------------------------------------

Comment By: David Chelimsky (dchelimsky)
Date: 2007-08-22 14:11

Message:
Thanks for the props - but do you have an opinion about modifying the comparison?

----------------------------------------------------------------------

Comment By: Daniel Neighman (hassox)
Date: 2007-08-22 14:08

Message:
I'm glad your on the case David.  I wouldn't have found that!

----------------------------------------------------------------------

Comment By: David Chelimsky (dchelimsky)
Date: 2007-08-22 14:05

Message:
OK - Here's the deal:

clip

@mock_clip == association_proxy_clip => false
association_proxy_clip == @mock_clip => true

RSpec happens to be doing the comparison the first way:

expected == actual

I'm not sure of the best way to handle this. We could have rspec try == in both directions:

actual == expected || expected == actual

But that seems slimy to me. Anybody have an opinion about that?

Incidentally, mocha also compares [expected == actual], so the same problem would occur w/ mocha - and I'd bet the same is true of flexmock. In the end, the source of this problem is AssociationProxy, which seems to violate the Principle of Least Surprise in most sinister ways.

----------------------------------------------------------------------

Comment By: David Chelimsky (dchelimsky)
Date: 2007-08-22 12:35

Message:
I'm on to something, but haven't gotten there yet.

Modifying the example in the pastie, the following passes:

    @book.should_receive(:can_store?) do |clip|
      @clip.object_id.should == clip.object_id
    end

But the following fails:

    @book.should_receive(:can_store?) do |clip|
      @clip.should == clip
    end

And this fails:

    @book.should_receive(:can_store?) do |clip|
      @clip.should equal(clip)
    end

I added specs for mocks and mock_models:

@mock.should == @mock
@mock.should equal(@mock)

@mock_model.should == @mock_model
@mock_model.should equal(@mock_model)

They all pass.

Here's my theory:

model.book.clip is not actually the clip object, but a Rails Association Proxy and it is not responding as you would expect to == or equal?.

Anybody else paying attention to this thread think that's a reasonable theory?

----------------------------------------------------------------------

Comment By: Daniel Neighman (hassox)
Date: 2007-08-22 07:24

Message:
Hi,

Thanx for looking at this David.  I've been away from my
machine.  I will zip it up and email it to you.

-Daniel

----------------------------------------------------------------------

Comment By: David Chelimsky (dchelimsky)
Date: 2007-08-22 05:38

Message:
Michael - The issue you described was specific to the way expect_render and stub_render delegated to a mock proxy (or to the the real method).

I've fixed that (see http://rubyforge.org/tracker/index.php?func=detail&aid=13271&group_id=797&atid=3149),
but I don't think it's related to this problem, which Daniel is seeing in model specs.

Daniel - I'll throw my request to you again - I'm having trouble reproducing the original problem in this report - it would help if you could give me a patch or a failing spec or an entire project that I can just apply, unzip, whatever, and go.

Cheers,
David

----------------------------------------------------------------------

Comment By: David Chelimsky (dchelimsky)
Date: 2007-08-22 01:03

Message:
Thanks Michael - patching rspec's own specs definitely makes it easy for me to see the problem you're trying to convey.

----------------------------------------------------------------------

Comment By: Michael Hamann (michitux)
Date: 2007-08-22 00:51

Message:
I just added an expectation in rspec_on_rails that shows the problem, here the diff: http://pastie.caboo.se/89880 - I hope that is enough and helps.

----------------------------------------------------------------------

Comment By: David Chelimsky (dchelimsky)
Date: 2007-08-22 00:41

Message:
Daniel - I started to look at this, but due to the dependencies on other models couldn't really get anything going w/o making some assumptions.

If you would wrap up your project in a zip and either post it here or email me directly, I'll be glad to take a look at it. I just need to be able to unzip a file, create a db, run migrations and go.

----------------------------------------------------------------------

Comment By: David Chelimsky (dchelimsky)
Date: 2007-08-22 00:28

Message:
Michael - would you mind posting your spec please? Maybe a pastie.

----------------------------------------------------------------------

Comment By: Michael Hamann (michitux)
Date: 2007-08-22 00:19

Message:
The problem that stub overwrites should_receive/expect occurs also in view-specs with template.stub_render and template.expect_render. Example: I have a partial that is rendered in a template. I test several things where I don't want to pay any attention to this partial. But then I want to test that the partial is actually rendered. Unfortunately this doesn't work. I am using rspec_on_rails revision 2419.

----------------------------------------------------------------------

Comment By: Daniel Neighman (hassox)
Date: 2007-07-24 15:38

Message:
The migrations are here
http://pastie.caboo.se/81756

----------------------------------------------------------------------

Comment By: David Chelimsky (dchelimsky)
Date: 2007-07-24 15:29

Message:
Would you mind posting the relevant migrations as well? Then I can build a subset of your app and see the error happen.

----------------------------------------------------------------------

Comment By: Daniel Neighman (hassox)
Date: 2007-07-24 15:23

Message:
A bit more information

I have rspec installed as plugins in rails

rspec 2174
rspec_on_rails 2174

----------------------------------------------------------------------

You can respond by visiting: 
http://rubyforge.org/tracker/?func=detail&atid=3149&aid=12547&group_id=797