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