Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Deprecate the function ctrl.Finish() #50

Merged
merged 1 commit into from
Sep 5, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 0 additions & 9 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -137,9 +137,6 @@ func SUT(f Foo) {
func TestFoo(t *testing.T) {
ctrl := gomock.NewController(t)

// Assert that Bar() is invoked.
defer ctrl.Finish()

m := NewMockFoo(ctrl)

// Asserts that the first and only call to Bar() is passed 99.
Expand All @@ -153,11 +150,6 @@ func TestFoo(t *testing.T) {
}
```

If you are using a Go version of 1.14+, a mockgen version of 1.5.0+, and are
passing a *testing.T into `gomock.NewController(t)` you no longer need to call
`ctrl.Finish()` explicitly. It will be called for you automatically from a self
registered [Cleanup](https://pkg.go.dev/testing?tab=doc#T.Cleanup) function.

## Building Stubs

```go
Expand All @@ -174,7 +166,6 @@ func SUT(f Foo) {
```go
func TestFoo(t *testing.T) {
ctrl := gomock.NewController(t)
defer ctrl.Finish()

m := NewMockFoo(ctrl)

Expand Down
19 changes: 7 additions & 12 deletions gomock/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,19 +53,16 @@ type cleanuper interface {
//
// func TestFoo(t *testing.T) {
// ctrl := gomock.NewController(t)
// defer ctrl.Finish()
// // ..
// }
//
// func TestBar(t *testing.T) {
// t.Run("Sub-Test-1", st) {
// ctrl := gomock.NewController(st)
// defer ctrl.Finish()
// // ..
// })
// t.Run("Sub-Test-2", st) {
// ctrl := gomock.NewController(st)
// defer ctrl.Finish()
// // ..
// })
// })
Expand All @@ -81,11 +78,10 @@ type Controller struct {
finished bool
}

// NewController returns a new Controller. It is the preferred way to create a
// Controller.
// NewController returns a new Controller. It is the preferred way to create a Controller.
//
// New in go1.14+, if you are passing a *testing.T into this function you no
// longer need to call ctrl.Finish() in your test methods.
// Passing [*testing.T] registers cleanup function to automatically call [Controller.Finish]
// when the test and all its subtests complete.
func NewController(t TestReporter, opts ...ControllerOption) *Controller {
h, ok := t.(TestHelper)
if !ok {
Expand Down Expand Up @@ -238,12 +234,11 @@ func (ctrl *Controller) Call(receiver any, method string, args ...any) []any {
return rets
}

// Finish checks to see if all the methods that were expected to be called
// were called. It should be invoked for each Controller. It is not idempotent
// and therefore can only be invoked once.
// Finish checks to see if all the methods that were expected to be called were called.
// It is not idempotent and therefore can only be invoked once.
//
// New in go1.14+, if you are passing a *testing.T into NewController function you no
// longer need to call ctrl.Finish() in your test methods.
// Deprecated: Calling this function in test methods is not required starting from Go 1.14.
// It will be called automatically from a self registered [testing.T.Cleanup] function.
func (ctrl *Controller) Finish() {
// If we're currently panicking, probably because this is a deferred call.
// This must be recovered in the deferred function.
Expand Down
46 changes: 3 additions & 43 deletions gomock/controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,8 @@ package gomock_test
import (
"fmt"
"reflect"
"testing"

"strings"
"testing"

"go.uber.org/mock/gomock"
)
Expand Down Expand Up @@ -177,8 +176,7 @@ func createFixtures(t *testing.T) (reporter *ErrorReporter, ctrl *gomock.Control
}

func TestNoCalls(t *testing.T) {
reporter, ctrl := createFixtures(t)
ctrl.Finish()
reporter, _ := createFixtures(t)
reporter.assertPass("No calls expected or made.")
}

Expand All @@ -189,7 +187,6 @@ func TestNoRecordedCallsForAReceiver(t *testing.T) {
reporter.assertFatal(func() {
ctrl.Call(subject, "NotRecordedMethod", "argument")
}, "Unexpected call to", "there are no expected calls of the method \"NotRecordedMethod\" for that receiver")
ctrl.Finish()
}

func TestNoRecordedMatchingMethodNameForAReceiver(t *testing.T) {
Expand All @@ -213,7 +210,6 @@ func TestExpectedMethodCall(t *testing.T) {

ctrl.RecordCall(subject, "FooMethod", "argument")
ctrl.Call(subject, "FooMethod", "argument")
ctrl.Finish()

reporter.assertPass("Expected method call made.")
}
Expand All @@ -225,8 +221,6 @@ func TestUnexpectedMethodCall(t *testing.T) {
reporter.assertFatal(func() {
ctrl.Call(subject, "FooMethod", "argument")
})

ctrl.Finish()
}

func TestRepeatedCall(t *testing.T) {
Expand All @@ -241,7 +235,6 @@ func TestRepeatedCall(t *testing.T) {
reporter.assertFatal(func() {
ctrl.Call(subject, "FooMethod", "argument")
})
ctrl.Finish()
reporter.assertFail("After calling one too many times.")
}

Expand Down Expand Up @@ -388,7 +381,6 @@ func TestAnyTimes(t *testing.T) {
ctrl.Call(subject, "FooMethod", "argument")
}
reporter.assertPass("After 100 method calls.")
ctrl.Finish()
}

func TestMinTimes1(t *testing.T) {
Expand Down Expand Up @@ -514,8 +506,6 @@ func TestDo(t *testing.T) {
if wantArg != argument {
t.Error("Do callback received wrong argument.")
}

ctrl.Finish()
}

func TestDoAndReturn(t *testing.T) {
Expand Down Expand Up @@ -551,8 +541,6 @@ func TestDoAndReturn(t *testing.T) {
} else if ret != 5 {
t.Errorf("DoAndReturn return value: got %d, want 5", ret)
}

ctrl.Finish()
}

func TestSetArgSlice(t *testing.T) {
Expand All @@ -574,8 +562,6 @@ func TestSetArgSlice(t *testing.T) {
if !reflect.DeepEqual(in, set) {
t.Error("Expected SetArg() to modify input slice argument as any")
}

ctrl.Finish()
}

func TestSetArgMap(t *testing.T) {
Expand All @@ -597,8 +583,6 @@ func TestSetArgMap(t *testing.T) {
if !reflect.DeepEqual(in, set) {
t.Error("Expected SetArg() to modify input map argument as any")
}

ctrl.Finish()
}

func TestSetArgPtr(t *testing.T) {
Expand All @@ -620,7 +604,6 @@ func TestSetArgPtr(t *testing.T) {
if in != set {
t.Error("Expected SetArg() to modify value pointed to by argument as any")
}
ctrl.Finish()
}

func TestReturn(t *testing.T) {
Expand All @@ -640,7 +623,6 @@ func TestReturn(t *testing.T) {
t,
[]any{5},
ctrl.Call(subject, "FooMethod", "five"))
ctrl.Finish()
}

func TestUnorderedCalls(t *testing.T) {
Expand Down Expand Up @@ -707,7 +689,6 @@ func TestPanicOverridesExpectationChecks(t *testing.T) {

func TestSetArgWithBadType(t *testing.T) {
rep, ctrl := createFixtures(t)
defer ctrl.Finish()

s := new(Subject)
// This should catch a type error:
Expand All @@ -719,7 +700,6 @@ func TestSetArgWithBadType(t *testing.T) {

func TestTimes0(t *testing.T) {
rep, ctrl := createFixtures(t)
defer ctrl.Finish()

s := new(Subject)
ctrl.RecordCall(s, "FooMethod", "arg").Times(0)
Expand All @@ -735,7 +715,6 @@ func TestVariadicMatching(t *testing.T) {
s := new(Subject)
ctrl.RecordCall(s, "VariadicMethod", 0, "1", "2")
ctrl.Call(s, "VariadicMethod", 0, "1", "2")
ctrl.Finish()
rep.assertPass("variadic matching works")
}

Expand All @@ -750,7 +729,6 @@ func TestVariadicNoMatch(t *testing.T) {
}, "expected call at", "doesn't match the argument at index 0",
"Got: 1 (int)\nWant: is equal to 0 (int)")
ctrl.Call(s, "VariadicMethod", 0)
ctrl.Finish()
}

func TestVariadicMatchingWithSlice(t *testing.T) {
Expand All @@ -771,7 +749,6 @@ func TestVariadicMatchingWithSlice(t *testing.T) {
args[i+1] = arg
}
ctrl.Call(s, "VariadicMethod", args...)
ctrl.Finish()
rep.assertPass("slices can be used as matchers for variadic arguments")
})
}
Expand All @@ -798,7 +775,6 @@ func TestVariadicArgumentsGotFormatter(t *testing.T) {
}, "expected call to", "doesn't match the argument at index 0",
"Got: test{1}\nWant: is equal to 0")
ctrl.Call(s, "VariadicMethod", 0)
ctrl.Finish()
}

func TestVariadicArgumentsGotFormatterTooManyArgsFailure(t *testing.T) {
Expand All @@ -823,7 +799,6 @@ func TestVariadicArgumentsGotFormatterTooManyArgsFailure(t *testing.T) {
}, "expected call to", "doesn't match the argument at index 1",
"Got: test{[2 3]}\nWant: is equal to 1")
ctrl.Call(s, "VariadicMethod", 0, "1")
ctrl.Finish()
}

func TestNoHelper(t *testing.T) {
Expand Down Expand Up @@ -854,22 +829,7 @@ func TestMultipleDefers(t *testing.T) {
reporter.Cleanup(func() {
reporter.assertPass("No errors for multiple calls to Finish")
})
ctrl := gomock.NewController(reporter)
ctrl.Finish()
}

// Equivalent to the TestNoRecordedCallsForAReceiver, but without explicitly
// calling Finish.
func TestDeferNotNeededFail(t *testing.T) {
reporter := NewErrorReporter(t)
subject := new(Subject)
var ctrl *gomock.Controller
reporter.Cleanup(func() {
reporter.assertFatal(func() {
ctrl.Call(subject, "NotRecordedMethod", "argument")
}, "Unexpected call to", "there are no expected calls of the method \"NotRecordedMethod\" for that receiver")
})
ctrl = gomock.NewController(reporter)
_ = gomock.NewController(reporter)
}

func TestDeferNotNeededPass(t *testing.T) {
Expand Down
2 changes: 1 addition & 1 deletion gomock/doc.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@
// (2) Use mockgen to generate a mock from the interface.
// (3) Use the mock in a test:
// func TestMyThing(t *testing.T) {
// mockCtrl := gomock.NewController(t)//
// mockCtrl := gomock.NewController(t)
// mockObj := something.NewMockMyInterface(mockCtrl)
// mockObj.EXPECT().SomeMethod(4, "blah")
// // pass mockObj to a real object and play with it.
Expand Down
4 changes: 0 additions & 4 deletions gomock/matchers_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,6 @@ func TestMatchers(t *testing.T) {
// A more thorough test of notMatcher
func TestNotMatcher(t *testing.T) {
ctrl := gomock.NewController(t)
defer ctrl.Finish()

mockMatcher := mock_gomock.NewMockMatcher(ctrl)
notMatcher := gomock.Not(mockMatcher)
Expand All @@ -94,9 +93,6 @@ type ctxKey struct{}

// A thorough test of assignableToTypeOfMatcher
func TestAssignableToTypeOfMatcher(t *testing.T) {
ctrl := gomock.NewController(t)
defer ctrl.Finish()

aStr := "def"
anotherStr := "ghi"

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ import (
// TestValidInterface assesses whether or not the generated mock is valid
func TestValidInterface(t *testing.T) {
ctrl := gomock.NewController(t)
defer ctrl.Finish()

s := NewMockSource(ctrl)
s.EXPECT().Method().Return("")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ import (

func TestGreeter_Greet(t *testing.T) {
ctrl := gomock.NewController(t)
defer ctrl.Finish()

input := client.GreetInput{
Name: "Foo",
Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
package bugreport

import (
"go.uber.org/mock/gomock"
"testing"

"go.uber.org/mock/gomock"
)

func TestExample_Method(t *testing.T) {
Expand All @@ -11,8 +12,6 @@ func TestExample_Method(t *testing.T) {
m.EXPECT().Method(1, 2, 3, 4)

m.Method(1, 2, 3, 4)

ctrl.Finish()
}

func TestExample_VarargMethod(t *testing.T) {
Expand All @@ -21,6 +20,4 @@ func TestExample_VarargMethod(t *testing.T) {
m.EXPECT().VarargMethod(1, 2, 3, 4, 6, 7)

m.VarargMethod(1, 2, 3, 4, 6, 7)

ctrl.Finish()
}
3 changes: 0 additions & 3 deletions mockgen/internal/tests/generics/source/mock_external_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ import (

func TestMockEmbeddingIface_One(t *testing.T) {
ctrl := gomock.NewController(t)
defer ctrl.Finish()

m := NewMockEmbeddingIface[int, float64](ctrl)
m.EXPECT().One("foo").Return("bar")
Expand All @@ -21,7 +20,6 @@ func TestMockEmbeddingIface_One(t *testing.T) {

func TestMockUniverse_Water(t *testing.T) {
ctrl := gomock.NewController(t)
defer ctrl.Finish()

m := NewMockUniverse[int](ctrl)
m.EXPECT().Water(1024)
Expand All @@ -30,7 +28,6 @@ func TestMockUniverse_Water(t *testing.T) {

func TestNewMockGroup_Join(t *testing.T) {
ctrl := gomock.NewController(t)
defer ctrl.Finish()

m := NewMockGroup[generics.Generator[any]](ctrl)
ctx := context.TODO()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ import (
// TestValidInterface assesses whether or not the generated mock is valid
func TestValidInterface(t *testing.T) {
ctrl := gomock.NewController(t)
defer ctrl.Finish()

s := NewMockSource(ctrl)
s.EXPECT().Ersatz().Return("")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ import (
// TestValidInterface assesses whether or not the generated mock is valid
func TestValidNetInterface(t *testing.T) {
ctrl := gomock.NewController(t)
defer ctrl.Finish()

s := NewMockNet(ctrl)
s.EXPECT().WriteHeader(10)
Expand Down
1 change: 0 additions & 1 deletion mockgen/internal/tests/overlapping_methods/overlap_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ import (
// TestValidInterface assesses whether or not the generated mock is valid
func TestValidInterface(t *testing.T) {
ctrl := gomock.NewController(t)
defer ctrl.Finish()

s := NewMockReadWriteCloser(ctrl)
s.EXPECT().Close().Return(errors.New("test"))
Expand Down
Loading