I totally agree. This sort of comment/guidance/opinion should be findable
from (or included on) the rspec.info page on models..
Pat - thanks a bunch for taking the time!
Tim.
On 03/04/2008, David Chelimsky <dchelimsky at gmail.com> wrote:
>> On Wed, Apr 2, 2008 at 10:40 PM, Zach Dennis <zach.dennis at gmail.com>
> wrote:
> > Pat,
> >
> > That was a wonderfully thought out reply and at first read your
> > reasoning makes a lot of sense.
>>> +1
>>> > Thank you for taking the time to write
> > it.
>>> +1 more
>> Cheers,
>> David
>>> > I am going to let it sink in over the next few days,
> >
> > Zach
> >
> >
> >
> >
> > On Wed, Apr 2, 2008 at 8:11 PM, Pat Maddox <pergesu at gmail.com> wrote:
> > >
> > > On Tue, Apr 1, 2008 at 4:20 PM, Zach Dennis <zach.dennis at gmail.com>
> wrote:
> > > >
> > > > On Tue, Apr 1, 2008 at 6:38 PM, Pat Maddox <pergesu at gmail.com>
> wrote:
> > > > > On Tue, Apr 1, 2008 at 3:09 PM, David Chelimsky <
>dchelimsky at gmail.com> wrote:
> > > > > > > Example:
> > > > > > >
> > > > > > > describe SomeModel do
> > > > > > > it_has_many :widgets, :destroy => :null, :class_name =>
> "BaseWidget"
> > > > > > > it_has_one :fuzzbucket
> > > > > > > it_belongs_to :another_model
> > > > > > > end
> > > > > >
> > > > > > I see more and more structures appearing like this. I have
> very mixed
> > > > > > feelings about them. This is about structure, not
> behaviour. Even if
> > > > > > the underlying code is actually approaching this in a more
> behavioural
> > > > > > way, it's still expressing structure at the high level.
> > > > >
> > > > > I don't have mixed feelings about this. I think this type of
> spec is
> > > > > terrible. It completely duplicates the implementation. It's
> not even
> > > > > testing anything.
> > > > >
> > > > > This is not a value judgment against you though, Zach. I
> think when
> > > > > people do stuff like this they genuinely have good
> intentions. It's
> > > > > just that it seems to be quite difficult to test highly
> declarative
> > > > > stuff like AR associations.
> > > > >
> > > > > Now that I've given my rather harsh opinion, I have to get
> back to
> > > > > work :) I'll try follow up later with something more helpful
> like
> > > > > thoughts on how to write better specs.
> > > > >
> > > >
> > > > I don't like the fact that it tests the structure of the
> association
> > > > (as opposed to testing the behavior), but I do like that it tests
> the
> > > > conceptual relationship between models. I find value in this.
> Even
> > > > though it is declarative it is very clear and meaningful to the
> next
> > > > guy looking at the code, and if someone changes something
> incidentally
> > > > they are quickly pointed to the fact that they broke a conceptual
> > > > relationship between two models.
> > > >
> > > > Please do respond with more thoughts, as this is a topic I'd like
> to
> > > > get hammered out as it will provide value to every developer on
> this
> > > > list,
> > >
> > > I've put some more thought into this and have a bit of time to
> reply.
> > > So here goes.
> > >
> > > The first thing I'm going to do is demonstrate why I feel this is a
> > > bad spec. Please understand that this is not a criticism of anyone
> in
> > > particular. I'm merely using this as an example of specs that don't
> > > add any value.
> > >
> > > Take a look at the spec again:
> > >
> > >
> > > describe SomeModel do
> > > it_has_many :widgets, :destroy => :null, :class_name =>
> "BaseWidget"
> > > it_has_one :fuzzbucket
> > > it_belongs_to :another_model
> > > end
> > >
> > > 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.
> > >
> > > The concrete benefits of object-level specification are, in my mind,
> that it
> > > - helps you design your code well
> > > - leaves behind regression tests
> > > - provides executable examples of how to use code
> > >
> > > Often when we write specs we have to balance those goals...for
> > > example, one major criticism of using mocks is that tests that use
> > > mocks don't act as effective regression tests. That's a valid
> > > criticism in certain contexts, but you'll find that most people who
> > > make such criticisms are being myopic - they either don't understand
> > > or don't share the other goals, and so write the technique off all
> > > together.
> > >
> > > Besides the fact that the given association helpers duplicate the
> > > implementation to an i-t-underscore, what else is wrong with
> them? I
> > > would argue that they provide 0 value in all three categories.
> > >
> > > They don't help drive the design. You either need widgets or you
> > > don't. If you decide you do, you add a declaration to the
> > > implementation. Done. There was never any question about how to
> > > design it. Rails made that decision for you.
> > >
> > > They have no value as regression tests. How likely is it that any
> of
> > > that code will break? Not likely at all. It's not like anyone's
> ever
> > > going to go in there and change the behavior, because there is no
> > > behavior, other than that which is abstracted away by Rails (thus
> > > already thoroughly tested). If you make any change to the
> > > implementation then the specs will fail...so they're brittle without
> > > providing any value.
> > >
> > > The lack of documentation value should be obvious. The specs don't
> > > show you how to use the objects together. You have to know how
> Rails
> > > associations work. And you get absolutely no information from the
> > > spec that you don't get from the implementation itself.
> > >
> > > Hopefully that clarifies why I don't feel that specs like these are
> > > valuable. I also hope that this helps others develop heuristics for
> > > when to write/delete/ignore tests.
> > >
> > > With that out of the way, how would I specify this SomeModel class?
> > > Well, if the class is as given and there's no business logic, then I
> > > would only write a couple specs for SomeModel directly. These would
> > > be specs for the widgets association.
> > >
> > > describe SomeModel, "widgets" do
> > > before(:each) do
> > > @model = SomeModel.create!
> > > BaseWidget.create! :some_model_id => @model.id
> > > end
> > >
> > > it "should find BaseWidgets" do
> > > @model.should have(1).widget
> > > end
> > >
> > > it "should nullify keys when deleting the widgets" do
> > > lambda { @model.widgets.destroy_all }.should_not
> change(BaseWidget, :count)
> > > @model.should have(0).widgets
> > > end
> > > end
> > >
> > > The reason I would do this is because there's a greater chance that
> > > some of this stuff could break. None of the other associations
> will.
> > >
> > > Looking back at this, I'm not sure that I would write the second
> > > example. The reason I might not write it is that I don't think I
> have
> > > enough information. The foreign keys get nulled out when I call
> > > #destroy_all, but that's something that I know from the has_many
> > > declaration, and that I know works. My real question at this point
> is
> > > WHY I want the keys nulled out instead of just deleting the records.
> > > Do I have some requirement in the system that these associations
> > > should be broken, but the child records should stick around? If so,
> I
> > > should have another spec that demonstrates that behavior.
> > >
> > > These associations *should* be tested, but they should be tested
> > > indirectly from _somewhere else_. They're not important enough to
> > > deserve tests at such a low granularity. They should be tested via
> an
> > > acceptance test where a view iterates through them, or some test
> which
> > > calls a DB aggregate method on the proxy. They don't have any
> > > interesting behavior on their own, and we are concerned primarily
> with
> > > behavior.
> > >
> > > Does that help?
> > >
> > > Pat
> > >
> >
> >
> >
> >
> > --
> > Zach Dennis
> > http://www.continuousthinking.com> > _______________________________________________
> >
> >
> > rspec-devel mailing list
> > rspec-devel at rubyforge.org> > http://rubyforge.org/mailman/listinfo/rspec-devel> >
> _______________________________________________
> rspec-devel mailing list
>rspec-devel at rubyforge.org>http://rubyforge.org/mailman/listinfo/rspec-devel>-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://rubyforge.org/pipermail/rspec-devel/attachments/20080403/4e695e25/attachment-0001.html