search

smarty / assertions

Fluent assertion-style functions used by goconvey and gunit. Can also be used in any test or application.
Other
100 stars 34 forks source link

ShouldEqual could be made to work with structs that define an equality method (like time.Time) #20

Closed mdwhatcott closed 7 years ago

mdwhatcott commented 7 years ago

Thought: What if ShouldEqual (and ShouldNotEqual) worked on struct types that implemented an equality method for their own type:

func (a <struct>) Equal(b <struct>) bool

The time.Time struct has:

func (t Time) Equal(u Time) bool

Is this pattern common enough that it would be helpful and/or obvious?

joliver commented 7 years ago

I like the idea. It definitely makes it feel better than ShouldResemble.

SunSparc commented 7 years ago

ShouldResemble works fine for me, but perhaps ShouldEqual would be better since it is more intuitive for those that are new to the assertions package. Or perhaps the ShouldEqual could just detect if it is trying to be used on structs and print a message that redirects users to ShouldResemble.

mdwhatcott commented 7 years ago

The ShouldResemble function sometimes fall over when comparing time values that are in different time zones. The .Equal() method compares the actual instant, regardless of time zone, so that's why it's a compelling option.

rana commented 7 years ago

I get "Equal" immediately. "Resemble" implies kind of equal? Perhaps in some circumstances? I'm sure i could look up the docs, but for something as basic as time.Time I just wrote my own assertion to make sure time is really equal.

mdwhatcott commented 7 years ago

Yes @rana, in retrospect, it probably just should have been ShouldDeepEqual rather than ShouldResemble. The word "resemble" doesn't have as strong a connotation as "equal". But I liked the definition, which referenced two things that have similar qualities/features (fields!):

re·sem·ble rəˈzembəl verb have qualities or features, especially those of appearance, in common with (someone or something);

rana commented 7 years ago

Sounds good!

rana commented 7 years ago

Btw, great package! I don't know how i lived without it before!

rana commented 7 years ago

You might find https://github.com/google/go-cmp useful for future works on the assertions package. Google created a new package for DeepEqual:

This package is intended to be a more powerful and safer alternative to reflect.DeepEqual for comparing whether two values are semantically equal.

Love the goconvey package, thanks for the awesome efforts!

mdwhatcott commented 7 years ago

Thanks for the tip @rana - I saw that recently (and now that I step back and think about it, that's probably where I got the idea to make use of the Equal method). I like that they are calling the Equal method from both directions to root out non-deterministic and/or non-symmetric scenarios (https://github.com/google/go-cmp/blob/master/cmp/compare.go#L369).