A bit of a delayed reply, but I appreciate all the feedback everyone.
What a helpful list this is!
Nicolás Sanguinetti [email protected] on 2010-01-28 at 15:45:
You’re definitely testing too much implementation and not enough behavior.
This was the overwhelming opinion of the group, and I see what you are
all saying.
Basically, what you want to spec, is that provided some options, when
you call a certain method of your form builder, you get a certain html
output. At least that’s how I would approach the problem.
it “produces a correctly formatted FOO input” do
html = @builder.foo_text(…)
html.should have_tag(“label”, :for => “foo”)
html.should have_tag(“input”, :id => “foo”)
end
This is more or less exactly the structure that current specs have. And
the (mistakenly-designed, I now realize) refactor I was hoping to make
was to address the problem of re-verifying all of the behavior of
question
in every formbuilder method.
Here is how we’re currently doing it:
describe ‘question’, :shared_behavior => true do
it ‘marks the question required’ do
# stub question required in model
xhtml = @builder.send(@current_method) # [A]
xhtml.should have_tag(‘li.required’)
end
… more examples describing question behavior [B]
end
describe ‘#question’ do
before { @current_method = ‘question’ }
it_should_behave_like ‘question’
end
describe ‘#text’ do
before { @current_method = ‘text’ }
it_should_behave_like ‘question’
… examples describing text behavior
end
This worked great at first, until we got to the point of there being
more than 50 examples at [B] for question
, and there being more than
30 other methods. I also realized that some methods that share question
behavior require certain options to properly pass through the question
shared example group. So I had to add another argument to [A] and
maintain this knowledge across the suite.
Not only that, but I realized that question is not the only shared
behavior. Add that to the mix and you start to get structures like
this:
describe ‘collection’, :shared => true do
… examples describing collection behavior
end
describe ‘#collection’ do
before do
@current_method = :collection
@default_options = { :collection => [:foo, :bar, :baz] }
end
it_should_behave_like ‘question’
it_should_behave_like ‘collection’
end
describe ‘#dependent_collection’ do
before do
@current_method = :dependent_collection
@default_options = { :collection => [:foo, :bar, :baz], :depends_on
=> :qux }
end
it_should_behave_like ‘question’
it_should_behave_like ‘collection’
… examples describing dependent_collection behavior
end
This gets unruly and difficult to read and maintain very quickly, and
because our group has several devs who touch this code, the pattern is
not always followed correctly.
So now I sit with these disadvantages:
- a relatively slow test suite for my formbuilder, since every detail
of any shared behavior is verified over and over
- some mysterious instance variables i need to maintain across all
example groups in order to keep shared groups testing the right
methods
- strange and unintuitive coupling across the test suite
The advantage we’re getting, of course, is that behavior is properly
speced with this structure (when we pull it off properly at least).
I started this thread with the thought that I could simplify this by
basically flipping out ‘it_should_behave_like’ with ‘it_should_call’,
but you folks identified that as slipping down into verifying
implementation rather than behavior.
So the question that I pose to you, list, is this: is there any way to
change the way we’re dealing with this problem to minimize the
disadvantages I mentioned above or is the answer “yup, that’s pretty
much how you would need to set this up”?
Thanks again for all your feedback and attention folks, it has been
incredibly helpful.
Cheers,
Paul