Mechanism of operation aborting valid codepath

[ghost]

gomock is jumping out of a normal code execution, causing code that would normally be executed a normal program to be bypassed. This means any clean-up after a failed mock call will have unexpected consequences such as:

• Files not being flushed or closed
• Signals not being sent over channels
• Mutexes being left in incorrect states

The last point is used as an example below. The code being tested is a series of functions that use a global mutex for synchronization.

Code to be tested:

package test

import "sync"
import "io"

var m sync.Mutex

func init() {
    m = sync.Mutex{}
}

func f(a io.Writer) {
    m.Lock()
    a.Write([]byte{})
    m.Unlock()
}

func g(a io.Writer) {
    m.Lock()
    a.Write([]byte{})
    m.Unlock()
}

Test code

package test

import (
    "testing"

    "github.com/golang/mock/gomock"
)

func TestF(t *testing.T) {
    ctrl := gomock.NewController(t)
    defer ctrl.Finish()
    mw := NewMockWriter(ctrl)
    mw.EXPECT().Write([]byte{'a'})
    f(mw)
}

func TestG(t *testing.T) {
    ctrl := gomock.NewController(t)
    defer ctrl.Finish()
    mw := NewMockWriter(ctrl)
    mw.EXPECT().Write([]byte{'b'})
    g(mw)
}

Command to generate mock:

mockgen -package test io Writer

Output

➜  test go test .
--- FAIL: TestF (0.00s)
        controller.go:150: Unexpected call to *test.MockWriter.Write([[]]) at /home/ser/workspace/src/test/test_mock.go:32 because:
                Expected call at /home/ser/workspace/src/test/test_mock.go:39 doesn't match the argument at index 0.
                Got: []
                Want: is equal to [97]
        asm_amd64.s:573: missing call(s) to *test.MockWriter.Write(is equal to [97]) /home/ser/workspace/src/test/test_mock.go:39
        asm_amd64.s:573: aborting test due to missing call(s)
fatal error: all goroutines are asleep - deadlock!
(stack trace)

Expected behavior

The mutex would in practice only be left in a locked state if the mocked functions panic'd, which is not the behavior being tested. This results in incorrect execution of the tests, including in some cases tests hanging, tests being skipped, and an inability to properly execute tests.

If this is known behavior, and gomock is incompatible with some packages or requires special handling when using those functions, it'd be great to have this documented. E.g., a blurb saying that gomock uses panic to manage code flow would help people understand what can and can't be tested using gomock.

[balshetzer]

First, yes, gomock needs better documentation.

Second, gomock uses the passed in TestReporter, which has Errorf and Fatalf.

The most common way to use gomock is to pass in the testing.T for the test, which implements TestReporter. testing.T.Fatalf behaves a bit like panic (I think it actually calls runtime.Goexit). So that's where that comes from.

You could implement your own TestHelper with a Fatalf that doesn't panic and isn't really fatal. Then execution will continue but it's not clear if that will be better because gomock doesn't know what to do for a function call that hasn't been registered.

It's hard to come up with a general solution for this problem. One could easily write code that deadlocks if the tested function returns zero and the real implementation never returns zero. If the function call is not specified for the mock and execution is not aborted then the mock has to return some value, probably zero.

Finally, I'm also not sure if I understand the negative consequences of this issue. In this case the test fails, which is what you would want. I imagine hanging tests would time out and then fail that way. Can you show me a case where the test is skipped? That would be bad. Or maybe elaborate on the negative consequences?

Thanks for the report!

[ghost]

Thanks for the response. To answer your last question(s):

The issue is that, because the gomock uses exceptions for flow control, it bypasses the defer -- and any other clean-up code, such as closing files, mutexes, database connections -- possibly leaving the test suite in an invalid state that might normally never happen IRL.

To make it more clear, change g() to write []byte{"b"} as expected by TestG(). TestG() should, then, pass, except it doesn't because TestF() left the library in an inconsistent state -- a state that would not have been entered even in an error case in application code.

My use of io.Writer was contrived for simplicity. An example could be produced where all of the code under test is guaranteed to never panic, such that it is provable that valid state will be maintained. gomock introduces behavior that would violate this guarantee and, at the very least, causing misleading and confusing results (as in this case, a bad mutex state).

A tester might spend several hours trying to track down the cause of the deadlock, thinking it might be in the code path being tested, when the error was actually introduced by the mocking framework. For example :-)