Quit testing the framework!

|

I think one of the things that I appreciate most about Ruby on Rails is the fact that it provides an accessible framework for testing. Test Driven Development is a snap with Rails. Unfortunately, I think the ease with which we can create test cases with Rails can lead to creating either too many or the wrong kind of tests. Many times I see tests that are actually testing the Rails framework.

I think the easiest way to provide more detail is to work through an example. I'll be using RSpec for the testing framework. If you're not familiar with RSpec, you should still be able to translate the concepts with Test::Unit and Mocha (or similar stubbing/mocking framework).

Let's start with a simple model: user.rb

  
  class User < ActiveRecord::Base
    def activate!
      self.update_attribute(:status, "Active")
    end

    def active?
      self.status == "Active"
    end

    def deactivate!
      self.update_attribute(:status, "Inactive")
    end
  end
  

First, let's write a spec to test the activate! method.

  
  describe User do
    before(:each) do
      @user = User.new
    end  
  
    it "should update the user's status attribute when activating" do
      @user.activate!
      @user.should be_active
    end
  end
  

And the log from running the test:

  
  SQL (0.000142)   BEGIN
  User Columns (0.002444)   SHOW FIELDS FROM `users`
  SQL (0.000159)   ROLLBACK
  SQL (0.000181)   BEGIN
  User Create (0.000406)   INSERT INTO `users` (`status`, `updated_at`, `created_at`) 
  ...
  

Now, the spec looks innocent enough. We are calling the activate! method and testing that the status attribute was properly set . Wait! Wait! Wait! Think about that for a second... testing that the status attribute was properly set... Doesn't that mean that we are actually testing the results of update_attribute? Well, yes it does.

The thing is, update_attribute is part of the Rails framework. Do we really need to be writing more tests for the Rails framework? My opinion is no. What we really need to verify is that update_attribute is being called with the proper values. Let Rails handle the rest.

Let's modify our previous spec a bit:

  
    it "should update the user's status attribute when activating" do
      @user.should_receive(:update_attribute).with(:status, "Active")
      @user.activate!
    end
  

Now, this change is closer to what I like to see. We are verifying that update_attribute is being called. We are also verifying that we are asking Rails to update the status attribute with the value "Active". That should be all we need to test. Let Rails deal with update_attribute, you've got business logic to test...

A quick sidenote... Can you see a second benefit from our change?. Here is the log of our new test:

  
  SQL (0.000087)   BEGIN
  User Columns (0.002240)   SHOW FIELDS FROM `users`
  SQL (0.000133)   ROLLBACK
  

Did you see it? There is no call to the database for updating the status attribute. By letting Rails handle the framework testing, we are also saving ourselves a little bit of time by not accessing the database.

Mmmmm. Tasty! But, I want more (I've got an insatiable sweet tooth)... Let's move on and test the active? method.

  
  describe User do
    before(:each) do
      @user = User.new
    end  
  
    it "should update the user's status attribute when activating" do
      @user.should_receive(:update_attribute).with(:status, "Active")
      @user.activate!
    end

    it "should be active if status is 'Active'"do
      @user.activate!
      @user.should be_active
    end
  end
  

Once again, this looks fairly straightforward, doesn't it? Look again. We are relying on the results of the activate! method. Do we really want to be wasting the time it takes for Rails to call update_attribute to properly set the status? I certainly don't!

We know what our method should be doing. If status is "Active", return true. We don't need to navigate the Rails plumbing to test that. Let's make another small change:

  
    it "should be active if status is 'Active'" do
      @user.stub!(:status).and_return("Active")
      @user.should be_active
    end
  

Ah, that's better. We are verifying that we wrote self.status == "Active" correctly. No need to worry about update_attribute or the rest of the Rails core. We are now limiting our tests to code we wrote ourselves. And again, we aren't hitting the database to set the user's status. It's a win/win.

Here is the rundown of where our code is tested:

MethodTested
@user.activate!user_spec.rb
@user.active?user_spec.rb
@user.update_attributeRails framework

I've included the user model and spec files here: user.rb, user_spec.rb

Enjoy!