From bd33a982fdb971421c9f537e17e68d129c5af209 Mon Sep 17 00:00:00 2001 From: eolmedo Date: Wed, 6 Sep 2023 17:41:01 +0000 Subject: [PATCH 1/5] Allow type-safe calls to be used in InOrder This modifies `InOrder` to receive variadic `any` instead of `*Call`, allowing users to use this function with type-safe generated Calls. This implementation uses a type assertion to check if the provided arguments are `*Call`s or reflection to get the `*Call` when the arguments wrap one (generated code). If neither of the two cases are fullfiled then that argument is ignored. Fix #70. --- gomock/call.go | 30 ++++- gomock/call_test.go | 26 ++++ mockgen/internal/tests/typed_inorder/input.go | 14 +++ .../tests/typed_inorder/input_test.go | 26 ++++ mockgen/internal/tests/typed_inorder/mock.go | 114 ++++++++++++++++++ 5 files changed, 209 insertions(+), 1 deletion(-) create mode 100644 mockgen/internal/tests/typed_inorder/input.go create mode 100644 mockgen/internal/tests/typed_inorder/input_test.go create mode 100644 mockgen/internal/tests/typed_inorder/mock.go diff --git a/gomock/call.go b/gomock/call.go index b8a06ac..5e13dc5 100644 --- a/gomock/call.go +++ b/gomock/call.go @@ -435,12 +435,40 @@ func (c *Call) call() []func([]any) []any { } // InOrder declares that the given calls should occur in order. -func InOrder(calls ...*Call) { +// Ignores every argument that isn't a *Call or wraps one. +func InOrder(args ...any) { + var calls []*Call + for i := 0; i < len(args); i++ { + if call := getCall(args[i]); call != nil { + calls = append(calls, call) + } + } for i := 1; i < len(calls); i++ { calls[i].After(calls[i-1]) } } +func getCall(arg any) *Call { + if call, ok := arg.(*Call); ok { + return call + } + t := reflect.ValueOf(arg) + if t.Kind() != reflect.Ptr && t.Kind() != reflect.Interface { + return nil + } + t = t.Elem() + for i := 0; i < t.NumField(); i++ { + f := t.Field(i) + if !f.CanInterface() { + continue + } + if call, ok := f.Interface().(*Call); ok { + return call + } + } + return nil +} + func setSlice(arg any, v reflect.Value) { va := reflect.ValueOf(arg) for i := 0; i < v.Len(); i++ { diff --git a/gomock/call_test.go b/gomock/call_test.go index 1e7d034..410f106 100644 --- a/gomock/call_test.go +++ b/gomock/call_test.go @@ -38,6 +38,10 @@ type b struct { foo string } +type c struct { + *Call +} + func (testObj b) Foo() string { return testObj.foo } @@ -616,3 +620,25 @@ func TestCall_DoAndReturn(t *testing.T) { }) } } + +func TestInOrder(t *testing.T) { + t.Run("process only *Call or its wrappers", func(t *testing.T) { + tr1 := &mockTestReporter{} + tr2 := &mockTestReporter{} + c1 := &Call{t: tr1} + c2 := &c{Call: &Call{t: tr2}} + b := &b{foo: "bar"} + a := a{name: "Joe"} + InOrder(c1, c2) // This is the only correct relationship + InOrder("a", c1) // Should do nothing + InOrder(a, c2) // Should do nothing + InOrder(nil, c2) // Should do nothing + InOrder(b, c2) // Should do nothing + if len(c2.preReqs) != 1 { + t.Fatalf("expected 1 preReq in c2, found %d", len(c2.preReqs)) + } + if len(c1.preReqs) != 0 { + t.Fatalf("expected 0 preReq in c1, found %d", len(c1.preReqs)) + } + }) +} diff --git a/mockgen/internal/tests/typed_inorder/input.go b/mockgen/internal/tests/typed_inorder/input.go new file mode 100644 index 0000000..9a96029 --- /dev/null +++ b/mockgen/internal/tests/typed_inorder/input.go @@ -0,0 +1,14 @@ +package typed_inorder + +//go:generate mockgen -package typed_inorder -source=input.go -destination=mock.go -typed +type Animal interface { + GetSound() string + Feed(string) error +} + +func Interact(a Animal, food string) (string, error){ + if err := a.Feed(food); err != nil { + return "", err + } + return a.GetSound(), nil +} diff --git a/mockgen/internal/tests/typed_inorder/input_test.go b/mockgen/internal/tests/typed_inorder/input_test.go new file mode 100644 index 0000000..5a0c190 --- /dev/null +++ b/mockgen/internal/tests/typed_inorder/input_test.go @@ -0,0 +1,26 @@ +package typed_inorder + +import ( + "testing" + "fmt" + "go.uber.org/mock/gomock" +) + +func TestInteract(t * testing.T) { + ctrl := gomock.NewController(t) + + mockAnimal := NewMockAnimal(ctrl) + gomock.InOrder( + mockAnimal.EXPECT().Feed("burguir").DoAndReturn(func(s string) error { + if s != "chocolate" { + return nil + } + return fmt.Errorf("Dogs can't eat chocolate!") + }), + mockAnimal.EXPECT().GetSound().Return("Woof!"), + ) + _, err := Interact(mockAnimal, "burguir") + if err != nil { + t.Fatalf("sad") + } +} diff --git a/mockgen/internal/tests/typed_inorder/mock.go b/mockgen/internal/tests/typed_inorder/mock.go new file mode 100644 index 0000000..39de997 --- /dev/null +++ b/mockgen/internal/tests/typed_inorder/mock.go @@ -0,0 +1,114 @@ +// Code generated by MockGen. DO NOT EDIT. +// Source: input.go +// +// Generated by this command: +// +// mockgen -package typed_inorder -source=input.go -destination=mock.go -typed +// +// Package typed_inorder is a generated GoMock package. +package typed_inorder + +import ( + reflect "reflect" + + gomock "go.uber.org/mock/gomock" +) + +// MockAnimal is a mock of Animal interface. +type MockAnimal struct { + ctrl *gomock.Controller + recorder *MockAnimalMockRecorder +} + +// MockAnimalMockRecorder is the mock recorder for MockAnimal. +type MockAnimalMockRecorder struct { + mock *MockAnimal +} + +// NewMockAnimal creates a new mock instance. +func NewMockAnimal(ctrl *gomock.Controller) *MockAnimal { + mock := &MockAnimal{ctrl: ctrl} + mock.recorder = &MockAnimalMockRecorder{mock} + return mock +} + +// EXPECT returns an object that allows the caller to indicate expected use. +func (m *MockAnimal) EXPECT() *MockAnimalMockRecorder { + return m.recorder +} + +// Feed mocks base method. +func (m *MockAnimal) Feed(arg0 string) error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "Feed", arg0) + ret0, _ := ret[0].(error) + return ret0 +} + +// Feed indicates an expected call of Feed. +func (mr *MockAnimalMockRecorder) Feed(arg0 any) *AnimalFeedCall { + mr.mock.ctrl.T.Helper() + call := mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Feed", reflect.TypeOf((*MockAnimal)(nil).Feed), arg0) + return &AnimalFeedCall{Call: call} +} + +// AnimalFeedCall wrap *gomock.Call +type AnimalFeedCall struct { + *gomock.Call +} + +// Return rewrite *gomock.Call.Return +func (c *AnimalFeedCall) Return(arg0 error) *AnimalFeedCall { + c.Call = c.Call.Return(arg0) + return c +} + +// Do rewrite *gomock.Call.Do +func (c *AnimalFeedCall) Do(f func(string) error) *AnimalFeedCall { + c.Call = c.Call.Do(f) + return c +} + +// DoAndReturn rewrite *gomock.Call.DoAndReturn +func (c *AnimalFeedCall) DoAndReturn(f func(string) error) *AnimalFeedCall { + c.Call = c.Call.DoAndReturn(f) + return c +} + +// GetSound mocks base method. +func (m *MockAnimal) GetSound() string { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetSound") + ret0, _ := ret[0].(string) + return ret0 +} + +// GetSound indicates an expected call of GetSound. +func (mr *MockAnimalMockRecorder) GetSound() *AnimalGetSoundCall { + mr.mock.ctrl.T.Helper() + call := mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetSound", reflect.TypeOf((*MockAnimal)(nil).GetSound)) + return &AnimalGetSoundCall{Call: call} +} + +// AnimalGetSoundCall wrap *gomock.Call +type AnimalGetSoundCall struct { + *gomock.Call +} + +// Return rewrite *gomock.Call.Return +func (c *AnimalGetSoundCall) Return(arg0 string) *AnimalGetSoundCall { + c.Call = c.Call.Return(arg0) + return c +} + +// Do rewrite *gomock.Call.Do +func (c *AnimalGetSoundCall) Do(f func() string) *AnimalGetSoundCall { + c.Call = c.Call.Do(f) + return c +} + +// DoAndReturn rewrite *gomock.Call.DoAndReturn +func (c *AnimalGetSoundCall) DoAndReturn(f func() string) *AnimalGetSoundCall { + c.Call = c.Call.DoAndReturn(f) + return c +} From d4e8666228bff889a368d132d6e08996c7327f5b Mon Sep 17 00:00:00 2001 From: eolmedo Date: Thu, 7 Sep 2023 06:28:17 +0000 Subject: [PATCH 2/5] InOrder panics when no *Call provided This make `InOrder` to panic when any of its arguments isn't either a `*Call` or a mock generated type which should already embed `*Call`. --- gomock/call.go | 53 ++++++++++++++++++++++++--------------------- gomock/call_test.go | 21 ++++++++++++------ 2 files changed, 42 insertions(+), 32 deletions(-) diff --git a/gomock/call.go b/gomock/call.go index 5e13dc5..41ef643 100644 --- a/gomock/call.go +++ b/gomock/call.go @@ -435,38 +435,41 @@ func (c *Call) call() []func([]any) []any { } // InOrder declares that the given calls should occur in order. -// Ignores every argument that isn't a *Call or wraps one. +// It panics if the type of any of the arguments isn't *Call or a generated +// mock with an embedded *Call. func InOrder(args ...any) { - var calls []*Call - for i := 0; i < len(args); i++ { - if call := getCall(args[i]); call != nil { - calls = append(calls, call) - } - } + var calls []*Call + for i := 0; i < len(args); i++ { + if call := getCall(args[i]); call != nil { + calls = append(calls, call) + continue + } + panic("InOrder parameters type must be either a *Call or a generated mock with an embedded *Call") + } for i := 1; i < len(calls); i++ { calls[i].After(calls[i-1]) } } func getCall(arg any) *Call { - if call, ok := arg.(*Call); ok { - return call - } - t := reflect.ValueOf(arg) - if t.Kind() != reflect.Ptr && t.Kind() != reflect.Interface { - return nil - } - t = t.Elem() - for i := 0; i < t.NumField(); i++ { - f := t.Field(i) - if !f.CanInterface() { - continue - } - if call, ok := f.Interface().(*Call); ok { - return call - } - } - return nil + if call, ok := arg.(*Call); ok { + return call + } + t := reflect.ValueOf(arg) + if t.Kind() != reflect.Ptr && t.Kind() != reflect.Interface { + return nil + } + t = t.Elem() + for i := 0; i < t.NumField(); i++ { + f := t.Field(i) + if !f.CanInterface() { + continue + } + if call, ok := f.Interface().(*Call); ok { + return call + } + } + return nil } func setSlice(arg any, v reflect.Value) { diff --git a/gomock/call_test.go b/gomock/call_test.go index 410f106..489956a 100644 --- a/gomock/call_test.go +++ b/gomock/call_test.go @@ -627,13 +627,7 @@ func TestInOrder(t *testing.T) { tr2 := &mockTestReporter{} c1 := &Call{t: tr1} c2 := &c{Call: &Call{t: tr2}} - b := &b{foo: "bar"} - a := a{name: "Joe"} - InOrder(c1, c2) // This is the only correct relationship - InOrder("a", c1) // Should do nothing - InOrder(a, c2) // Should do nothing - InOrder(nil, c2) // Should do nothing - InOrder(b, c2) // Should do nothing + InOrder(c1, c2) if len(c2.preReqs) != 1 { t.Fatalf("expected 1 preReq in c2, found %d", len(c2.preReqs)) } @@ -641,4 +635,17 @@ func TestInOrder(t *testing.T) { t.Fatalf("expected 0 preReq in c1, found %d", len(c1.preReqs)) } }) + t.Run("panic when the argument isn't a *Call or has one embeded", func(t *testing.T) { + defer func() { + if r := recover(); r == nil { + t.Error("expected InOrder to panic") + } + }() + tr := &mockTestReporter{} + c := &Call{t: tr} + a := &a{ + name: "Foo", + } + InOrder(c, a) + }) } From e6e653d455ed79d4d74f700f12fe88639b5135e9 Mon Sep 17 00:00:00 2001 From: eolmedo Date: Fri, 8 Sep 2023 06:03:10 +0000 Subject: [PATCH 3/5] Add details to panic message for inorder This adds to the panic message the index and type of the first invalid type (if any) passed as parameter to `InOrder`. --- gomock/call.go | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/gomock/call.go b/gomock/call.go index 41ef643..2588277 100644 --- a/gomock/call.go +++ b/gomock/call.go @@ -444,7 +444,12 @@ func InOrder(args ...any) { calls = append(calls, call) continue } - panic("InOrder parameters type must be either a *Call or a generated mock with an embedded *Call") + errMsg := fmt.Sprintf( + "invalid argument at position %d of type %T, InOrder expects *gomock.Call or generated mock types with an embedded *gomock.Call", + i, + args[i], + ) + panic(errMsg) } for i := 1; i < len(calls); i++ { calls[i].After(calls[i-1]) From 0135608640d4cf78628bf87ca2fc11d309c98e63 Mon Sep 17 00:00:00 2001 From: eolmedo Date: Fri, 8 Sep 2023 06:20:16 +0000 Subject: [PATCH 4/5] go fmt test files --- mockgen/internal/tests/typed_inorder/input.go | 14 +++---- .../tests/typed_inorder/input_test.go | 39 ++++++++++--------- 2 files changed, 27 insertions(+), 26 deletions(-) diff --git a/mockgen/internal/tests/typed_inorder/input.go b/mockgen/internal/tests/typed_inorder/input.go index 9a96029..a1ed291 100644 --- a/mockgen/internal/tests/typed_inorder/input.go +++ b/mockgen/internal/tests/typed_inorder/input.go @@ -2,13 +2,13 @@ package typed_inorder //go:generate mockgen -package typed_inorder -source=input.go -destination=mock.go -typed type Animal interface { - GetSound() string - Feed(string) error + GetSound() string + Feed(string) error } -func Interact(a Animal, food string) (string, error){ - if err := a.Feed(food); err != nil { - return "", err - } - return a.GetSound(), nil +func Interact(a Animal, food string) (string, error) { + if err := a.Feed(food); err != nil { + return "", err + } + return a.GetSound(), nil } diff --git a/mockgen/internal/tests/typed_inorder/input_test.go b/mockgen/internal/tests/typed_inorder/input_test.go index 5a0c190..94fc5ed 100644 --- a/mockgen/internal/tests/typed_inorder/input_test.go +++ b/mockgen/internal/tests/typed_inorder/input_test.go @@ -1,26 +1,27 @@ package typed_inorder import ( - "testing" - "fmt" - "go.uber.org/mock/gomock" + "fmt" + "testing" + + "go.uber.org/mock/gomock" ) -func TestInteract(t * testing.T) { - ctrl := gomock.NewController(t) +func TestInteract(t *testing.T) { + ctrl := gomock.NewController(t) - mockAnimal := NewMockAnimal(ctrl) - gomock.InOrder( - mockAnimal.EXPECT().Feed("burguir").DoAndReturn(func(s string) error { - if s != "chocolate" { - return nil - } - return fmt.Errorf("Dogs can't eat chocolate!") - }), - mockAnimal.EXPECT().GetSound().Return("Woof!"), - ) - _, err := Interact(mockAnimal, "burguir") - if err != nil { - t.Fatalf("sad") - } + mockAnimal := NewMockAnimal(ctrl) + gomock.InOrder( + mockAnimal.EXPECT().Feed("burguir").DoAndReturn(func(s string) error { + if s != "chocolate" { + return nil + } + return fmt.Errorf("Dogs can't eat chocolate!") + }), + mockAnimal.EXPECT().GetSound().Return("Woof!"), + ) + _, err := Interact(mockAnimal, "burguir") + if err != nil { + t.Fatalf("sad") + } } From 1b3cb79c5f80f048818e700d921b114746f32726 Mon Sep 17 00:00:00 2001 From: Sung Yoon Whang Date: Wed, 13 Sep 2023 09:38:56 -0700 Subject: [PATCH 5/5] misc. fixes --- gomock/call.go | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/gomock/call.go b/gomock/call.go index 2588277..e1ea826 100644 --- a/gomock/call.go +++ b/gomock/call.go @@ -438,24 +438,25 @@ func (c *Call) call() []func([]any) []any { // It panics if the type of any of the arguments isn't *Call or a generated // mock with an embedded *Call. func InOrder(args ...any) { - var calls []*Call + calls := make([]*Call, 0, len(args)) for i := 0; i < len(args); i++ { if call := getCall(args[i]); call != nil { calls = append(calls, call) continue } - errMsg := fmt.Sprintf( + panic(fmt.Sprintf( "invalid argument at position %d of type %T, InOrder expects *gomock.Call or generated mock types with an embedded *gomock.Call", i, args[i], - ) - panic(errMsg) + )) } for i := 1; i < len(calls); i++ { calls[i].After(calls[i-1]) } } +// getCall checks if the parameter is a *Call or a generated struct +// that wraps a *Call and returns the *Call pointer - if neither, it returns nil. func getCall(arg any) *Call { if call, ok := arg.(*Call); ok { return call