n.times { code! }

Thursday, August 30, 2007

BDD, isolation, integration

I have been reading a lot of posts about skinny controllers, isolation and good behaviour-driven development. Let me explain from the beginning:

The typical approach to spec a 'create' action looks like this (Taken from rspec documentation):

describe PeopleController do
it "with a valid person should redirect to index on successful POST to create" do
Person.should_receive(:create).with({"name" => 'Aslak'}).and_return(@person)

post 'create', {:person => {:name => 'Aslak'}}

response.should redirect_to(:action => 'index')

That is too tied to the implementation. What if I want to use the 'create' method in my controller? And what about assigning attributes in a block?
What you are spec-ing here, is how your controller should look, not how it should behave.

Then I saw this in one of the blogs that I linked above:

it "should create a new thing" do
lambda { do_post }.should change { Thing.count }.by(1)

This is really testing behaviour. The main problem with this (although I really liked it the first time I saw it) is, of course, that interacts with the database and the models. I don't want to hit the database from my controller specs at all, I want full isolation.

Add a pair of helpers in spec_helper.rb:

def mock_valid_model(klass)
mock_model(klass, :save => true, :save! => true)

def mock_invalid_model(klass)
m = mock_model(klass, :save => false)

Then stub Model.new to return the mock you want. Spec your controller 'create' action in order to redirect in case that the mock is valid, and to render a template if it is not valid (of course you can add specs for flash or whatever):

describe ThingsController, '"create" a valid model by POST' do
before do
@thing = mock_valid_model(Thing)

it 'should redirect to model show' do
post :create
response.should redirect_to(thing_url(@thing))

describe ThingsController, '"create" an invalid model by POST' do
before do
@thing = mock_invalid_model(Thing)

it 'should render "create" template' do
post :create
response.should render_template('things/create')

There are various interesting things here:
  1. You don't need to keep adding expectations to your mocks. Expectations couple your specs with the implementation. With my approach you can use create, new + save, new + block assignement, whatever. And the spec still passes.
  2. The database isn't touched at all. That's good stuff, I guess.
  3. You are really testing the behaviour. You know that the only way your controller will know if it should redirect or render, is to create a new instance of Thing, and call save or save!. That is actually an implied expectation, but without the need of should_receive(), just stubs.
Some of you may have noticed that I don't need to use the params() hash in the controller in order to make the specs pass. Right. But you know, that your controller is creating a new instance of Thing, and calling save, save!, create or whatever. That's when integration/acceptance tests come in

The only moment that you want your controller mess with params, is when you are not isolating. Because, if you are isolating, you can't validate parameters, since you stubbed save and save!.

With this approach, you keep your specs to a minimum amount of lines, you isolate, you test just behaviour.

Anyway, let me know of any issues you find with this.

Thanks for reading and sorry about the syntax non-highlighting.