Validations are behavior, associations are structure
February 12th, 2012
TL;DR:
- TDD is about specifying behavior, not structure.
- Validations are behavior, and should be specified.
- Associations are structure, and need not be.
Disclaimer
This is my personal viewpoint, though it is not mine alone. YMMV.
Declarations
ActiveRecord provides a declarative interface for describing the structure and behavior of a model:
While syntactically similar, these two declarations do fundamentally different things.
Validations are behavior
The validates_presence_of :title declaration changes the behavior of
the save method (and other methods that use save), and should be specified
explicitly. Here’s an example using shoulda matchers:
Even though the matcher’s name looks just like the likely implementation, the
validate_presence_of matcher specifies that you can not save an Article
without a non-nil value for title, not that the
validates_presence_of(:title) declaration exists.
Associations are structure
The has_many declaration exposes a comments method to clients that appears
to be a collection of Comment objects. Doing Test-Driven Development, you
would add this declaration when a specified behavior requires it e.g.
This example needs a comments method that returns a collection in order to
pass. If it doesn’t exist already (because no other example drove you to add
it), this would be all the motivation you need to introduce it. You don’t need
an example that says it "should have_many(:comments)".
Testing the framework
Some will argue that we don’t need to spec validations either, suggesting that
it "should validate_presence_of(:title)" is testing the Rails framework,
which we trust is already tested. If you think of TDD as a combination of
specification, documentation, and regression testing, then this argument falls
short on the specification/documentation front because the validation is
behavior and, thus, the spec should specify the validation.
Even if you view testing as nothing more than a safety net against regressions,
the argument still falls down in the face of refactoring. If we add a Review
class that also has_many(:comments) and validates_presence_of(:title), and
we want to extract that behavior to a Postable module that gets included in
both Article and Review, we’d want a regression test to fail if we failed
to include either of those declarations in the Postable module.
But declarations are already declarative!
Another argument is that declarations supply sufficient documentation. e.g. we
can look at rental_contract.rb and know that it validates the presence of
:rentable:
This is an interesting argument that I think has some merit, but I think it would require an extraordinarily disciplined and consistent approach of using declarations 100% of the time in model files such that each one is the spec for that model, e.g.
100% may sound extreme, but as soon as we define a single method body in any one of the models, the declarative nature of the file begins to degrade, and so does its fitness for the purpose of specification. Plus, if we can only understand the expected behavior of a model by looking at both its spec and its implementation, we’ve lost some of the power of a test-driven approach.
What do you think?
Do you spec associations? If so, what value do you get from doing so? If not, have you run into situations where you wished you had?
Same questions for validations.
February 12th, 2012 at 6:12 pm
Dave, You’re spot on.
Nearly every time I start to refactor a piece of an existing Rails app I usually with the specs covered the associations and validations for one simple reason: refactoring. More often than not I have to add some in when refactoring a complicated class.
Most people assume that if your validation or association lines are removed from the class, that other tests will fail. I cringe at that argument because of the behavior changes validations and associations introduce… and in the worst cases I’ve seen “mystery” associations in Models and Factories that are very poorly spec’d or documented that affect the behavior of the Model (i.e, not able to save, not to mention LoD violations)
-Mark
February 12th, 2012 at 6:14 pm
Hey David, I’ve pinged you about this in the past and we didn’t see eye to eye on it, but I’d like to bring up my ValidAttribute gem as an alternative to ShouldaMatchers. I feel that ShouldaMatchers ties the tests too closely to the implementation. My tests shouldn’t care what specific validations are on my model, just that a given attribute is valid or invalid under certain circumstances.
February 12th, 2012 at 6:45 pm
I make the most mistakes with associations, especially when the models are namespaced. So, when seeing specs as verification, I prefer to have specs for validations. ShouldaMatchers also don’t really catch this properly, so you’ll need to use factories like you showed.
Furthermore, I agree with Brian Cardarella: I’d rather use “should_not allow_value(nil).for(:title)” to test my validations.
February 12th, 2012 at 7:37 pm
I’ve tested validations and declarations in the past, but lately I’m of the opinion that those kinds of tests are unnecessary.
If you’re relying on a single “post should have many comments” test to ensure that a Post has_many comments, you’re in bad shape.
That being said, I just changed some of the validation rules on a model in our signup/billing system at work, and you better believe I added some new tests while I was in there. I’d rather err on the side of caution (and a few extra tests) especially when dealing with something like that.
What’s the right balace? I don’t know — test as much as it increases your confidence, I suppose.
February 12th, 2012 at 11:41 pm
Great post. I suppose in the end, that I (theoretically) like to think of AR models as data structures, and that I don’t want them to have behaviour (I grant them exceptions for persistence). As you pointed out, validations are behaviour, so I tend to think they should exist outside of the model. Other objects would take this data and do things to it like validating and calculating default monthly charges, and so forth.
In reality, I rarely achieve this goal, but it is what I strive towards. If it is achieved, I think it is reasonable to expect that models would be 100% declarative. They would really only have methods on them that discuss their relationships, and these are already declarative.
February 13th, 2012 at 10:19 am
I agree with your viewpoint David. I don’t spec associations directly; they exist to wire things up and they usually get tested indirectly through other specs as well as through integration tests (I think working outside-in exposes this, whereas working inside-out has a harder time doing this). The associations themselves are structural declarations of how things are related to one another. At a lower-level that relationship is behavior and I find that ActiveRecord does a good job of testing the mechanics of that itself.
However, there are times I spec associations although the focus is not on the structure of the association; it is on the behavior that the association supports. For example, when the relationship between models is not plain vanilla. In these cases, since they are specific to my application code I add specs around the the relationships I expect to be there and then I implement the association. This has nothing to do with ensuring the literal line of code “has_many :foobars, :finder_sql => ‘SELECT…’” exists. It is about ensuring that “foobars” is related in the way I expect it to be and it is custom and potentially complicated logic that is specific to my application that ActiveRecord tests wouldn’t help me with verifying anyways.
Another example of spec’ing behavior which drives associations is things like :dependent => :destroy. I add specs around those to ensure that when X is removed so is Y. This isn’t about ensuring that the option :dependent => :destroy is added declaratively. No, instead, it actually creating an X and a Y and then deleting X and ensuring Y got deleted to. I just use the :dependent => :destroy as an easy way to implement the behavior I’m looking for.
Validations are very much behavior to me. I always spec those because they are specific to my application. Similar to the times when I do spec behavior around associations, I don’t spec the mechanics that make validations work; I spec the behavior that I expect to exist. This work great when an integration test shows that errors are displayed properly and then relying on the much faster spec counterpart to have examples of all of the specific validations and types of errors that could be possible to propagate up.
I don’t think that the implementation of behavior (whether declarative or not) is at all related with spec’ing the behavior itself. It’s an entirely separate area. If we can find a nice simple way to implement something declaratively, then perhaps we should also push ourselves to find a similarly elegant way to specify that behavior, not simply omit it.
February 13th, 2012 at 4:02 pm
Seems reasonable to me. I like the double-entry book-keeping nature of checking validations.
February 13th, 2012 at 6:34 pm
your snippets show up “escaped” for some reason.
February 14th, 2012 at 10:47 am
I do intend to test the associations too. Just as you point it out, it is for documentation/specification purpose. Make a few nice macros you can test the association easily.
Also just make my philosophy of writing a test before you write your code be more consistent
Also it does help me spotting bugs. You may not believe but people from time to time just forget these associations.
February 15th, 2012 at 8:23 am
Thanks for the feedback, everyone.
Yi - I’m not saying that associations should just be added without a failing example. I’m just saying that, in my view, that example should be about some behavior that requires the association to exist vs simply declaring its existence..
February 16th, 2012 at 3:36 pm
David - art, got it. and yes, agreed.
April 11th, 2012 at 8:54 am
Very interesting read. I get that you rather spec the behaviour rather then merely verifying having the statement
has_many :commentsthere.So, if I setup a post with two comments, I could test that
post.commentswill return both those comments. Yet: then I am testing Rails code. On the other hand, if I writepost.should have_many :commentsI have very readable documentation/spec that I expect a post to be linked to comments and that those can be retrieved in the rails way.So I would argue that associations are not purely structure, but also behaviour. It adds methods to our model, of which the behaviour is known, and in the simplest cases guaranteed to work (because it is implemented in Rails).
Also, using these high-level matchers limit duplication in my specs/code: I have a clear and concise way of expressing that an association should exist.
Does that make sense, or am I completely missing the point you are trying to make?
April 11th, 2012 at 9:06 am
Nathan - I don’t think you’re missing the point, but you’ve stumbled on the gray area of what’s domain-specific behavior and what’s framework behavior. Yes,
post.commentsadds methods, but those methods are already well tested in the Rails test suite. What I try to do is find something interesting in my own domain, and spec that. e.g. imagine that a list of popular articles is based on the number of comments each has. In that case, we have to use thecommentsmethod(s) to specify that an article with 0 comment is less popular than one with 20. It may be thatcommentsdoesn’t exist before I write that spec, but when I get theNoMethodErroroncommentsI can add the declaration without having to specify it explicitly. Make sense?