add default calling of ctrl.Finish() in go1.14+ (#422)

In go1.14 the testing package added a new cleanup method that is
called at the end of test runs. By taping into this gomock can
remove the need for users to call ctrl.Finish() if they pass a
*testing.T into gomock.NewController(...).

Fixes #407

Author: codyoss

Diff for: .travis.yml

@@ -2,8 +2,8 @@ language: go
 
 go:
   - 1.11.x
-  - 1.12.x
   - 1.13.x
+  - 1.14.x
 
 env:
   - GO111MODULE=on

Diff for: README.md

@@ -141,6 +141,9 @@ func TestFoo(t *testing.T) {
 }
 ```
 
+In Go versions 1.14+, if you pass a *testing.T into `gomock.NewController(t)`
+you no longer need to call `ctrl.Finish()`.
+
 Building Stubs
 --------------
 

Diff for: gomock/controller.go

@@ -50,9 +50,6 @@
 //         mockObj.EXPECT().SomeMethod(2, "second"),
 //         mockObj.EXPECT().SomeMethod(3, "third"),
 //     )
-//
-// TODO:
-//	- Handle different argument/return types (e.g. ..., chan, map, interface).
 package gomock
 
 import (
@@ -77,6 +74,15 @@ type TestHelper interface {
 	Helper()
 }
 
+// cleanuper is used to check if TestHelper also has the `Cleanup` method. A
+// common pattern is to pass in a `*testing.T` to
+// `NewController(t TestReporter)`. In Go 1.14+, `*testing.T` has a cleanup
+// method. This can be utilized to call `Finish()` so the caller of this library
+// does not have to.
+type cleanuper interface {
+	Cleanup(func())
+}
+
 // A Controller represents the top-level control of a mock ecosystem.  It
 // defines the scope and lifetime of mock objects, as well as their
 // expectations.  It is safe to call Controller's methods from multiple
@@ -115,45 +121,66 @@ type Controller struct {
 
 // 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
 func NewController(t TestReporter) *Controller {
 	h, ok := t.(TestHelper)
 	if !ok {
-		h = nopTestHelper{t}
+		h = &nopTestHelper{t}
 	}
-
-	return &Controller{
+	ctrl := &Controller{
 		T:             h,
 		expectedCalls: newCallSet(),
 	}
+	if c, ok := isCleanuper(ctrl.T); ok {
+		c.Cleanup(func() {
+			ctrl.T.Helper()
+			ctrl.Finish()
+		})
+	}
+
+	return ctrl
 }
 
 type cancelReporter struct {
-	TestHelper
+	t      TestHelper
 	cancel func()
 }
 
 func (r *cancelReporter) Errorf(format string, args ...interface{}) {
-	r.TestHelper.Errorf(format, args...)
+	r.t.Errorf(format, args...)
 }
 func (r *cancelReporter) Fatalf(format string, args ...interface{}) {
 	defer r.cancel()
-	r.TestHelper.Fatalf(format, args...)
+	r.t.Fatalf(format, args...)
+}
+
+func (r *cancelReporter) Helper() {
+	r.t.Helper()
 }
 
 // WithContext returns a new Controller and a Context, which is cancelled on any
 // fatal failure.
 func WithContext(ctx context.Context, t TestReporter) (*Controller, context.Context) {
 	h, ok := t.(TestHelper)
 	if !ok {
-		h = nopTestHelper{t}
+		h = &nopTestHelper{t: t}
 	}
 
 	ctx, cancel := context.WithCancel(ctx)
-	return NewController(&cancelReporter{h, cancel}), ctx
+	return NewController(&cancelReporter{t: h, cancel: cancel}), ctx
 }
 
 type nopTestHelper struct {
-	TestReporter
+	t TestReporter
+}
+
+func (h *nopTestHelper) Errorf(format string, args ...interface{}) {
+	h.t.Errorf(format, args...)
+}
+func (h *nopTestHelper) Fatalf(format string, args ...interface{}) {
+	h.t.Fatalf(format, args...)
 }
 
 func (h nopTestHelper) Helper() {}
@@ -236,7 +263,15 @@ func (ctrl *Controller) Finish() {
 	defer ctrl.mu.Unlock()
 
 	if ctrl.finished {
-		ctrl.T.Fatalf("Controller.Finish was called more than once. It has to be called exactly once.")
+		if _, ok := isCleanuper(ctrl.T); !ok {
+			ctrl.T.Fatalf("Controller.Finish was called more than once. It has to be called exactly once.")
+		}
+		// provide a log message to guide users to remove `defer ctrl.Finish()` in Go 1.14+
+		tr := unwrapTestReporter(ctrl.T)
+		if l, ok := tr.(interface{ Log(args ...interface{}) }); ok {
+			l.Log("In Go 1.14+ you no longer need to `ctrl.Finish()` if a *testing.T is passed to `NewController(...)`")
+		}
+		return
 	}
 	ctrl.finished = true
 
@@ -262,3 +297,27 @@ func callerInfo(skip int) string {
 	}
 	return "unknown file"
 }
+
+// isCleanuper checks it if t's base TestReporter has a Cleanup method.
+func isCleanuper(t TestReporter) (cleanuper, bool) {
+	tr := unwrapTestReporter(t)
+	c, ok := tr.(cleanuper)
+	return c, ok
+}
+
+// unwrapTestReporter unwraps TestReporter to the base implementation.
+func unwrapTestReporter(t TestReporter) TestReporter {
+	tr := t
+	switch nt := t.(type) {
+	case *cancelReporter:
+		tr = nt.t
+		if h, check := tr.(*nopTestHelper); check {
+			tr = h.t
+		}
+	case *nopTestHelper:
+		tr = nt.t
+	default:
+		// not wrapped
+	}
+	return tr
+}

Diff for: gomock/controller_113_test.go

@@ -0,0 +1,28 @@
+// Copyright 2020 Google LLC
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+// +build !go1.14
+
+package gomock_test
+
+import "testing"
+
+func TestDuplicateFinishCallFails(t *testing.T) {
+	rep, ctrl := createFixtures(t)
+
+	ctrl.Finish()
+	rep.assertPass("the first Finish call should succeed")
+
+	rep.assertFatal(ctrl.Finish, "Controller.Finish was called more than once. It has to be called exactly once.")
+}

Diff for: gomock/controller_114_test.go

@@ -0,0 +1,132 @@
+// Copyright 2020 Google LLC
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+// +build go1.14
+
+package gomock_test
+
+import (
+	"testing"
+
+	"github.com/golang/mock/gomock"
+)
+
+func (e *ErrorReporter) Cleanup(f func()) {
+	e.t.Helper()
+	e.t.Cleanup(f)
+}
+
+func TestMultipleDefers(t *testing.T) {
+	reporter := NewErrorReporter(t)
+	reporter.Cleanup(func() {
+		reporter.assertLogf("In Go 1.14+ you no longer need to `ctrl.Finish()` if a *testing.T is passed to `NewController(...)`")
+	})
+	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)
+}
+
+func TestDeferNotNeededPass(t *testing.T) {
+	reporter := NewErrorReporter(t)
+	subject := new(Subject)
+	var ctrl *gomock.Controller
+	reporter.Cleanup(func() {
+		reporter.assertPass("Expected method call made.")
+	})
+	ctrl = gomock.NewController(reporter)
+	ctrl.RecordCall(subject, "FooMethod", "argument")
+	ctrl.Call(subject, "FooMethod", "argument")
+}
+
+func TestOrderedCallsInCorrect(t *testing.T) {
+	reporter := NewErrorReporter(t)
+	subjectOne := new(Subject)
+	subjectTwo := new(Subject)
+	var ctrl *gomock.Controller
+	reporter.Cleanup(func() {
+		reporter.assertFatal(func() {
+			gomock.InOrder(
+				ctrl.RecordCall(subjectOne, "FooMethod", "1").AnyTimes(),
+				ctrl.RecordCall(subjectTwo, "FooMethod", "2"),
+				ctrl.RecordCall(subjectTwo, "BarMethod", "3"),
+			)
+			ctrl.Call(subjectOne, "FooMethod", "1")
+			// FooMethod(2) should be called before BarMethod(3)
+			ctrl.Call(subjectTwo, "BarMethod", "3")
+		}, "Unexpected call to", "Subject.BarMethod([3])", "doesn't have a prerequisite call satisfied")
+	})
+	ctrl = gomock.NewController(reporter)
+}
+
+// Test that calls that are prerequisites to other calls but have maxCalls >
+// minCalls are removed from the expected call set.
+func TestOrderedCallsWithPreReqMaxUnbounded(t *testing.T) {
+	reporter := NewErrorReporter(t)
+	subjectOne := new(Subject)
+	subjectTwo := new(Subject)
+	var ctrl *gomock.Controller
+	reporter.Cleanup(func() {
+		reporter.assertFatal(func() {
+			// Initially we should be able to call FooMethod("1") as many times as we
+			// want.
+			ctrl.Call(subjectOne, "FooMethod", "1")
+			ctrl.Call(subjectOne, "FooMethod", "1")
+
+			// But calling something that has it as a prerequite should remove it from
+			// the expected call set. This allows tests to ensure that FooMethod("1") is
+			// *not* called after FooMethod("2").
+			ctrl.Call(subjectTwo, "FooMethod", "2")
+
+			ctrl.Call(subjectOne, "FooMethod", "1")
+		})
+	})
+	ctrl = gomock.NewController(reporter)
+}
+
+func TestCallAfterLoopPanic(t *testing.T) {
+	reporter := NewErrorReporter(t)
+	subject := new(Subject)
+	var ctrl *gomock.Controller
+	reporter.Cleanup(func() {
+		firstCall := ctrl.RecordCall(subject, "FooMethod", "1")
+		secondCall := ctrl.RecordCall(subject, "FooMethod", "2")
+		thirdCall := ctrl.RecordCall(subject, "FooMethod", "3")
+
+		gomock.InOrder(firstCall, secondCall, thirdCall)
+
+		defer func() {
+			err := recover()
+			if err == nil {
+				t.Error("Call.After creation of dependency loop did not panic.")
+			}
+		}()
+
+		// This should panic due to dependency loop.
+		firstCall.After(thirdCall)
+	})
+	ctrl = gomock.NewController(reporter)
+}