Test behavior, not implementation!

This is a based on a comment I made on a post on thoughtbot’s blog. I suggest you read the original post for some background on what I’m talking about.

If you’re too lazy to read, here’s the basic gist: You could potentially test your associations or plugins (such as acts_as_solr) by simply checking whether or not your object responds to the messages that the plugins generate when their class methods are called.

The problem is, we should be 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:

# 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:

# 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?