Previously passing tests now fail if they have missing expected calls. v1.4.4 -> v1.5.0

[wafer-bw]

Actual behavior

Some tests which pass using github.com/golang/mock v1.4.4 now fail when using github.com/golang/mock v1.5.0. Prior to v1.5.0, missed (uncalled) expected calls to mocked interfaces do not fail tests.
As of v1.5.0 any mock.EXPECT() call that is missed now fails the test with a message like:

... missing call(s) to ...
... aborting test due to missing call(s)

Expected behavior

Tests which are passing using v1.4.4 or earlier should still pass when using v1.5.0.

To Reproduce

1. Ensure you are using gomock v1.4.4 or earlier in your go.mod file
2. Anywhere in a passing test with a mock.EXPECT(), add the same line again. Ex:

   mock.EXPECT().Get(id).Return("val", nil).Times(1)
   mock.EXPECT().Get(id).Return("val", nil).Times(1) // Duplicate of the above line

3. Run your tests, they should still pass.
4. Switch to gomock v1.5.0 in your go.mod file.
5. Run the tests again, they should fail this time with a message like:

   ... missing call(s) to ...
   ... aborting test due to missing call(s)

Additional Information

• gomock mode (reflect or source): source
• gomock version or git ref: v1.5.0
• golang version: 1.15

Triage Notes for the Maintainers

[codyoss]

Hey @wafer-bw thanks for the report.

In general this sounds like a good thing to me as the test should not pass if there are calls that are expected to be made, but are never actually called. When I wrote a simple example with 1.4.4 it also failed with the same error: "aborting test due to missing call(s)"

package m530

//go:generate mockgen -source=m530.go -destination=m530_mock_test.go -package=m530

type Foo interface {
    Bar() string
}

func Baz(f Foo) string {
    return f.Bar()
}

package m530

import (
    "testing"

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

func TestFooBar(t *testing.T) {
    ctrl := gomock.NewController(t)
    mockFoo := NewMockFoo(ctrl)
    defer ctrl.Finish()
    mockFoo.EXPECT().Bar().Return("abc").Times(1)
    mockFoo.EXPECT().Bar().Return("xyz").Times(1)
    want := "abc"
    got := Baz(mockFoo)
    if got != want {
        t.Fatalf("got %v, want %v", got, want)
    }
}

[wafer-bw]

@codyoss, I agree it is better this way but it could be considered breaking in a sense. I just wanted to bring attention to the change. Here is a more complete way to reproduce, hopefully this helps!

1. Init a new module

   mkdir test
   cd test
   go mod init
   go mod init example.com/test

2. In go.mod paste:

   module example.com/test
   
   go 1.15
   
   require (
       github.com/golang/mock v1.4.4
       github.com/stretchr/testify v1.7.0
   )

3. Setup an interface to test in a new file main.go

   package main
   
   type msg struct {
       content string
   }
   
   type MsgInterface interface {
       GetContent() string
   }
   
   func (m *msg) GetContent() string {
       return m.content
   }

4. Generate mocks:

   PATH=$(PATH):$(HOME)/go/bin mockgen -source=main.go -destination=mock/mock.go -package=mock

5. Make a test in main_test.go

   package main
   
   import (
       "testing"
   
       "example.com/test/mock"
       "github.com/golang/mock/gomock"
       "github.com/stretchr/testify/require"
   )
   
   func TestGetContent(t *testing.T) {
       ctrl := gomock.NewController(t)
       mockMsg := mock.NewMockMsgInterface(ctrl)
       expect := "hello"
       msg := &msg{content: expect}
       t.Run("success", func(t *testing.T) {
           mockMsg.EXPECT().GetContent().Return(expect).Times(1)
           mockMsg.EXPECT().GetContent().Return(expect).Times(1)
           c := msg.GetContent()
           require.Equal(t, expect, c)
       })
   }

6. Run the test (It should pass):

   go test -coverprofile=cover.out `go list ./... | grep -v mock`
   # ok      example.com/test        0.093s  coverage: 100.0% of statements

7. Update gomock version in go.mod from v1.4.4 to v1.5.0
8. Rerun the test (It should fail):

   go test -coverprofile=cover.out `go list ./... | grep -v mock`
   # --- FAIL: TestGetContent (0.00s)
   #     controller.go:137: missing call(s) to *mock.MockMsgInterface.GetContent() /redacted/main_test.go:17
   #     controller.go:137: missing call(s) to *mock.MockMsgInterface.GetContent() /redacted/main_test.go:18
   #     controller.go:137: aborting test due to missing call(s)
   # FAIL
   # coverage: 100.0% of statements
   # FAIL    example.com/test        0.119s
   # FAIL

[codyoss]

@wafer-bw Thank you for bring this up. I am going to close this as by design though.

Previously there were some edge cases where some panics were silently being swallowed by gomock. Now they are resurfaced to the user to help provide more details of failures.