[rspec-devel] rspec_on_rails, speccing models, adding it helpers...

[David Chelimsky]

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
>