diff --git a/.travis.yml b/.travis.yml index 34539ba4..83c7da98 100644 --- a/.travis.yml +++ b/.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 --git a/README.md b/README.md index 2a6f0001..4550c25e 100644 --- a/README.md +++ b/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 --git a/gomock/controller.go b/gomock/controller.go index d7c3c656..0a8366b9 100644 --- a/gomock/controller.go +++ b/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,29 +121,43 @@ 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 @@ -145,15 +165,22 @@ func (r *cancelReporter) Fatalf(format string, args ...interface{}) { 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 --git a/gomock/controller_113_test.go b/gomock/controller_113_test.go new file mode 100644 index 00000000..44a7e4b5 --- /dev/null +++ b/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 --git a/gomock/controller_114_test.go b/gomock/controller_114_test.go new file mode 100644 index 00000000..b6c325b1 --- /dev/null +++ b/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) +} diff --git a/gomock/controller_test.go b/gomock/controller_test.go index c22908b8..5f6e7427 100644 --- a/gomock/controller_test.go +++ b/gomock/controller_test.go @@ -54,6 +54,17 @@ func (e *ErrorReporter) assertFail(msg string) { } } +func (e *ErrorReporter) assertLogf(expectedErrMsgs ...string) { + if len(e.log) < len(expectedErrMsgs) { + e.t.Fatalf("got %d Logf messages, want %d", len(e.log), len(expectedErrMsgs)) + } + for i, expectedErrMsg := range expectedErrMsgs { + if !strings.Contains(e.log[i], expectedErrMsg) { + e.t.Errorf("Error message:\ngot: %q\nwant to contain: %q\n", e.log[i], expectedErrMsg) + } + } +} + // Use to check that code triggers a fatal test failure. func (e *ErrorReporter) assertFatal(fn func(), expectedErrMsgs ...string) { defer func() { @@ -107,6 +118,10 @@ func (e *ErrorReporter) recoverUnexpectedFatal() { } } +func (e *ErrorReporter) Log(args ...interface{}) { + e.log = append(e.log, fmt.Sprint(args...)) +} + func (e *ErrorReporter) Logf(format string, args ...interface{}) { e.log = append(e.log, fmt.Sprintf(format, args...)) } @@ -651,59 +666,6 @@ func TestOrderedCallsCorrect(t *testing.T) { reporter.assertPass("After finish") } -func TestOrderedCallsInCorrect(t *testing.T) { - reporter, ctrl, subjectOne, subjectTwo := commonTestOrderedCalls(t) - - ctrl.Call(subjectOne, "FooMethod", "1") - reporter.assertFatal(func() { - // 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") -} - -// 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, ctrl, subjectOne, subjectTwo := commonTestOrderedCalls(t) - - // 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") - - // Therefore this call should fail: - reporter.assertFatal(func() { - ctrl.Call(subjectOne, "FooMethod", "1") - }) -} - -func TestCallAfterLoopPanic(t *testing.T) { - _, ctrl := createFixtures(t) - - subject := new(Subject) - - 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) -} - func TestPanicOverridesExpectationChecks(t *testing.T) { ctrl := gomock.NewController(t) reporter := NewErrorReporter(t) @@ -787,15 +749,6 @@ func TestVariadicMatchingWithSlice(t *testing.T) { } } -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.") -} - func TestNoHelper(t *testing.T) { ctrlNoHelper := gomock.NewController(NewErrorReporter(t))