its not your responsibility

Posted by jcarroll

Sep 18

Sometimes in Rails you ask yourself: how do I test this?

One example is ActiveRecord associations.

1
2
3
4
5
6
7
8
class Post < ActiveRecord::Base

  has_many :comments

end

class Comment < ActiveRecord::Base
end

Now from the documentation for ActiveRecord::Base#has_many we know it’s going to generate a method for us called #comments, so let’s test that.

1
2
3
4
5
def test_should_have_many_comments
  post = Post.new

  assert_respond_to post, :comments
end

Here all we’re testing is that a Post object understands #comments.

Maybe thats not enough?

1
2
3
4
5
6
def test_should_have_many_comments
  post = Post.new

  assert_respond_to post, :comments
  assert_kind_of Array, post.comments
end

Now we’ve added an additional assertion to check that the object returned from Post#comments is an Array.

Can we take it any further?

1
2
3
4
5
6
7
8
def test_should_have_many_comments
  post = Post.new

  assert_respond_to post, :comments
  assert_kind_of Array, post.comments
  assert_raises(ActiveRecord::AssociationTypeMismatch) { post.comments << Post.new }
  assert_nothing_raised { post.comments << Comment.new }
end

Now we’ve added 2 additional assertions that check that the object returned from Post#comments only allows Comment objects to be added to it via #<<.

OK, so maybe that might have been overkill, but at the very least you should check the object understands the given message a la our first attempt.

Another example is testing methods added to a class via a plugin.

1
2
3
4
5
class Article < ActiveRecord::Base
 
  acts_as_solr

end

From the acts_as_solr documentation it looks like calling the acts_as_solr class method on an ActiveRecord::Base class object will add a #find_by_solr method to it.

Here’s our test.

1
2
3
def test_should_understand_find_by_solr
  assert_respond_to Article, :find_by_solr
end

There is no need to test specific solr queries and then run assertions on the results. Once you go that route, you are now testing the acts_as_solr plugin and the solr web application; this is not your application’s responsibility, its the author(s) of acts_as_solr and solr to test their own code.

You’re also introducing an external dependency in your tests (that a Java application server is running the solr web application). Like any external dependency, this is going to slow your tests down considerably. And once your tests start getting slower developers are not going to be running them as much. And if you aren’t running your tests your code is going to go south.


Comments on this post

Steve Tooke

Sep 19

Steve Tooke said,

Doesn’t this encourage brittle tests though? Here your tests are relying on the implementation of ActiveRecord or acts_as_solr. If this changes, or if you need to change the underlying implementation (e.g. for performance) your tests will likely break, and more importantly you don’t have a safety net to prove that your application is still behaving in the same way.

You don’t want to retest the whole of the library you are using, but you do want to make sure your application is behaving how you want it to.

This is where I struggle to be honest. How do you draw the line between testing a single object and testing the behaviour that you want.

Perhaps with your two examples you’ve got it right in the case of AR associations, but with acts_as_solr perhaps you need to test the methods on the object that you will use to wrap find_with_solr, and stub acts_as_solr at that point?

Brandon Dimcheff

Sep 19

Brandon Dimcheff said,

I agree with Steve. We are trying to test behavior, not simply whether or not acts_as_solr or has_many are being called. The problem with the approach described above is that it assumes too much about implementation details and doesn’t actually make sure your app is doing what it’s supposed to be doing.

In this case, I think that find_by_solr should be called find_by_content or something, since it doesn’t really matter that it’s solr that’s doing the lookup. All the developer using the API cares about is that when they pass a particular query into the method, the proper results are returned. And that is what our tests should test.

I am not convinced that (as many test/rspec examples show) simply checking for association methods (has_many, belongs_to, etc.) or plugin methods (acts_as_solr) are sufficient, or even a good idea at all. Nor do I think that those sorts of tests qualify as BDD. For instance, I have something resembling the following in an application per someone’s suggestion:


Person.reflect_on_association(:addresses).should_not be_nil

I really don’t like this, though. I don’t care one bit that there’s an association called “addresses” on my Person object. What I care about is that Person responds to addresses and that addresses returns an array of the proper addresses. This is the whole point in BDD. Care about the behavior of your objects, not their implementation.

To explore this further, I’ll use a slightly more complicated example. Say I have the following in my Person class:

1
2
3
# app/models/person.rb
has_many :addresses
has_many :cool_addresses, :foreign_key => "address_id", :conditions => ["foo = ? AND bar = ?", foo, bar], :order => :zipcode

And the corresponding test case:


Person.reflect_on_association(:cool_addresses).should_not be_nil

Well, guess what. This association exists. Our tests pass. But it’s wrong. That :foreign_key is supposed to be person_id and not address_id. Well, we can solve that! Just test to make sure the has_many receives the appropriate parameters. Something like this (made up) helper would work:


Person.reflect_on_association(:cool_addresses).should have_foreign_key("person_id"))

And we could go about our business, basically duplicating all the parameters supplied to has_many in our tests. But in the end, this is just going to make our tests horribly brittle and is not actually testing anything useful. It’s not testing behavior at all.

The whole point in BDD is to make our tests poke and prod our application in a certain way and have them spit back the correct output. Yes, the plugins/associations are well tested and I shouldn’t be testing them again. I know that if my has_many call supplies the correct parameters, I will get the objects I expect to get. But I still need to make sure that I’m calling has_many properly. It’s simply not sufficient for me to know that has_many is called, I need to know that when it’s called, the proper “stuff” happens. I need to make sure the association does what I expect it to do. Here’s what I think my tests should do to ensure cool_addresses is working properly (no real code this time):

  • Add a few objects to the Addresses table, either using fixtures or in some kind of before callback. (Yes, fixtures suck, etc.)
  • Make sure cool_addresses returns addresses that correspond to the ‘foo’ and ‘bar’ conditions above.
  • Make sure that cool_addresses returns the addresses ordered by zipcode.

And that’s it. Yes, it will take the tests slightly longer to run, since they’re using the database (and maybe fixtures). Yes, I’m partially testing ActiveRecord. But I’m testing that my object behaves like I want it to. That’s the point in BDD, right?

As an added benefit, the tests are much more flexible now. Check it out:

1
2
3
4
5
6
7
# app/models/person.rb
has_many :addresses
#has_many :cool_addresses, :foreign_key => "address_id", :conditions => ["foo = ? AND bar = ?", foo, bar], :order => :zipcode

def cool_addresses
  addresses.find_all do {|a| a.foo == foo && a.bar == bar}.sort(&:zipcode)
end

That passes my tests, too. And it should. But my previous example where I used association introspection would fail miserably.

This is a major complaint about a lot of the test code examples I see floating around. Everybody seems to be mocking and stubbing and introspecting to their heart’s content, but all they seem to be doing in the end is writing the same code twice: once in their implementation, and once in their tests. And so when they change the way their application is implemented (NOTE: I did not say their application’s behavior) their tests break.

There are two sides to this BDD thing. First, your tests need to ensure that if you change the behavior of your code, they will fail. Second, your tests need to still work when your application still behaves the right way, even if you change every single line of code in your application. Of course, this is nearly impossible to achieve, but at least we can try.

Thoughts?

PS – This rant has become much longer than I had predicted. It will probably end up on my blog in one form or another.

Brandon Dimcheff

Sep 19

Brandon Dimcheff said,

Now that I’m re-reading the post, it looks like we might actually be getting at the same thing. You are indeed testing that Post responds to comments and not simply that the association exists. That’s definitely a good first step, but I think that often you need to take it a little further like you did in your example. Maybe not for extremely simple associations, but certainly for the association I described in my above response.

Also, I’d like to point out that it’s definitely possible to write some good helpers that will generate tests that will test the appropriate behavior. It’s just important to make sure the helpers test behavior and not implementation details, which is how I’ve seen it done a lot in the past.

jare care

Sep 19

jare care said,

I agree with both of you.

Interaction based testing using mocks ties your test exactly to your implementation. And your tests are very brittle. And you are absolutely correct in saying you could implement something entirely invalid, as long as your test for it duplicates the whole implementation your test passes, even though its totally wrong.

I used to go all interaction based testing like this but in order to really know if your application works you need to go back to browser based testing. That’s a total waste of automated testing.

When a framework like Rails assumes things like a test environment, a test db and fixtures, doing simple, straight-forward state based testing is the way to go. Send a message to an object, get back some results, run some assertions against the results. That’s a good test and you’re not dependent on implementation at all.

In my above examples the ActiveRecord associations should of been tested using simple state based testing because its quick and introduces no external dependencies.

However, I stand by my acts_as_solr tests because solr is an external dependency. Having it be required during testing slows down your whole rhythm. Test, code, refactor becomes test, wait a few minutes, code, refactor. I like my tests to be quick. Rails already requires all developers who run its tests to have a test database. That’s fine and its become accepted practice (this would never fly in the Java/.Net world most likely because of a lack of a complete stack like Rails).

In my experience, I say start out with traditional state based testing. When external dependencies creep into your app e.g. external APIs, the file system, the internet, etc. look at the effects they have on your development cycle and if its negatively impacting it then mock the dependency out using interaction based testing. The resulting code will be brittle and refactoring it will be impossible without breaking its test but your tests will be fast and your developers will be happy.

Like everything in software, its a trade-off.

Brandon Dimcheff

Sep 19

Brandon Dimcheff said,

Jare,

“Like everything in software, its a trade-off.” Truer words have never been spoken. ;)

I think I may have to disagree with you about testing acts_as_solr. In the very simple case, it may be enough to assume that if your object responds to find_by_solr, everything is dandy. But suppose you have a more complex solr setup with some facets and whatnot, and you need to be sure that everything works correctly.

Now… I would never suggest that every test case that deals with the results of a solr query should go through the whole solr stack. Mocks are perfectly reasonable here. But I do think that you do need to actually make sure you’re getting the proper data back in the cases that really matter to your application. Just like with cool_addresses above, how would you ever know that your crazy solr setup is actually returning the proper data?

All this seems like it might be some sort of integration testing. I need to test to make sure solr is properly integrated with my application. Once I’m sure of that, I can mock it out so most of my tests are quick. But I don’t think I want to avoid interacting with solr in every case.

Steve Tooke

Sep 20

Steve Tooke said,

I guess that is the crux of the whole thing. Simple unit testing isn’t enough – you have to do full integration testing, with something like Watir or Selenium to ensure that your app actually behaves how you expect as a whole.


Sorry, comments are closed for this article.

© 2000 - 2009 by thoughtbot, inc.
written by a bushel of tiny robots