onsi
Formatting Cyclic Types produces infinite output
When using gomega (and, by extension, ginkgo) with cyclic types, the output from formatting the objects from e.g. Expect(foo).To(Equal(bar)) is infinite, and if the cyclic pointer field is early in the struct, the actual differing fields may never actually be printed. This renders this output nearly useless.
I would like to propose and implement adding a new struct tag gomega:"omit" which replaces the formatting output of that field with a message indicating it was omitted, plus the address if it was a pointer. This would allow cyclic types to be formatted finitely.
hey thanks for bringing this up and proposing a solution (and including a PR!)
i'm open to the approach you've taken though we'll need to add some documentation to make it discoverable.
some thoughts though:
gomega:"omit"is scoped to formatting output - but on the face of it - i could imagine confusion about the scope. e.g. "I want Gomega's equality matcher to ignore this field when making comparisons - why doesn'tgomega:"omit"work?"- for the specific issue of cyclic types it might make more sense to support this in
format.Object"out of the box." Namely,Objectcould keep track of every pointer address it encounters and only descend into, and format, the pointer if it hasn't already seen it. This would solve the cyclic issue without the user having to explicitly annotate their code with agomegatag.
Thoughts? Would you be up for giving the latter a try in the PR?
That's a situation I'm constantly battling with in my lxkns and ghostwire packages, with their cyclic data structures. As onsi points out, an annotation probably doesn't help, because the annotation's placement would need to be test-specific, depending on where in the probably cyclic graph your test "starts". The cycle detection looks much more promising.
The cycle detection would be a better long-term solution, which I did consider at first, and I've be happy to try that as well, however, this was more of a quick-and-dirty solution to get me unblocked in my particular use case that didn't require re-doing any of the internals of the package. That said, I think the tag approach is still useful in the short term (albleit with a better name, maybe gomega/format:"omit"?), as well as a more general use case for hiding large, obstructive fields that the user knows they aren't interested in having in output.
the more i think about it the less confident i am about an omit tag that controls output without also controlling equality. While I wholeheartedly agree about the irritating reality of noisy output the reality is that reflect.DeepEqual() which asserts equality even on those noisy bits will fail to match the noisy bits don't match. By hiding the noise we inadvertently make it harder to debug that sort of failure.
Someday™ building out Gomega's own equality library would be a good investment as it would enable this sort of usecase (so many times i've wanted to just suppress checking for equality on "uninteresting" fields like timestamps or coordinates). But that's too much scope for this, imo.
So... I think an approach that does the cycle detection, scoped to each invocation of format.Object() is probably the best approach. Would you be up for giving it a try?