Examples of Behaviour Spec'n

Sun Jun 29 07:02:48 -0700 2008

In my previous post (Spec Behaviour not Implementation) I went on a froth roll about why you should treat controller actions as black boxes. Here I give an all to common example of why this is good and how you can write specs that won’t break at the most trivial change.

1
2
3
4
5
6

class AddressesControler < ApplicationController
  def index
    @person = Person.find(params[:person_id])
    @addresses = Address.find_by_person_id(params[:person_id])
  end
end

Now obviously this should be refactored (we’ll get to that in a sec) but what are we doing here? We are assigning person to the view and the person’s addresses to the view.

Using a mocks and stubs, I might do something like this:

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33

describe AddressesController do
  describe 'GET index' do

    it "should ask the Person model for a person" do
      @person = mock_model(Person)
      Person.should_receive(:find).with("1").and_return(@person)
      get :index, :person_id => 1
    end

    it "should assign a person to the view" do
      @person = mock_model(Person)
      Person.stub!(:find).and_return(@person)
      get :index, :person_id => 1
      assigns[:person].should == @person
    end
    
    it "should ask the Address model for the addresses" do
      Person.stub!(:find)
      @addrs = [mock_model(Address), mock_model(Address)]
      Address.should_receive(:find_by_person_id).with("1").and_return(@addrs)
      get :index, :person_id => 1
    end

    it "should assign the person's addresses to the view" do
      Person.stub!(:find)
      @addrs = [mock_model(Address), mock_model(Address)]
      Address.should_receive(:find_by_person_id).with("1").and_return(@addrs)
      get :index, :person_id => 1
      assigns[:addresses].should == @addrs
    end

  end
end

Now, no one would complain about that (except that it is not very dry), pretty straight forward. But I say, my fellow RSpecers, what are we doing poking around the internals of that controller? Why do we give two hoots from a behavioral viewpoint on HOW the controller goes about it’s business? Can’t we just assign the controller responsibility here?

Well, no, you say, I need to know it is getting the addresses properly!. Well, ok, let’s make one trivial change to one line of that controller and see how many specs break:

1
2
3
4
5
6

class AddressesControler < ApplicationController
  def index
    @person = Person.find(params[:id])
    @addresses = @person.addresses
  end
end

Now, you would agree with me that the external behaviour of that controller action has not changed, we are assigning the requested person to the view and we are assigning the person’s addresses to the view, but guess what, every one of our specs now fail Why!? Because we spec’d implementation details. We setup our stubs and mocks to provide data in only certain ways.

The first and second specs fail because @person is a mock model and gets an unexpected message :addresses, the second and third specs fail because we don’t return anything from the Person.find and so you then get nil.addresses which fails. Even if that didn’t fail, we then have a failing spec because the Address model is never called.

Evil. Madness. Insanity, but most importantly, lost time. Now you need to go and dig in, remember what that spec did, what you were trying to do and re-write the spec, hoping that you aren’t changing the required behaviour.

And what is worse, our behaviour driven development specs are failing when the behaviour didn’t change!

Now there is a lot to be said for mock objects and stub methods. They are useful and have their place, they definitely speed up spec execution time, but I think this whole thing of mock everything in the controllers to make the specs go faster is over rated.

I work in a development team. We user factory objects and real objects when we can get away with it. And our specs run slower. On an app that is about 50% done we have 1500 specs passing, and it takes about 70 seconds to run them. Not too bad. About half of them use real objects (from the database). Sure, if I went through and mocked and stubbed everything out and spent the time doing that, it might get down to 50 seconds, or maybe 40. But it is pretty rare that waiting for the test suite to pass is blocking my development time.

So, if we re-wrote the above spec using factories, we would get something like this:

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19

describe AddressesController do
  describe 'GET index' do

    it "should assign the requested person to the view" do
      @person = Person.build_valid!(:id => 1)
      get :index, :person_id => 1
      assigns[:person].should == @person
    end

    it "should assign the person's addresses to the view" do
      @person = Person.build_valid!(:id => 1)
      @address = Address.build_valid(:person_id => @person.id)
      @person.addresses << @address
      get :index, :person_id => 1
      assigns[:addresses].should == [@address]
    end

  end
end

Now, even though we have reduced this down to two specs, in reality, that is all the behaviour our controller is showing. It has no error handling etc.

If there were decision path’s in the behaviour of the controller, I would stub these out to do what I wanted for the spec, like @person.stub!(:save).and_return(false) and the like.

One other thing that I often read is that you don’t want to do a spec like this because some other developer could go and change the underlying API on how person and address talk to each other or what constitutes a valid object.

Well, I work in a team and I humbly submit that:

(a) this doesn’t happen too often, especially if you are using factories, because if you change the valid requirements of a person, then you should also be updating the factory.

(b) I want to know if my controller broke because of an API change.

Anyway, that’s just how I do it, your mileage may vary.

blogLater

Mikel

  1. Zach Dennis Says:
    Mon Jun 30 01:19:16 -0700 2008

    “But I say, my fellow RSpecers, what are we doing poking around the internals of that controller”

    In my view controllers are simply application coordinators. It’s not that I care about the internals of the controller so much that I care that controller coordinates correctly between the incoming request and the appropriate object to do the work.

    Since the controller is just a coordinator I use it to let me drive the design of the domain/application objects that actually have behavior I’m interested in. I am coming from an outside-in style of development where you haven’t yet created any models, services/managers, etc to do the work and you’re letting the design emerge as you go based on customer driven stories/features.

  2. David Chelimsky Says:
    Wed Jul 02 11:21:51 -0700 2008

    I’m kind of with Mikel on this one from a philosophical stance – black box testing is less brittle.

    But I disagree about the time savings. I run my examples ALL DAY LONG and 20 seconds savings means 20 seconds times hundreds of runs in a single day. So I tend to suffer the maintenance burden in favor of the time savings.

    I think what we really need to solve this problem to satisfy both sets of desires (black box w/o database) would be an in-memory connection adapter. I’d like to make this my next project, but I think it’s going to be too far in the future to satisfy me :) Anybody else interested?

  3. mikel Says:
    Wed Jul 02 17:29:41 -0700 2008

    David, I think the best compromise here would be some sort of intelligent mock model.

    That is, something that you can call which creates a “real” object, using reflection or the like to determine valid calls of some sort, but the biggest #1 pain on this sort of thing is getting id values into the instances that will be found in relationship and .find calls.

    Really, at the end of the day, that’s the only reason why I end up doing real objects in the DB. Otherwise, I use objects that are not saved, but real objects otherwise. These are fast :)

    Topfunky had something here when he put the sqlite3 in memory for his tests. I haven’t looked at this recently, but that might be a good compromise.

    Mikel

  4. Zach Dennis Says:
    Fri Jul 04 03:32:23 -0700 2008

    David, I’m a little surprised by your response, seeing as you’re an advocate of outside-in development. How can you blackbox test a controller by building up valid models for models which may not exist yet?

    My approach is actually a mixture of Mikel’s examples above. I would have the “it” examples from Mikel’s black box example, but I would use mocks to make sure that Person.find got called. I’m going to see if I can get a code example in this comment box, bare with me if this doesn’t work out.

    1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19

    describe AddressesController do
  describe 'GET index' do

    it "should assign the requested person to the view" do
      person = mock_model(Person)
      Person.should_receive(:find).with(199).and_return(person)
      get :index, :person_id => 199
      assigns[:person].should == person
    end

    it "should assign the person's addresses to the view" do
      address = mock_model(Address)
      person = mock_model(Person, :address => address)
      get :index, :person_id => 199
      assigns[:addresses].should == [address]
    end

  end
end

    Beside the example though, how would you blackbox test a controller for things may not exist in the system (like models) driving from the outside-in?

Leave a Reply