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

Ben Mabey ben at benmabey.com
Wed Apr 2 23:11:35 EDT 2008


Thanks Pat, that is a very good argument and I totally agree with it.  
When I began down the BDD path I found myself wanting/adding specs like 
the ones that we are talking about.
I'm realizing now that a better way to approach associations is like you 
suggested write some spec that tests the actual behaviour of those 
associations.  You will then see red until the correct association is 
set up and they are exercised. 

-Ben


Zach Dennis wrote:
> Pat,
>
> That was a wonderfully thought out reply and at first read your
> reasoning makes a lot of sense. Thank you for taking the time to write
> it. 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
>>
>>     
>
>
>
>   



More information about the rspec-devel mailing list