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;
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:
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" - @jrockwayAnd another one by James Shore that expresses the same idea but uses the term, nullables.