Recently I have gotten to work on a greenfield application, this has led to some discussions about the best way to test things. I personally have been taking time to write tests that allow me to take small steps giving me a better sense of direction. These small steps have allowed me to write a test, make it pass, write the next test, make it pass then refactor.
Continuously refactoring my code keeps it clean and maintainable. I’m not going for cleverness or golfing to the lowest number of lines of code. Instead I’m going for code that is flexible and allows me to continue to add new features with ease.
Below is an example of a controller test, written in two different styles. We’ll walk through a small refactoring scenario and see how we can keep our test simple and testing result rather than the implementation by comparing the two styles.
Disclaimer this example has some assumptions such as I am using FactoryGirl and Rspec.
Here is our starting controller, we are going to focus on the edit action. This is pretty strait forward, we’re going to edit an instance of ‘Foo’ so we’ll return it to the view.
class FooController > ApplicationController
def edit
@foo = Foo.find(params[:id])
end
end
One way we can test this is to build an object with FactoryGirl and then stub out the find method on Foo to return that object. This will allow us to test our edit action insuring that foo is always the same thing.
describe "GET 'edit'" do
it 'should receive the STUBBED Foo instance' do
@foo = FactoryGirl.build(:foo)
Foo.stub(:find).and_return(@foo)
get :edit, :id => @foo.id
expect(assigns(:foo)).to eql @foo
end
end
Another way we could test this trivial example is to just use FactoryGirl to create the object rather than just build it. This takes a little less code, but does require two hits to the database, one for saving the record and one for retrieving.
The benefit of this is we are setting up that if we send in the ‘id’ of ‘@foo’ we’ll get back an identical ‘@foo’ from the database.
Our controller test now is just saying hey we expect to get ‘@foo’ back, we really don’t care how you get it but in order for this edit form to work we need this ‘@foo’ back.
describe "GET 'edit'" do
it 'should receive the NON stubbed Foo instance' do
@foo = FactoryGirl.create(:foo)
get :edit, :id => @foo.id
expect(assigns(:foo)).to eql @foo
end
end
Now let’s refactor our Foo controller to do something a little different. Now we decided that really the ‘@foo’ should be retrieved by sending the ‘@foo.id’ to a method called ‘by_bar’
class FooController < ApplicationController
def edit
@foo = Foo.by_bar(params[:id])
end
end
Here is our new class method that replaces the normal Foo.find.
class Foo
def self.by_bar(id)
Bar.find_by_foo_id(id).foo
end
end
Now in our first version of the test we are going to get an error because we are not actually creating and saving the object and it is only stubbing out the find method making it brittle and tied to the implementation.
Let’s refactor our tests, first we’ll start with the test using a stub, we’ll need to modify the stub to use the ‘by_bar’ method.
Next we’ll look at the test not using a stub, this one does not need any work. Again we are testing to make sure that the instance of ‘Foo’ we are expecting is returned. In this case it is, so we don’t need to do anything.
describe "GET 'edit'" do
it 'should receive the STUBBED Foo instance' do
@foo = FactoryGirl.build(:foo)
Foo.stub(:by_bar).and_return(@foo)
get :edit, :id => @foo.id
expect(assigns(:foo)).to eql @foo
end
it 'should receive the NON stubbed Foo instance' do
@foo = FactoryGirl.create(:foo)
get :edit, :id => @foo.id
expect(assigns(:foo)).to eql @foo
end
end
While looking at our refactoring we realize that we didn’t really need a separate method for our ‘Bar.find_by_foo’ and we can just move that into our controller like so.
class FooController < ApplicationController
def edit
@foo = Bar.find_by_foo_id(id).foo
end
end
Now our stubbed test breaks again, let’s see what it will take to fix it. We’ll have to change our stub again. This time the stub needs to be done on the ‘Bar’ class and ‘find_by_foo_id’ method. Again our non-stubbed test continues to work because we are still returning an instance of ‘Foo’
describe "GET 'edit'" do
it 'should receive the STUBBED Foo instance' do
@foo = FactoryGirl.build(:foo)
Bar.stub(:find_by_foo_id).and_return(@foo)
get :edit, :id => @foo.id
expect(assigns(:foo)).to eql @foo
end
it 'should receive the NON stubbed Foo instance' do
@foo = FactoryGirl.create(:foo)
get :edit, :id => @foo.id
expect(assigns(:foo)).to eql @foo
end
end
As we have seen here taking small steps and limiting our stubs and mocks will save us time when refactoring. In all of these examples the things that broke the test were just breaking test setup code rather than actually failing the test giving us a false positive.
