Pat, I appreciate your time in forming your thoughts and putting
together a well reasoned reply. Yet there are a few areas where I
think tests like these are valuable.
(Please note, this is not the original poster, even if we share the
same first name)
> Change 'describe' to 'class' and remove 'do' from the first line.
> Then remove the 'it_' from the next three lines. At this point you
> have the exact implementation of the class.
>> I don't know about you, but that bothers the hell out of me.
This doesn't actually bug me as much. My company sometimes writes
software for some pretty dumb users, and I mean that with kindness.
Imagine a form with the following fields:
- First name
- Last name
- Email
I don't know how many submissions I've seen where they've entered
"Bob " or "jacob at foo.com ". Notice the spaces at the end. So most
often I write classes like:
class SomeObj < ActiveRecord::Base
def first_name=(first_name)
self[:first_name] = first_name.nil? ? nil : first_name.strip
end
... (repeated for last_name and email)
end
And from writing this same code many many times, I've moved to.
class SomeObj < ActiveRecord::Base
strips :first_name, :last_name, :email
end
Naturally my spec was
describe SomeObj do
it "should set the first name to nil if nil" do
obj = SomeObj.new(:first_name => nil)
obj.first_name.should be_nil
end
it "should strip the first name if containing spaces " do
obj = SomeObj.new(:first_name => "foo ")
obj.first_name.should == "foo"
end
...(repeated for last name and email)
end
Obviously you could (and maybe should) test "obj[:first_name]" to be
more specific.
So being a lazy developer, I changed this common occurring spec into
describe SomeObj do
should_strip :first_name, :last_name, :email
end
This is an example where the implementation and the spec both appear
clones, yet I feel they are valuable.
> -helps you design your code well
Not going to argue this point, because I don't think the spec does
squat for this goal.
> - provides executable examples of how to use code
Although the implementation of the helper "should_strip()" is a little
hard to read from the code standpoint, it does create the individual
tests. And the spec output is:
User
- should set the first name to nil if nil
- should strip the first name if containing spaces
- should set the last name to nil if nil
- should strip the last name if containing spaces
- should set the email to nil if nil
- should strip the email if containing spaces
So if the spec breaks I get feedback on what is broken. Which also
feels pretty descriptive.
That doesn't explain _how_ it can break though, and I feel that ties
into the next goal:
> leaves behind regression tests
It came to pass that another developer added this:
class SomeObj < ActiveRecord::Base
strips :first_name, :last_name, :email
def first_name=(first_name)
self[:first_name] = first_name
do_some_business_logic_on_setter_of_first_name(first_name)
end
end
Which broke the behavior of the object with which the "should strip
the first name if containing spaces" failed.
> If you make any change to the implementation then the specs will
> fail...so they're brittle without providing any value.
I have a hard time grappling with this topic, as it's a very recurring
theme in mock testing. But I guess I don't see
describe SomeModel do
it_has_many :widgets, :destroy => :null, :class_name => "BaseWidget"
end
any more brittle than
class SomeModel do
def self.foo(bar)
AnotherClass.baz(bar)
end
end
describe SomeModel do
describe "#foo" do
it "should delegate to AnotherClass#baz" do
AnotherClass.stubs(:baz).with("something").returns("something
different")
SomeModel.foo("something").should == "something different"
end
end
end
Yet I find a lot of value in that test (given adequate reasons why
it's designed that way).
I think this reasoning still applies to the model relationships. I
agree there isn't a lot of bang for the buck, but I see there is
value. I like the fact it stresses the relationship at a basic level.
So to bring the argument full circle:
> The concrete benefits of object-level specification are, in my mind,
> that it
> - helps you design your code well
Not disagreeing
> - leaves behind regression tests
- The scenario above (i.e. adding a method that collides with rails-
added behavior)
- Stressing the basic add / remove functionality as you develop more
complex validation rules on that collection. (e.g. Can't have more
than x, Can't have a member in the set of y)
- Stressing foreign key constraints from bubbled exceptions from the
database (though I think our company is in the minority on that one)
> - provides executable examples of how to use code
- Declarative error messages (from the spec titles). An example spec
doc from our existing project using our version of model behaviors:
User relationships
- should have many descriptions
- should destroy descriptions when deleted
- should have many photos
- should destroy photos when deleted
- should have many projects
A couple final notes:
- I don't disagree that some of these scenarios could be tested at a
higher level, or if something changes, higher level tests will catch
them. It's just been my experience that when leaning on higher level
tests too much, too many things slip through. Or the verbosity of
individual tests becomes a bitch.
- Our version of "should_have_many" "should_belong_to", etc, touches
the database, and doesn't just check manipulated methods on the model.
I find that much more valuable.
- I think these helpers needs to remain with basic functionality
(Defining the relationships and effects of deletion), any more needs
to be in custom specs.
- In a perfect world, I would see Rails Core, and other plugin
developers to write such declarative behavior helpers along with their
declarative code, but I don't think that's at all practical.
- While I'm not totally sold on the "Double-entry bookkeeping"
argument with regards to testing, I feel there is a bit of that
creeping in. For good or worse.
- Not intending this reply to be a straw man, but can't articulate
what's in my head without bringing another example into the mix. Sorry
if it confuses more than helps.
-Zach