[rspec-devel] Fwd: [ rspec-Bugs-6905 ] Mocking rails associations where the association is validated

David Chelimsky dchelimsky at gmail.com
Mon Nov 27 23:55:35 EST 2006


On 11/27/06, Pat Maddox <pergesu at gmail.com> wrote:
> On 11/27/06, David Chelimsky <dchelimsky at gmail.com> wrote:
> > As usual, I said nothing..... (accidentally hit send instead of
> > discard). Anyhow - comments below....
> >
> > On 11/27/06, David Chelimsky <dchelimsky at gmail.com> wrote:
> > > On 11/27/06, aslak hellesoy <aslak.hellesoy at gmail.com> wrote:
> > > > On 11/27/06, Pat Maddox <pergesu at gmail.com> wrote:
> > > > > On 11/27/06, David Chelimsky <dchelimsky at gmail.com> wrote:
> > > > > > On 11/27/06, Pat Maddox <pergesu at gmail.com> wrote:
> > > > > > > Another question :)
> > > > > > >
> > > > > > > When I first wrote the add_person method, my spec looked like this:
> > > > > > >
> > > > > > >   specify "should accept a new person" do
> > > > > > >     @user.people.size.should_be 0
> > > > > > >     @user.add_person Person.new
> > > > > > >     @user.people.size.should_be 1
> > > > > > >   end
> > > > > > >
> > > > > > > That works, of course, and lets me infer that I have the new user.
> > > > > > > However I think it's ugly that I'm accessing people to verify
> > > > > > > something about the user...
> > > > > > >
> > > > > > > So then I decided to change it to this:
> > > > > > >
> > > > > > >   specify "should accept a new person" do
> > > > > > >     mock_person = mock("person")
> > > > > > >     mock_people = mock("people")
> > > > > > >     @user.should_receive(:people).and_return(mock_people)
> > > > > > >     mock_people.should_receive(:<<).with(mock_person)
> > > > > > >     @user.add_person mock_person
> > > > > > >   end
> > > > > > >
> > > > > > > That feels like the more RSpecy way to do this, despite being longer.
> > > > > > > In both cases they dig into people (either checking the size or
> > > > > > > checking that it calls <<), but in the latter case it verifies
> > > > > > > behavior, rather than inferring it from state.  Does that sound right?
> > > > > >
> > > > > > This one is tricky. There are multiple perspectives on how much
> > > > > > mocking you should do in model specs. One view is that model specs are
> > > > > > really small integration specs (model class + database). Another view
> > > > > > is that you should do what you did above, mocking out the other
> > > > > > players where possible.
> > > > > >
> > > > > > I'm leaning towards the first view - that model specs are like little
> > > > > > integration specs. My thinking is that because the way AR is designed,
> > > > > > the things you end up mocking are often internal to AR, which is a
> > > > > > huge mocking no-no (mock YOUR code, not other people's).
> > > > >
> > > > > Weren't we mocking other people's code in the controller specs as well?
> > > > > ARClass.should_receive(:find).with(1).and_return(some_mock_object)
> > > > > ARClass.should_receive(:new).with(:name => "name").and_return(other_mock_object)
> >
> > I think one *could* argue that mocking :find is closer to mocking your
> > own code (as it is a method that appears on *your* AR subclass, even
> > though it is implemented above) than mocking the collection that AR
> > makes available when you ask it for a collection property. As I said
> > before, this is very gray area, thanks to being cornered into
> > inheritance rather than composition. That sounds whiny, I know. What
> > can I say? When something is SO close to being really great, you're
> > more likely to notice its flaws than its lesser counterparts.
>
> I guess that's where I'm confused...the collection is defined on
> *your* AR subclass as well.  Would you mind clarifying the difference
> you see between MyClass.find and my_instance.items?  The latter case
> is even more custom because it's not automatically implemented higher
> in the heirarchy, but rather is defined only after you call "has_many
> :items"

I'm confused too.

>
>
> > > > > I'm not really sure what the difference is between mocking that out
> > > > > and mocking players.  At first I thought that it's because the find
> > > > > and new calls appear in my code, but so does players <<.
> > > > >
> > > > >
> > > > > > In this case you could use this to make it feel more RSpecy without
> > > > > > mocking anything:
> > > > > >
> > > > > > specify "should accept a new person" do
> > > > > >   lambda do
> > > > > >     @user.add_person
> > > > > >   end.should_change(@user.people.size).by(1)
> > > > > > end
> > > > > >
> > > > > > I'm not a huge fan of going after @user.people.size here, but I think
> > > > > > that @user.number_of_people might be overkill.
> > > > >
> > > > > After thinking about this some more, I think I like the other version
> > > > > with lots of mocks.  To me, it boils down to what I think is a
> > > > > revelation I had last night...using the specs do define behavior,
> > > > > rather than infer it from state.
> > > > >
> > > > > The one problem I see with my example was what if the User object
> > > > > doesn't define a players method?  Then the spec passes fine, but the
> > > > > code blows up of course.  This would be easily solved if you ensure
> > > > > that partial mocks can only mock methods that the object already has.
> > > > >
> > > > > for example, I've got a user object that has players in this case...when I do
> > > > > user.should_receive(:players)
> > > > > then I think the should_receive call out to call
> > > > > user.respond_to?(players) before mocking the method.  That seems
> > > > > reasonable to me for partial mocks - we're just substituting in some
> > > > > code for behavior that should already exist, because the existing code
> > > > > is too complex or expensive and doesn't need to be executed at the
> > > > > moment.  At the very least, perhaps it could generate a warning such
> > > > > as "WARNING - 'people' not defined on user object."  WDYT?
> > > > >
> > > >
> > > > That would be possible to bake into RSpec, but it would violate one of
> > > > the main principles of BDD. BDD is (among other things) a technique to
> > > > *discover* the interfaces of neighbouring objects N while speccing a
> > > > class C. And while you're speccing C, N might not even exist.
> > > >
> > > > The WARNING idea is interesting though.
> >
> > RFE [#5064] (Let mock() take a class argument) seems to be addressing
> > the similar issue.
> >
> > If we do something like this, I'd like to see it be modal. Under
> > normal spec runs it has no effect, but w/ a command line switch you
> > can get a report of what's being mocked that doesn't exist yet. I also
> > added a comment describing a vague vision (I don't really know how
> > this would work) in which if you use a class argument you could get
> > the specs to run in an integration mode in which real instances of the
> > class get substituted for the mocks, thereby allowing the specs to
> > serve as both isolation and integration specs. This is not an original
> > idea - Dan introduced something similar in JBehave, and someone on one
> > of these lists wrote about it as well some time ago. But it's worth
> > investigating, I think.
>
> I agree that it would be better to use a command line switch to report
> warnings.  One problem I initially saw is when some methods are
> handled by method_missing, such as find_by_login.  We wouldn't want
> warnings polluting the spec output when the method call is in fact
> valid, albeit not defined on the object itself.

Should be ok if we use object.respond_to?

>
> I remember reading about the integration mode, and I think it would be
> very good.  You can use the current way to drive your design well, and
> then use integration mode to ensure that the entire stack works
> properly.  This would serve a different purpose from the Rails
> integration tests, which imo are to prove workflow through your
> application.

Right. Although, this will likely get VERY complicated because you'd
have to have potentially two setups for every spec - one that uses
mocks and one that uses the real objects. It's an interesting idea,
but pretty low in priority.

Doing something simpler, like running in a verification mode just to
get a report on what methods aren't being implemented by the classes
submitted as initialization args is more likely to happen sooner.

So you'd do this:

  mock_thing = mock(Thing)

When you do a normal spec run nothing happens. When you do spec
--verify_mocked (or something like that) it would produce a simple
report:

Thing does not implement the following methods:
- blah
- ugh
- doh
- eek

Or we could make it work more like the normal output:

Thing should respond to the following messages:
- blah
- ugh (FAILED - NOT IMPLEMENTED)
- doh
- eek

Any preference there?

Cheers,
David

>
> Pat
> _______________________________________________
> rspec-devel mailing list
> rspec-devel at rubyforge.org
> http://rubyforge.org/mailman/listinfo/rspec-devel
>


More information about the rspec-devel mailing list