rspec users - Jul 2009 - how to spec a recursive method that creates db records?

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

On Tue, Jul 21, 2009 at 2:05 AM, Barun Singh<barunio at gmail.com>
wrote:
> 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).
Why would they have to be public?

> 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?
My answer for that remains the same: spec it from the outside.  Treat
it as a black box.  Set up an initial state, then write specs that
express the expectation that inputs A, B, and C should yield output D.
 (Where D might be a database record, or a returned value, or both, or
whatever.)  The number of specs you write in this manner will depend
on the number of interesting ways that your method could go right or
wrong.  You may need to provide multiple initial states as well, but
that''s easy in RSpec with nested ''describe'' or
''context'' blocks.

That the method calls ''add_something'' and
''find_something'' on the
inside is immaterial.  You don''t have to isolate everything from
everything else.  If we stubbed every method call inside every method
we tested, we''d soon get to the point where we were stubbing
''+'' or
''gsub'' or ''print.''

Your specs shouldn''t have to care how
''find_and_add_something'' works.
What you are expressing is that it DOES work.  Test the inputs and
outputs, and let it call whatever it wants to get from one to the
other.



-- 
Have Fun,
   Steve Eley (sfeley at gmail.com)
   ESCAPE POD - The Science Fiction Podcast Magazine
   http://www.escapepod.org

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
>

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
>

On Tue, Jul 21, 2009 at 3:18 AM, Barun Singh<barunio at gmail.com>
wrote:
> I get your point, and I already understood it well before my original
> email.? We all know that generic advice isn''t always applicable in
every
> instance, however, and this is a case where the number of distinct specs
> required to test all input combinations that are of interest is simply too
> large to make it worth doing if I don''t use stubs at all.
That''s the kind of problem I like to avoid.  My experience is that it
can _usually_ be avoided by reexamining something up the chain.
Sometimes the method can be turned into a class, or the data model can
be restructured, or the business rules that make it overly complex
aren''t as inflexible as I first thought they were.  Not always --
sometimes we really do have to work with what we''re given, and what
we''re given is a mess.  That may be your case, but you didn''t
give
enough information for me _not_ to raise "Look at the big picture" as
a possibility.

If that''s really the case, though, then the kind of testing you need
to do on this algorithm changes.  "A bunch of pieces with complex
states need to work together" feels more like an integration test to
me than a unit test.  And I don''t understand how stubbing helps you
get better integration test coverage.  Stubbing means locking down the
state of something.  If you stub something internal to your code,
you''re no longer testing cases for states your stub doesn''t
return.
If those cases matter during ''black box'' testing, but you
don''t hit
them when you abstract something out, you''ve just created a gap.  (And
if you *do* hit them despite abstracting out, then why do you need the
code that got stubbed?)


> It''s obviously hyperbolic and a bit
> silly to suggest that stubbing out a public method from one of my models in
> order to simplify a spec would lead to stubbing "every method call
inside
> every method".? You might as well argue that a person should never
stub
> anything in any spec.
You''re right that I was hyperbolic.  But I treat it as a red flag if I
feel a need to stub code I''ve already written in the same class or
module as what I''m testing.  I usually stub for two reasons:

1.) As a design placeholder for stuff I haven''t written yet (and I
take those stubs out after I''ve written them); or
2.) As a placeholder for *external* interface points to services,
libraries, or functional spheres that I need to couple with but don''t
want to get highly entangled with.  Stubbing improves speed for slow
stuff, but it also compels me to keep my interactions simple.
(Examples: stubbing out results from a SOAP service, or stubbing out
"current_user" on controller specs.)

I do a lot more mocking, but usually just to confirm that particular
side effects were triggered.  (And again: I take having to do it as a
cue to at least consider whether I can write the code in a way that
minimizes side effects.)

I''m not trying to be an ass about this.  I think this is a good
conversation.  You know your problem domain better than I do, and
you''re clearly not a novice at this stuff.  But telling me
"Thanks,
but you''re wrong" when you didn''t say a lot about the
problem domain
makes it hard to be helpful.

> My statement of the refactored methods being public was predicated on the
> assumption that I would be stubbing them out (I don''t stub private
methods),
> since there''s no other useful reason to refactor in this instance
(the code
> isn''t hard to read, I don''t need to reuse any of it
elsewhere, and if I
> don''t stub them out in testing the outer method then refactoring
hasn''t made
> any tests any easier either).
But a useful reason to refactor *did* come up.  Your subject line for
this thread was about how to spec a recursive method.  You''ve had
several demonstrations of ways to make spec''ing easier by not
recursing.  Your second block of code, with "add_something" and
"find_something," is more readable *and* more testable, whether or not
you stub.

That''s a win, right?

-- 
Have Fun,
   Steve Eley (sfeley at gmail.com)
   ESCAPE POD - The Science Fiction Podcast Magazine
   http://www.escapepod.org