Home   About Me   Blog  


Don't use a different interface in tests. (05 May 2022)

This is probably going to be an unpopular opinion, but;
Your test code should not be using a different interface implementation compared to your non test code.
Using different implementations(sometimes called 'mocking') is one of the best ways to lie to yourself that you are testing your code whereas you really aren't.

An example will suffice to drive my point; let's say we have some functionality that is meant to be used in logging:


// logg writes msg to the data stream provided by w
func logg(w io.Writer, msg string) error {
    msg = msg + "\n"
    _, err := w.Write([]byte(msg))
    return err
}
            
And the way we use it in our application(ie non-test code) is;

// httpLogger logs messages to a HTTP logging service
type httpLogger struct{}

// Write fulfills io.Writer interface
func (h httpLogger) Write(p []byte) (n int, err error) {
    tr := &http.Transport{
        MaxIdleConns:       10,
        IdleConnTimeout:    10 * time.Second,
        DisableCompression: true,
    }
    client := &http.Client{
        Transport:     tr,
        Timeout:       10 * time.Second,
    }
    // assume httpbin.org is an actual logging service.
    resp, err := client.Post("https://httpbin.org/post", "application/json", bytes.NewReader(p))
    return int(resp.Request.ContentLength), err
}

func main() {
	err := logg(httpLogger{}, "hey-httpLogger")
	if err != nil {
		log.Fatal(err)
	}
}
            
At this point we would now like to write some test for our application's code. What usually happens in practice is that people will create a 'mock' that fulfills the interface expected by the logg function instead of using the type that their non-test code is already using(ie httpLogger)

func Test_logg(t *testing.T) {
    msg := "hey"
    mockWriter := &bytes.Buffer{}

    err := logg(mockWriter, msg)
    if err != nil {
        t.Fatalf("logg() got error = %v, wantErr = %v", err, nil)
        return
    }

    gotMsg := mockWriter.String()
    wantMsg := msg + "\n"
    if gotMsg != wantMsg {
        t.Fatalf("logg() got = %v, want = %v", gotMsg, wantMsg)
    }
}
            
And it is not just developers that will write code like this. I checked both VS Code & JetBrains GoLand; they both used a &bytes.Buffer{} when I asked them to Generate test for this code.

The problem with this code is; the only thing we have tested is that bytes.Buffer implements the io.Writer interface and that httpLogger also implements the same.
In other words, the only thing that our test/s have done is just duplicate the compile-time checks that Go is already giving us for free. The bulk of our application's implementation(the Write method of httpLogger) is still wholly untested.
A photo of the test coverage as produced by go test -cover illustrates the situation more clearly;

test coverage for mock interfaces

I do understand why we do use alternate interface implementations for our tests. For example, we may not be in control of the HTTP logging service in question & do not want to send them any unwanted/spam traffic from our tests.
But I think we can do better. We can use the same implementation for both test code and non-test code without spamming the logging service.

One (but not the only) way of doing that is by having one implementation that switches conditionally based on whether it is been used in test;

// betterhttpLogger logs messages to a HTTP logging service
type betterhttpLogger struct {
    test struct {
        enabled bool
        written string
    }
}

// Write fulfills io.Writer interface
func (h *betterhttpLogger) Write(p []byte) (n int, err error) {
    tr := &http.Transport{
        MaxIdleConns:       10,
        IdleConnTimeout:    10 * time.Second,
        DisableCompression: true,
    }
    client := &http.Client{
        Transport:     tr,
        Timeout:       10 * time.Second,
    }
    if h.test.enabled {
        mockWriter := &bytes.Buffer{}
        n, err := mockWriter.Write(p)
        h.test.written = mockWriter.String()
        return n, err
    }
    // assume httpbin.org is an actual logging service.
    resp, err := client.Post("https://httpbin.org/post", "application/json", bytes.NewReader(p))
    return int(resp.Request.ContentLength), err
}

func main() {
	err := logg(&betterhttpLogger{}, "hey-httpLogger")
	if err != nil {
		log.Fatal(err)
	}
}
            
And the test code will be,

func Test_logg(t *testing.T) {
    msg := "hey"
    w := &betterhttpLogger{test: struct {
        enabled bool
        written string
    }{enabled: true}}

    err := logg(w, msg)
    if err != nil {
        t.Fatalf("logg() got error = %v, wantErr = %v", err, nil)
        return
    }

    gotMsg := w.test.written
    wantMsg := msg + "\n"
    if gotMsg != wantMsg {
        t.Fatalf("logg() got = %v, want = %v", gotMsg, wantMsg)
    }
}
            
Notice that in this version, the only piece of code that is not tested is just one line;

http.Client.Post("https://httpbin.org/post", "application/json", bytes.NewReader(p))
            
And guess what? Since that line is calling code from the standard library, you can bet that it is one of the most heavily tested functionality out there.
Here's the test coverage of this version:

test coverage for mock interfaces

Of course this example is a bit silly - it is for illustration purposes - because most of the code in the Write method is just setting up a http transport and client, and thus you could make the argument that those are already tested in the standard library. But you can imagine a situation where most of the code inside the Write method is our own custom code; and using a mock interface in tests would mean that our custom code is never tested.

In conclusion, if you have one interface implementation that you use in your 'real' application and another implementation that you use in tests; you are probably doing it wrong.
You should, in my opinion, use just one interface implementation for both application code and test code.

As I said, file this under the unpopular opinion category.

All the code in this blogpost, including the full source code, can be found at: https://github.com/komuw/komu.engineer/tree/master/blogs/10

You can comment on this article by clicking here.

Update(04 July 2022);
One main argument against the ideas in here has been along the lines; "you should not let test code leak into production code."
It is a fair comment. I just wanted to point out that the Go source repo does this(leak test to production) a number of times.
The example of http.Server.Serve(l net.Listener) error is particulary interesting. Here's what it looks like:

var testHookServerServe func(*Server, net.Listener) // used if non-nil
...
func (srv *Server) Serve(l net.Listener) error {
	if fn := testHookServerServe; fn != nil {
		fn(srv, l) // call hook with unwrapped listener
	}
...
            

Here's also a good blogpost that makes a similar kind of argument
"Do not use interfaces at all, just add test hooks to your real-life structs" - @jrockway
And another one by James Shore that expresses the same idea but uses the term, nullables.