From 5dfd10767b10ef96217cd3e171cde39d1e1b53bf Mon Sep 17 00:00:00 2001 From: Saswata Mukherjee Date: Wed, 26 Oct 2022 09:48:06 +0530 Subject: [PATCH 1/4] Add EqualsWithNaN to testutil Signed-off-by: Saswata Mukherjee --- go.mod | 5 +++- go.sum | 2 ++ testutil/testutil.go | 19 ++++++++++++++ testutil/testutil_test.go | 54 ++++++++++++++++++++++++++++++++++++++- 4 files changed, 78 insertions(+), 2 deletions(-) diff --git a/go.mod b/go.mod index 26f3f26..0621f69 100644 --- a/go.mod +++ b/go.mod @@ -2,4 +2,7 @@ module github.com/efficientgo/core go 1.17 -require github.com/davecgh/go-spew v1.1.1 +require ( + github.com/davecgh/go-spew v1.1.1 + github.com/google/go-cmp v0.5.9 +) diff --git a/go.sum b/go.sum index b5e2922..8e9a525 100644 --- a/go.sum +++ b/go.sum @@ -1,2 +1,4 @@ github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c= github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= +github.com/google/go-cmp v0.5.9 h1:O2Tfq5qg4qc4AmwVlvv0oLiVAGB7enBSJ2x2DqQFi38= +github.com/google/go-cmp v0.5.9/go.mod h1:17dUlkBOakJ0+DkrSSNjCkIjxS6bF9zb3elmeNGIjoY= diff --git a/testutil/testutil.go b/testutil/testutil.go index c74f368..9c3db89 100644 --- a/testutil/testutil.go +++ b/testutil/testutil.go @@ -14,6 +14,8 @@ import ( "github.com/davecgh/go-spew/spew" "github.com/efficientgo/core/errors" "github.com/efficientgo/core/testutil/internal" + "github.com/google/go-cmp/cmp" + "github.com/google/go-cmp/cmp/cmpopts" ) // Assert fails the test if the condition is false. @@ -76,6 +78,23 @@ func Equals(tb testing.TB, exp, act interface{}, v ...interface{}) { tb.Fatal(sprintfWithLimit("\033[31m%s:%d:"+msg+"\n\n\texp: %#v\n\n\tgot: %#v%s\033[39m\n\n", filepath.Base(file), line, exp, act, diff(exp, act))) } +// EqualsWithNaN fails the test if exp is not equal to act and ensures NaN == NaN. +// Keep in mind that this implementation cannot check unexported fields in structs on its own +// and needs setting custom options for it. +func EqualsWithNaN(tb testing.TB, exp, act interface{}, opts cmp.Options, v ...interface{}) { + tb.Helper() + if cmp.Equal(exp, act, cmpopts.EquateNaNs(), opts) { + return + } + _, file, line, _ := runtime.Caller(1) + + var msg string + if len(v) > 0 { + msg = fmt.Sprintf(v[0].(string), v[1:]...) + } + tb.Fatal(sprintfWithLimit("\033[31m%s:%d:"+msg+"\n\n\texp: %#v\n\n\tgot: %#v%s\033[39m\n\n", filepath.Base(file), line, exp, act, diff(exp, act))) +} + // FaultOrPanicToErr returns error if panic of fault was triggered during execution of function. func FaultOrPanicToErr(f func()) (err error) { // Set this go routine to panic on segfault to allow asserting on those. diff --git a/testutil/testutil_test.go b/testutil/testutil_test.go index ac42702..27251bd 100644 --- a/testutil/testutil_test.go +++ b/testutil/testutil_test.go @@ -3,7 +3,12 @@ package testutil -import "testing" +import ( + "math" + "testing" + + "github.com/google/go-cmp/cmp" +) func TestContains(t *testing.T) { tests := map[string]struct { @@ -68,3 +73,50 @@ func TestContains(t *testing.T) { }) } } + +type child struct { + Val float64 +} + +type parent struct { + C child +} + +type unexp struct { + val float64 +} + +func TestEqualsWithNaN(t *testing.T) { + for _, tc := range []struct { + name string + a interface{} + b interface{} + opts cmp.Options + }{ + { + a: math.NaN(), + b: math.NaN(), + name: "Simple NaN value comparison", + }, + { + a: child{Val: math.NaN()}, + b: child{Val: math.NaN()}, + name: "NaN value as struct member comparison", + }, + { + a: parent{C: child{Val: math.NaN()}}, + b: parent{C: child{Val: math.NaN()}}, + name: "NaN value in nested struct comparison", + }, + { + a: unexp{val: math.NaN()}, + b: unexp{val: math.NaN()}, + name: "NaN value as unexported struct member comparison", + opts: cmp.Options{cmp.AllowUnexported(unexp{})}, + }, + } { + t.Run(tc.name, func(t *testing.T) { + EqualsWithNaN(t, tc.a, tc.b, tc.opts) + }) + } +} From 3cfb3388c0d25c9ef10720579556eafd094ca0c1 Mon Sep 17 00:00:00 2001 From: Saswata Mukherjee Date: Tue, 1 Nov 2022 19:28:00 +0530 Subject: [PATCH 2/4] Make API cleaner Signed-off-by: Saswata Mukherjee --- README.md | 2 +- testutil/testutil.go | 18 ++++++++++++------ testutil/testutil_test.go | 8 ++++++-- 3 files changed, 19 insertions(+), 9 deletions(-) diff --git a/README.md b/README.md index 24849ac..2796127 100644 --- a/README.md +++ b/README.md @@ -2,7 +2,7 @@ [![core module docs](https://img.shields.io/badge/go.dev-reference-007d9c?logo=go&logoColor=white&style=flat-square)](https://pkg.go.dev/github.com/efficientgo/core) -Go module with set of core packages **every** Go project needs. Minimal API, battle-tested, strictly versioned and with only one transient dependency--[davecgh/go-spew](https://github.com/davecgh/go-spew). +Go module with set of core packages **every** Go project needs. Minimal API, battle-tested, strictly versioned and with only two transient dependencies-- [davecgh/go-spew](https://github.com/davecgh/go-spew) and [google/go-cmp](https://github.com/google/go-cmp). Maintained by experienced Go developers, including author of the [Efficient Go book](https://www.oreilly.com/library/view/efficient-go/9781098105709/). diff --git a/testutil/testutil.go b/testutil/testutil.go index 9c3db89..113b947 100644 --- a/testutil/testutil.go +++ b/testutil/testutil.go @@ -15,7 +15,6 @@ import ( "github.com/efficientgo/core/errors" "github.com/efficientgo/core/testutil/internal" "github.com/google/go-cmp/cmp" - "github.com/google/go-cmp/cmp/cmpopts" ) // Assert fails the test if the condition is false. @@ -78,12 +77,19 @@ func Equals(tb testing.TB, exp, act interface{}, v ...interface{}) { tb.Fatal(sprintfWithLimit("\033[31m%s:%d:"+msg+"\n\n\texp: %#v\n\n\tgot: %#v%s\033[39m\n\n", filepath.Base(file), line, exp, act, diff(exp, act))) } -// EqualsWithNaN fails the test if exp is not equal to act and ensures NaN == NaN. -// Keep in mind that this implementation cannot check unexported fields in structs on its own -// and needs setting custom options for it. -func EqualsWithNaN(tb testing.TB, exp, act interface{}, opts cmp.Options, v ...interface{}) { +type goCmp struct { + opts cmp.Options +} + +func WithCmpOpts(opts ...cmp.Option) goCmp { + return goCmp{opts: opts} +} + +// Equals uses go-cmp for comparing equality between two structs, and can be used with +// various options defined in go-cmp/cmp and go-cmp/cmp/cmpopts. +func (o goCmp) Equals(tb testing.TB, exp, act interface{}, v ...interface{}) { tb.Helper() - if cmp.Equal(exp, act, cmpopts.EquateNaNs(), opts) { + if cmp.Equal(exp, act, o.opts) { return } _, file, line, _ := runtime.Caller(1) diff --git a/testutil/testutil_test.go b/testutil/testutil_test.go index 27251bd..5cf4b4a 100644 --- a/testutil/testutil_test.go +++ b/testutil/testutil_test.go @@ -8,6 +8,7 @@ import ( "testing" "github.com/google/go-cmp/cmp" + "github.com/google/go-cmp/cmp/cmpopts" ) func TestContains(t *testing.T) { @@ -97,26 +98,29 @@ func TestEqualsWithNaN(t *testing.T) { a: math.NaN(), b: math.NaN(), name: "Simple NaN value comparison", + opts: cmp.Options{cmpopts.EquateNaNs()}, }, { a: child{Val: math.NaN()}, b: child{Val: math.NaN()}, name: "NaN value as struct member comparison", + opts: cmp.Options{cmpopts.EquateNaNs()}, }, { a: parent{C: child{Val: math.NaN()}}, b: parent{C: child{Val: math.NaN()}}, name: "NaN value in nested struct comparison", + opts: cmp.Options{cmpopts.EquateNaNs()}, }, { a: unexp{val: math.NaN()}, b: unexp{val: math.NaN()}, name: "NaN value as unexported struct member comparison", - opts: cmp.Options{cmp.AllowUnexported(unexp{})}, + opts: cmp.Options{cmp.AllowUnexported(unexp{}), cmpopts.EquateNaNs()}, }, } { t.Run(tc.name, func(t *testing.T) { - EqualsWithNaN(t, tc.a, tc.b, tc.opts) + WithCmpOpts(tc.opts).Equals(t, tc.a, tc.b) }) } } From 04e25dc1d98014d6e8c87b6a5db87a085e797b67 Mon Sep 17 00:00:00 2001 From: Saswata Mukherjee Date: Tue, 6 Dec 2022 11:46:07 +0530 Subject: [PATCH 3/4] Rename to WithGoCmp Signed-off-by: Saswata Mukherjee --- testutil/testutil.go | 2 +- testutil/testutil_test.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/testutil/testutil.go b/testutil/testutil.go index 113b947..8f631d9 100644 --- a/testutil/testutil.go +++ b/testutil/testutil.go @@ -81,7 +81,7 @@ type goCmp struct { opts cmp.Options } -func WithCmpOpts(opts ...cmp.Option) goCmp { +func WithGoCmp(opts ...cmp.Option) goCmp { return goCmp{opts: opts} } diff --git a/testutil/testutil_test.go b/testutil/testutil_test.go index 5cf4b4a..08e8fb0 100644 --- a/testutil/testutil_test.go +++ b/testutil/testutil_test.go @@ -120,7 +120,7 @@ func TestEqualsWithNaN(t *testing.T) { }, } { t.Run(tc.name, func(t *testing.T) { - WithCmpOpts(tc.opts).Equals(t, tc.a, tc.b) + WithGoCmp(tc.opts).Equals(t, tc.a, tc.b) }) } } From 66aa6362b4e97dc1589b0d7e2d7a9233143e4515 Mon Sep 17 00:00:00 2001 From: Saswata Mukherjee Date: Tue, 6 Dec 2022 19:55:45 +0530 Subject: [PATCH 4/4] Add comment Signed-off-by: Saswata Mukherjee --- testutil/testutil.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/testutil/testutil.go b/testutil/testutil.go index 8f631d9..4f432d4 100644 --- a/testutil/testutil.go +++ b/testutil/testutil.go @@ -81,6 +81,9 @@ type goCmp struct { opts cmp.Options } +// WithGoCmp allows specifying options and using https://github.com/google/go-cmp +// for equality comparisons. The compatibility guarantee of this function's arguments +// are the same as go-cmp i.e none due to v0.x. func WithGoCmp(opts ...cmp.Option) goCmp { return goCmp{opts: opts} }