Test: December 2007 Archives

In my last post, Quit testing the framework!, I talked about my philosophy of testing without second guessing the Rails framework. I'd like to continue the conversation, this time focusing on the controller side of things. If you haven't read the previous post, I suggest you do it now. Otherwise, you might be a tad lost.

Ok, our controller: users_controller.rb

  
  class UsersController < ApplicationController
    def index
    end
  
    def activate
      @user = User.find(params[:id])
      @user.activate! if @user
      render :action => :index
    end
  end
  

We are going to focus on testing the activate action. Specifically, I want to focus on finding and activating the user. I'll save the rest of the specs for the download.

Here is what our initial specs might look like: users_controller_spec.rb

  
  describe UsersController, "responding to POST /activate" do
    before(:each) do
      @user = User.create(:status => "Some Status")
    end
    
    def do_post
      post :activate, :id => @user.id
    end
  
    it "should find a user" do
      do_post
      assigns[:user].should_not be_nil
    end
  
    it "should activate a user" do
      do_post
      assigns[:user].should be_active
    end  
  end
  

Let's look at the first spec. We are creating a post to the activate action and verifying that a user was actually found.

Do we actually need to verify that a User was found? Not really. How about we trust ActiveRecord and concentrate on making sure we call the proper find method. While we are at it, let's get rid of that nasty assigns[:user] junk. Personally, I think assigns should be reserved for confirming that we actually set an instance variable.

A little stubbing, a little mocking , a smidge of refactoring...

  
  describe UsersController, "responding to POST /activate" do
    before(:each) do
      @user = mock_model(User, :id => "1")
      @user.stub!(:activate!)
      User.stub!(:find).and_return(@user)
    end
  
    def do_post
      post :activate, :id => @user.id
    end
  
    it "should find a user" do
      User.should_receive(:find).with(@user.id)
      do_post
    end
  end
  

Okay, that feels a little better to me. We are just making sure that User.find is being called properly. Notice that we've stubbed User.find. As long as Rails is doing it's job, we shouldn't have to worry about testing the result of a call to find. A nice side effect is that we are no longer creating and destroy an actual database model for each spec.

Now that we've focused on finding the user, let's turn our attention to actually activating the user. Our initial spec was a lil' ikky. We still had that ugly assigns call. And, besides, we've already tested the activate! method back when we tested our model. How about we just make sure that activate! is called? Sounds good to me.

A bit more refactoring:

  
    it "should activate a user" do
      @user.should_receive(:activate!)
      do_post
    end
  

Ahoy! New hotness! Just make sure that activate! was called. Leave the rest of the details to our model spec.

And our testing rundown:

I've included the user model, controller and controller spec for your here: user.rb, users_controller.rb, users_controller_spec.rb

If you'd like the user model spec, it was included in my previous post.

As always, I welcome any and all comments. Please see the sidebar for instructions on sending me a message.

Enjoy!

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:

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

Enjoy!