diff --git a/README.md b/README.md index db8ac2d..eba715b 100644 --- a/README.md +++ b/README.md @@ -139,9 +139,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. @@ -155,11 +152,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 @@ -176,7 +168,6 @@ func SUT(f Foo) { ```go func TestFoo(t *testing.T) { ctrl := gomock.NewController(t) - defer ctrl.Finish() m := NewMockFoo(ctrl) diff --git a/gomock/controller.go b/gomock/controller.go index 5879a2f..943828e 100644 --- a/gomock/controller.go +++ b/gomock/controller.go @@ -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() // // .. // }) // }) @@ -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 { @@ -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. diff --git a/gomock/controller_test.go b/gomock/controller_test.go index 0c2e297..2489226 100644 --- a/gomock/controller_test.go +++ b/gomock/controller_test.go @@ -17,9 +17,8 @@ package gomock_test import ( "fmt" "reflect" - "testing" - "strings" + "testing" "go.uber.org/mock/gomock" ) @@ -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.") } @@ -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) { @@ -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.") } @@ -225,8 +221,6 @@ func TestUnexpectedMethodCall(t *testing.T) { reporter.assertFatal(func() { ctrl.Call(subject, "FooMethod", "argument") }) - - ctrl.Finish() } func TestRepeatedCall(t *testing.T) { @@ -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.") } @@ -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) { @@ -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) { @@ -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) { @@ -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) { @@ -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) { @@ -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) { @@ -640,7 +623,6 @@ func TestReturn(t *testing.T) { t, []any{5}, ctrl.Call(subject, "FooMethod", "five")) - ctrl.Finish() } func TestUnorderedCalls(t *testing.T) { @@ -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: @@ -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) @@ -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") } @@ -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) { @@ -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") }) } @@ -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) { @@ -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) { @@ -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) { diff --git a/gomock/doc.go b/gomock/doc.go index f1a304f..696dda3 100644 --- a/gomock/doc.go +++ b/gomock/doc.go @@ -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. diff --git a/gomock/matchers_test.go b/gomock/matchers_test.go index 8de86a2..0a85906 100644 --- a/gomock/matchers_test.go +++ b/gomock/matchers_test.go @@ -74,7 +74,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) @@ -98,9 +97,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" diff --git a/mockgen/internal/tests/aux_imports_embedded_interface/bugreport_test.go b/mockgen/internal/tests/aux_imports_embedded_interface/bugreport_test.go index cef03ed..b0984f7 100644 --- a/mockgen/internal/tests/aux_imports_embedded_interface/bugreport_test.go +++ b/mockgen/internal/tests/aux_imports_embedded_interface/bugreport_test.go @@ -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("") diff --git a/mockgen/internal/tests/custom_package_name/greeter/greeter_test.go b/mockgen/internal/tests/custom_package_name/greeter/greeter_test.go index b34d05f..1e50aef 100644 --- a/mockgen/internal/tests/custom_package_name/greeter/greeter_test.go +++ b/mockgen/internal/tests/custom_package_name/greeter/greeter_test.go @@ -9,7 +9,6 @@ import ( func TestGreeter_Greet(t *testing.T) { ctrl := gomock.NewController(t) - defer ctrl.Finish() input := client.GreetInput{ Name: "Foo", diff --git a/mockgen/internal/tests/generated_identifier_conflict/bugreport_test.go b/mockgen/internal/tests/generated_identifier_conflict/bugreport_test.go index 3b53248..7a48f0d 100644 --- a/mockgen/internal/tests/generated_identifier_conflict/bugreport_test.go +++ b/mockgen/internal/tests/generated_identifier_conflict/bugreport_test.go @@ -1,8 +1,9 @@ package bugreport import ( - "go.uber.org/mock/gomock" "testing" + + "go.uber.org/mock/gomock" ) func TestExample_Method(t *testing.T) { @@ -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) { @@ -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() } diff --git a/mockgen/internal/tests/generics/source/mock_external_test.go b/mockgen/internal/tests/generics/source/mock_external_test.go index c345164..8b98678 100644 --- a/mockgen/internal/tests/generics/source/mock_external_test.go +++ b/mockgen/internal/tests/generics/source/mock_external_test.go @@ -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") @@ -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) @@ -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() diff --git a/mockgen/internal/tests/import_embedded_interface/bugreport_test.go b/mockgen/internal/tests/import_embedded_interface/bugreport_test.go index 2767840..ae4773b 100644 --- a/mockgen/internal/tests/import_embedded_interface/bugreport_test.go +++ b/mockgen/internal/tests/import_embedded_interface/bugreport_test.go @@ -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("") diff --git a/mockgen/internal/tests/import_embedded_interface/net_test.go b/mockgen/internal/tests/import_embedded_interface/net_test.go index bf98dd8..d181019 100644 --- a/mockgen/internal/tests/import_embedded_interface/net_test.go +++ b/mockgen/internal/tests/import_embedded_interface/net_test.go @@ -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) diff --git a/mockgen/internal/tests/overlapping_methods/overlap_test.go b/mockgen/internal/tests/overlapping_methods/overlap_test.go index 0bb8a3d..be9f8bf 100644 --- a/mockgen/internal/tests/overlapping_methods/overlap_test.go +++ b/mockgen/internal/tests/overlapping_methods/overlap_test.go @@ -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")) diff --git a/mockgen/internal/tests/panicing_test/panic_test.go b/mockgen/internal/tests/panicing_test/panic_test.go index 436ce8e..f825d3a 100644 --- a/mockgen/internal/tests/panicing_test/panic_test.go +++ b/mockgen/internal/tests/panicing_test/panic_test.go @@ -25,7 +25,6 @@ import ( func TestDanger_Panics_Explicit(t *testing.T) { ctrl := gomock.NewController(t) - defer ctrl.Finish() mock := NewMockFoo(ctrl) mock.EXPECT().Bar().Return("Bar") mock.EXPECT().Bar().Return("Baz") diff --git a/mockgen/internal/tests/typed/bugreport_test.go b/mockgen/internal/tests/typed/bugreport_test.go index 5b60484..cce8a4f 100644 --- a/mockgen/internal/tests/typed/bugreport_test.go +++ b/mockgen/internal/tests/typed/bugreport_test.go @@ -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("") diff --git a/mockgen/internal/tests/unexported_method/bugreport_test.go b/mockgen/internal/tests/unexported_method/bugreport_test.go index b328685..fc6b42f 100644 --- a/mockgen/internal/tests/unexported_method/bugreport_test.go +++ b/mockgen/internal/tests/unexported_method/bugreport_test.go @@ -8,7 +8,6 @@ import ( func TestCallExample(t *testing.T) { ctrl := gomock.NewController(t) - defer ctrl.Finish() e := NewMockExample(ctrl) e.EXPECT().someMethod(gomock.Any()).Return("it works!") diff --git a/sample/concurrent/concurrent_test.go b/sample/concurrent/concurrent_test.go index 4c4ae27..3dc8822 100644 --- a/sample/concurrent/concurrent_test.go +++ b/sample/concurrent/concurrent_test.go @@ -1,12 +1,10 @@ package concurrent import ( - "testing" - "context" + "testing" "go.uber.org/mock/gomock" - mock "go.uber.org/mock/sample/concurrent/mock" ) @@ -30,7 +28,6 @@ func call(ctx context.Context, m Math) (int, error) { func TestConcurrentFails(t *testing.T) { t.Skip("Test is expected to fail, remove skip to trying running yourself.") ctrl, ctx := gomock.WithContext(context.Background(), t) - defer ctrl.Finish() m := mock.NewMockMath(ctrl) if _, err := call(ctx, m); err != nil { t.Error("call failed:", err) @@ -39,7 +36,6 @@ func TestConcurrentFails(t *testing.T) { func TestConcurrentWorks(t *testing.T) { ctrl, ctx := gomock.WithContext(context.Background(), t) - defer ctrl.Finish() m := mock.NewMockMath(ctrl) m.EXPECT().Sum(1, 2).Return(3) if _, err := call(ctx, m); err != nil { diff --git a/sample/user_test.go b/sample/user_test.go index 55856e3..f963651 100644 --- a/sample/user_test.go +++ b/sample/user_test.go @@ -12,7 +12,6 @@ import ( func TestRemember(t *testing.T) { ctrl := gomock.NewController(t) - defer ctrl.Finish() mockIndex := NewMockIndex(ctrl) mockIndex.EXPECT().Put("a", 1) // literals work @@ -65,7 +64,6 @@ func TestRemember(t *testing.T) { func TestVariadicFunction(t *testing.T) { ctrl := gomock.NewController(t) - defer ctrl.Finish() mockIndex := NewMockIndex(ctrl) mockIndex.EXPECT().Ellip("%d", 5, 6, 7, 8).Do(func(format string, nums ...int) { @@ -123,7 +121,6 @@ func TestVariadicFunction(t *testing.T) { func TestGrabPointer(t *testing.T) { ctrl := gomock.NewController(t) - defer ctrl.Finish() mockIndex := NewMockIndex(ctrl) mockIndex.EXPECT().Ptr(gomock.Any()).SetArg(0, 7) // set first argument to 7 @@ -136,7 +133,6 @@ func TestGrabPointer(t *testing.T) { func TestEmbeddedInterface(t *testing.T) { ctrl := gomock.NewController(t) - defer ctrl.Finish() mockEmbed := NewMockEmbed(ctrl) mockEmbed.EXPECT().RegularMethod() @@ -153,7 +149,6 @@ func TestExpectTrueNil(t *testing.T) { // Make sure that passing "nil" to EXPECT (thus as a nil interface value), // will correctly match a nil concrete type. ctrl := gomock.NewController(t) - defer ctrl.Finish() mockIndex := NewMockIndex(ctrl) mockIndex.EXPECT().Ptr(nil) // this nil is a nil any @@ -163,7 +158,6 @@ func TestExpectTrueNil(t *testing.T) { func TestDoAndReturnSignature(t *testing.T) { t.Run("wrong number of return args", func(t *testing.T) { ctrl := gomock.NewController(t) - defer ctrl.Finish() mockIndex := NewMockIndex(ctrl) @@ -182,7 +176,6 @@ func TestDoAndReturnSignature(t *testing.T) { t.Run("wrong type of return arg", func(t *testing.T) { ctrl := gomock.NewController(t) - defer ctrl.Finish() mockIndex := NewMockIndex(ctrl)