gomega
gomega copied to clipboard
onsi
DeepEqual comparison should tell you what's not equal
Right now when you use Expect(..).To(Equal(...)) and it uses go's DeepEqual function, it doesn't really help you find what is not equal. I just spent a bunch of time trying to figure out that an interface field was using a value type instead of a pointer.
Along the way I found https://godoc.org/github.com/juju/testing/checkers#DeepEqual, which is an alternate implementation of DeepEqual that actually will show you the differences. Would you consider a PR that changed gomega Equal to use this instead?
@benmoss would that introduce a new, third-party dependency to Gomega? If so, I foresee this causing pain for consumers of this library.
Is it better to re-implement that logic inside gomega than pull in a dependency?
+1 to reimplementing. This is dangerous territory... asserting equality can be tricky business in Go and relying on DeepEqual is a safe way to do so. Maintaining control over the code would be key.
My response wasn't very clear. I would love to have improved diff identification in Gomega. I'd want the code that does equality to be:
- a part of Gomega so we can be more responsive to issues
- forked off of Golang's deep equal
- super well-tested. I'd want to at least use Go's own tests as a starting point
- in it's own package under Gomega so it can be used as a standalone thing
Does that make sense?
ok after looking at this for a lil bit it seems way too hard to reimplement, my reflect-fu is far too lacking
I ended up making my own lib for this, in case anyone wants it: https://github.com/benmoss/matchers/
The burden of the criteria needed to be included in Gomega itself was just too high, so it's better to be a buyer-beware third party thing :).
Did you ever consider to depend on https://github.com/google/go-cmp?
hey go-cmp looks pretty good. I can imagine a Compare or BeComparableTo matcher that can accept go-cmp options and simply runs go-cmp in the background. I don't have a lot of bandwidth to work on that right now but would be open to a PR.
I hesitate to change the Equal matcher, though. Not without bumping the major version of Gomega...
A separate comparer sounds good. We have set it up like this for now:
https://github.com/kubernetes-sigs/kueue/blob/main/pkg/util/testing/equal_cmp.go
yes, exactly like that. I could also imagine a configurable flag on Gomega that would cause Equal to behave like go-cmp and so folks could try switching over more incrementally.
Hey, the Gomega equality operation still fails on time comparison as @sedyh noted on https://github.com/onsi/gomega/issues/264#issuecomment-985317474. @onsi Might be another incentive to use the Google comparison package to fix this bug.
I'm on board with moving in this direction - but I'm a bit underwater with other projects at the moment. If there's anyone with bandwidth and interest to work on a PR I'd be happy to talk through a design that would allow users to use go-cmp with a new matcher and optionally configure Gomega's Equal matcher (and anywhere that Gomega uses Equal under the hood for comparisons - e.g. ContainElement) to use go-cmp. I think that would give folks an onramp to switch to go-cmp.
@onsi I find the description of BeComparableTo to be quite confusing. Is it capable of deep comparison of two structs?
Hey @ilearnio - which description are you referring to? The godocs are simply trying to say "this matcher uses gocmp.Equal to make comparisons."
@onsi Yeah, I'm referring to that description. It took me a while to understand that gocmp is a separate go module (though it was something from gomega)
Thanks @ilearnio - I've updated the docs to try and make it a bit clearer and pushed out a change.