stretchr
Using mock struct as arg to mocked method causes data race
If a mock struct with a method is passed as an argument to a method on another mock struct, and both methods are invoked simultaneously in separate goroutines, a data race occurs. In one goroutine, the first mock instance is written to update call counts or the list of calls on its method, while in another goroutine the second mock instance is logging its arguments, which includes a walk of all the data on the first instance.
For example, test the following files with go test -race:
main.go
package main
import "sync"
type IInterface1 interface {
SomeFunc1() error
}
type IInterface2 interface {
SomeFunc2(IInterface1) error
}
type Implementation1 struct {
}
func (i Implementation1) SomeFunc1() error {
return nil
}
type Implementation2 struct {
}
func (i Implementation2) SomeFunc2(i1 IInterface1) error {
return nil
}
func DoSomeStuff(i1 IInterface1, i2 IInterface2) error {
var wg sync.WaitGroup
wg.Add(1)
go func() {
for j := 0; j < 10000; j++ {
i2.SomeFunc2(i1)
}
wg.Done()
}()
for i := 0; i < 10000; i++ {
i1.SomeFunc1()
}
wg.Wait()
return nil
}
func main() {
var i1 Implementation1
var i2 Implementation2
_ = DoSomeStuff(i1, i2)
}
main_test.go
package main
import (
"testing"
"github.com/stretchr/testify/mock"
"github.com/stretchr/testify/suite"
)
type (
MockBugTestSuite struct {
suite.Suite
}
)
type IInterface1Mock struct {
mock.Mock
}
func (_m IInterface1Mock) SomeFunc1() error {
ret := _m.Called()
var r0 error
if rf, ok := ret.Get(0).(func() error); ok {
r0 = rf()
} else {
r0 = ret.Error(0)
}
return r0
}
type IInterface2Mock struct {
mock.Mock
}
func (_m IInterface2Mock) SomeFunc2(i1 IInterface1) error {
ret := _m.Called(i1)
var r0 error
if rf, ok := ret.Get(0).(func(IInterface1) error); ok {
r0 = rf(i1)
} else {
r0 = ret.Error(0)
}
return r0
}
func (s *MockBugTestSuite) TestDoSomeStuff() {
var i1 IInterface1Mock
var i2 IInterface2Mock
i1.On("SomeFunc1").Return(nil)
i2.On("SomeFunc2", mock.Anything).Return(nil)
err := DoSomeStuff(i1, i2)
s.Nil(err)
}
func TestPermTestSuite(t *testing.T) {
suite.Run(t, new(MockBugTestSuite))
}
I hit the same issue and prepared a test case for it, which I'll add here since it's a bit shorter than the original post. @lance-burton's description above is accurate.
func TestConcurrentArgumentRead(t *testing.T) {
methodUnderTest := func(c caller, u user) {
go u.Use(c)
c.Call()
}
c := &mockCaller{}
defer c.AssertExpectations(t)
u := &mockUser{}
defer u.AssertExpectations(t)
done := make(chan struct{})
c.On("Call").Return().Once()
u.On("Use", c).Return().Once().Run(func(args Arguments) { close(done) })
methodUnderTest(c, u)
<-done
}
Assuming the following interfaces and mocks:
type caller interface {
Call()
}
type mockCaller struct{ Mock }
func (m *mockCaller) Call() { m.Called() } // line
type user interface {
Use(caller)
}
type mockUser struct{ Mock }
func (m *mockUser) Use(c caller) { m.Called(c) }
The race checker output is: go test -race . -run TestConcurrentArgumentRead
WARNING: DATA RACE
Read at 0x00c0000d2338 by goroutine 7:
reflect.typedmemmove()
/usr/lib/go/src/runtime/mbarrier.go:177 +0x0
reflect.packEface()
/usr/lib/go/src/reflect/value.go:119 +0x103
reflect.valueInterface()
/usr/lib/go/src/reflect/value.go:1008 +0x16f
reflect.Value.Interface()
/usr/lib/go/src/reflect/value.go:978 +0x51
fmt.(*pp).printValue()
/usr/lib/go/src/fmt/print.go:699 +0x3b32
fmt.(*pp).printValue()
/usr/lib/go/src/fmt/print.go:853 +0x27ae
fmt.(*pp).printArg()
/usr/lib/go/src/fmt/print.go:689 +0x2fb
fmt.(*pp).doPrintf()
/usr/lib/go/src/fmt/print.go:1099 +0x93f
fmt.Sprintf()
/usr/lib/go/src/fmt/print.go:203 +0x73
github.com/stretchr/testify/mock.Arguments.Diff()
/home/neil/go/src/github.com/stretchr/testify/mock/mock.go:678 +0x1001
github.com/stretchr/testify/mock.(*Mock).findExpectedCall()
/home/neil/go/src/github.com/stretchr/testify/mock/mock.go:267 +0x177
github.com/stretchr/testify/mock.(*Mock).MethodCalled()
/home/neil/go/src/github.com/stretchr/testify/mock/mock.go:343 +0xb3
github.com/stretchr/testify/mock.(*Mock).Called()
/home/neil/go/src/github.com/stretchr/testify/mock/mock.go:333 +0x18c
github.com/stretchr/testify/mock.(*mockUser).Use()
/home/neil/go/src/github.com/stretchr/testify/mock/mock_test.go:1534 +0xaa
Previous write at 0x00c0000d2338 by goroutine 6:
github.com/stretchr/testify/mock.(*Mock).MethodCalled()
/home/neil/go/src/github.com/stretchr/testify/mock/mock.go:376 +0x3c1
github.com/stretchr/testify/mock.(*Mock).Called()
/home/neil/go/src/github.com/stretchr/testify/mock/mock.go:333 +0x18c
github.com/stretchr/testify/mock.(*mockCaller).Call()
/home/neil/go/src/github.com/stretchr/testify/mock/mock_test.go:1526 +0x4b
github.com/stretchr/testify/mock.TestConcurrentArgumentRead.func1()
/home/neil/go/src/github.com/stretchr/testify/mock/mock_test.go:1502 +0x78
github.com/stretchr/testify/mock.TestConcurrentArgumentRead()
/home/neil/go/src/github.com/stretchr/testify/mock/mock_test.go:1516 +0x34e
testing.tRunner()
/usr/lib/go/src/testing/testing.go:827 +0x162
@georgelesica-wf
Can you please check this out?
This issue and corresponding PR seem to have a consistent repro and fix.
The consistent repro is very similar to what I'm stumbling in my codebase (nested mocks across go routines)
Note: This repro doesn't seem to be fixed with the patch proposed in: https://github.com/stretchr/testify/pull/654/commits/365386cb02c44b632277e859d737c3456b1630a2
Sure, I'll take a look this evening or tomorrow morning.
On Mon, May 13, 2019, 16:53 Kshitij Gupta [email protected] wrote:
@georgelesica-wf https://github.com/georgelesica-wf Can you please check this out? This issue and corresponding PR seem to have a consistent repro and fix. The consistent repro is very similar to what I'm stumbling in my codebase (nested mocks across go routines)
Note: This repro doesn't seem to be fixed with the patch proposed in: 365386c https://github.com/stretchr/testify/commit/365386cb02c44b632277e859d737c3456b1630a2
— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/stretchr/testify/issues/423?email_source=notifications&email_token=AB4JODSVFXAXD2JB3PLCOH3PVHWPVA5CNFSM4DGOYROKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODVJY7AQ#issuecomment-492015490, or mute the thread https://github.com/notifications/unsubscribe-auth/AB4JODRJAMHGMTSWNIGABFTPVHWPVANCNFSM4DGOYROA .
@kshitij8 would you mind confirming that the PR here fixes the issue you're having? A quick comment on the PR would be super helpful. Thanks!
I believe this is still a problem. The following code results in a data race when running go test -v ./... -race -count=100:
package main
import (
"sync"
"testing"
"github.com/stretchr/testify/mock"
)
type MyMockedObject struct {
mock.Mock
}
type data struct {
foo string
}
func (m *MyMockedObject) DoSomething(d *data) (bool, error) {
args := m.Called(d)
return args.Bool(0), args.Error(1)
}
func TestSomething(t *testing.T) {
testObj := &MyMockedObject{}
var wg sync.WaitGroup
d := &data{}
wg.Add(1)
testObj.On("DoSomething", d).Run(func(args mock.Arguments) {
wg.Done()
}).Return(true, nil).Once()
d2 := &data{}
wg.Add(1)
testObj.On("DoSomething", d2).Run(func(args mock.Arguments) {
wg.Done()
}).Return(true, nil).Once()
go func() {
d.foo = "bar"
testObj.DoSomething(d)
}()
go func() {
d2.foo = "baz"
testObj.DoSomething(d2)
}()
wg.Wait()
testObj.AssertExpectations(t)
}
To avoid a data race, change the calls to testObj.On to:
testObj.On("DoSomething", mock.Anything)
I believe this is happening because Testify ranges over and reads arguments of expected calls (*data in this case) which triggers a detected data race.
If I understand your intention; the code under test is passed a value which it unpredictably mutates and then passes to a mocked interface.
In this case you can use a custom argument matcher to assert that you have been passed the same struct pointer by comparing only the pointer values, which is not a race:
package kata_test
import (
"math/rand"
"strconv"
"sync"
"testing"
"github.com/stretchr/testify/mock"
)
type SomethingDoer interface {
DoSomething(*data) (bool, error)
}
type MyMockedObject struct {
mock.Mock
}
type data struct {
foo string
}
func (m *MyMockedObject) DoSomething(d *data) (bool, error) {
args := m.Called(d)
return args.Bool(0), args.Error(1)
}
func TestSomething(t *testing.T) {
testObj := &MyMockedObject{}
var wg sync.WaitGroup
d := &data{}
wg.Add(1)
testObj.On("DoSomething", mock.MatchedBy(func(arg *data) bool {
return d == arg // just compare the pointer value
})).Run(func(args mock.Arguments) {
wg.Done()
}).Return(true, nil).Once()
d2 := &data{}
wg.Add(1)
testObj.On("DoSomething", mock.MatchedBy(func(arg *data) bool {
return d2 == arg
})).Run(func(args mock.Arguments) {
wg.Done()
}).Return(true, nil).Once()
CodeUnderTest(d, d2, testObj)
wg.Wait()
testObj.AssertExpectations(t)
}
func CodeUnderTest(d, d2 *data, s SomethingDoer) {
go func() {
d.foo = strconv.FormatInt(rand.Int63(), 10) // can't predict this mutation
s.DoSomething(d)
}()
go func() {
d2.foo = strconv.FormatInt(rand.Int63(), 10)
s.DoSomething(d2)
}()
}
The original panic in this issue was finally fixed by #596
@fterrag I think your case is a limitation and I don't think there is anything testify can do to avoid that race. If my above suggested workaround doesn't fix your real cases, please do open a new issue. 🙏