On Tue, Jul 21, 2009 at 8:15 AM, Barun Singh<baru...@gmail.com> wrote: > I completely agree that testability is a huge benefit, and if changing code > structure makes that easier then I'm all for it. My statement was only > meant to imply that I didn't find testability to become any easier by > refactoring this particular method, and absent that I didn't see any reason > to split it up. But if testability is actually improved, I'd be eager to > make changes appropriately. > > I looked over your code in the gist (which, by the way I really appreciate > you taking the time to write out), but I don't see how this tests the method > properly. The result of the else condition is never actually tested. > Specifically, I thing the tests would all pass, even if > find_or_add_something looked like this: > > def self.find_or_add_something(inputs) > if x = self.find_something(inputs) > x > else > self.add_something(inputs) > "blah" > end > end > (if it isn't clear, the difference is that I'm not calling find_something in > the else condition and am instead just returning "blah"). > > This is because the last test "#find_or_add_something when find something > returns nil returns the result of find_something" doesn't actually specify > that find_something returns nil the first time it is called. So that last > spec describes what I want to test but what it actually tests is the same > thing as "#find_or_add_something when find something returns a Thing returns > the Thing" [line 39]. > > At least this is what it seems like to me; if I'm missing something > glaringly obvious (possible) please do point it out to me. > > Hopefully with this concrete example of a set of specs that you've provided > my question is a bit clearer.
I changed the very last example in the gist so that the first time it receives find_something it returns nil, then the 2nd time it returns the thing. Try again: http://gist.github.com/151226 > > > > On Tue, Jul 21, 2009 at 5:49 AM, David Chelimsky <dchelim...@gmail.com> > wrote: >> >> Hi Barun, >> >> On Tue, Jul 21, 2009 at 1:05 AM, Barun Singh<baru...@gmail.com> wrote: >> > I certainly appreciate the thoughtful and lengthy reply, but I think it >> > misses the main part of my problem -- perhaps my original explanation of >> > my >> > issue was lacking in clarity. This isn't a question of refactoring; I >> > can >> > easily refactor the method as you describe but this doesn't resolve the >> > issue (indeed, it just leads to an increased number of public methods in >> > the >> > model with no real benefit). >> >> You want to know your code works, right? Testability _is_ a real >> benefit. Obviously everything needs to be balanced, and if you can do >> things in a pure API, non-invasive way, you're going to end up with a >> more flexible result. But sometimes we have to make tradeoffs. >> >> > So, to more clearly illustrate my question, >> > forget about my original method and instead let's say I have three >> > methods >> > as follows: >> > >> > def self.find_something(inputs) >> > .. do stuff here >> > end >> > >> > def self.add_something(inputs) >> > .. do stuff here >> > end >> > >> > def self.find_or_add_something(inputs) >> > x = self.find_something(inputs) >> > if !x >> > self.add_something(inputs) >> > x = self.find_something(inputs) >> > end >> > x >> > end >> > >> > I can easily spec the find_something and add_something methods so I know >> > that they work correctly. My question is, how do I write a spec for >> > find_or_add_something in a way where I don't have to re-test the >> > behavior of >> > the individual find_something and add_something methods a second time, >> > and >> > can just test the overall logic of that method? >> >> This is _very_ invasive and brittle, and I'd likely not approach it >> this way myself, but it works: >> >> http://gist.github.com/151226 >> >> HTH, >> David >> >> > >> > It's helpful to know that Mocha is available to assist with this, but >> > I'd >> > rather not switch to an entirely different framework for mocks & stubs >> > just >> > because of this one method... >> > >> > >> > >> > On Mon, Jul 20, 2009 at 11:45 PM, Stephen Eley <sfe...@gmail.com> wrote: >> >> >> >> On Mon, Jul 20, 2009 at 8:33 PM, Barun Singh<baru...@gmail.com> wrote: >> >> > >> >> > I want to find a way to write a spec for this method that does both >> >> > of >> >> > these >> >> > things: >> >> > (1) stub out the do_something method (that method has its own specs) >> >> > (2) not stub out the logic in the else statement, (there is some >> >> > complex >> >> > logic in there involving sql queries on multiple tables and i >> >> > explicitly >> >> > want to make it touch the database so that I can examine the state of >> >> > the >> >> > database after the method is run in my spec. a lot of complex stubs >> >> > would >> >> > be required to do this while also having the spec that is actually >> >> > useful) >> >> >> >> Okay, first off -- I think part of your problem is that your focus is >> >> too short. It sounds like you're asking how to write different specs >> >> for various lines of the _internals_ of this method. That's too >> >> fine-grained. Spec the method from the _outside:_ write specs that >> >> say "If I give the method these inputs, this should happen." Then >> >> test that the end result of the method's execution was what you >> >> expect. >> >> >> >> I'm betting that probably sounds really difficult, given the "complex >> >> logic" you're describing. And that would be my second, more important >> >> tip: if your method is so complicated that you can't spec it from the >> >> outside, you should take that as a sign. Spaghetti code is usually >> >> very hard to spec. >> >> >> >> Reconsider your problem. Try breaking it down into smaller, more >> >> easily specified pieces, and then recompose them into the logic that >> >> you need. I don't know your business rules; it looks to me like the >> >> basic task here is "Keep twiddling with the database until I get what >> >> I'm looking for, as many times as that takes." I don't know why >> >> that's necessary, but if it is, recursion doesn't seem like the most >> >> elegant answer to me. Recursive calls are usually made in small >> >> functions that don't have side effects. This is the opposite. >> >> >> >> What would I do instead? Again, without knowing your specific >> >> business, I'd probably break it into pieces, and then turn the >> >> requirement inside out and do it with a loop: >> >> >> >> def self.find_something(inputs) >> >> <find something in db based on inputs> >> >> end >> >> >> >> def self.add_something(inputs) >> >> <add something to db based on inputs> >> >> end >> >> >> >> And then, wherever you *would* have called mymethod(inputs), you can >> >> instead write this, which will loop zero or more times: >> >> >> >> add_something_to_db(inputs) until x = find_something(inputs) >> >> x.do_something >> >> >> >> ...Whether or not you still want to wrap that in a method probably >> >> depends on whether you need to call it more than once. But I'll bet >> >> you that the "add_something" and "find_something" methods are both >> >> easier to spec from the outside. If they're still too complex, then >> >> break them down too. >> >> >> >> >> >> Oh, and final trivia note: >> >> >> >> > any thoughts on how to accomplish this? two approaches that would >> >> > make >> >> > sense but don't seem to be possible as far as i know are: >> >> > (A) stub out the do_something method for any instance of the class >> >> > (i >> >> > can't >> >> > stub it out for a particular record because the records don't exist >> >> > at >> >> > the >> >> > time of calling the method and I'm not stubbing the creation process) >> >> > (B) stub out "mymethod" only for the second time it is called >> >> > (obviously i >> >> > can't stub the method out entirely since that's the method i'm >> >> > testing) >> >> >> >> You can do both of these with the Mocha mocking library, if you really >> >> want to. It puts an "any_instance" method on Class that you can use >> >> to mock or stub; and you can specify ordered chains of results to >> >> return. >> >> >> >> But again, you shouldn't need to if your methods are atomic enough. >> >> If 'do_something' properly belongs outside the scope of what you're >> >> testing, then maybe that's a sign that you shouldn't put it in the >> >> same block of code. And if you find a need to declare that your >> >> method should behave entirely differently on a second invocation, >> >> maybe it shouldn't be one method. >> >> >> >> Simplify relentlessly. Once I figured out how to make my code >> >> simpler, I found that I wasn't mocking nearly as much as I used to. >> >> >> >> >> >> -- >> >> Have Fun, >> >> Steve Eley (sfe...@gmail.com) >> >> ESCAPE POD - The Science Fiction Podcast Magazine >> >> http://www.escapepod.org >> >> _______________________________________________ >> >> 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 >> > >> _______________________________________________ >> 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 > _______________________________________________ rspec-users mailing list rspec-users@rubyforge.org http://rubyforge.org/mailman/listinfo/rspec-users
