Skip to content

Commit

Permalink
fix: stringer deadlock uber-go#116
Browse files Browse the repository at this point in the history
  • Loading branch information
nbgraham committed Jan 19, 2024
1 parent aaa9d14 commit 2f9e900
Show file tree
Hide file tree
Showing 6 changed files with 74 additions and 2 deletions.
6 changes: 5 additions & 1 deletion gomock/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -206,7 +206,11 @@ func (ctrl *Controller) Call(receiver any, method string, args ...any) []any {
// and this line changes, i.e. this code is wrapped in another anonymous function.
// 0 is us, 1 is controller.Call(), 2 is the generated mock, and 3 is the user's test.
origin := callerInfo(3)
ctrl.T.Fatalf("Unexpected call to %T.%v(%v) at %s because: %s", receiver, method, args, origin, err)
stringArgs := make([]string, len(args))
for i, arg := range args {
stringArgs[i] = getString(arg)
}
ctrl.T.Fatalf("Unexpected call to %T.%v(%v) at %s because: %s", receiver, method, stringArgs, origin, err)
}

// Two things happen here:
Expand Down
16 changes: 16 additions & 0 deletions gomock/controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -203,6 +203,22 @@ func TestNoRecordedMatchingMethodNameForAReceiver(t *testing.T) {
})
}

func TestNoStringerDeadlockOnError(t *testing.T) {
reporter, ctrl := createFixtures(t)
subject := new(Subject)
mockFoo := NewMockFoo(ctrl)
var _ fmt.Stringer = mockFoo

ctrl.RecordCall(subject, "FooMethod", mockFoo)
reporter.assertFatal(func() {
ctrl.Call(subject, "NotRecordedMethod", mockFoo)
}, "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)
Expand Down
2 changes: 1 addition & 1 deletion gomock/matchers.go
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,7 @@ func (e eqMatcher) Matches(x any) bool {
}

func (e eqMatcher) String() string {
return fmt.Sprintf("is equal to %v (%T)", e.x, e.x)
return fmt.Sprintf("is equal to %s (%T)", getString(e.x), e.x)
}

type nilMatcher struct{}
Expand Down
19 changes: 19 additions & 0 deletions gomock/mock_test.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

25 changes: 25 additions & 0 deletions gomock/string.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
package gomock

import "fmt"

type mockInstance interface {
ISGOMOCK() struct{}
}
type mockedStringer interface {
fmt.Stringer
mockInstance
}

// getString is a safe way to convert a value to a string for printing results
// If the value is a a mock, getString avoids calling the mocked String() method,
// which avoids potential deadlocks
func getString(x any) string {
switch v := x.(type) {
case mockedStringer:
return fmt.Sprintf("%T", v)
case fmt.Stringer:
return v.String()
default:
return fmt.Sprintf("%v", v)
}
}
8 changes: 8 additions & 0 deletions mockgen/mockgen.go
Original file line number Diff line number Diff line change
Expand Up @@ -502,6 +502,14 @@ func (g *generator) GenerateMockInterface(intf *model.Interface, outputPackagePa
g.out()
g.p("}")

// XXX: possible name collision here if someone has ISGOMOCK in their interface.
g.p("// ISGOMOCK indicates that this struct is a gomock mock.")
g.p("func (m *%v%v) ISGOMOCK() struct{} {", mockType, shortTp)
g.in()
g.p("return struct{}{}")
g.out()
g.p("}")

g.GenerateMockMethods(mockType, intf, outputPackagePath, longTp, shortTp, *typed)

return nil
Expand Down

0 comments on commit 2f9e900

Please sign in to comment.