On Mar 18, 2008, at 6:36 AM, Alan Larkin wrote: > David Schmidt wrote: >> Hello fellow RSpec users. >> >> Before you all start warming up your flame throwers please let me >> explain my Subject line. >> >> I've been working over 4 months on a large Rails project with a few >> other developers. Test coverage was spotty at best, though they >> *were* >> RSpec tests. One of the other developers and I had started adding >> more >> tests, mostly controller tests using the methodology given at >> rspec.info >> for writing controller tests isolated from the model and view layers >> using stubs and mocks. >> >> Recently a new project manager was put in place and he brought in >> another developer. This developer promptly started to re-write all >> the >> *existing* controller (and later view) tests, removing all mocks and >> stubs and replacing them with code to use fixtures. (He also deletes >> many comments he finds in the code if *he* thinks they're obvious, >> but >> that's another story...). His commit messages include comments >> like "Stop mocking around" and "More fixes due to our test mockery". >> >> When challenged on why he's re-writing these tests instead of writing >> new, missing tests (even tests using fixtures) he replied with this >> e-mail with the subject "Why not MockEverything". (Note that I >> *do* use >> fixtures for model tests but follow the RSpec documentation and use >> mocks/stubs for controller and view tests for isolation.) In the >> email >> this developer mentions tests broken by the addition of conditional >> to >> the view. This conditional used a model method not previously used >> in >> the view, and the addition of one stub was sufficient to fix the view >> test in question. >> >> Here is his email to me, less his signature as I don't want to make >> this >> personal. I'd like to see what the RSpec user community has to say in >> response to his comments, below: >> >> --- Why not MockEverything --- >> David I've removed the mocks on purpuse. Not that I have sufficient >> ills >> with them to meddle without a *need*. We committed simple template >> fixes >> adding a conditional and there, yet the tests broke. >> >> Now this was to be expected, the tests were constructed by >> exhaustively >> mocking out all methods called on the object. Add a simple >> conditional >> be it harmless as it is now means another method needs to be mocked >> out. >> >> The MockEverything approach is not healthy, judicious use is >> preferable. >> One thing is to write a short sample in a blog and another is to >> have a >> working app with lots of tests. From all my apps that I have worked >> on >> this has by far the lowest coverage both in profile and in test >> value. >> There is no discussion we are all committed to tests. >> >> To better see what constitutes good practice I recommend you to >> inspect the source of RadiantCMS a beautiful and well engineered app >> recently rewrote to use rspec instead of Test::Unit: >> >> http://dev.radiantcms.org/browser/trunk/radiant/spec >> >> Observe how the code is restrained in mocking, real objects are >> preferred wherever possible. Incidentally they don't use fixtures >> rather >> factories to create *real* objects. Now the factory part is a >> separate >> issue I'll don't discuss here, as it has its own disadvantages >> especially a project with many models ... >> >> With real objects your test will not be brittle, and their value >> will be >> kept even after adjusting the templates or doing other small >> refactorings. >> Contrary to common misconception test speed will not be affected >> either. >> Especially for view tests where you don't even have to save to the db >> upon preparing the test rig. >> >> Beside Radiant there where efforts to rspec Typo and Mephisto (both >> noted rails blog engines). Still these were half harted conversions >> so >> my arguments based on them would not have the same weight. >> RadiantCMS is >> enough - they are used on ruby-lang.org and have converted 100% to >> rspec ... plus they also have good coverage showing that they >> actually >> believe in tests. So please look into Radiant, you'll find it most >> helpful I think. >> --- END OF EMAIL--- >> >> Thank you, >> >> David Schmidt >> >> >> ------------------------------------------------------------------------ >> >> _______________________________________________ >> rspec-users mailing list >> rspec-users@rubyforge.org >> http://rubyforge.org/mailman/listinfo/rspec-users > > I was going to start a thread about mocks and fixtures this morning > too, so Ill > use this one. > > Let me first say that I am a very very recent comer to RSpec so my > opinions dont > carry much weight, but ... > > I have come to the tentative conclusion that mocking is fine in view > specs where > you are really only interested that certain assigns have been made > and that they > respond to certain messages. In fact mocks are ideal. Possibly in > models too. > However in controller specs I think you find examples where fixtures > are just > the best way to go. In these cases, from what I have seen, mocking > leads to > brittle and frankly worthless tests (a half-arsed test is worse than > no test at > all, right?). > > The case that crystalised that opinion for me was a spec for a > destroy action. > In spec::rails scaffold (and in many examples I see online) this > action is > tested by asserting that the instance receives a destroy message. I > personally > think thats inadequate. It makes assumptions about implementation > and doesnt > guard against unwanted side effects. > > I should be able to delete a record any way I please (delete, destroy, > connection().execute(...)) and the test should pass.
However, each of those have different behaviour, and that is what I am spec'ing when I do model.should_receive(:destroy) James Deville > This is BDD after all. We > should be testing the behaviour of the action, not the > implementation, and the > desired behaviour is that the corresponding record *and only that > record* are > deleted ... no one cares how its achieved. The only correct way to > test this > IMHO is to assert that TheModel.find(:all) before the action is > equal to > TheModel.find(:all) after the action less the record in question. > For this I see > fixtures as the way to go. > > Just my opinion. Commence flaming. > _______________________________________________ > rspec-users mailing list > rspec-users@rubyforge.org > http://rubyforge.org/mailman/listinfo/rspec-users _______________________________________________ rspec-users mailing list rspec-users@rubyforge.org http://rubyforge.org/mailman/listinfo/rspec-users
