maraino / go-mock

A mocking framework for the Go Programming Language.
MIT License
35 stars 12 forks source link

Use cmp.Equal before trying reflect.DeepEqual #15

Closed electricmonk closed 6 years ago

electricmonk commented 6 years ago

reflect.DeepEqual is a problematic function that does not support providing customized equality functions. We had a use case where our mocks were failing to register a call because one of the arguments did not pass DeepEqual's equality test because of unexported fields that should not be taken into account for equality purposes. This PR aims to help work around this issue.

maraino commented 6 years ago

I'm totally willing to integrate a PR that supports custom equality methods, but I think there are cases where we want to take into account unexported fields and directly using go-cmp Equal without any option doesn't look like the best option. According to go-cmd docs, unexported fields can result in panics, I never used go-cmp before, so I'm not really sure if that's the case:

If no custom equality functions are used and no Equal method is defined, equality is 
determined by recursively comparing the primitive kinds on both values, much like 
reflect.DeepEqual. Unlike reflect.DeepEqual, unexported fields are not compared by 
default; they result in panics unless suppressed by using an Ignore option (see 
cmpopts.IgnoreUnexported) or explicitly compared using the AllowUnexported option.

Right now you should be able to pass your test cases using something like:

sut.When("Call", mock.AnyIf(func(i interface{}){
  v, ok := i.(*EqualMe)
  return ok && foo.Equal(v)
}).Return(struct{}{}).Times(1)

You might want to consider to use a helper that returns a mock.AnyIf with a custom parameter:

func anyEqualMe(a *EqualMe) mock.AnyIfType {
    return mock.AnyIf(func(i interface{}) {
        b, ok := i.(*EqualMe)
        return ok && a.Equal(b)
    })
}
// and then use
sut.When("Call", anyEqualMe(foo)).Times(1)

As you can see, there are right now ways to go around, but there might be good options to improve, and looking at go-cmp there are good ideas there. I like for example the option of registering custom comparers.

Support for custom equality methods, can be integrated with a comparer, but if a more generic Option methods is added to go-mock we can even support enabling (T) Equal(T) bool methods, and you can even register your own Option methods.

electricmonk commented 6 years ago

Hmm. My bad. I think that the correct thing to do would be to pass the AllowUnexported option so that the behavior is on par with reflect.DeepEqual. Would you be willing to accept this solution as an interim one?

Re: registering custom comparators and other improvements to the API - I'm not up to doing this at the moment.