I spy

There has been discussion lately about moving away from code generated mocks towards just writing simple, easy to read test doubles. The main motivation behind this is that our tests are difficult to read and understand. Often times there will be massive blocks of code in our tests just to setup the mocks in a certain way. Tricks like closing a channel to get the mock to return the zero value or using reflect.Select to pull a single value off of multiple mocks are hostile to newcomers. Would we expect an open source contributor to understand these tricks just to write a test? Are we just trying to be clever and sacrificing test readability?

We have experimented with a few different ideas of how to implement these spies. As we discover good ideas in writing spies my hope is to use this wiki page to document them.

A Toy Example

Say you have a function that receives an interface. Let's drive out the implementation of both the code and a test double. Here is our starting point:

package something

// this is the interface we will need a test double for
type ReadReporter interface {
    Read(p []byte) (n int, err error)
    Report(err error)
}

// and the code we need to drive out
func Do(r ReadReporterer) {
}

Start by just passing in nil for the interface.

package something_test

import "something"

func TestItDoesSomething(t *testing.T) {
    something.Do(nil)
}

Well that builds but doesn't help us much. We need to go red. Let's create a basic spy that implements the interface but will panic if any of its methods are called:

type SpyReadReporter struct {
    something.ReadReporter
}

func TestItDoesSomething(t *testing.T) {
    spy := SpyReadReporter{}
    something.Do(spy)
}

This is our basic spy. Simple, right!

Embedding the interface into the struct allows our test double to comply with the interface but not implement any of the methods. If methods are not being called by the code under test they should not be implemented on the double. The requirements of the tests should drive out the capabilities of your spy not the other way around.

Why make the spy uppercase? Well, this is subjective but I think it reads better. If we name a type say spyReader it might collide with instances of that type that might also be called spyReader. That is confusing. Besides, test packages can not export anything so why not?

Our basic spy will panic as Read is not implemented on the Spy, lets do that:

type SpyReadReporter struct {
    something.ReadReporter
}

func (*SpyReader) Read([]byte) (int, error) {
    return 0, nil
}

We no longer panic but have no way to assert the read method was called:

type SpyReadReporter struct {
    something.ReadReporter
    called bool
}

func (s *SpyReader) Read([]byte) (int, error) {
    s.called = true
    return 0, nil
}

We can now assert against the method being called:

func TestItDoesSomething(t *testing.T) {
    spy := SpyReadReporter{}
    something.Do(spy)
    if !spy.called {
        t.Fatal("spy's read method was not called")
    }
}

This gives us our first failing test. Let's get green:

func Do(r ReadReporterer) {
    r.Read(nil)
}

We should force the code under test to pass in a valid buffer:

type SpyReadReporter struct {
    something.ReadReporter
    called bool
    calledWith []byte
}

func (s *SpyReader) Read(p []byte) (int, error) {
    s.called = true
    s.calledWith = p
    return 0, nil
}

func TestItDoesSomething(t *testing.T) {
    spy := SpyReadReporter{}
    something.Do(spy)
    if !spy.called {
        t.Fatal("spy's read method was not called")
    }
    if spy.calledWith == nil {
        t.Fatal("spy's read method was called with invalid buffer")
    }
}

func Do(r ReadReporterer) {
    p := make([]byte, 256)
    r.Read(p)
}

And force out error handling:

type SpyReadReporter struct {
    something.ReadReporter
    called bool
    calledWith []byte
    err error
    reportedErr
}

func (s *SpyReader) Read(p []byte) (int, error) {
    s.called = true
    s.calledWith = p
    return 0, s.err
}

func (s *SpyReader) Report(err error) {
    s.reportedErr = err
}

func TestItReportsErrors(t *testing.T) {
    expectedErr := errors.New("test-error")
    spy := SpyReadReporter{
        err: expectedErr,
    }
    something.Do(spy)
    if spy.reportedErr != expectedErr {
        t.Fatal("reportedErr != expectedErr")
    }
}

func Do(r ReadReporterer) {
    p := make([]byte, 256)
    _, err := r.Read(p)
    if err != nil {
        r.Report(err)
    }
}

You might have noticed I did not add any synchronization to the spy. If you are spawning goroutines you should add synchronization, but only where it is required. You should discover this by having a failing test with go test -race. Don't just slap mutexes around everything. This just adds bloat to the spy and signals to the reader that these mutexes are required.

If you need to add a method on the spy that only tests will call it should be lowercase to indicate that it isn't called by the code under test.

Another thing to note, if the spy is primarily implementing a single method it seems overly obsessive to prefix everything with the method name. Something like spyReader.called reads better to me than spyReader.readCalled.