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

David Chelimsky dchelimsky at gmail.com
Mon Nov 27 07:09:32 EST 2006


On 11/27/06, Pat Maddox <pergesu at gmail.com> wrote:
> Hey David,
>
> Thanks for the info.  I know that it wasn't a bug in RSpec at all...I
> was just describing something that I wanted to do, but couldn't at the
> time.  I posted it to say, "Hey this is my problem, I'm working on a
> solution," and perhaps spark discussion if warranted.

That's really what the mailing lists are for. If you really think it
is an RSpec deficiency, by all means stick it in a Bug report or
Feature Request, but in this case the mailing list is the right place
to start. Perhaps the discussion ends up leading to RFEs and that's
fine.

>  As I mentioned
> in the report, I knew that the problem was I was using an array, and
> actually I began writing a class that would act like the Rails
> collection class for me.  Then my girlfriend interrupted me...
>
> Glad she did :)  Had another aha moment reading your post.
>
> On 11/26/06, David Chelimsky <dchelimsky at gmail.com> wrote:
> > One goal of splitting specs into model, view, controller specs is to
> > decouple the underlying AR behaviour from what you specify in a
> > controller or view spec.
> >
> > What I would do in the example that Pat provided is specify
> > validations in the model specs and then mock out the models in the
> > controller specs. Given your current implementation, you could have
> > the following spec:
> >
> > specify "should create a new Person and stick it in the users people" do
> >   mock_user = mock("user")
> >   mock_person = mock("person")
> >   mock_people = mock("user people")
> >   mock_user.should_receive(:people).and_return(mock_people)
> >   mock_people.should_receive(:<<).with(mock_person)
> >
> >   User.should_receive(:find_by_login).with("pat").and_return(mock_user)
> >   Person.should_receive(:new).with(:name => "BJ").and_return(mock_person)
> >
> >   post :create, :user_id => "pat", :person => { :name => "BJ" }
> > end
> >
> > If that seems like a lot of mocking to you (it does to me), that's
> > because adding something to @user.people in this context is a
> > violation of Tell, Don't Ask and also smells like Feature Envy (when
> > one object operates on the data of another).
>
> I see what you mean.  At first I thought it wasn't a big deal, because
> we're just telling the people collection to add another person.  But
> really, we're asking for the people collection, and then telling it to
> do something, when we're really interacting with the user object.
> There's no need to know that there's a collection of people.
>
> Interestingly, if I were to see
> people = user.people
> people << person
>
> I'd instantly identify that as a code smell. Perhaps it all being on
> the same line and the syntactic sugar of << hid it from me.

Totally. That's something we'll all fall sucker to at one point or another.

>
> > This could be solved by
> > adding an add_person method to User. Now your controller spec could
> > look like this:
> >
> > specify "should create a new Person and stick it in the users people" do
> >   mock_user = mock("user")
> >   mock_person = mock("person")
> >   mock_user.should_receive(:add_person).with(mock_person)
> >
> >   User.should_receive(:find_by_login).with("pat").and_return(mock_user)
> >   Person.should_receive(:new).with(:name => "BJ").and_return(mock_person)
> >
> >   post :create, :user_id => "pat", :person => { :name => "BJ" }
> > end
> >
> > and the create method:
> >
> > def create
> >   @user = User.find_by_login params[:user_id]
> >   @person = Person.new params[:person]
> >   @user.add_person @person
> >   redirect_to person_path(@user, @person)
> > end
> >
> > Small change to the create method.  Not so small implications in terms
> > of decoupling the design.
> >
> > No intent to dis your design here, Pat. I've certainly done much worse
> > that that along the way! The problem is that ActiveRecord encourages
> > this sort of highly coupled  design, and the problems that go along
> > with it (like having to do many layers of mocking in your specs). It's
> > one of those situations in which we get so much good from AR that we
> > have to live w/ the design problems it encourages, and try to address
> > them through discussion and education. And that's why I believe so
> > strongly in the approach we're trying to take w/ the rails plugin.
>
> No problem at all, I'm glad you brought this up.  In other languages
> or frameworks I would create an add_person method...but for some
> reason this case didn't jump out at me as breaking encapsulation.  Of
> course it'll be easier to recognize and avoid the problem in the
> future though.
>
> So I think I've learned a lesson here...if it's hard for me to do
> something, it's likely a deficiency in my design.  I couldn't get the
> expectations to work like I wanted, because I was using mocks wrong,
> because my design was too tightly coupled.
>
> Thank you very much for taking the time to discuss this.  I'm looking
> forward to making more (different!) mistakes :)

No worries. As I said above, that's what these lists are for. Perhaps
this particular thread might even be better on the rspec-users list.
That's the problem w/ multiple lists. Maybe you'd consider blogging
some of this as it becomes more clear to you.

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