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:

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?

Posted on September 20th in Tips with tags , , , , , , . | 1 comment | read on

Predefined models? No different than any others.

Earlier today I was trying to figure out how to make a list of potential models to be created. For example: I have some Users and some Groups. I have a many-to-many Membership join model with some has_many :throughs. The fundamental goal is to provide the user an interface to add Membership models that link a User to a Group. I could use a bunch of checkboxes, but I’d rather pull up a list of possible Groups with little “add” links next to them.

Since I’m using RESTful controllers, we have a MembershipsController that implements all the standard methods. We’re also nesting our routes such that MembershipsController nested beneath the UsersController. In order to create the Membership, we need to POST to /users/1/memberships. But how do we get a list of Memberships that we can quickly add?

How about modifying MembershipsController.new? We don’t need the normal definition of new, since we’re never going to be manually creating a new Membership.

1
2
3
4
5
6
# controllers/memberships_controller.rb
def new
  @memberships = Groups.find(:all).collect do |group|
    Membership.new(:user_id => params[:user_id], :group_id => group.id)
  end
end

Now we have a list of potential Membership objects available to our view. Remember, the Memberships haven’t been saved yet. They’re just there for convenience for holding attributes. We are doing object-oriented programming after all.

1
2
3
4
5
6
7
8
# views/memberships/new.html.erb
<% @memberships.each do |membership| %>
  <% form_for :membership, membership, :url => memberships_path do |f| %>
    <%= f.hidden_field :user_id %>
    <%= f.hidden_field :group_id %>
    <%= f.submit membership.group.name %>
  <% end %>
<% end %>

Basically what we have here is a big list of predefined join models wrapped up in form tags. When you click on one of them, you’ll end up submitting the form that actually creates the model. Eventually, we could make this into an AJAX widget that uses form_remote_for with very little effort.

Nothing I have described here is particularly revolutionary. Rather than just returning a single new Membership in the new method, we return a list of objects. Rather than rendering one form with editable fields, we render multiple forms with predefined, hidden fields. Whichever you submit creates the corresponding object. These two very simple changes to the standard REST actions allow us to easily and elegantly create our join models.

Posted on August 30th in Tips with tags , , , , , . | 0 comments | read on

View archives for September 2007.