Testify Arguments.Diff method modify google.type.Money

[bigspawn]

Context

Dependence:

github.com/golang/protobuf v1.4.3
github.com/stretchr/testify v1.6.1

Problem line: github.com/stretchr/testify@v1.6.1/mock/mock.go:768

Example:

Proto file

syntax = "proto3";
option go_package = "mockserver";

import "google/type/money.proto";

message Object {
    google.type.Money Amount = 1;
}

mockserver was generate by prototool.

Test file

package sepa

import (
    "github.com/golang/protobuf/proto"
    "github.com/stretchr/testify/assert"
    "github.com/stretchr/testify/mock"
    "google.golang.org/genproto/googleapis/type/money"
    "mockserver"
    "reflect"
    "testing"
)

type Doing interface {
    Do(object *mockserver.Object)
}

type Mock struct {
    mock.Mock
}

func (m *Mock) Do(a1 *mockserver.Object) {
    m.Called(a1)
}

func Test1(t *testing.T) {
    expected := &mockserver.Object{
        Amount: &money.Money{
            CurrencyCode: "EUR",
            Units:        1,
            Nanos:        0,
        },
    }

    obj := &Mock{}
    obj.On("Do", mock.Anything).
        Run(func(args mock.Arguments) {
            get, ok := args.Get(0).(*mockserver.Object)
            if !ok {
                t.FailNow()
            }
            assert.True(t, reflect.DeepEqual(expected, get))
        })

    actual := &mockserver.Object{
        Amount: &money.Money{
            CurrencyCode: "EUR",
            Units:        1,
            Nanos:        0,
        },
    }
    obj.Do(actual)
    t.Logf("proto eq: %t", proto.Equal(expected.Amount, actual.Amount))
}

Result

=== RUN   Test1
    booking_test.go:896: 
            Error Trace:    booking_test.go:896
                                        booking_test.go:877
                                        booking_test.go:906
            Error:          Should be true
            Test:           Test1
    booking_test.go:907: proto eq: true
--- FAIL: Test1 (0.00s)

FAIL

Diff

expected
--------
expected = {*my.company.com/api/mockserver.Object | 0xc0008124e0} 
 Amount = {*google.golang.org/genproto/googleapis/type/money.Money | 0xc00007e140} 
  state = {google.golang.org/protobuf/internal/impl.MessageState} 
   NoUnkeyedLiterals = {google.golang.org/protobuf/internal/pragma.NoUnkeyedLiterals} 
   DoNotCompare = {google.golang.org/protobuf/internal/pragma.DoNotCompare} len:0
   DoNotCopy = {google.golang.org/protobuf/internal/pragma.DoNotCopy} len:0
   atomicMessageInfo = {*google.golang.org/protobuf/internal/impl.MessageInfo} nil
  sizeCache = {int32} 0
  unknownFields = {[]uint8} nil
  CurrencyCode = {string} "EUR"
  Units = {int64} 1
  Nanos = {int32} 0
 XXX_NoUnkeyedLiteral = {struct {}} 
 XXX_unrecognized = {[]uint8} nil
 XXX_sizecache = {int32} 0
------
actual
------
get = {*my.company.com/mockserver.Object | 0xc0008125d0} 
 Amount = {*google.golang.org/genproto/googleapis/type/money.Money | 0xc00007e1e0} 
  state = {google.golang.org/protobuf/internal/impl.MessageState} 
   NoUnkeyedLiterals = {google.golang.org/protobuf/internal/pragma.NoUnkeyedLiterals} 
   DoNotCompare = {google.golang.org/protobuf/internal/pragma.DoNotCompare} len:0
   DoNotCopy = {google.golang.org/protobuf/internal/pragma.DoNotCopy} len:0
   atomicMessageInfo = {*google.golang.org/protobuf/internal/impl.MessageInfo | 0xc0007283c0} 
    GoReflectType = {reflect.Type | *reflect.rtype} 
    Desc = {google.golang.org/protobuf/reflect/protoreflect.MessageDescriptor | *google.golang.org/protobuf/internal/filedesc.Message} 
    Exporter = {google.golang.org/protobuf/internal/impl.exporter} nil
    OneofWrappers = {[]interface {}} nil
    initMu = {sync.Mutex} 
    initDone = {uint32} 1
    reflectMessageInfo = {google.golang.org/protobuf/internal/impl.reflectMessageInfo} 
    coderMessageInfo = {google.golang.org/protobuf/internal/impl.coderMessageInfo} 
  sizeCache = {int32} 0
  unknownFields = {[]uint8} nil
  CurrencyCode = {string} "EUR"
  Units = {int64} 1
  Nanos = {int32} 0
 XXX_NoUnkeyedLiteral = {struct {}} 
 XXX_unrecognized = {[]uint8} nil
 XXX_sizecache = {int32} 0

variable atomicMessageInfo was modified.

[bigspawn]

And if I try compare object by assert.Equal(t, expected, get) instead of assert.True(t, reflect.DeepEqual(expected, get)) then the test freezes

[dolmen]

Please, make some more effort to explain the issue. Also adding a simpler test case running on play.golang.org would be helpful. Here is a template: https://go.dev/play/p/AsRNcxCRdWx

[bigspawn]

When a method argument is or contains a proto message, after passing it to the mock structure it modifies its internal fields, which causes the given objects to no longer be equals.

Here is an example, but go.dev failed to launch it: https://go.dev/play/p/x45sLsUKS9U

package main

import (
    "reflect"
    "testing"

    "github.com/stretchr/testify/assert"
    "github.com/stretchr/testify/mock"
    "google.golang.org/genproto/googleapis/type/money"
)

type Mock struct {
    mock.Mock
}

func (mock *Mock) Do(money *money.Money) {
    mock.Called(money)
}

func TestName(t *testing.T) {
    expected := &money.Money{
        CurrencyCode: "EUR",
        Units:        1,
        Nanos:        0,
    }
    actual := &money.Money{
        CurrencyCode: "EUR",
        Units:        1,
        Nanos:        0,
    }
    assert.Truef(t, reflect.DeepEqual(expected, actual), "must be equal: expected: %v, actual: %v", expected, actual)

    obj := &Mock{}
    obj.On("Do", mock.Anything).
        Run(func(args mock.Arguments) {
            get, ok := args.Get(0).(*money.Money)
            if !ok {
                t.FailNow()
            }
            assert.Equalf(t, actual, get, "must be equal: actual: %v, get argumet: %v", actual, get)
            // fails here
            assert.Equalf(t, expected, get, "must be equal: expected: %v, get argumet: %v", expected, get)
        })
    obj.Do(actual)
    obj.AssertExpectations(t)
}

As I see that is a problem in String method implementation of proto message. It loads and stores them.

impl.(*messageState).LoadMessageInfo (pointer_unsafe.go:160) google.golang.org/protobuf/internal/impl
money.(*Money).ProtoReflect (money.pb.go:77) google.golang.org/genproto/googleapis/type/money
prototext.MarshalOptions.Format (encode.go:90) google.golang.org/protobuf/encoding/prototext
impl.Export.MessageStringOf (api_export.go:176) google.golang.org/protobuf/internal/impl
money.(*Money).String (money.pb.go:68) google.golang.org/genproto/googleapis/type/money
fmt.(*pp).handleMethods (print.go:673) fmt
fmt.(*pp).printArg (print.go:756) fmt
fmt.(*pp).doPrintf (print.go:1176) fmt
fmt.Sprintf (print.go:239) fmt
mock.Arguments.Diff (mock.go:768) github.com/stretchr/testify/mock
mock.(*Mock).findExpectedCall (mock.go:287) github.com/stretchr/testify/mock
mock.(*Mock).MethodCalled (mock.go:366) github.com/stretchr/testify/mock
mock.(*Mock).Called (mock.go:356) github.com/stretchr/testify/mock
main.(*Mock).Do (main_test.go:17) github.com/bigspawn/learn-go/testifybug
main.TestName (main_test.go:44) github.com/bigspawn/learn-go/testifybug
testing.tRunner (testing.go:1576) testing
testing.(*T).Run.func1 (testing.go:1629) testing
runtime.goexit (asm_arm64.s:1172) runtime

It seems to be a problem with the protobuf code generator and not the library, you can close the ticket.