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.
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
