proposal: use a better printer/differencer for failed matches

[Capstan]

See also issue #233 which is related.

When a gomock call fails an expectation, if the argument is an interface, the output, based on the %v format character, is not terribly helpful, e.g.,

   controller.go:150: Unexpected call to *jobs_mock.MockJobs.Insert([testproject 0xc0001f2420]) at …/jobs_mock.go:62 because: 
        Expected call at …/my_test.go:132 doesn't match the argument at index 1.
        Got: &{0xc000206300   0xc00020c120   <nil> <nil>  {0 map[]} [] []}
        Want: is equal to &{0xc000206100   0xc0000f5020   <nil> <nil>  {0 map[]} [] []}

This doesn't help isolate the problem, which is only visible to reflect.DeepEqual and is not obvious to me, the viewer of the test results. (The problem from the above turned out to be two levels deep into an embedded struct.)

Please consider either or both of the following:

Pretty-print the values

This would involve changing the code in calls.go to use a formatter other than %v for Got, and matchers's String implementation to use the same on the embedded interface. e.g.,

• https://godoc.org/github.com/kylelemons/godebug/pretty
• https://godoc.org/github.com/davecgh/go-spew/spew

Print a diff of the values

This would involve adding a new Matcher method, e.g., Explain or MatchAndExplain to be able to more thoroughly specify what happened. To preserve backwards incompatibility, this would be in a new interface, e.g.,

type Explainer interface {
  // MatchAndExplain returns whether x is a match and why.
  MatchAndExplain(x interface{}) (bool, string)
}

func (e eqMatcher) MatchAndExplain(x interface{}) (bool, string) {
  negation := ""
  diff := ""
  match := e.Matches(x)
  if !match {
    negate = "not "
    diff = fmt.Sprintf("\nGot: %v\nWant: %v\nDiff: %s", x, e.x, cmp.Diff(e.x, x))
  }
  return match, fmt.Sprintf("is %sequal to %v%s", negation, e.x, diff)
}

Then, in lieu of using Matcher.String and explicit Got/Want errors in call.go, rely on MatchAndExplain to yield the deep difference.

• https://godoc.org/github.com/google/go-cmp/cmp,
• https://godoc.org/github.com/kylelemons/godebug/diff

---

I'm not familiar with a go built-in pretty printer, so I think either of these would require either extending a core library, or allowing an import to do pretty printing / diffing. Not sure how feasible that is.

This approach is modeled at least partly after the C++ Google Test Matchers's MatcherInterface; many examples of implementations of which are in the Google Mock matchers.

[maelvls]

Very good idea! For now, it's hard to tell where a struct differs. It's even worse when the struct is deep and has nested structs.

Regarding diff outputing: Testify has human readable diffs; but go-testdeep goes a step beyond with colors!

[codyoss]

https://github.com/golang/mock#modifying-failure-messages I think covers this. @Capstan Thoughts?

[danielo515]

well @codyoss not completely.
It can not be used with calls to expect, it doesn't implement the required interface:

	*gomock.Call does not implement gomock.Matcher (missing Matches method)
		have gomock.matches([]interface {}) error
		want Matches(interface {}) bool
FAIL	gitlab.com/pentoapp/pento/pkg/uk_employee/usecases [build failed]

[codyoss]

I think this is less of a problem with some more recent changes: #559 and #236.
We are not pretty printing but we now show types when there is an error and there are options to format got/want. I will leave this open for a little longer to see if there are issues these two are not solving for people. If you have use cases that are still hard to debug please post a full example in this issue so they can be examined further.

[codyoss]

Closing this issue as resolved, see above comment.

[austinhyde]

I think two big pain points were missed here:

1. The default formatting of Got/Want values is lackluster in most cases
2. There's no diff between Got/Want

#236 makes it so we can control formatting of specific values/matchers on a per-assertion basis, which sort of fixes the first point, but not really. I don't know about you, but I don't particularly want to go wrap all the values in all the expectations in all my codebase's tests just so I can tell what's actually breaking. If I absolutely had to, I'd wind up writing a handful of utilities that would basically look like the following, then put it absolutely everywhere:

func fmteq(v interface{}) gomock.Matcher {
	return gomock.WantFormatter(
		gomock.StringerFunc(func() string {
			return prettyprint(v)
		}),
		gomock.GotFormatterAdapter(
			gomock.GotFormatterFunc(prettyprint),
			gomock.Eq(v),
		),
	)
}

All in all, not great. It would be cool to be able to insert a default formatter somehow, maybe on the controller or through a global var in the package. If there's a way to do this already, it's not clear from the current documentation.

As to the second issue, as far as I know, there's no mechanism at all that would let you do a diff of the two values. As noted by @maelvls, github.com/stretchr/testify does this out of the box, and it's great. IIRC, it literally just runs the diff algorithm on spew output, but it makes investigating test failures a breeze.

As it stands, I'm looking at this mess trying to figure out what about it isn't equal:

Got: &{1m0s <nil> 2022-01-14 15:30:21.98344173 +0000 UTC 12.34 [{foo_id foo foo message false true true [{string param true 0xc0005729f0 <nil> <nil>} {double param true <nil> 0xc00070d5d8 <nil>}]} {bar_id bar bar message false true true [{bool param true <nil> <nil> 0xc00070d5e0}]}] [{foo module foo {1234 22.22 1.1 12.34 43.21}} {bar module bar {2222 22.22 1 22.21 22.23}}] pass} (*models.Foo)
Want: is equal to &{1m0s <nil> 2022-01-14 10:30:21.98344173 -0500 EST m=+0.015514841 12.34 [{foo_id foo foo message false true true [{string param true 0xc000572980 <nil> <nil>} {double param true <nil> 0xc00070d5a8 <nil>}]} {bar_id bar bar message false true true [{bool param true <nil> <nil> 0xc00070d5b0}]}] [{foo module foo {1234 22.22 1.1 12.34 43.21}} {bar module bar {2222 22.22 1 22.21 22.23}}] pass} (*models.Foo)

(yes, I know it's the m=+0.01..., but that's only because I went and manually formatted the output)

Sure, I can drop spew or similar in here for cases like this, but a) doing that by default means I need to get approval for the new dependency from legal, and b) doing that on-off means I need to waste a ton of time adding the dependency to bazel for a one-off test.

It'd be so much easier if gomock just defaulted to human-friendly output.

[codyoss]

Thanks for the thoughtful comment @austinhyde. I still think the original issue was addressed here, but we can alway strive to be better. Would you mind summarizing your thoughts in a new issue and provide a small code example the demonstrates how/why this would be useful.

[austinhyde]

Honestly, I don't believe the original issue was addressed in any meaningful way:

• It was requested that the default formatting of values be improved beyond just %v. You provided a way to override this formatting on a case-by-case basis, but that doesn't address the original ask of changing the default.
• It was requested that there be a way to render diffs between got/want, at least in the case of Eq, and not even by default. I don't see any way at all, even case-by-case, that this is possible with gomock, or even acknowledged in this issue, let alone addressed.

OP provided a possible route to providing these features. If you don't like that approach for whatever reason, that's one thing, but that doesn't invalidate the overall ask. You asked for examples where the linked issues are not sufficient, except that neither issue actually addresses any of the OP's asks.

You ask me to provide a code example of why this would be useful - that's exactly what I posted. It's not readable for any sane definition of the word. Do you want me to actually write up a full test case that just... outputs the exact same thing? How is that at all a good use of my time to write it or yours to read it? The issue is around the default formatting of the library's output, the usefulness of such a feature should be apparent in the output, not the code that generates that output. Literally any EXPECT().Whatever(whatever) with any sort of non-trivial struct will reproduce this sort of output.

[alnvdl]

There's a workaround using the current gomock API (as of v1.6.0): #616 (comment)

[c4milo]

I agree with @austinhyde, the issue is not addressed. gomock is great but its output is not great at helping you iterate faster due to the cryptic messages when it fails at trying to match structs.