From 521aed8f8109da778bb77423e49c50be9c966005 Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Thu, 16 Jan 2025 13:52:42 +0000 Subject: [PATCH 1/3] test(coderd/database/dbauthz): compare outputs with cmp This produces a more readable diff than that of the assert library. --- coderd/database/dbauthz/setup_test.go | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/coderd/database/dbauthz/setup_test.go b/coderd/database/dbauthz/setup_test.go index fc01e39330d7d..1d6ed990dcb7c 100644 --- a/coderd/database/dbauthz/setup_test.go +++ b/coderd/database/dbauthz/setup_test.go @@ -9,6 +9,7 @@ import ( "strings" "testing" + "github.com/google/go-cmp/cmp" "github.com/google/uuid" "github.com/open-policy-agent/opa/topdown" "github.com/stretchr/testify/require" @@ -198,11 +199,9 @@ func (s *MethodTestSuite) Subtest(testCaseF func(db database.Store, check *expec s.Equal(len(testCase.outputs), len(outputs), "method %q returned unexpected number of outputs", methodName) for i := range outputs { a, b := testCase.outputs[i].Interface(), outputs[i].Interface() - if reflect.TypeOf(a).Kind() == reflect.Slice || reflect.TypeOf(a).Kind() == reflect.Array { - // Order does not matter - s.ElementsMatch(a, b, "method %q returned unexpected output %d", methodName, i) - } else { - s.Equal(a, b, "method %q returned unexpected output %d", methodName, i) + // Use cmp.Diff to get a nice diff of the two values. + if diff := cmp.Diff(a, b); diff != "" { + s.Failf("compare outputs failed", "method %q returned unexpected output %d (-want +got):\n%s", methodName, i, diff) } } } From 7ed901bc89118c2c474a3463157118b2ad4856e9 Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Thu, 16 Jan 2025 14:10:43 +0000 Subject: [PATCH 2/3] equate empty for nil/zero slice --- coderd/database/dbauthz/setup_test.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/coderd/database/dbauthz/setup_test.go b/coderd/database/dbauthz/setup_test.go index 1d6ed990dcb7c..44996e9f9a863 100644 --- a/coderd/database/dbauthz/setup_test.go +++ b/coderd/database/dbauthz/setup_test.go @@ -10,6 +10,7 @@ import ( "testing" "github.com/google/go-cmp/cmp" + "github.com/google/go-cmp/cmp/cmpopts" "github.com/google/uuid" "github.com/open-policy-agent/opa/topdown" "github.com/stretchr/testify/require" @@ -200,7 +201,7 @@ func (s *MethodTestSuite) Subtest(testCaseF func(db database.Store, check *expec for i := range outputs { a, b := testCase.outputs[i].Interface(), outputs[i].Interface() // Use cmp.Diff to get a nice diff of the two values. - if diff := cmp.Diff(a, b); diff != "" { + if diff := cmp.Diff(a, b, cmpopts.EquateEmpty()); diff != "" { s.Failf("compare outputs failed", "method %q returned unexpected output %d (-want +got):\n%s", methodName, i, diff) } } From 6d2b94dcb531ec052b23175d179c9518c04923ee Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Fri, 17 Jan 2025 10:22:21 +0000 Subject: [PATCH 3/3] compare disregarding order --- coderd/database/dbauthz/setup_test.go | 27 ++++++++++++++++++++++++--- 1 file changed, 24 insertions(+), 3 deletions(-) diff --git a/coderd/database/dbauthz/setup_test.go b/coderd/database/dbauthz/setup_test.go index 44996e9f9a863..4faac05b4746e 100644 --- a/coderd/database/dbauthz/setup_test.go +++ b/coderd/database/dbauthz/setup_test.go @@ -2,6 +2,7 @@ package dbauthz_test import ( "context" + "encoding/gob" "errors" "fmt" "reflect" @@ -200,9 +201,29 @@ func (s *MethodTestSuite) Subtest(testCaseF func(db database.Store, check *expec s.Equal(len(testCase.outputs), len(outputs), "method %q returned unexpected number of outputs", methodName) for i := range outputs { a, b := testCase.outputs[i].Interface(), outputs[i].Interface() - // Use cmp.Diff to get a nice diff of the two values. - if diff := cmp.Diff(a, b, cmpopts.EquateEmpty()); diff != "" { - s.Failf("compare outputs failed", "method %q returned unexpected output %d (-want +got):\n%s", methodName, i, diff) + + // To avoid the extra small overhead of gob encoding, we can + // first check if the values are equal with regard to order. + // If not, re-check disregarding order and show a nice diff + // output of the two values. + if !cmp.Equal(a, b, cmpopts.EquateEmpty()) { + if diff := cmp.Diff(a, b, + // Equate nil and empty slices. + cmpopts.EquateEmpty(), + // Allow slice order to be ignored. + cmpopts.SortSlices(func(a, b any) bool { + var ab, bb strings.Builder + _ = gob.NewEncoder(&ab).Encode(a) + _ = gob.NewEncoder(&bb).Encode(b) + // This might seem a bit dubious, but we really + // don't care about order and cmp doesn't provide + // a generic less function for slices: + // https://github.com/google/go-cmp/issues/67 + return ab.String() < bb.String() + }), + ); diff != "" { + s.Failf("compare outputs failed", "method %q returned unexpected output %d (-want +got):\n%s", methodName, i, diff) + } } } }