Testing the Right Stuff

Posted by Koz Monday, August 20, 2007 02:24:00 GMT

I’m going to take a slightly different tack here, and review some of the unit tests in rails itself. They show up two common anti patterns, spurious assertions and coupling your tests to the implementation.

Perhaps the biggest benefit of a suite of unit tests is that they can provide a safety net, preventing you from accidentally adding new bugs or introducing regressions of old bugs. With a large codebase, the unit tests can also help new developers understand your intent, though they’re no substitute for comments. However if you’re not careful with what gets included in your test cases, you can end up with a liability.

Be careful what you assert

Whenever you add an assertion to your test suite you’re sending a signal to future developers that the behaviour you’re asserting is both intentional and desired. Future developers who try to refactor your code will see a failing test, and either give up, or waste time trying to figure out if the assertion is ‘real’ or whether it was merely added because that’s what the code happened to do at present.

For an example, take the test_hatbm_attribute_access_and_respond_to from associations_test.rb , especially the assertions that the project responds to access_level= and joined_on=. Because of the current implementation of respond_do?, those assertions pass. But should they?

In reality while those values will get stored in the object, they’ll never be written back to the database. This is a surprising result for some developers, and removing those accessor methods would go a long way to helping avoid some frustrating moments.

Mock and Stub with care

Mock object frameworks like flexmock and mocha make it really easy to test how your code interacts with another system or a third party library. However you should make sure that the thing that you’re mocking doesn’t merely reflect the current implementation of a method. To take a case from rails, take a look at setup_for_named_route in routing_test.rb.

It takes the seemingly sensible approach of building a stubbed-out implementation of url_for instead of trying to build a full implementation into the test cases. The stubbed version of url_for simply returns the arguments it was passed, this makes it extremely easy to work with and to test.

The problem is not with stubbing out the method, but in the way it is used in all the named route test cases. Take test_named_route_with_nested_controller.

1
2
3
4
5
6
def test_named_route_with_nested_controller
  rs.add_named_route :users, 'admin/user', :controller => '/admin/user', :action => 'index'
  x = setup_for_named_route.new
  assert_equal({:controller => '/admin/user', :action => 'index', :use_route => :users, :only_path => false},
  x.send(:users_url))
end

The strange hash value you see in the assertion is the result of the named route method calling url_for, and returning that. The current implementation of the named route helpers does this, but what if you wanted to implement a new version of named routes which completely avoids the costly call to url_for? Every single named route test fails, even though applications which use those methods will work fine.

In this situation you have two options, you could make your tests depend on the full implementation of url_for. This would probably slow down your test cases, and require a lot more setup code, but because the return values are correct you’re not likely to impede future refactoring.

The other option is to use different stubs for every test case. Leaving you with something like this:

1
2
3
4
5
6
7
def test_named_route_with_nested_controller
  rs.add_named_route :users, 'admin/user', :controller => '/admin/user', :action => 'index'
  generated_url = "http://test.named.routes/admin/user"
  x = setup_for_named_route.new
  x.stubs(:url_for).returns(generated_url)
  assert_equal(generated_url,  x.send(:users_url))
end

Doing this for each and every test case is going to be quite time consuming and make your test cases extremely verbose. As with all things in software you’ll have to make a judgement call on this trade off and make a choice between coupling or verbosity.

Whatever approach you choose, just remember that misleading test ‘failures’ can slow down refactoring, and end up reducing your ability to respond to change. As satisfying as ‘100% coverage’ or 2:1 ratios may be, don’t blindly add assertions or mock objects just to satisfy a tool. Every line in your test cases should be there for a reason, and should be placed there with just as much care as you’d use for a line of application code.

Comments

Leave a response

  1. janAugust 28, 2007 @ 10:47 AM

    This is very interesting and useful article, thanks. Writing good tests is not as easy as it might seem when you’re starting learning them.

  2. DiegoSeptember 19, 2007 @ 07:20 PM

    Let me see if I got it right: In the last example you’re testing the process but not the outcome. You are making sure that when calling users_url the url_for methods of that object (x) get called in order to generate the url. But you are not testing the outcome of it because you already tested url_for for correctness somewhere else. If I got it right, It makes sense :D

  3. massiveOctober 23, 2007 @ 01:37 PM

    I’m a bit puzzled too. If Diego’s assumption isn’t correct, then I don’t see any sense in the last example. Isn’t it like testing that generated_url equals generated_url or what?

Comment