Jay Fields' Thoughts

Yesterday, Mike Clark posted a blog entry asking "How Would You Test This?" The topic of the entry is how to test controllers. Mike's article is good, and you should start there for context.

I think Mike has the Functional tests well covered in his entry. However, I've never been able to accept that Controllers can only be functionally tested. In my earlier Rails days I would probably have written something similar to the example below and considered it a Unit Test. (Sorry Mike, I prefer Mocha to Flexmock)

require File.dirname(__FILE__) + '/../test_helper'

class MenuItemsControllerTest < Test::Unit::TestCase
  
  def test_create_with_valid_menu_item
    controller = MenuItemsController.new
    controller.expects(:params).returns(:menu_item => {:title => 'Classic'})
    controller.expects(:flash).returns(flash_mock = mock)
    controller.expects(:menu_items_url).returns(:menu_items_url)
    controller.expects(:redirect_to).with(:menu_items_url)
    flash_mock.expects(:[]=).with(:notice, "MenuItem was successfully created.")
    controller.create
    assert_not_nil controller.instance_eval { @menu_item }
  end
  
  def test_create_with_invalid_menu_item
    controller = MenuItemsController.new
    controller.expects(:params).returns(:menu_item => {})
    controller.expects(:render).with(:action => :new)
    controller.create
    assert_not_nil controller.instance_eval { @menu_item }
  end

end

There's a few concerns with the above example. Probably the largest concern is that a lot of mocking almost always results in brittle tests. Another concern is that there's so much noise in the test that it's hard to determine what the intent of the test is. Furthermore, the test is clearly specifying not what should be done, but how it should be done.

On my current project, David Vollbracht created something called an IsolatedControllerTest. The IsolatedControllerTest class mocks the render method and does a few other things to give you the ability to test your controllers in isolation. While David had the right idea, it simply didn't catch on. We have a few IsolatedControllerTests; however, the majority of controller tests are all Functional.

As Mike points out, the current situation often leads to a lack of testing controllers. I'm no more okay with that option than Mike is, but we've had other more interesting problems to solve so I put the thought on a back burner. Luckily, Mike isn't willing to settle.

I think part of the problem is that controllers are not Good Citizens. Controllers violate the first rule of Good Citizenship. Upon creation (Controller.new), controllers are not in a valid state. Instead, controllers depend on being initialized within the framework and having their state set post construction time. That ends up being a problem for unit testing since it requires each test to set additional state on newly created controllers.

Another problem is that methods are marked as protected. For example the *_url and *_path methods are all only accessible from within a controller. I'm sure this was done with the best intentions; however, I subscribe to the philosophy that making something public to increase testability is a good idea.

One of the larger shortcomings that controllers have is that they don't behave like POROs (Plain Old Ruby Objects). Ideally, testing controllers should be as easy as writing the following tests.

require File.dirname(__FILE__) + '/../test_helper'

class MenuItemsControllerTest < Test::Unit::TestCase
  
  def test_redirection_to_menu_items_on_success
    controller = MenuItemsController.new(:params => { :menu_item => {:title => 'Classic'} })
    controller.expects(:redirect_to).with(controller.menu_items_url)
    controller.create
  end
  
  def test_flash_notice_on_success
    controller = MenuItemsController.new(:params => { :menu_item => {:title => 'Classic'} })
    controller.create
    assert_equal "MenuItem was successfully created.", controller.flash[:notice]
  end
  
  def test_menu_item_is_available_for_view
    controller = MenuItemsController.new(:params => { :menu_item => {:title => 'Classic'} })
    controller.create
    assert_instance_variable(:menu_item).exists_in(controller)
  end
  
end

I look forward to the day when I can create controllers (via Controller.new) and I will have a valid object that I can easily test (with tests similar to the ones above).

Labels: controller, rails, testing

Traditional Rails Controller tests begin with code very similar to the following code.

require File.dirname(__FILE__) + '/../test_helper'

# Re-raise errors caught by the controller.
class SomeController; def rescue_action(e) raise e end; end

class SomeControllerTest < Test::Unit::TestCase

  def setup
    @controller = SomeController.new
    @request    = ActionController::TestRequest.new
    @response   = ActionController::TestResponse.new
  end

If you use generators you may not notice or care about all this boiler plate code; however, if you generally create your files from scratch you may be interested in the following method that I add to Object (in my test_helper.rb).

class Object
  def controller_tests(&block)
    full_path_file_name = eval "__FILE__", block.binding
    test_name = File.basename(full_path_file_name, ".rb")
    controller_name = test_name.chomp("_test")
    require controller_name
    
    controller = controller_name.camelize.constantize
    controller.class_eval do
      def rescue_action(e)
        raise e
      end
    end
    
    test_class = eval "module Functionals; class #{test_name.camelize} < Test::Unit::TestCase; self; end; end"
    test_class.class_eval do
      define_method :setup do 
        @controller = controller.new
        @request    = ActionController::TestRequest.new
        @response   = ActionController::TestResponse.new
      end
    end
    test_class.class_eval &block
  end
end

The above code does basically the same thing, but it allows me to define my controller tests in a much cleaner way.

require File.dirname(__FILE__) + '/../test_helper'

controller_tests do    
  test "should display order number index" do
    get :index
    ...
  end    

  test "should create new order on new" do
    get :new
    ...
  end    
end

This change also provides the benefit of automatic test class renames when the test file name changes.

Labels: controller, rails, ruby, test