On 9/2/07, David Chelimsky <[EMAIL PROTECTED]> wrote:
> On 9/2/07, Jay Levitt <[EMAIL PROTECTED]> 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
> >
> > I can write specs like that all day and ensure absolutely nothing about
> > my code.
> >
> > I like to think of specs as a form of N-version programming where N=2
> > (or maybe N=3 now with Story Runner). By using a different vocabulary
> > to express the specs than the actual code, we are more likely to think
> > of the problem differently, and thus find places where the two versions
> > of our code differ. Sometimes, it means we miswrote the spec;
> > sometimes, it means we miswrote the code.
> >
> > But if all your spec does is guarantee that your code reads a certain
> > way, you've done nothing but protect against accidental edits. And if
> > you're gonna go that way, why not go all the way:
> >
> > it "shouldn't change unless I change the spec too" do
> > MD5.new(my_model).should == "0xDEADBEEF0FFD2FFE4..."
> > end
> >
> > I'd much rather see:
> >
> > it "should prevent me from entering anything but digits" do
> > PhoneNumber.new("800-MATTRESS").should_not be_valid
> > end
> >
> > And then, every time I find an edge case, I add another spec:
> >
> > it "should allow me to enter dashes" do
> > PhoneNumber.new("800-555-1212").should be_valid
> > end
> >
> > it "should only allow 10 digits" do
> > PhoneNumber.new("800-555-12121212").should_not be_valid
> > 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!).
>
> 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.
>
> 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.
>
> In AR, the validators are added declaratively. This is a Rails design
> decision that we have to either live with or write other code around.
> Choosing to live with it, it seems to me that mocking the call to
> validates_presence_of :digits is no different than mocking validate on
> an injected validator.
>
> That all make sense?
There's nothing technically *wrong* with it, and logically it holds weight.
It just doesn't feel right.
Your key point is that we're making an API call, which I agree with.
We also agree that AR probably does too much, and I think this is a
situation where we should go with the flow. We call my_record.valid?
and end up with my_record.errors if it's not valid. An AR object is
in fact responsible for its own validation (even if you feel it's too
much responsibility). It makes sense to specify the object's behavior
in the same way.
Personally I can't find a strong argument either way. I'm sure it's a
matter of taste here. I would prefer to look at a spec and get as
much info on how to use an object as possible. In that case, creating
an object, calling valid?, and inspecting errors is probably more
helpful. But after giving this a lot of thought, I'm not sure it
warrants a ton of thought :)
Pat
_______________________________________________
rspec-users mailing list
rspec-users@rubyforge.org
http://rubyforge.org/mailman/listinfo/rspec-users
