Home · Blog · Talks · Projects

Mocha Test Spies

Introduction

Today I finally got round to looking at Joe Ferris’ Test Spies fork of Mocha. Joe explained how Test Spies work and why he thinks they are useful in Spy vs spy on the ThoughtBot blog. I apologise to Joe for not getting round to this sooner.

I have a number of hesitations about merging these changes into Mocha as they stand. My thinking hasn’t really crystallized, but in these situations I often find it’s useful to force myself to try and explain myself in writing. Hopefully it’ll also generate some feedback.

I’d like to thank Joe for taking the time to fork Mocha and add this functionality. I’m particularly grateful that he’s taken the trouble to write a bunch of high-level acceptance tests.

Concept

In this section I’ve tried to write up some comments on Joe’s blog post. I’ve tried to break it down into roughly the same headings that Joe used :-

Why would I let spies in my code?

Joe correctly points out that setting up expectations before exercising the object under test is a bit of a departure from the traditional Four-Phase Test regime. I can see how some people might see this as counter-intuitive, but it doesn’t seem particularly so to me. Also the idea of Expected Behaviour Specification (i.e. setting up expectations before the Exercise phase) has a couple of advantages :-

Sharing stubs in a context

describe PostsController, "on GET show" do
  before(:each) do
    @post = stub('a post', :to_param => '1')
    Post.stubs(:find => @post)

    get :show, :id => @post.to_param
  end

  it { should render_template(:show) }

  it "should find and assign the given post" do
    Post.should have_received(:find).with(@post.to_param)
    should assign_to(:post).with(@post)
  end
end

I’m going to struggle a bit when commenting on this code example for a couple of reasons :-

  1. stubbing Post.find is setting up the environment and making the instance of Post available
  2. the assertion then checks that the instance of Post is assigned to the @post instance variable
  3. the combination of (1) and (2) is enough to tell me that the Post.find method has been called – where else would the controller have got the instance of Post from?

So I’m not convinced I would need Test Spies for this example.

Sharing stubs between tests

module PostHelpers
  def stub_post(post_attrs = {})
    post_attrs = {
      :to_param => '1',
      :published? => true,
      :title => 'a title'
    }.update(post_attrs)
    stub('a post', post_attrs)
  end
  def stub_post!(post_attrs = {})
    returning stub_post do |post|
      Post.stubs(:find => post)
    end
  end
end
describe PostsController do
  include PostHelpers
  it "should show a published post on GET show" do
    post = stub_post!
    get :show, :id => post.to_param
    Post.should have_received(:find).with(post.to_param)
    post.should have_received(:published?)
    should render_template(:show)
    should assign_to(:post).with(post)
  end
end
describe "/posts/show" do
  include PostHelpers
  it "should display a post" do
    assigns[:post] = stub_post
    render '/posts/show'
    template.should have_tag('h1', assigns[:post].title)
  end
end

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?

Steve Freeman makes a similar point in Test Smell: Everything is mocked.

Reusable matchers and assertions

# post.class.should have_received(:find).with(post.to_param)
should find(post)
# Post.should have_received(:new).with(post_attrs)
should build(Post, post_attrs)
# Post.should have_received(:new).with(post_attrs)
# post.should have_received(:save)
should build_and_save(Post, post_attrs)

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.

Syntax

I think the suggested syntax extension is inconsistent with the existing Mocha API and a little clunky. See James Rosen’s comment on the mailing list. Looking at the following examples given in the documentation :-

assert_received(mock, :to_s)
assert_received(Radio, :new) {|expect| expect.with(1041) }
assert_received(radio, :volume) {|expect| expect.with(11).twice }

I think I’d prefer to see something like :-

assert_that mock.received(:to_s)
assert_that Radio.received(:new).with(1041)
assert_that radio.received(:volume).with(11).twice

Or something like :-

mock.assert_received(:to_s)
Radio.assert_received(:new).with(1041)
radio.assert_received(:volume).with(11).twice

Note that I haven’t completely thought through the implementation implications of these suggestions i.e. whether they are even possible!

Implementation

The code looks good and, since Joe has written acceptance tests, there shouldn’t be too much difficulty in doing a bit of refactoring. I like the idea of introducing a model for an Invocation , but I think the Invocation instances should be stored by the relevant Mock instance rather than the Mockery. Similarly I’d push more of the logic in the HaveReceived class into relevant objects. And I’d like to avoid having to make Expectation#invocation_count writeable.

One slight concern is what happens if someone mixes up Expected Behaviour Specification expectations with Test Spies stubs and assertions? I’d probably at least want some acceptance tests to convince me that there are no complications on this front.

Conclusion

I’m sure that some people find the changes useful, but I’m not yet convinced I want to merge the Test Spies changes into Mocha. Please feel free to try and convince me, either here in the comments or in the mailing list thread.

Contact · History · Colophon · Links