From 1b704440c83bf032e0aa892e5397e0f51a55ef87 Mon Sep 17 00:00:00 2001 From: Jacob Meixner Date: Mon, 7 May 2018 21:52:03 -0700 Subject: [PATCH 01/12] Add a property on the Mock Controller to allow for unexpected calls --- gomock/controller.go | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/gomock/controller.go b/gomock/controller.go index a7b79188..94e260d2 100644 --- a/gomock/controller.go +++ b/gomock/controller.go @@ -57,10 +57,11 @@ package gomock import ( "fmt" - "golang.org/x/net/context" "reflect" "runtime" "sync" + + "golang.org/x/net/context" ) // A TestReporter is something that can be used to report test failures. @@ -78,6 +79,7 @@ type Controller struct { t TestReporter expectedCalls *callSet finished bool + LooseMode bool } func NewController(t TestReporter) *Controller { @@ -144,11 +146,17 @@ func (ctrl *Controller) Call(receiver interface{}, method string, args ...interf ctrl.mu.Lock() defer ctrl.mu.Unlock() + var actions []func([]interface{}) []interface{} expected, err := ctrl.expectedCalls.FindMatch(receiver, method, args) - if err != nil { + if err != nil && !ctrl.LooseMode { origin := callerInfo(2) ctrl.t.Fatalf("Unexpected call to %T.%v(%v) at %s because: %s", receiver, method, args, origin, err) } + // this is to protect against nil dereference for calls that are not + // expected + if expected == nil && ctrl.LooseMode { + return actions + } // Two things happen here: // * the matching call no longer needs to check prerequite calls, @@ -158,7 +166,8 @@ func (ctrl *Controller) Call(receiver interface{}, method string, args ...interf ctrl.expectedCalls.Remove(preReqCall) } - actions := expected.call(args) + actions = expected.call(args) + if expected.exhausted() { ctrl.expectedCalls.Remove(expected) } From 451e3aed11198d819d8c0e2053ea212ceabb9574 Mon Sep 17 00:00:00 2001 From: Jacob Meixner Date: Tue, 8 May 2018 21:05:54 -0700 Subject: [PATCH 02/12] Add a test to prove that LooseMode should make unexpected calls safe --- gomock/controller.go | 3 +++ gomock/controller_test.go | 14 +++++++++++++- 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/gomock/controller.go b/gomock/controller.go index 94e260d2..0d46a5d8 100644 --- a/gomock/controller.go +++ b/gomock/controller.go @@ -148,10 +148,13 @@ func (ctrl *Controller) Call(receiver interface{}, method string, args ...interf var actions []func([]interface{}) []interface{} expected, err := ctrl.expectedCalls.FindMatch(receiver, method, args) + fmt.Printf("Here is the result: '%v','%v'\n", expected, err) if err != nil && !ctrl.LooseMode { + fmt.Println("where I expect to be") origin := callerInfo(2) ctrl.t.Fatalf("Unexpected call to %T.%v(%v) at %s because: %s", receiver, method, args, origin, err) } + fmt.Println("How am I here") // this is to protect against nil dereference for calls that are not // expected if expected == nil && ctrl.LooseMode { diff --git a/gomock/controller_test.go b/gomock/controller_test.go index b3bb4de2..c0c4480c 100644 --- a/gomock/controller_test.go +++ b/gomock/controller_test.go @@ -168,8 +168,9 @@ func TestNoCalls(t *testing.T) { reporter.assertPass("No calls expected or made.") } -func TestNoRecordedCallsForAReceiver(t *testing.T) { +func TestNoRecordedCallsForAReceiverStrictMode(t *testing.T) { reporter, ctrl := createFixtures(t) + // ctrl.LooseMode = true subject := new(Subject) reporter.assertFatal(func() { @@ -178,6 +179,17 @@ func TestNoRecordedCallsForAReceiver(t *testing.T) { ctrl.Finish() } +func TestNoRecordedCallsForAReceiverLooseMode(t *testing.T) { + reporter, ctrl := createFixtures(t) + ctrl.LooseMode = true + subject := new(Subject) + + ctrl.Call(subject, "FooMethod", "argument") + + reporter.assertPass("No calls expected but LooseMode does not cause failures") + ctrl.Finish() +} + func TestNoRecordedMatchingMethodNameForAReceiver(t *testing.T) { reporter, ctrl := createFixtures(t) subject := new(Subject) From 88eac4487831339603de582436d37da7d4aee4bb Mon Sep 17 00:00:00 2001 From: Jacob Meixner Date: Sat, 12 May 2018 15:24:28 -0700 Subject: [PATCH 03/12] Add some more comprehensive tests and check the old ones --- gomock/controller.go | 24 ++++++++++---- gomock/controller_test.go | 67 ++++++++++++++++++++++++++++++++++----- 2 files changed, 77 insertions(+), 14 deletions(-) diff --git a/gomock/controller.go b/gomock/controller.go index 0d46a5d8..9b5ae1a3 100644 --- a/gomock/controller.go +++ b/gomock/controller.go @@ -58,6 +58,7 @@ package gomock import ( "fmt" "reflect" + "regexp" "runtime" "sync" @@ -148,13 +149,24 @@ func (ctrl *Controller) Call(receiver interface{}, method string, args ...interf var actions []func([]interface{}) []interface{} expected, err := ctrl.expectedCalls.FindMatch(receiver, method, args) - fmt.Printf("Here is the result: '%v','%v'\n", expected, err) - if err != nil && !ctrl.LooseMode { - fmt.Println("where I expect to be") - origin := callerInfo(2) - ctrl.t.Fatalf("Unexpected call to %T.%v(%v) at %s because: %s", receiver, method, args, origin, err) + // fmt.Printf("Here is the result: '%v','%v'\n", expected, err) + // fmt.Printf("This is: %v!!\n", err.Error() == "there are no expected calls of the method \"FooMethod\" for that receiver") + if err != nil { + ok, _ := regexp.MatchString("there are no expected calls of the method \".*\" for that receiver", err.Error()) + if !ok || !ctrl.LooseMode{ + origin := callerInfo(2) + ctrl.t.Fatalf("Unexpected call to %T.%v(%v) at %s because: %s", receiver, method, args, origin, err) + // fmt.Println("This is an not unexpected call") + } + // if } - fmt.Println("How am I here") + + // if err != nil && !ctrl.LooseMode { + // // fmt.Println("where I expect to be") + // origin := callerInfo(2) + // ctrl.t.Fatalf("Unexpected call to %T.%v(%v) at %s because: %s", receiver, method, args, origin, err) + // } + // fmt.Println("How am I here") // this is to protect against nil dereference for calls that are not // expected if expected == nil && ctrl.LooseMode { diff --git a/gomock/controller_test.go b/gomock/controller_test.go index c0c4480c..3b32c03c 100644 --- a/gomock/controller_test.go +++ b/gomock/controller_test.go @@ -159,6 +159,16 @@ func createFixtures(t *testing.T) (reporter *ErrorReporter, ctrl *gomock.Control // successful or failed. reporter = NewErrorReporter(t) ctrl = gomock.NewController(reporter) + ctrl.LooseMode = false + return +} + +func createLooseFixtures(t *testing.T) (reporter *ErrorReporter, ctrl *gomock.Controller) { + // Same as above only this one enables LooseMode which won't + // fail for unexpected calls + reporter = NewErrorReporter(t) + ctrl = gomock.NewController(reporter) + ctrl.LooseMode = true return } @@ -170,7 +180,7 @@ func TestNoCalls(t *testing.T) { func TestNoRecordedCallsForAReceiverStrictMode(t *testing.T) { reporter, ctrl := createFixtures(t) - // ctrl.LooseMode = true + ctrl.LooseMode = false subject := new(Subject) reporter.assertFatal(func() { @@ -180,18 +190,18 @@ func TestNoRecordedCallsForAReceiverStrictMode(t *testing.T) { } func TestNoRecordedCallsForAReceiverLooseMode(t *testing.T) { - reporter, ctrl := createFixtures(t) - ctrl.LooseMode = true + reporter, ctrl := createLooseFixtures(t) subject := new(Subject) - ctrl.Call(subject, "FooMethod", "argument") + ctrl.Call(subject, "NotRecordedMethod", "argument") + ctrl.Finish() reporter.assertPass("No calls expected but LooseMode does not cause failures") - ctrl.Finish() } -func TestNoRecordedMatchingMethodNameForAReceiver(t *testing.T) { +func TestNoRecordedMatchingMethodNameForAReceiverStrictMode(t *testing.T) { reporter, ctrl := createFixtures(t) + ctrl.LooseMode = false subject := new(Subject) ctrl.RecordCall(subject, "FooMethod", "argument") @@ -204,6 +214,20 @@ func TestNoRecordedMatchingMethodNameForAReceiver(t *testing.T) { }) } +func TestNoRecordedMatchingMethodNameForAReceiverLooseMode(t *testing.T) { + reporter, ctrl := createLooseFixtures(t) + subject := new(Subject) + + ctrl.RecordCall(subject, "FooMethod", "argument") + // reporter.assertFatal(func() { + ctrl.Call(subject, "NotRecordedMethod", "argument") + // }, "Unexpected call to", "there are no expected calls of the method \"NotRecordedMethod\" for that receiver") + reporter.assertFatal(func() { + // The expected call wasn't made. + ctrl.Finish() + }) +} + // This tests that a call with an arguments of some primitive type matches a recorded call. func TestExpectedMethodCall(t *testing.T) { reporter, ctrl := createFixtures(t) @@ -216,8 +240,9 @@ func TestExpectedMethodCall(t *testing.T) { reporter.assertPass("Expected method call made.") } -func TestUnexpectedMethodCall(t *testing.T) { +func TestUnexpectedMethodCallStrict(t *testing.T) { reporter, ctrl := createFixtures(t) + ctrl.LooseMode = false subject := new(Subject) reporter.assertFatal(func() { @@ -227,7 +252,17 @@ func TestUnexpectedMethodCall(t *testing.T) { ctrl.Finish() } -func TestRepeatedCall(t *testing.T) { +func TestUnexpectedMethodCallLoose(t *testing.T) { + reporter, ctrl := createLooseFixtures(t) + subject := new(Subject) + + ctrl.Call(subject, "FooMethod", "argument") + reporter.assertPass("No expected calls in loose mode are allowed") + + ctrl.Finish() +} + +func TestRepeatedCallStrict(t *testing.T) { reporter, ctrl := createFixtures(t) subject := new(Subject) @@ -243,6 +278,22 @@ func TestRepeatedCall(t *testing.T) { reporter.assertFail("After calling one too many times.") } +func TestRepeatedCallLoose(t *testing.T) { + reporter, ctrl := createLooseFixtures(t) + subject := new(Subject) + + ctrl.RecordCall(subject, "FooMethod", "argument").Times(3) + ctrl.Call(subject, "FooMethod", "argument") + ctrl.Call(subject, "FooMethod", "argument") + ctrl.Call(subject, "FooMethod", "argument") + reporter.assertPass("After expected repeated method calls.") + reporter.assertFatal(func() { + ctrl.Call(subject, "FooMethod", "argument") + }) + ctrl.Finish() + reporter.assertFail("After calling one too many times.") +} + func TestUnexpectedArgCount(t *testing.T) { reporter, ctrl := createFixtures(t) defer reporter.recoverUnexpectedFatal() From 1499fae2b5d202a1b5642276eab04f0151848be1 Mon Sep 17 00:00:00 2001 From: Jacob Meixner Date: Sat, 12 May 2018 15:25:07 -0700 Subject: [PATCH 04/12] Clean up some test code --- gomock/controller.go | 1 + gomock/controller_test.go | 5 ----- 2 files changed, 1 insertion(+), 5 deletions(-) diff --git a/gomock/controller.go b/gomock/controller.go index 9b5ae1a3..12c5cf4c 100644 --- a/gomock/controller.go +++ b/gomock/controller.go @@ -167,6 +167,7 @@ func (ctrl *Controller) Call(receiver interface{}, method string, args ...interf // ctrl.t.Fatalf("Unexpected call to %T.%v(%v) at %s because: %s", receiver, method, args, origin, err) // } // fmt.Println("How am I here") + // this is to protect against nil dereference for calls that are not // expected if expected == nil && ctrl.LooseMode { diff --git a/gomock/controller_test.go b/gomock/controller_test.go index 3b32c03c..b946ad62 100644 --- a/gomock/controller_test.go +++ b/gomock/controller_test.go @@ -180,7 +180,6 @@ func TestNoCalls(t *testing.T) { func TestNoRecordedCallsForAReceiverStrictMode(t *testing.T) { reporter, ctrl := createFixtures(t) - ctrl.LooseMode = false subject := new(Subject) reporter.assertFatal(func() { @@ -201,7 +200,6 @@ func TestNoRecordedCallsForAReceiverLooseMode(t *testing.T) { func TestNoRecordedMatchingMethodNameForAReceiverStrictMode(t *testing.T) { reporter, ctrl := createFixtures(t) - ctrl.LooseMode = false subject := new(Subject) ctrl.RecordCall(subject, "FooMethod", "argument") @@ -219,9 +217,7 @@ func TestNoRecordedMatchingMethodNameForAReceiverLooseMode(t *testing.T) { subject := new(Subject) ctrl.RecordCall(subject, "FooMethod", "argument") - // reporter.assertFatal(func() { ctrl.Call(subject, "NotRecordedMethod", "argument") - // }, "Unexpected call to", "there are no expected calls of the method \"NotRecordedMethod\" for that receiver") reporter.assertFatal(func() { // The expected call wasn't made. ctrl.Finish() @@ -242,7 +238,6 @@ func TestExpectedMethodCall(t *testing.T) { func TestUnexpectedMethodCallStrict(t *testing.T) { reporter, ctrl := createFixtures(t) - ctrl.LooseMode = false subject := new(Subject) reporter.assertFatal(func() { From e95b3563c0e7f743b3d2f1f813555de20415b1e9 Mon Sep 17 00:00:00 2001 From: Jacob Meixner Date: Sat, 12 May 2018 15:44:44 -0700 Subject: [PATCH 05/12] Clean up code to check for call errors --- gomock/controller.go | 15 ++------------- 1 file changed, 2 insertions(+), 13 deletions(-) diff --git a/gomock/controller.go b/gomock/controller.go index 12c5cf4c..7d53d8b3 100644 --- a/gomock/controller.go +++ b/gomock/controller.go @@ -149,25 +149,14 @@ func (ctrl *Controller) Call(receiver interface{}, method string, args ...interf var actions []func([]interface{}) []interface{} expected, err := ctrl.expectedCalls.FindMatch(receiver, method, args) - // fmt.Printf("Here is the result: '%v','%v'\n", expected, err) - // fmt.Printf("This is: %v!!\n", err.Error() == "there are no expected calls of the method \"FooMethod\" for that receiver") if err != nil { - ok, _ := regexp.MatchString("there are no expected calls of the method \".*\" for that receiver", err.Error()) - if !ok || !ctrl.LooseMode{ + isUnexpectedCallsError, _ := regexp.MatchString("there are no expected calls of the method \".*\" for that receiver", err.Error()) + if !isUnexpectedCallsError || !ctrl.LooseMode { origin := callerInfo(2) ctrl.t.Fatalf("Unexpected call to %T.%v(%v) at %s because: %s", receiver, method, args, origin, err) - // fmt.Println("This is an not unexpected call") } - // if } - // if err != nil && !ctrl.LooseMode { - // // fmt.Println("where I expect to be") - // origin := callerInfo(2) - // ctrl.t.Fatalf("Unexpected call to %T.%v(%v) at %s because: %s", receiver, method, args, origin, err) - // } - // fmt.Println("How am I here") - // this is to protect against nil dereference for calls that are not // expected if expected == nil && ctrl.LooseMode { From be4106fd5f40686656a3ee5af7e9c6747ebecdb8 Mon Sep 17 00:00:00 2001 From: Jacob Meixner Date: Sun, 13 May 2018 14:16:55 -0700 Subject: [PATCH 06/12] Fix the go formatting I thought that VS Code did this for me but for some reason it seemed to miss this one thing --- gomock/controller_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gomock/controller_test.go b/gomock/controller_test.go index b946ad62..f7203566 100644 --- a/gomock/controller_test.go +++ b/gomock/controller_test.go @@ -164,7 +164,7 @@ func createFixtures(t *testing.T) (reporter *ErrorReporter, ctrl *gomock.Control } func createLooseFixtures(t *testing.T) (reporter *ErrorReporter, ctrl *gomock.Controller) { - // Same as above only this one enables LooseMode which won't + // Same as above only this one enables LooseMode which won't // fail for unexpected calls reporter = NewErrorReporter(t) ctrl = gomock.NewController(reporter) From 1179e2bffb2dbf4a1fb212b0a54164e20f1dff37 Mon Sep 17 00:00:00 2001 From: Jacob Meixner Date: Sat, 2 Jun 2018 10:54:08 -0700 Subject: [PATCH 07/12] Add a few tests One will explicitly assert what I meant for it too and there is a new one to catch the fact that there are more possible matching errors that exist which must be handled accordingly --- gomock/controller_test.go | 40 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 40 insertions(+) diff --git a/gomock/controller_test.go b/gomock/controller_test.go index f7203566..47536590 100644 --- a/gomock/controller_test.go +++ b/gomock/controller_test.go @@ -224,6 +224,46 @@ func TestNoRecordedMatchingMethodNameForAReceiverLooseMode(t *testing.T) { }) } +func TestMakingAnUnMatchingCallWhereSpecificCallsAreExpectedLooseMode(t *testing.T) { + reporter, ctrl := createLooseFixtures(t) + subject := new(Subject) + + ctrl.RecordCall(subject, "FooMethod", "argument") + ctrl.Call(subject, "NotRecordedMethod", "argument") + reporter.assertFatal(func() { + ctrl.Call(subject, "FooMethod", "argument", "more arg") + }) + reporter.assertFatal(func() { + ctrl.Call(subject, "FooMethod", "argument1000") + }) + ctrl.Call(subject, "FooMethod", "argument") + reporter.assertPass("Expected method call made eventually") +} + +func TestMakingAnUnexpectedCallWhereCallsAreExpectedLooseMode(t *testing.T) { + reporter, ctrl := createLooseFixtures(t) + subject := new(Subject) + + ctrl.RecordCall(subject, "FooMethod", "argument") + ctrl.Call(subject, "NotRecordedMethod", "argument") + ctrl.Call(subject, "FooMethod", "argument") + reporter.assertPass("Expected method call made eventually") +} + +func TestMakingAnUnexpectedCallWhereCallsAreExpectedStrictMode(t *testing.T) { + reporter, ctrl := createFixtures(t) + subject := new(Subject) + + ctrl.RecordCall(subject, "FooMethod", "argument") + reporter.assertFatal(func() { + ctrl.Call(subject, "NotRecordedMethod", "argument") + }) + ctrl.Call(subject, "FooMethod", "argument") + ctrl.Finish() + reporter.assertFail("Expected method call made eventually but only after other unexecpted " + + "calls (This is not permitted in loose mode)") +} + // This tests that a call with an arguments of some primitive type matches a recorded call. func TestExpectedMethodCall(t *testing.T) { reporter, ctrl := createFixtures(t) From eb129e41013cfbfcd93145d421f1303c6416377c Mon Sep 17 00:00:00 2001 From: Jacob Meixner Date: Sat, 2 Jun 2018 11:46:56 -0700 Subject: [PATCH 08/12] Tweak the tests In the previous commit I have a test which is broken partially because it is and partially because it is the wrong test. This is fixed in this commit. As well as another test which discovered during development. --- gomock/controller_test.go | 24 +++++++++++++++++------- 1 file changed, 17 insertions(+), 7 deletions(-) diff --git a/gomock/controller_test.go b/gomock/controller_test.go index 47536590..62e0fc98 100644 --- a/gomock/controller_test.go +++ b/gomock/controller_test.go @@ -224,20 +224,30 @@ func TestNoRecordedMatchingMethodNameForAReceiverLooseMode(t *testing.T) { }) } -func TestMakingAnUnMatchingCallWhereSpecificCallsAreExpectedLooseMode(t *testing.T) { +func TestMakingUnMatchingCallWhereASpecificCallAreExpectedLooseMode(t *testing.T) { reporter, ctrl := createLooseFixtures(t) subject := new(Subject) ctrl.RecordCall(subject, "FooMethod", "argument") ctrl.Call(subject, "NotRecordedMethod", "argument") + ctrl.Call(subject, "FooMethod", "argument", "morearg2") + ctrl.Call(subject, "FooMethod", "argument1000") + ctrl.Call(subject, "FooMethod", "argument") + reporter.assertPass("Expected method call never made") +} + +func TestMakingUnMatchingCallWhereSpecificCallsAreExpectedNTimesLooseMode(t *testing.T) { + reporter, ctrl := createLooseFixtures(t) + subject := new(Subject) + + ctrl.RecordCall(subject, "FooMethod", "argument") + ctrl.Call(subject, "NotRecordedMethod", "argument") + ctrl.Call(subject, "FooMethod", "argument", "morearg2") + ctrl.Call(subject, "FooMethod", "argument1000") + ctrl.Call(subject, "FooMethod", "argument") reporter.assertFatal(func() { - ctrl.Call(subject, "FooMethod", "argument", "more arg") - }) - reporter.assertFatal(func() { - ctrl.Call(subject, "FooMethod", "argument1000") + ctrl.Call(subject, "FooMethod", "argument") }) - ctrl.Call(subject, "FooMethod", "argument") - reporter.assertPass("Expected method call made eventually") } func TestMakingAnUnexpectedCallWhereCallsAreExpectedLooseMode(t *testing.T) { From d4b107314494f996909c896d5586fb0a6e0d244d Mon Sep 17 00:00:00 2001 From: Jacob Meixner Date: Sat, 2 Jun 2018 12:13:28 -0700 Subject: [PATCH 09/12] Get it working After adding some failing test cases I made the necessary changes to get it working now some refactor --- gomock/callset.go | 15 ++++++++++----- gomock/callset_test.go | 3 ++- gomock/controller.go | 10 +++------- 3 files changed, 15 insertions(+), 13 deletions(-) diff --git a/gomock/callset.go b/gomock/callset.go index c44a8a58..1a324d59 100644 --- a/gomock/callset.go +++ b/gomock/callset.go @@ -63,7 +63,7 @@ func (cs callSet) Remove(call *Call) { } // FindMatch searches for a matching call. Returns error with explanation message if no call matched. -func (cs callSet) FindMatch(receiver interface{}, method string, args []interface{}) (*Call, error) { +func (cs callSet) FindMatch(receiver interface{}, method string, args []interface{}, isLooseMode bool) (*Call, error) { key := callSetKey{receiver, method} // Search through the expected calls. @@ -71,9 +71,9 @@ func (cs callSet) FindMatch(receiver interface{}, method string, args []interfac var callsErrors bytes.Buffer for _, call := range expected { err := call.matches(args) - if err != nil { + if err != nil && !isLooseMode { fmt.Fprintf(&callsErrors, "\n%v", err) - } else { + } else if err == nil { return call, nil } } @@ -87,11 +87,16 @@ func (cs callSet) FindMatch(receiver interface{}, method string, args []interfac } } - if len(expected)+len(exhausted) == 0 { + if len(expected)+len(exhausted) == 0 && !isLooseMode { fmt.Fprintf(&callsErrors, "there are no expected calls of the method %q for that receiver", method) } - return nil, fmt.Errorf(callsErrors.String()) + errString := callsErrors.String() + if errString == "" { + return nil, nil + } else { + return nil, fmt.Errorf(callsErrors.String()) + } } // Failures returns the calls that are not satisfied. diff --git a/gomock/callset_test.go b/gomock/callset_test.go index 7fc711a4..0c39a1bd 100644 --- a/gomock/callset_test.go +++ b/gomock/callset_test.go @@ -33,7 +33,8 @@ func TestCallSetAdd(t *testing.T) { cs.Add(newCall(t, receiver, method, reflect.TypeOf(receiverType{}.Func))) } - call, err := cs.FindMatch(receiver, method, []interface{}{}) + isLoose := false + call, err := cs.FindMatch(receiver, method, []interface{}{}, isLoose) if err != nil { t.Fatalf("FindMatch: %v", err) } diff --git a/gomock/controller.go b/gomock/controller.go index 7d53d8b3..d3c6e35e 100644 --- a/gomock/controller.go +++ b/gomock/controller.go @@ -58,7 +58,6 @@ package gomock import ( "fmt" "reflect" - "regexp" "runtime" "sync" @@ -148,13 +147,10 @@ func (ctrl *Controller) Call(receiver interface{}, method string, args ...interf defer ctrl.mu.Unlock() var actions []func([]interface{}) []interface{} - expected, err := ctrl.expectedCalls.FindMatch(receiver, method, args) + expected, err := ctrl.expectedCalls.FindMatch(receiver, method, args, ctrl.LooseMode) if err != nil { - isUnexpectedCallsError, _ := regexp.MatchString("there are no expected calls of the method \".*\" for that receiver", err.Error()) - if !isUnexpectedCallsError || !ctrl.LooseMode { - origin := callerInfo(2) - ctrl.t.Fatalf("Unexpected call to %T.%v(%v) at %s because: %s", receiver, method, args, origin, err) - } + origin := callerInfo(2) + ctrl.t.Fatalf("Unexpected call to %T.%v(%v) at %s because: %s", receiver, method, args, origin, err) } // this is to protect against nil dereference for calls that are not From fb03c774f66374c52406d21961e1e9ac4ad359df Mon Sep 17 00:00:00 2001 From: Jacob Meixner Date: Sat, 2 Jun 2018 13:09:33 -0700 Subject: [PATCH 10/12] Slight refactor to remove the need to pass booleans arround I have always undetstood passing booleans into functions to be a codesmell so instead of calling `FindMatch` with the isLoose flag I have opted to call two different functions bassed on weather the isLoose flag is set. This chould be taken one step further and be made part of the `CallSet` itself but was not sure if that abstraction made sense --- gomock/callset.go | 34 ++++++++++++++++++++++++++++++---- gomock/callset_test.go | 3 +-- gomock/controller.go | 8 +++++++- 3 files changed, 38 insertions(+), 7 deletions(-) diff --git a/gomock/callset.go b/gomock/callset.go index 1a324d59..b6aa6448 100644 --- a/gomock/callset.go +++ b/gomock/callset.go @@ -63,7 +63,7 @@ func (cs callSet) Remove(call *Call) { } // FindMatch searches for a matching call. Returns error with explanation message if no call matched. -func (cs callSet) FindMatch(receiver interface{}, method string, args []interface{}, isLooseMode bool) (*Call, error) { +func (cs callSet) FindMatch(receiver interface{}, method string, args []interface{}) (*Call, error) { key := callSetKey{receiver, method} // Search through the expected calls. @@ -71,9 +71,9 @@ func (cs callSet) FindMatch(receiver interface{}, method string, args []interfac var callsErrors bytes.Buffer for _, call := range expected { err := call.matches(args) - if err != nil && !isLooseMode { + if err != nil { fmt.Fprintf(&callsErrors, "\n%v", err) - } else if err == nil { + } else { return call, nil } } @@ -87,10 +87,36 @@ func (cs callSet) FindMatch(receiver interface{}, method string, args []interfac } } - if len(expected)+len(exhausted) == 0 && !isLooseMode { + if len(expected)+len(exhausted) == 0 { fmt.Fprintf(&callsErrors, "there are no expected calls of the method %q for that receiver", method) } + return nil, fmt.Errorf(callsErrors.String()) +} + +// FindMatch searches for a matching call. Returns error with explanation message if no call matched. +func (cs callSet) FindLooseMatch(receiver interface{}, method string, args []interface{}) (*Call, error) { + key := callSetKey{receiver, method} + + // Search through the expected calls. + expected := cs.expected[key] + var callsErrors bytes.Buffer + for _, call := range expected { + err := call.matches(args) + if err == nil { + return call, nil + } + } + + // If we haven't found a match then search through the exhausted calls so we + // get useful error messages. + exhausted := cs.exhausted[key] + for _, call := range exhausted { + if err := call.matches(args); err != nil { + fmt.Fprintf(&callsErrors, "\n%v", err) + } + } + errString := callsErrors.String() if errString == "" { return nil, nil diff --git a/gomock/callset_test.go b/gomock/callset_test.go index 0c39a1bd..7fc711a4 100644 --- a/gomock/callset_test.go +++ b/gomock/callset_test.go @@ -33,8 +33,7 @@ func TestCallSetAdd(t *testing.T) { cs.Add(newCall(t, receiver, method, reflect.TypeOf(receiverType{}.Func))) } - isLoose := false - call, err := cs.FindMatch(receiver, method, []interface{}{}, isLoose) + call, err := cs.FindMatch(receiver, method, []interface{}{}) if err != nil { t.Fatalf("FindMatch: %v", err) } diff --git a/gomock/controller.go b/gomock/controller.go index d3c6e35e..dc9d1f15 100644 --- a/gomock/controller.go +++ b/gomock/controller.go @@ -147,7 +147,13 @@ func (ctrl *Controller) Call(receiver interface{}, method string, args ...interf defer ctrl.mu.Unlock() var actions []func([]interface{}) []interface{} - expected, err := ctrl.expectedCalls.FindMatch(receiver, method, args, ctrl.LooseMode) + var expected *Call + var err error + if !ctrl.LooseMode { + expected, err = ctrl.expectedCalls.FindMatch(receiver, method, args) + } else { + expected, err = ctrl.expectedCalls.FindLooseMatch(receiver, method, args) + } if err != nil { origin := callerInfo(2) ctrl.t.Fatalf("Unexpected call to %T.%v(%v) at %s because: %s", receiver, method, args, origin, err) From a4f6f46bc761c1ad60ac5050d3dda0ade1d46c40 Mon Sep 17 00:00:00 2001 From: Jacob Meixner Date: Sat, 2 Jun 2018 13:20:39 -0700 Subject: [PATCH 11/12] Add test for ordering in LooseMode After some further dive down I have realized that After does not work in LooseMode assuming we want to maintain a close to exact feature set I have added a test to demonstrate this failing behavior. Fix to come. --- gomock/controller_test.go | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/gomock/controller_test.go b/gomock/controller_test.go index 62e0fc98..15f9f17f 100644 --- a/gomock/controller_test.go +++ b/gomock/controller_test.go @@ -229,7 +229,6 @@ func TestMakingUnMatchingCallWhereASpecificCallAreExpectedLooseMode(t *testing.T subject := new(Subject) ctrl.RecordCall(subject, "FooMethod", "argument") - ctrl.Call(subject, "NotRecordedMethod", "argument") ctrl.Call(subject, "FooMethod", "argument", "morearg2") ctrl.Call(subject, "FooMethod", "argument1000") ctrl.Call(subject, "FooMethod", "argument") @@ -241,7 +240,6 @@ func TestMakingUnMatchingCallWhereSpecificCallsAreExpectedNTimesLooseMode(t *tes subject := new(Subject) ctrl.RecordCall(subject, "FooMethod", "argument") - ctrl.Call(subject, "NotRecordedMethod", "argument") ctrl.Call(subject, "FooMethod", "argument", "morearg2") ctrl.Call(subject, "FooMethod", "argument1000") ctrl.Call(subject, "FooMethod", "argument") @@ -260,6 +258,20 @@ func TestMakingAnUnexpectedCallWhereCallsAreExpectedLooseMode(t *testing.T) { reporter.assertPass("Expected method call made eventually") } +func TestMakingExpectedCallsInOrderLooseMode(t *testing.T) { + reporter, ctrl := createLooseFixtures(t) + subject := new(Subject) + + ctrl.RecordCall(subject, "FooMethod", "argument2").After( + ctrl.RecordCall(subject, "FooMethod", "argument"), + ) + reporter.assertFatal(func() { + ctrl.Call(subject, "FooMethod", "argument2") + ctrl.Call(subject, "FooMethod", "argument") + }) + reporter.assertFail("Expected method calls in order even in LooseMode") +} + func TestMakingAnUnexpectedCallWhereCallsAreExpectedStrictMode(t *testing.T) { reporter, ctrl := createFixtures(t) subject := new(Subject) From cab11cc367d9fade2271df860be406ca5449c3a3 Mon Sep 17 00:00:00 2001 From: Jacob Meixner Date: Sat, 2 Jun 2018 13:58:38 -0700 Subject: [PATCH 12/12] Add method to check Call preReqs and take the check out of match weather a calls prereqs are met doesn't seem semntically signifigant to weather it is a match so I removed that as a dependancy and now call it in the `callSet`s `FindMatch` methods which allows each version to decide if that is necessary. This change enables us to fix the fact that `After` does not work with LooseMode --- gomock/call.go | 16 +++++++++------- gomock/callset.go | 8 +++++++- 2 files changed, 16 insertions(+), 8 deletions(-) diff --git a/gomock/call.go b/gomock/call.go index a3fa1ae4..4a8a11f1 100644 --- a/gomock/call.go +++ b/gomock/call.go @@ -380,19 +380,21 @@ func (c *Call) matches(args []interface{}) error { } } - // Check that all prerequisite calls have been satisfied. + // Check that the call is not exhausted. + if c.exhausted() { + return fmt.Errorf("Expected call at %s has already been called the max number of times.", c.origin) + } + + return nil +} + +func (c *Call) arePreReqsSatisfied() error { for _, preReqCall := range c.preReqs { if !preReqCall.satisfied() { return fmt.Errorf("Expected call at %s doesn't have a prerequisite call satisfied:\n%v\nshould be called before:\n%v", c.origin, preReqCall, c) } } - - // Check that the call is not exhausted. - if c.exhausted() { - return fmt.Errorf("Expected call at %s has already been called the max number of times.", c.origin) - } - return nil } diff --git a/gomock/callset.go b/gomock/callset.go index b6aa6448..1556204a 100644 --- a/gomock/callset.go +++ b/gomock/callset.go @@ -73,6 +73,8 @@ func (cs callSet) FindMatch(receiver interface{}, method string, args []interfac err := call.matches(args) if err != nil { fmt.Fprintf(&callsErrors, "\n%v", err) + } else if err := call.arePreReqsSatisfied(); err != nil { + fmt.Fprintf(&callsErrors, "\n%v", err) } else { return call, nil } @@ -103,7 +105,11 @@ func (cs callSet) FindLooseMatch(receiver interface{}, method string, args []int var callsErrors bytes.Buffer for _, call := range expected { err := call.matches(args) - if err == nil { + if err != nil { + continue + } else if err = call.arePreReqsSatisfied(); err != nil { + fmt.Fprintf(&callsErrors, "\n%v", err) + } else { return call, nil } }