[rspec-users] testing behaviour or testing code?

Jay Levitt lists-rspec at shopwatch.org
Thu Sep 6 23:36:41 EDT 2007

On 9/2/2007 11:49 PM, David Chelimsky wrote:
> On 9/2/07, Jay Levitt <lists-rspec at shopwatch.org> wrote:
>> On 9/2/2007 12:43 PM, David Chelimsky wrote:
>>> The problem we face is that AR promises huge productivity gains for
>>> the non TDD-er, and challenges the thinking of the die-hard TDD-er.
>>> I've gone back and forth about whether it's OK to test validations like this:
>>> it "should validate_presence_of digits" do
>>>   PhoneNumber.expects(:validates_presence_of).with(:digits)
>>>   load "#{RAILS_ROOT}/app/models/phone_number.rb"
>>> end
>>> On the one hand, it looks immediately like we're testing
>>> implementation. On the other, we're not really - we're mocking a call
>>> to an API. The confusion is that the API is represented in the same
>>> object as the one we're testing (at least its class object). I haven't
>>> really done this in anger yet, but I'm starting to think it's the
>>> right way to go - especially now that we have Story Runner to cover
>>> things end to end. WDYT of this approach?
>> Personally, I don't much like it.  It feels too much like:
>> it "should validate_presence_of digits" do
>>    my_model.line(7).should_read "validates_presence_of :digits"
>> end
> A couple of things to consider:
> There's a very useful guideline in TDD that says "test YOUR code, not
> everyone elses." The validation library we're testing here is
> ActiveRecord's. It's already tested (we hope!).

Right... and I'm not testing that ActiveRecord's validation works.  I'm 
testing that my model works as I expect it to work.

For instance, in your example, you just verify that you call 
validates_presence_of with fieldname :digits.  You're not verifying that 
that's the right thing to do, or that it behaves the way you expect it to.

Also, I think this conflicts with "test behavior, not implementation". 
All I care about is the behavior of the model; I don't care if it calls 
validates_presence_of, or if it calls acts_as_phone_number.

> Also - there's a difference between the behaviour of a system and the
> behaviour of an object. The system's job is to validate that the phone
> number is all digits. So it makes sense to have examples like that in
> high level examples using Story Runner, rails integration tests, or an
> in-browser suite like Selenium or Watir.

Ah, but (as Pat pointed out) in Rails, validations are, in fact, the job 
of the model.  They may be done with validates_* "declarations", or with 
custom code, or with plugins.

> This model object's job is to make sure the input gets validated, not
> to actually validate it. If the model made a more OO-feeling call out
> to a message library - something like this:
> class PhoneNumber
>   def validators
>     @validators ||= []
>   end
>   def add_validator (validator)
>     validators << validator
>   end
>   def validate(input)
>     validators.each {|v| v.validate (input)}
>   end
> end
> Then submitting mock validators via add_validator and setting mock
> expectations that they get called would be totally par for the course.

Yeah, and I guess I still haven't swallowed that part of mocking - 
because, again, it's brittle and tied to implementation.  I have no 
problem mocking out ActiveRecord, because that's a major part of any 
Rails app and it's a given that you'll be using it in a certain way. 
Ditto for any other major library.  But validations are so simplistic 
that you might write a given validation in five different ways, and 
specifying -which- of those five ways the code should use just feels wrong.

And even for AR, as someone pointed out - what if I want to use .new or 
.build instead of .create, or .update_attributes instead of the setter 

The ideal answer for that is to build a more sophisticated AR mock that 
lets you write expectations that work in any of those cases.  I want to 
know that User.register.should do_something_that_creates_a_new_record, 
not that it explicitly called .create.

And interestingly, in the case of .update_attributes vs. direct 
assignment, it seems to me that the proper way to "test the behavior, 
not the implementation" is to check the value of the field after the 
fact - which of course apparently conflicts with "test behavior, not 
state".  But, when the behavior IS to set a certain state, I feel like 
it's OK.


More information about the rspec-users mailing list