From 2ee015452c39c83371e823543509aa46ea3fdb3e Mon Sep 17 00:00:00 2001 From: Preston Van Loon Date: Sun, 23 Feb 2025 15:06:07 -0600 Subject: [PATCH] Use go-cmp and protocmp for assertion diff printing (#14978) Fix assertion tests after switching to cmp Changelog fragment --- changelog/pvl_go-cmp.md | 3 +++ testing/assertions/BUILD.bazel | 2 ++ testing/assertions/assertions.go | 11 +++++---- testing/assertions/assertions_test.go | 34 ++++++++++++++++++++------- 4 files changed, 36 insertions(+), 14 deletions(-) create mode 100644 changelog/pvl_go-cmp.md diff --git a/changelog/pvl_go-cmp.md b/changelog/pvl_go-cmp.md new file mode 100644 index 0000000000..324fd8021b --- /dev/null +++ b/changelog/pvl_go-cmp.md @@ -0,0 +1,3 @@ +### Changed + +- Use go-cmp for printing better diffs for assertions.DeepEqual diff --git a/testing/assertions/BUILD.bazel b/testing/assertions/BUILD.bazel index ae67367cf2..c80dbfe51b 100644 --- a/testing/assertions/BUILD.bazel +++ b/testing/assertions/BUILD.bazel @@ -8,8 +8,10 @@ go_library( deps = [ "//encoding/ssz/equality:go_default_library", "@com_github_d4l3k_messagediff//:go_default_library", + "@com_github_google_go_cmp//cmp:go_default_library", "@com_github_sirupsen_logrus//hooks/test:go_default_library", "@org_golang_google_protobuf//proto:go_default_library", + "@org_golang_google_protobuf//testing/protocmp:go_default_library", ], ) diff --git a/testing/assertions/assertions.go b/testing/assertions/assertions.go index 5bf5303967..0b566880f4 100644 --- a/testing/assertions/assertions.go +++ b/testing/assertions/assertions.go @@ -10,9 +10,11 @@ import ( "strings" "github.com/d4l3k/messagediff" + "github.com/google/go-cmp/cmp" "github.com/prysmaticlabs/prysm/v5/encoding/ssz/equality" "github.com/sirupsen/logrus/hooks/test" "google.golang.org/protobuf/proto" + "google.golang.org/protobuf/testing/protocmp" ) // AssertionTestingTB exposes enough testing.TB methods for assertions. @@ -52,13 +54,12 @@ func DeepEqual(loggerFn assertionLoggerFn, expected, actual interface{}, msg ... if !isDeepEqual(expected, actual) { errMsg := parseMsg("Values are not equal", msg...) _, file, line, _ := runtime.Caller(2) - var diff string + opts := cmp.Options{cmp.AllowUnexported(expected), cmp.AllowUnexported(actual)} if _, isProto := expected.(proto.Message); isProto { - diff = ProtobufPrettyDiff(expected, actual) - } else { - diff, _ = messagediff.PrettyDiff(expected, actual) + opts = append(opts, protocmp.Transform()) } - loggerFn("%s:%d %s, want: %#v, got: %#v, diff: %s", filepath.Base(file), line, errMsg, expected, actual, diff) + diff := cmp.Diff(expected, actual, opts...) + loggerFn("%s:%d %s, expected != actual, diff: %s", filepath.Base(file), line, errMsg, diff) } } diff --git a/testing/assertions/assertions_test.go b/testing/assertions/assertions_test.go index 118c06642a..0550e24569 100644 --- a/testing/assertions/assertions_test.go +++ b/testing/assertions/assertions_test.go @@ -5,6 +5,7 @@ import ( "fmt" "strings" "testing" + "unicode" eth "github.com/prysmaticlabs/prysm/v5/proto/prysm/v1alpha1" testpb "github.com/prysmaticlabs/prysm/v5/proto/testing" @@ -188,7 +189,7 @@ func TestAssert_DeepEqual(t *testing.T) { expected: struct{ i int }{42}, actual: struct{ i int }{41}, }, - expectedErr: "Values are not equal, want: struct { i int }{i:42}, got: struct { i int }{i:41}", + expectedErr: "Values are not equal, expected != actual, diff: struct{ i int }{\n- \ti: 42,\n+ \ti: 41,\n }\n", }, { name: "custom error message", @@ -198,7 +199,7 @@ func TestAssert_DeepEqual(t *testing.T) { actual: struct{ i int }{41}, msgs: []interface{}{"Custom values are not equal"}, }, - expectedErr: "Custom values are not equal, want: struct { i int }{i:42}, got: struct { i int }{i:41}", + expectedErr: "Custom values are not equal, expected != actual, diff: struct{ i int }{\n- \ti: 42,\n+ \ti: 41,\n }", }, { name: "custom error message with params", @@ -208,24 +209,39 @@ func TestAssert_DeepEqual(t *testing.T) { actual: struct{ i int }{41}, msgs: []interface{}{"Custom values are not equal (for slot %d)", 12}, }, - expectedErr: "Custom values are not equal (for slot 12), want: struct { i int }{i:42}, got: struct { i int }{i:41}", + expectedErr: "Custom values are not equal (for slot 12), expected != actual, diff: struct{ i int }{\n- \ti: 42,\n+ \ti: 41,\n }\n", }, } for _, tt := range tests { - verify := func() { - if tt.expectedErr == "" && tt.args.tb.ErrorfMsg != "" { - t.Errorf("Unexpected error: %v", tt.args.tb.ErrorfMsg) - } else if !strings.Contains(tt.args.tb.ErrorfMsg, tt.expectedErr) { + verify := func(t testing.TB) { + // Trim unicode space characters for an easier comparison. + got := strings.Map(func(r rune) rune { + if unicode.IsSpace(r) { + return -1 + } + return r + }, tt.args.tb.ErrorfMsg) + want := strings.Map(func(r rune) rune { + if unicode.IsSpace(r) { + return -1 + } + return r + }, tt.expectedErr) + if want == "" && got != "" { + t.Errorf("Unexpected error: %v", got) + } else if !strings.Contains(got, want) { + t.Logf("got=%q", got) + t.Logf("want=%q", want) t.Errorf("got: %q, want: %q", tt.args.tb.ErrorfMsg, tt.expectedErr) } } t.Run(fmt.Sprintf("Assert/%s", tt.name), func(t *testing.T) { assert.DeepEqual(tt.args.tb, tt.args.expected, tt.args.actual, tt.args.msgs...) - verify() + verify(t) }) t.Run(fmt.Sprintf("Require/%s", tt.name), func(t *testing.T) { require.DeepEqual(tt.args.tb, tt.args.expected, tt.args.actual, tt.args.msgs...) - verify() + verify(t) }) } }