Test Spies
floehopper
Joe Ferris from Thoughtbot sent me a pull request on GitHub a while
ago, but unfortunately I haven't had much spare time to look at his
changes. From my brief look it seems he has implemented an alternative
way of asserting that a stubbed method has been invoked. The
difference as I understand it is that instead of using "expects", you
use "stubs" and then later call "assert_received". I think this is
along similar lines to Mockito [2].
I'd be interested in what people think of this addition to the API. Is
it useful? Is it more useful than the existing API? I'm quite keen to
keep Mocha as lean as possible and avoid a plethora of alternative
ways of doing things, but I'm always open to suggestions. I wonder if
there is a case for having a Mocha-core with alternative external
APIs.
Thanks, James.
----
http://blog.floehopper.org/
[1] http://github.com/jferris/mocha/
[2] http://mockito.org/
Ken Collins
I'd be interested in knowing more about why go thru the extra steps.
Typically when I am black white box testing internal implementation
chains I just go for the quick and simple expects. However, if I do
think about it, perhaps the stubbing of a method could be done in a
setup and while most tests focus on that return value, one assertion
later on might just be seeing if that stub was actually called. So I
can see the uses, but I'm not seeing why quite yet.
Heck, it can not hurt to have the alternate. But I'd have little
feedback on the total abstraction of a core with external alternate
external APIs.
- Ken
> --~--~---------~--~----~------------~-------~--~----~
> You received this message because you are subscribed to the Google
> Groups "mocha-developer" group.
> To post to this group, send email to mocha-d...@googlegroups.com
> To unsubscribe from this group, send email to mocha-develop...@googlegroups.com
> For more options, visit this group at http://groups.google.com/group/mocha-developer?hl=en
> -~----------~----~----~----~------~----~------~--~---
>
Elliot Winkler
obj.stub_methods(:foo => "some return value") # you can also say obj.track_methods(:foo) if you don't want to stub, although I rarely use that
obj.main_method
obj.should have_received(:foo)
If I'm writing a test for a method, and that method calls another and also returns something, I'll often put the method expectation and the value expectation in the same test simply because it's easier, and that's where the consistency really comes in:
obj.stub_methods(:foo => "some return value")
value = obj.main_method
obj.should have_received(:foo)
value.should == "whatever"
At first, I thought having to stub the method beforehand was annoying, but after a while I just got used to it. As for stubbing multiple methods at once beforehand in a setup block, I almost never do this as it's more confusing to people that read the test, IMO. If I end up doing it, though, I'll put the setup block within the same "describe" block as the tests that make the method expectations ("describe" is used to group tests in RSpec; it's similar to an inner class in Test::Unit).
I can understand the concern of adding an alternate syntax to Mocha. One could say obj.expects(:foo) is "just how Mocha does it", no need to confuse people. However, one could also make the argument that RR[3] provides mocks, stubs, test spies, you name it, and that doesn't stop people from using that framework. Granted, RR and Mocha both stem from different ideas about testing (which is why I don't use RR). But I think test spies still fits within Mocha's identity.
Anyway, I thought I'd pitch in my two cents seeing as this interests me. I wouldn't be opposed to Mocha not supporting this syntax directly, as long as Joe Ferris kept his fork up to date...
-- Elliot
[1]: http://blog.jayfields.com/2008/11/ubiquitous-assertion-syntax.html
[2]: http://notahat.com/not_a_mock/
[3]: http://github.com/btakita/rr
James Mead
Joe just wrote a blog post about test spies:
http://giantrobots.thoughtbot.com/2009/8/6/spy-vs-spy
We've been using them with great success and enjoyment at thoughtbot
for months. We'd love to see spies make it into mocha so we're not
working off a non-standard branch.
I'd also add that we've found test spies to be a good way to teach
Rails students mocking and stubbing in our training classes.
We harp on the Four-Phase Test (setup, exercise, verification,
teardown) throughout the classes, so when it comes to mocha, it's a
more natural mental transition to use assert_received like a normal
verification. The necessary stubbing in the setup is still confusing
for students.
+1 test spies in mocha!
James
I use Joe's fork; it works quite well.
I would prefer assert_received(object, :method).never
over assert_received(object, :method) { |expects| expects.never }
but I'm sure that would take _lots_ of re-architecting. It's perfectly
usable as-is.
-James
On Aug 7, 10:44 am, James Mead <jamesmea...@gmail.com> wrote:
> 2009/5/11 Elliot Winkler <elliot.wink...@gmail.com>
bcardarella
We use this at Zendesk, never had a problem
Michał Szajbe
Joe's approach to test spies is more intuitive, however there are some
problems using his fork with original mocha also installed.
Rob Sanheim
James Mead
Cheers, James.
Rob Sanheim
I'll try and take another look at this in the next few days.
Cheers, James.
James Mead
> Hey James
> Any updates on this? We would love to see a new release of mocha with test
> spies in the mainline.
> Thanks,
> Rob
Hi Rob,
I wrote up my thoughts about Test Spies in a blog article [1]. I
thought I'd posted a link to that article on this list, but it looks
like I didn't - sorry. Anyway, the upshot is that I need more
convincing to add Test Spies into Mocha. Do you have some compelling
examples we could see?
Cheers, James.
[1] http://blog.floehopper.org/articles/2009/09/14/mocha-test-spies
Joe Ferris
Thanks for the detailed response on test spies. I can understand your
hesitation to introduce such a fundamentally different approach, or to
support the extra code. I'd like to respond to a few of your points.
"Using Test Spies, you have to (a) set up a stub so the mock object
knows what value to return and (b) assert that the stubbed method was
invoked the requisite number of times with the appropriate parameters.
Whereas using Expected Behaviour Specification, these are combined
into setting up an expectation on the mock object. I think this
results in terser code with less duplication."
This is true, except that if you want to reuse either the stub or the
expectation, you must reuse both. I find that this results in testing
the same thing over and over, and it makes it harder to produce clear
test failures.
"Using Expected Behaviour Specification means that the test can fail
fast if an unexpected invocation is made against a mock object.
Failing fast is generally a good idea, because you are more likely to
be able to identify the root cause of the problem."
Fast-failing also has the side-effect of taking over the control of
failure order. Sometimes the first assertion in the test would produce
a clear failure, but it's difficult to tell why a mock was called with
an unexpected value. I believe there are use cases for both.
"I dislike the use of setup methods (before in this RSpec example). I
prefer to do my setup in-line in the test to make the test as
independent and as explicit as possible. I particularly dislike
exercising the object under test within the setup method. To me this
seems like a more fundamental breakage of the Four-Phase Test
concept."
Lots of people feel this way, and I tend to use "flatter" tests as
well. However, this approach is popular in the Ruby testing community
and test spies have the same benefits whether you use setup methods or
not.
"In this example, I would not want to assert that Post.find has been
called. Nat Pryce (one of the authors of jMock) has a rule of thumb
that says: “stub queries and expect commands”. In this example, the
call to Post.find is clearly a query, so I would just stub it."
I agree with the general rule of thumb, but that only works out for
extremely simple interfaces. Post.find must be called with a specific
value in order for the behavior to be at all correct - Post.find(:all)
or Post.find('other_id') would be a total failure, so only stubbing it
out in this case is inadequate.
"Some languages might allow you to stub a value, but what’s the point?
It is easier and less error prone to use the real type. If it’s not
easier to use the real type then, I think, you should investigate why.
What can you do to make the value type easier to use?"
In many cases using an actual object is much easier. However, test
spies also work with Mocha's partial stubs, and those are invaluable
in a world like Rails where many important methods are on classes, and
most classes carry a decent amount of behavior. I find that being able
to stub a method like User#approved_comments_count is invaluable -
otherwise, you'd need to construct an entire object graph. Although
this can be done with a library like factory_girl, it's time-consuming
and requires updating a number of factories when the top-level object
changes. Changing a stub just requires changing one factory.
"I’m not convinced that extracting the assertion part of an
expectation is sensible, because it’ll always need to be used in
conjunction with the stub part of the same expectation."
I don't think I can convince you philosophically that they're
reusable, but I can say that this has worked for us in practice.
Regarding syntax: I based the syntax from the similar assertions and
matchers found in RR, but accepted the method name as an argument
instead of using method_missing, which didn't seem to be the "mocha
way." RR's method_missing also caused some problems for us (for
example, #attributes= in Ruby will always return the argument passed
to it, so you can't use it in a stub chain). I don't think the syntax
will be difficult to change if you dislike it, but there are a number
of existing projects that use the syntax as-is, and I think it will be
difficult to find a much-improved syntax that's compatible with the
general Test::Unit assertion style.
Regarding implementation: I don't disagree with any of those points,
and I'd be happy to help out refactoring those parts if you're
interested in merging them.
Again, thanks for taking the time to look at these changes.
-Joe
On Oct 21, 5:36 am, James Mead <jamesmea...@gmail.com> wrote:
> 2009/10/21 Rob Sanheim <rsanh...@gmail.com>:
James Mead
Thanks for your response. I wanted to let you know I have seen it, but
I'm not sure when I'll have time to look at it in the detail that I
want. I do however have one quick comment regarding this paragraph :-
2009/10/21 Joe Ferris <joe.r....@gmail.com>:
> I agree with the general rule of thumb, but that only works out for
> extremely simple interfaces. Post.find must be called with a specific
> value in order for the behavior to be at all correct - Post.find(:all)
> or Post.find('other_id') would be a total failure, so only stubbing it
> out in this case is inadequate.
Are you aware that you can restrict a stubbed method using a parameter
matcher in the same way as you can restrict an expected method? In the
case you describe above, I would prefer to use
Post.stubs(:find).with('correct_id').returns(post) and then (if deemed
important) assert later in the test that the correct instance of Post
has been found. If Post.find is called with the wrong parameter (e.g.
'other_id'), the stubbed method will return nil and the later
assertion will fail. Since the call to Post.find does not change the
state of an object, I think it's overkill to "expect" the call and
leads to a brittle test and potentially confusing failures. What do
you think?
As always I think discussions like this are better when focussed on
concrete examples. Do you have any more examples of tests that are
improved by the use of Test Spies?
Cheers, James.
Ken Collins
Post.find method has been called – where else would the controller have got the instance of Post from?"
Curran Schiefelbein
Thanks for mentioning the possibility of restricting stubbed methods. It looks like it would solve a problem I've been having with test spies (the only problem, actually) -- when you write User.stubs(:find => @user), it screws with other calls to User.find (such as might be made within current_user.authenticated?) and breaks an otherwise sensible test setup. The reason for stubbing find() is so that the same instance of User is returned within the controller action, and an expectation on a different method (@user.foo) can be tested.
I tried replacing User.stubs(:find => @user) with User.stubs(:find).with(@user.id).returns(@user). The mocha documentation says that Mocha::ObjectMethods::stubs() "Adds an expectation that a method identified by method_name Symbol may be called any number of times with any parameters." I find that restricting the expectation's arguments will set the expectation that it is only called with those arguments. Therefore the find() call made within current_user.authenticated? still causes a test failure (unexpected invocation of User.find with a different user id).
The following code makes the test pass, but it's clunky. You even have to specify that the object id is passed in as a string, for one of the calls.
User.stubs(:find).with(@user.id.to_s).returns(@user)
User.stubs(:find).with(:first, :conditions => {:id => users(:quentin).id}).returns(users(:quentin))
Is it at all possible to word the stubbing in a more permissive way? I'd like the stub to "return this value if that id is passed in; otherwise, behave normally, and don't complain about expected or unexpected invocations". I'm not interested in validating the calls to User.find() at all in this case; the stubs are just there to facilitate the validation of an entirely different method.
Overall I much prefer the test spies approach, mainly due to separating the setup and verification phases of my tests. But this problem with stubbing find() keeps coming up. It almost seems easier to skip the Mocha stubbing and redefine find() on my own terms.
Thanks,
Curran
James Mead
Hi Curran,
Thanks for your questions. I don't claim to have all the answers, but
here are a few ideas :-
One way of relaxing the constraint on the database ID needing to be a
String would be to define your own Mocha parameter matcher for
matching integer equivalents. I've knocked up a quick (untested)
version [1]. This would allow you to do the following :-
User.stubs(:find).with(integer_equivalent(@user.id)).returns(@user)
Regarding the call to User.find made within
current_user.authenticated?, is there any reason this doesn't call
User.find with a database ID too? If you could change it so it did,
you could have the following two stubs :-
User.stubs(:find).with(integer_equivalent(@user.id)).returns(@user)
User.stubs(:find).with(integer_equivalent(users(:quentin).id)).returns(users(:quentin).id)
Another possibility would be to extract a method from
current_user.authenticated? ideally with a method name (better than
mine) expressing why it needs to be different from User.find(id) :-
class User
def self.special_find_by_id(id)
find(:first, :conditions => { :id => id })
end
end
Then you could have the following two stubs :-
User.stubs(:find).with(integer_equivalent(@user.id)).returns(@user)
User.stubs(:special_find_by_id).with(integer_equivalent(users(:quentin).id)).returns(users(:quentin).id)
If you can't change the current_user.authenticated? code, you could
extract the expected parameters of the User.find method into a private
explanatory method within your test (or maybe a module shared amongst
tests):-
def current_user_params_for(user)
[ :first, :conditions => { :id => users(:quentin).id } ]
end
User.stubs(:find).with(*current_user_params_for(users(:quentin))).returns(users(:quentin))
If you want this to be a more relaxed constraint, you could build a
combinatorial parameter matcher along the following lines :-
def current_user_matching(user)
[ all_of(includes(:first), includes(has_entry(:conditions =>
has_entry(:id => user.id)))) ]
end
User.stubs(:find).with(*current_user_matching(users(:quentin))).returns(users(:quentin))
Or you could even build another specialist custom parameter matcher.
I'll leave that as an exercise for the reader ;-)
In general, I think that custom parameter matchers are an under-used
aspect of Mocha. I'd be very open to adding new matchers to Mocha if
they are thought to be generally useful.
Something else to keep in mind is that if it's becoming difficult to
setup all the expectations you need in a test, it might be time to
think about simplifying the design. Extracting the
User.special_find_by_id is one (not very good) example of this.
Another option is that you might be able to stub a method higher up
the call chain (e.g. on the controller) to avoid the call to
current_user.authenticated? altogether. Depending on what you are
trying to test, this may also make your test more expressive and
focussed.
I hope some of the above is of some use. Please note that I haven't
had a chance to test any of the example code, so please take it with a
pinch of salt!
Cheers, James.
Curran Schiefelbein
Another option is that you might be able to stub a method higher up
the call chain (e.g. on the controller) to avoid the call to
current_user.authenticated? altogether. Depending on what you are
trying to test, this may also make your test more expressive and
focussed.
Ah, why didn't I think of that? :) Instead of having the test setup include
login_as :quentin
I could have done
@controller.stubs(:current_user => users(:quentin))
It feels wrong to make the object under test a Mock, though. The :current_user method comes from a mixin, but still...
After some more fooling around, I found that the "general case" stub needn't be so specific if it is constructed before the find-by-id stub:
User.stubs(:find).returns(users(:quentin))
User.stubs(:find).with(@user.id.to_s).returns(@user)
The first stub handles the current_user lookup, regardless of parameters, and the second stub handles the lookup of the user under manipulation. When the stubs were created in reverse order, the extra conditions were necessary (according to the failure message from mocha). I even found one test case where the expectation couldn't be satisfied, even with identical parameters cut and pasted from the failure message. :/ But creating the stubs in the right order fixed it and removed the need for specific parameters.
I'm ok with the status quo now, but I still dream of a stubbed method that invokes super() if the parameters don't match. :)
Thanks for your ideas!
Curran
jenshimmelreich
I want to add one additional aspect to the discussion: the cucumber
integration. In general: the integration in an BDD-Context. At the
Moment I add my expectations in the Given-Section. In the Given-
Section are two types of statements: setups and expectations.
With the new concept 'assert_received' it is possible to put
the expectations in the Then-Section, where they belong.
This is one of the motivations behind the rspec-mock
fork [1]. You will find a detailed discussion here: [2]
ciao
jens
[1] http://notahat.com/not_a_mock/
[2] http://www.ruby-forum.com/topic/165722