stretchr
mock: Deadlock if mocking String()
This is a fairly contrived example, I admit. But this was a real head-scratcher in the wild.
The following code will never complete.
package main
import (
"fmt"
"github.com/stretchr/testify/mock"
)
type K struct {
mock.Mock
}
func (f *K) String() string {
return f.Called().Get(0).(string)
}
func (f *K) Foo(b B) string {
return f.Called(b).Get(0).(string)
}
type B struct {
Kay *K
}
func main() {
k := &K{}
b := B{
Kay: k,
}
k.On("String").Return("string-return")
k.On("Foo", b).Return("foo-return")
fmt.Printf(k.Foo(b))
}
This fails because Mock.Called calls Mock.MethodCalled, which locks the Mutex, and eventually calls Arguments.Diff. When Arguments.Diff calls fmt.Sprintf, fmt.Sprintf sees the Kay field of B. Since K implements fmt.Stringer, fmt.Sprintf calls K.String. Since this is a mocked function, K.String jumps into Mock.Called, which calls Mock.MethodCalled which waits for the still-locked mutex to unlock, which never happens, because the lock is further up the call chain.
I don't know if this should be considered a bug or not, but it certainly was unexpected, and took a lot of digging to figure out.
Here's a stack trace from a debug session just before the mutex lock deadlock
github.com/stretchr/testify/mock.(*Mock).MethodCalled (github.com/stretchr/testify/mock/mock.go:318)
github.com/stretchr/testify/mock.(*Mock).Called (github.com/stretchr/testify/mock/mock.go:310)
main.(*K).String (main.go)
fmt.(*pp).handleMethods (/usr/local/Cellar/go/1.10/libexec/src/fmt/print.go:603)
fmt.(*pp).printValue (/usr/local/Cellar/go/1.10/libexec/src/fmt/print.go:700)
fmt.(*pp).printValue (/usr/local/Cellar/go/1.10/libexec/src/fmt/print.go:783)
fmt.(*pp).printValue (/usr/local/Cellar/go/1.10/libexec/src/fmt/print.go:853)
fmt.(*pp).printArg (/usr/local/Cellar/go/1.10/libexec/src/fmt/print.go:689)
fmt.(*pp).doPrintf (/usr/local/Cellar/go/1.10/libexec/src/fmt/print.go:1003)
fmt.Sprintf (/usr/local/Cellar/go/1.10/libexec/src/fmt/print.go:203)
github.com/stretchr/testify/mock.Arguments.Diff (github.com/stretchr/testify/mock/mock.go:675)
github.com/stretchr/testify/mock.(*Mock).findExpectedCall (github.com/stretchr/testify/mock/mock.go:242)
github.com/stretchr/testify/mock.(*Mock).MethodCalled (github.com/stretchr/testify/mock/mock.go:319)
github.com/stretchr/testify/mock.(*Mock).Called (github.com/stretchr/testify/mock/mock.go:310)
main.(*K).Foo (main.go)
main.main (main.go)
v1.11.0 resolved this in the case where the test passes because the formatting of non-matching arguments is now lazy. It will still deadlock in the case where the call is not expected.
Situations like this are why I do not like testify deferring to the stringer interface to format values for our test failure messages, we should use spew here. I think using spew would not resolve the race in #1597
It is likely that v1.11.1 will un-fix the passing version of this test case.
I think this can be fixed by using spew rather than fmt to format the expected and actual mock arguments. We should also lazily render these properly.