[rspec-users] how to spec a recursive method that creates db records?

David Chelimsky dchelimsky at gmail.com
Tue Jul 21 09:24:30 EDT 2009


On Tue, Jul 21, 2009 at 8:15 AM, Barun Singh<barunio at 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 <dchelimsky at gmail.com>
> wrote:
>>
>> Hi Barun,
>>
>> On Tue, Jul 21, 2009 at 1:05 AM, Barun Singh<barunio at 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 <sfeley at gmail.com> wrote:
>> >>
>> >> On Mon, Jul 20, 2009 at 8:33 PM, Barun Singh<barunio at 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 (sfeley at gmail.com)
>> >>   ESCAPE POD - The Science Fiction Podcast Magazine
>> >>   http://www.escapepod.org
>> >> _______________________________________________
>> >> rspec-users mailing list
>> >> rspec-users at rubyforge.org
>> >> http://rubyforge.org/mailman/listinfo/rspec-users
>> >
>> >
>> > _______________________________________________
>> > rspec-users mailing list
>> > rspec-users at rubyforge.org
>> > http://rubyforge.org/mailman/listinfo/rspec-users
>> >
>> _______________________________________________
>> rspec-users mailing list
>> rspec-users at rubyforge.org
>> http://rubyforge.org/mailman/listinfo/rspec-users
>
>
> _______________________________________________
> rspec-users mailing list
> rspec-users at rubyforge.org
> http://rubyforge.org/mailman/listinfo/rspec-users
>


More information about the rspec-users mailing list