Equality matcher should not compare types

[michaelklishin]

I have some code that looks roughly like this:

type User string

Ω(info.User).Should(Equal("guest"))

However, this leads to failures. Explicit casting helps:

type User string

Ω(string(info.User)).Should(Equal("guest"))

It may be that I'm too new to Go but I believe type aliases work the way I expect. But this is not the case with Ginkgo. So my theory is that the Equal matcher does type equality checking (not equivalence).

At first I thought that's how Go works and stopped using type aliases, even though they make a lot of sense in my library.

So, is it a Go limitation or a Ginkgo "feature"?

[onsi]

Ginkgo (the matcher library Gomega, actually) uses Golang's reflect.DeepEqual under the hood to do the equality matching (http://golang.org/pkg/reflect/#DeepEqual). DeepEqual's docs claim to use "==" first under the hood but a look at the code says otherwise. Looks like they perform a type assertion before attempting to do any sort of equality.

I think Gomega'sEqual matcher could use == first and then fallback on DeepEqual when that fails. That would at least work for cases where one is type aliasing primitives that work well with ==. Type aliasing a more complex type (e.g. a slice or a map or a custom type) however, would still require a cast as DeepEqual is more suited to copmaring complex types than ==.

If all goes well with == there should be a fix on master by EOD.

[onsi]

Hmm... so I've found a fix for this, but I'm conflicted about it. I can modify the Equality matcher so that it converts the actual value (the left hand side) to have the same value as the expected value (the right hand side) if such a conversion is possible. The converted actual is then passed in to reflect.DeepEqual.

This works with your string alias example, and doesn't break any other tests that I've written....

But it has the undesirable property that it will convert numbers... This would mean that the float64 5.1 will actually "Equal" the int 5... boo... this is terrible.

I'm inclined to leave the matcher as is: anal about types, and put the onus of converting types on to the developer. This does mean you'd need:

Ω(info.User).Should(Equal(User("guest"))

Which, while wordy, is actually nice and explicit -- (BTW I think User("guest") on the RHS would be preferred to string(info.User) on the LHS)

That said, I'm open to discussion (and would love to have some discussion) about this. I could imagine a separate equality matcher that doesn't care about types.... but I'm not sure what I'd call it.

[michaelklishin]

As for the new matcher name, Be and BeEqual come to mind. It may be worth renaming the existing matcher to DeepEqual, although I understand that it would break existing code. But it's now or never, you are only going to have more projects using Gomega in the future.

[onsi]

I think I'd prefer Equal to be strict. Since my main concern is that
converting numbers can lead to false positives, I'll look into making the
matcher strict about not performing numerical conversions... that's what
the BeNumerically matcher is for.

On Sat, Oct 26, 2013 at 9:19 PM, Michael Klishin
notifications@github.comwrote:

As for the new matcher name, Be and BeEqual come to mind. It may be worth
renaming the existing matcher to DeepEqual, although I understand that it
would break existing code. But it's now or never, you are only going to
have more projects using Gomega in the future.

—
Reply to this email directly or view it on GitHubhttps://github.com//issues/11#issuecomment-27162601
.

[michaelklishin]

I agree. What do you think about ValueEqual or BeEquivalent (for the new matcher)?

[onsi]

I think BeEquivalentTo works and conveys the intent (these are equivalent, but not - stritctly speaking - equal.). Will add it...

[onsi]

Just added BeEquivalentTo matcher to gomega:

onsi/gomega@27c7621

[michaelklishin]

Thank you!