Improve default failed equality match output to more clearly show diffs

[adamvictorclever]

Requested feature
Sometimes I'll generate a mock and EXPECT some calls. In cases where the test fails because the mock was called with a different input, it can be very hard to tell what the difference is. This is especially apparent in cases where some fields are pointers.

I've tried to set up an example in Go Playground to show what I mean: https://go.dev/play/p/pVxqpZpqiwz

Basically, I have a struct that looks like

type Obo struct {
	a *string
	b *bool
	c []*Obo
}

and an interface I generate a mock for that looks like

type TestThing interface {
	DoThing(obo Obo)
}

In my TestWithMock case, I expect a call with a certain Obo and provide a call with a slightly different one, causing a test failure. The only difference between the two provided is that the first one has an a value set to &"apple" and the second set to &"banana".

Here's the output I see when the test fails:

=== RUN   TestWithMock
    prog.go:25: Unexpected call to *main.MockTestThing.DoThing([{0xc0000a6f50 0xc0000c2998 [0xc0000a9440]}]) at /tmp/sandbox1909100993/prog.go:25 because: 
        expected call at /tmp/sandbox1909100993/prog.go:24 doesn't match the argument at index 0.
        Got: {0xc0000a6f50 0xc0000c2998 [0xc0000a9440]} (main.Obo)
        Want: is equal to {0xc0000a6ef0 0xc0000c2988 [0xc0000a93b0]} (main.Obo)
    controller.go:137: missing call(s) to *main.MockTestThing.DoThing(is equal to {0xc0000a6ef0 0xc0000c2988 [0xc0000a93b0]} (main.Obo)) /tmp/sandbox1909100993/prog.go:24
    controller.go:137: aborting test due to missing call(s)
--- FAIL: TestWithMock (0.00s)

In reality, the diff is quite small, just a single field with a different value. However, the output from the failure provides basically no information on this mismatch. Every field appears different because the only thing that it displays is the pointer, not the value. This makes the issue much more difficult to debug!

Compare this to the output of TestWithAssert, a similar case that directly compares the two Obo objects using assert.Equal from the github.com/stretchr/testify/assert package.

=== RUN   TestWithAssert
    prog.go:29: 
        	Error Trace:	prog.go:29
        	Error:      	Not equal: 
        	            	expected: main.Obo{a:(*string)(0xc000184000), b:(*bool)(0xc000180008), c:[]*main.Obo{(*main.Obo)(0xc000188000)}}
        	            	actual  : main.Obo{a:(*string)(0xc000184020), b:(*bool)(0xc000180009), c:[]*main.Obo{(*main.Obo)(0xc000188030)}}
        	            	
        	            	Diff:
        	            	--- Expected
        	            	+++ Actual
        	            	@@ -1,3 +1,3 @@
        	            	 (main.Obo) {
        	            	- a: (*string)((len=5) "apple"),
        	            	+ a: (*string)((len=6) "banana"),
        	            	  b: (*bool)(true),
        	Test:       	TestWithAssert
--- FAIL: TestWithAssert (0.00s)

With this output, the nice diff immediately makes it clear what the difference was between the two structs, vastly simplifying my debugging experience!

I believe this is related to #265, which was closed. If I understand correctly, the proposed solution would be to use WantFormatter and GotFormatter to generate a nicer output myself. Personally, having never used these features before, it feels like a lot of effort for a simple and common problem! Would I need to use these in every test case I have? I think it would be much more helpful to improve the default output when using an equality matcher.

(Optional) Proposed solution
Some potential ideas for how I'd like to see the output improved

• When outputting a failure, show the value for pointer fields. For example, something vaguely like

Got: {0xc0000a6f50("apple") ...} (main.Obo)
Want: is equal to {0xc0000a6ef0("banana") ...} (main.Obo)

This would make it easier to manually inspect and see what the difference is

• Output a diff, like the one included by the assert function in my example above. This is even better since it makes it really easy to tell the difference.

[palsivertsen]

I'm also running into this issue a lot. It would be great if the library exposed a way to set a diff function with a signature like type DiffFunc func(a, b interface{}) string. That way we could easily control how the diff is printed.

When a test fails and the got/want output is difficult to read, I like to use the following trick to get a pretty diff. It's not an elegant solution, especially when wrapping several values, but it works for debugging purposes.

example.go

package example

//go:generate mockgen -destination mocks/$GOFILE -package ${GOPACKAGE}_mocks -source $GOFILE

type Foo struct {
	Bar *string
}

type Fooer interface {
	DoSomething(Foo)
}

example_test.go

package example_test

import (
	"example"
	example_mocks "example/mocks"
	"log"
	"testing"

	"github.com/golang/mock/gomock"
	"github.com/kr/pretty"
)

func String(s string) *string { return &s }

func TestFoo_Standard(t *testing.T) {
	ctrl := gomock.NewController(t)
	defer ctrl.Finish()

	mockFooer := example_mocks.NewMockFooer(ctrl)
	mockFooer.EXPECT().
		DoSomething(
			example.Foo{Bar: String("expected")},
		)

	mockFooer.DoSomething(example.Foo{
		Bar: String("actual"),
	})
}

func TestFoo_Improved(t *testing.T) {
	ctrl := gomock.NewController(t)
	defer ctrl.Finish()

	mockFooer := example_mocks.NewMockFooer(ctrl)
	mockFooer.EXPECT().
		DoSomething(
			&diffWrapper{
				v: example.Foo{Bar: String("expected")},
			},
		)

	mockFooer.DoSomething(example.Foo{
		Bar: String("actual"),
	})
}

type diffWrapper struct {
	v interface{}
}

// Implement the gomock.Matcher interface
func (m *diffWrapper) Matches(v interface{}) bool {
	ok := gomock.Eq(m.v).Matches(v) // wrap the gomock.Eq matcher
	if !ok {                        // print the diff
		log.Printf("This is the diff: %s", pretty.Diff(m.v, v)) // any diff func here
	}
	return ok // return the result of the match
}

// Implement the gomock.Matcher interface
func (m *diffWrapper) String() string {
	return gomock.Eq(m.v).String() // again wrap the gomock.Eq matcher
}

When running the TestFoo_Improved test I get the diff printed above the test output like so:

2022/02/15 12:19:02 This is the diff: [Bar: "expected" != "actual"]
--- FAIL: TestFoo_Improved (0.00s)
    example_test.go:42: Unexpected call to *example_mocks.MockFooer.DoSomething([{0xc00004a590}]) at /tmp/tmp.Rxm4mU8eW1/example_test.go:42 because: 
        expected call at /tmp/tmp.Rxm4mU8eW1/example_test.go:36 doesn't match the argument at index 0.
        Got: {0xc00004a590} (example.Foo)
        Want: is equal to {0xc00004a530} (example.Foo)
    controller.go:269: missing call(s) to *example_mocks.MockFooer.DoSomething(is equal to {0xc00004a530} (example.Foo)) /tmp/tmp.Rxm4mU8eW1/example_test.go:36
    controller.go:269: aborting test due to missing call(s)
FAIL
exit status 1
FAIL	example	0.002s

[alnvdl]

While not perfect (and quite hackish to be honest), it's possible to have a diff in the output.
This works in v1.6.0 and uses google/go-cmp. It includes a diff while preserving the original gomock output style:

type eqMatcher struct {
	want interface{}
}

func EqMatcher(want interface{}) eqMatcher {
	return eqMatcher{
		want: want,
	}
}

func (eq eqMatcher) Matches(got interface{}) bool {
	return gomock.Eq(eq.want).Matches(got)
}

func (eq eqMatcher) Got(got interface{}) string {
	return fmt.Sprintf("%v (%T)\nDiff (-got +want):\n%s", got, got, strings.TrimSpace(cmp.Diff(got, eq.want)))
}

func (eq eqMatcher) String() string {
	return fmt.Sprintf("%v (%T)\n", eq.want, eq.want)
}

And then just use as you would with any other matcher:

m.EXPECT().DoSomething(EqMatcher(...))

Which outputs:

/home/ubuntu/dev/mockoutput/example.go:18: Unexpected call to *example.MockMyInterface.DoSomething([{1 map[abc:def xyz:mno] [0xc000026780]}]) at /home/ubuntu/dev/mockoutput/example.go:18 because: 
    expected call at /home/ubuntu/dev/mockoutput/example_test.go:42 doesn't match the argument at index 0.
    Got: {1 map[abc:def xyz:mno] [0xc000026780]} (defs.Object)
    Diff (-got +want):
    defs.Object{
      	ID: 1,
      	Mapping: map[string]string{
      		"abc": "def",
    - 		"xyz": "mno",
      	},
      	Subs: []*defs.SubObject{
      		&{
    - 			When:   s"2021-10-12 01:02:03.000000004 +0000 UTC",
    + 			When:   s"2022-10-12 01:02:03.000000004 +0000 UTC",
      			Name:   "sub1",
      			Things: {"a", "b"},
      		},
      	},
      }
    Want: {1 map[abc:def] [0xc000026740]} (defs.Object)

But I agree, some optional, native form of diffing (perhaps even using go-cmp?) would be better.

[palsivertsen]

Any chance we can rework the Controller to accept a diff function? Something along the lines of:

ctrl := gomock.NewController(t, gomock.WithDiffFunc(cmp.Diff))

The "got" and "want" formatters could be added in the same way.

[SpencerC]

I think cmp.Diff (-want +got) should be the default behavior. One design consideration is that cmp.Diff is most useful when you can give it options like cmp.AllowUnexported and protocmp.Transform.

@palsivertsen's solution by requiring developers to wrap cmp.Diff in an anonymous function that configures it to their liking. Alternatively:

ctrl := gomock.NewController(t, gomock.WithDiffOpts(cmp.AllowUnexported(&Something{}), protocmp.Transform()))

[vitaly-zdanevich]

Looks like DoAndReturn has better diff.