From 79433ace284e1270988886ec120ec5aa3d1d1776 Mon Sep 17 00:00:00 2001 From: Aoang Date: Tue, 22 Mar 2022 22:05:27 +0800 Subject: Run tests on Go 1.18 (#290) --- .github/workflows/test.yml | 13 ++----------- 1 file changed, 2 insertions(+), 11 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 5664da6..9a50673 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -2,14 +2,9 @@ on: [push, pull_request] name: Test jobs: test: - env: - GOPATH: ${{ github.workspace }} - defaults: - run: - working-directory: ${{ env.GOPATH }}/src/github.com/${{ github.repository }} strategy: matrix: - go-version: [1.11.x, 1.12.x, 1.13.x, 1.14.x, 1.15.x, 1.16.x, 1.17.x] + go-version: [1.11.x, 1.12.x, 1.13.x, 1.14.x, 1.15.x, 1.16.x, 1.17.x, 1.18.x] os: [ubuntu-latest, macos-latest] runs-on: ${{ matrix.os }} steps: @@ -19,12 +14,8 @@ jobs: go-version: ${{ matrix.go-version }} - name: Checkout code uses: actions/checkout@v2 - with: - path: ${{ env.GOPATH }}/src/github.com/${{ github.repository }} - - name: Checkout dependencies - run: go get golang.org/x/xerrors - name: Test run: go test -v -race ./... - name: Format - if: matrix.go-version == '1.17.x' + if: matrix.go-version == '1.18.x' run: diff -u <(echo -n) <(gofmt -d .) -- cgit v1.2.3 From 4664e24d52beca933d6f5378d36e1d405099115d Mon Sep 17 00:00:00 2001 From: Joe Tsai Date: Mon, 25 Apr 2022 09:26:47 -0700 Subject: Fix printing of types in reporter output (#293) When printing a pointer, only elide the type for unnamed pointers. Otherwise, we can run into situations where named and unnamed pointers format the same way in indistinguishable ways. When printing an interview, never skip the interface type. Whether we skip printing the type should be determined by the parent containers, and not locally determined. For examples, interface values within a struct, slice, or map will always be elided since they can be inferred. --- cmp/compare.go | 2 ++ cmp/compare_test.go | 34 ++++++++++++++++++++++++++++++++++ cmp/report_reflect.go | 8 ++++++-- cmp/testdata/diffs | 24 ++++++++++++++++++++++++ 4 files changed, 66 insertions(+), 2 deletions(-) diff --git a/cmp/compare.go b/cmp/compare.go index 2a54467..fd2b3a4 100644 --- a/cmp/compare.go +++ b/cmp/compare.go @@ -40,6 +40,8 @@ import ( "github.com/google/go-cmp/cmp/internal/value" ) +// TODO(≥go1.18): Use any instead of interface{}. + // Equal reports whether x and y are equal by recursively applying the // following rules in the given order to x and y and all of their sub-values: // diff --git a/cmp/compare_test.go b/cmp/compare_test.go index 9ad9456..7278485 100644 --- a/cmp/compare_test.go +++ b/cmp/compare_test.go @@ -104,6 +104,7 @@ func mustFormatGolden(path string, in []struct{ Name, Data string }) { var now = time.Date(2009, time.November, 10, 23, 00, 00, 00, time.UTC) +// TODO(≥go1.18): Define a generic function that boxes a value on the heap. func newInt(n int) *int { return &n } type Stringer string @@ -885,6 +886,7 @@ func reporterTests() []test { FloatsB []MyFloat FloatsC MyFloats } + PointerString *string ) return []test{{ @@ -1351,6 +1353,38 @@ using the AllowUnexported option.`, "\n"), "bar": true, }`, reason: "short multiline JSON should prefer triple-quoted string diff as it is more readable", + }, { + label: label + "/PointerToStringOrAny", + x: func() *string { + var v string = "hello" + return &v + }(), + y: func() *interface{} { + var v interface{} = "hello" + return &v + }(), + reason: "mismatched types between any and *any should print differently", + }, { + label: label + "/NamedPointer", + x: func() *string { + v := "hello" + return &v + }(), + y: func() PointerString { + v := "hello" + return &v + }(), + reason: "mismatched pointer types should print differently", + }, { + label: label + "/MapStringAny", + x: map[string]interface{}{"key": int(0)}, + y: map[string]interface{}{"key": uint(0)}, + reason: "mismatched underlying value within interface", + }, { + label: label + "/StructFieldAny", + x: struct{ X interface{} }{int(0)}, + y: struct{ X interface{} }{uint(0)}, + reason: "mismatched underlying value within interface", }} } diff --git a/cmp/report_reflect.go b/cmp/report_reflect.go index 76c04fd..d9d7323 100644 --- a/cmp/report_reflect.go +++ b/cmp/report_reflect.go @@ -282,7 +282,12 @@ func (opts formatOptions) FormatValue(v reflect.Value, parentKind reflect.Kind, } defer ptrs.Pop() - skipType = true // Let the underlying value print the type instead + // Skip the name only if this is an unnamed pointer type. + // Otherwise taking the address of a value does not reproduce + // the named pointer type. + if v.Type().Name() == "" { + skipType = true // Let the underlying value print the type instead + } out = opts.FormatValue(v.Elem(), t.Kind(), ptrs) out = wrapTrunkReference(ptrRef, opts.PrintAddresses, out) out = &textWrap{Prefix: "&", Value: out} @@ -293,7 +298,6 @@ func (opts formatOptions) FormatValue(v reflect.Value, parentKind reflect.Kind, } // Interfaces accept different concrete types, // so configure the underlying value to explicitly print the type. - skipType = true // Print the concrete type instead return opts.WithTypeMode(emitType).FormatValue(v.Elem(), t.Kind(), ptrs) default: panic(fmt.Sprintf("%v kind not handled", v.Kind())) diff --git a/cmp/testdata/diffs b/cmp/testdata/diffs index d207803..23781f7 100644 --- a/cmp/testdata/diffs +++ b/cmp/testdata/diffs @@ -1134,6 +1134,30 @@ """ ) >>> TestDiff/Reporter/ShortJSON +<<< TestDiff/Reporter/PointerToStringOrAny + any( +- &string("hello"), ++ &any(string("hello")), + ) +>>> TestDiff/Reporter/PointerToStringOrAny +<<< TestDiff/Reporter/NamedPointer + any( +- &string("hello"), ++ cmp_test.PointerString(&string("hello")), + ) +>>> TestDiff/Reporter/NamedPointer +<<< TestDiff/Reporter/MapStringAny + map[string]any{ +- "key": int(0), ++ "key": uint(0), + } +>>> TestDiff/Reporter/MapStringAny +<<< TestDiff/Reporter/StructFieldAny + struct{ X any }{ +- X: int(0), ++ X: uint(0), + } +>>> TestDiff/Reporter/StructFieldAny <<< TestDiff/EmbeddedStruct/ParentStructA/Inequal teststructs.ParentStructA{ privateStruct: teststructs.privateStruct{ -- cgit v1.2.3 From 71220fc3ca5513eaf3583d10fd18da893bc332d8 Mon Sep 17 00:00:00 2001 From: Joe Tsai Date: Mon, 25 Apr 2022 09:32:19 -0700 Subject: Use string formatting for slice of bytes (#294) If a slice of bytes is mostly text, format them as text instead of as []byte literal with hexadecimal digits. Avoid always printing the type. This is technically invalid Go code, but is unnecessary in many cases since the type is inferred from the parent concrete type. Fixes #272 --- cmp/cmpopts/example_test.go | 2 +- cmp/compare_test.go | 18 ++++++++++++++++++ cmp/example_test.go | 2 +- cmp/report_compare.go | 5 ++++- cmp/report_reflect.go | 2 +- cmp/testdata/diffs | 30 +++++++++++++++++++++++++++--- 6 files changed, 52 insertions(+), 7 deletions(-) diff --git a/cmp/cmpopts/example_test.go b/cmp/cmpopts/example_test.go index 0cf2513..4b9a8ab 100644 --- a/cmp/cmpopts/example_test.go +++ b/cmp/cmpopts/example_test.go @@ -39,7 +39,7 @@ func ExampleIgnoreFields_testing() { // SSID: "CoffeeShopWiFi", // - IPAddress: s"192.168.0.2", // + IPAddress: s"192.168.0.1", - // NetMask: {0xff, 0xff, 0x00, 0x00}, + // NetMask: s"ffff0000", // Clients: []cmpopts_test.Client{ // ... // 3 identical elements // {Hostname: "espresso", ...}, diff --git a/cmp/compare_test.go b/cmp/compare_test.go index 7278485..aafe441 100644 --- a/cmp/compare_test.go +++ b/cmp/compare_test.go @@ -1385,6 +1385,24 @@ using the AllowUnexported option.`, "\n"), x: struct{ X interface{} }{int(0)}, y: struct{ X interface{} }{uint(0)}, reason: "mismatched underlying value within interface", + }, { + label: label + "/SliceOfBytesText", + x: [][]byte{ + []byte("hello"), []byte("foo"), []byte("barbaz"), []byte("blahdieblah"), + }, + y: [][]byte{ + []byte("foo"), []byte("foo"), []byte("barbaz"), []byte("added"), []byte("here"), []byte("hrmph"), + }, + reason: "should print text byte slices as strings", + }, { + label: label + "/SliceOfBytesBinary", + x: [][]byte{ + []byte("\xde\xad\xbe\xef"), []byte("\xffoo"), []byte("barbaz"), []byte("blahdieblah"), + }, + y: [][]byte{ + []byte("\xffoo"), []byte("foo"), []byte("barbaz"), []byte("added"), []byte("here"), []byte("hrmph\xff"), + }, + reason: "should print text byte slices as strings except those with binary", }} } diff --git a/cmp/example_test.go b/cmp/example_test.go index e1f4338..9968149 100644 --- a/cmp/example_test.go +++ b/cmp/example_test.go @@ -37,7 +37,7 @@ func ExampleDiff_testing() { // SSID: "CoffeeShopWiFi", // - IPAddress: s"192.168.0.2", // + IPAddress: s"192.168.0.1", - // NetMask: {0xff, 0xff, 0x00, 0x00}, + // NetMask: s"ffff0000", // Clients: []cmp_test.Client{ // ... // 2 identical elements // {Hostname: "macchiato", IPAddress: s"192.168.0.153", LastSeen: s"2009-11-10 23:39:43 +0000 UTC"}, diff --git a/cmp/report_compare.go b/cmp/report_compare.go index 104bb30..1ef65ac 100644 --- a/cmp/report_compare.go +++ b/cmp/report_compare.go @@ -116,7 +116,10 @@ func (opts formatOptions) FormatDiff(v *valueNode, ptrs *pointerReferences) (out } // For leaf nodes, format the value based on the reflect.Values alone. - if v.MaxDepth == 0 { + // As a special case, treat equal []byte as a leaf nodes. + isBytes := v.Type.Kind() == reflect.Slice && v.Type.Elem() == reflect.TypeOf(byte(0)) + isEqualBytes := isBytes && v.NumDiff+v.NumIgnored+v.NumTransformed == 0 + if v.MaxDepth == 0 || isEqualBytes { switch opts.DiffMode { case diffUnknown, diffIdentical: // Format Equal. diff --git a/cmp/report_reflect.go b/cmp/report_reflect.go index d9d7323..287b893 100644 --- a/cmp/report_reflect.go +++ b/cmp/report_reflect.go @@ -211,7 +211,7 @@ func (opts formatOptions) FormatValue(v reflect.Value, parentKind reflect.Kind, if len(b) > 0 && utf8.Valid(b) && len(bytes.TrimFunc(b, isPrintSpace)) == 0 { out = opts.formatString("", string(b)) skipType = true - return opts.WithTypeMode(emitType).FormatType(t, out) + return opts.FormatType(t, out) } } diff --git a/cmp/testdata/diffs b/cmp/testdata/diffs index 23781f7..8bff76f 100644 --- a/cmp/testdata/diffs +++ b/cmp/testdata/diffs @@ -241,9 +241,9 @@ "string", }), Bytes: []uint8(Inverse(SplitBytes, [][]uint8{ - {0x73, 0x6f, 0x6d, 0x65}, - {0x6d, 0x75, 0x6c, 0x74, ...}, - {0x6c, 0x69, 0x6e, 0x65}, + "some", + "multi", + "line", { - 0x62, + 0x42, @@ -1158,6 +1158,30 @@ + X: uint(0), } >>> TestDiff/Reporter/StructFieldAny +<<< TestDiff/Reporter/SliceOfBytesText + [][]uint8{ +- "hello", + "foo", ++ "foo", + "barbaz", ++ "added", ++ "here", +- "blahdieblah", ++ "hrmph", + } +>>> TestDiff/Reporter/SliceOfBytesText +<<< TestDiff/Reporter/SliceOfBytesBinary + [][]uint8{ +- {0xde, 0xad, 0xbe, 0xef}, + {0xff, 0x6f, 0x6f}, ++ "foo", + "barbaz", ++ "added", ++ "here", +- "blahdieblah", ++ {0x68, 0x72, 0x6d, 0x70, 0x68, 0xff}, + } +>>> TestDiff/Reporter/SliceOfBytesBinary <<< TestDiff/EmbeddedStruct/ParentStructA/Inequal teststructs.ParentStructA{ privateStruct: teststructs.privateStruct{ -- cgit v1.2.3 From 63c2960be651bb95aaf14535bd5e36e86b6b5458 Mon Sep 17 00:00:00 2001 From: Tatsuya Kaneko Date: Wed, 27 Apr 2022 04:24:26 +0900 Subject: remove xerrors (#292) Versions older than Go 1.13 are no longer in use. Remove unnecessary dependencies. --- .github/workflows/test.yml | 2 +- cmp/cmpopts/errors_xerrors.go | 19 ------------------- cmp/cmpopts/util_test.go | 13 ++++++------- go.mod | 2 -- go.sum | 2 -- 5 files changed, 7 insertions(+), 31 deletions(-) delete mode 100644 cmp/cmpopts/errors_xerrors.go delete mode 100644 go.sum diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 9a50673..a93f058 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -4,7 +4,7 @@ jobs: test: strategy: matrix: - go-version: [1.11.x, 1.12.x, 1.13.x, 1.14.x, 1.15.x, 1.16.x, 1.17.x, 1.18.x] + go-version: [1.13.x, 1.14.x, 1.15.x, 1.16.x, 1.17.x, 1.18.x] os: [ubuntu-latest, macos-latest] runs-on: ${{ matrix.os }} steps: diff --git a/cmp/cmpopts/errors_xerrors.go b/cmp/cmpopts/errors_xerrors.go deleted file mode 100644 index 60b0727..0000000 --- a/cmp/cmpopts/errors_xerrors.go +++ /dev/null @@ -1,19 +0,0 @@ -// Copyright 2021, The Go Authors. All rights reserved. -// Use of this source code is governed by a BSD-style -// license that can be found in the LICENSE file. - -//go:build !go1.13 -// +build !go1.13 - -// TODO(≥go1.13): For support on Date: Tue, 26 Apr 2022 13:49:16 -0700 Subject: Additional cleanup with Go 1.13 as minimal version (#295) --- cmp/cmpopts/equate.go | 7 +++++++ cmp/cmpopts/errors_go113.go | 16 ---------------- go.mod | 2 +- 3 files changed, 8 insertions(+), 17 deletions(-) delete mode 100644 cmp/cmpopts/errors_go113.go diff --git a/cmp/cmpopts/equate.go b/cmp/cmpopts/equate.go index 62837c9..c49a756 100644 --- a/cmp/cmpopts/equate.go +++ b/cmp/cmpopts/equate.go @@ -6,6 +6,7 @@ package cmpopts import ( + "errors" "math" "reflect" "time" @@ -146,3 +147,9 @@ func areConcreteErrors(x, y interface{}) bool { _, ok2 := y.(error) return ok1 && ok2 } + +func compareErrors(x, y interface{}) bool { + xe := x.(error) + ye := y.(error) + return errors.Is(xe, ye) || errors.Is(ye, xe) +} diff --git a/cmp/cmpopts/errors_go113.go b/cmp/cmpopts/errors_go113.go deleted file mode 100644 index 8eb2b84..0000000 --- a/cmp/cmpopts/errors_go113.go +++ /dev/null @@ -1,16 +0,0 @@ -// Copyright 2021, The Go Authors. All rights reserved. -// Use of this source code is governed by a BSD-style -// license that can be found in the LICENSE file. - -//go:build go1.13 -// +build go1.13 - -package cmpopts - -import "errors" - -func compareErrors(x, y interface{}) bool { - xe := x.(error) - ye := y.(error) - return errors.Is(xe, ye) || errors.Is(ye, xe) -} diff --git a/go.mod b/go.mod index b570017..f55cea6 100644 --- a/go.mod +++ b/go.mod @@ -1,3 +1,3 @@ module github.com/google/go-cmp -go 1.11 +go 1.13 -- cgit v1.2.3 From a53d7e09b000ee6e0ca9f2676820299b5de8e89f Mon Sep 17 00:00:00 2001 From: Joe Tsai Date: Mon, 6 Jun 2022 10:31:27 -0700 Subject: Use reflect.Value.IsZero (#297) Now that Go 1.13 is the minimum version, we can use the reflect.Value.IsZero method instead of our own internal/value.IsZero function. Interestingly, our IsZero function pre-dates the IsZero method, but fortunately has the exact same semantics, since both are targetting semantics defined by the Go language specification. --- cmp/internal/value/zero.go | 48 ------------------------------------- cmp/internal/value/zero_test.go | 52 ----------------------------------------- cmp/report_compare.go | 8 +++---- cmp/report_reflect.go | 2 +- 4 files changed, 4 insertions(+), 106 deletions(-) delete mode 100644 cmp/internal/value/zero.go delete mode 100644 cmp/internal/value/zero_test.go diff --git a/cmp/internal/value/zero.go b/cmp/internal/value/zero.go deleted file mode 100644 index 9147a29..0000000 --- a/cmp/internal/value/zero.go +++ /dev/null @@ -1,48 +0,0 @@ -// Copyright 2017, The Go Authors. All rights reserved. -// Use of this source code is governed by a BSD-style -// license that can be found in the LICENSE file. - -package value - -import ( - "math" - "reflect" -) - -// IsZero reports whether v is the zero value. -// This does not rely on Interface and so can be used on unexported fields. -func IsZero(v reflect.Value) bool { - switch v.Kind() { - case reflect.Bool: - return v.Bool() == false - case reflect.Int, reflect.Int8, reflect.Int16, reflect.Int32, reflect.Int64: - return v.Int() == 0 - case reflect.Uint, reflect.Uint8, reflect.Uint16, reflect.Uint32, reflect.Uint64, reflect.Uintptr: - return v.Uint() == 0 - case reflect.Float32, reflect.Float64: - return math.Float64bits(v.Float()) == 0 - case reflect.Complex64, reflect.Complex128: - return math.Float64bits(real(v.Complex())) == 0 && math.Float64bits(imag(v.Complex())) == 0 - case reflect.String: - return v.String() == "" - case reflect.UnsafePointer: - return v.Pointer() == 0 - case reflect.Chan, reflect.Func, reflect.Interface, reflect.Ptr, reflect.Map, reflect.Slice: - return v.IsNil() - case reflect.Array: - for i := 0; i < v.Len(); i++ { - if !IsZero(v.Index(i)) { - return false - } - } - return true - case reflect.Struct: - for i := 0; i < v.NumField(); i++ { - if !IsZero(v.Field(i)) { - return false - } - } - return true - } - return false -} diff --git a/cmp/internal/value/zero_test.go b/cmp/internal/value/zero_test.go deleted file mode 100644 index ddaa337..0000000 --- a/cmp/internal/value/zero_test.go +++ /dev/null @@ -1,52 +0,0 @@ -// Copyright 2019, The Go Authors. All rights reserved. -// Use of this source code is governed by a BSD-style -// license that can be found in the LICENSE file. - -package value - -import ( - "archive/tar" - "math" - "reflect" - "testing" -) - -func TestIsZero(t *testing.T) { - tests := []struct { - in interface{} - want bool - }{ - {0, true}, - {1, false}, - {"", true}, - {"foo", false}, - {[]byte(nil), true}, - {[]byte{}, false}, - {map[string]bool(nil), true}, - {map[string]bool{}, false}, - {tar.Header{}, true}, - {&tar.Header{}, false}, - {tar.Header{Name: "foo"}, false}, - {(chan bool)(nil), true}, - {make(chan bool), false}, - {(func(*testing.T))(nil), true}, - {TestIsZero, false}, - {[...]int{0, 0, 0}, true}, - {[...]int{0, 1, 0}, false}, - {math.Copysign(0, +1), true}, - {math.Copysign(0, -1), false}, - {complex(math.Copysign(0, +1), math.Copysign(0, +1)), true}, - {complex(math.Copysign(0, -1), math.Copysign(0, +1)), false}, - {complex(math.Copysign(0, +1), math.Copysign(0, -1)), false}, - {complex(math.Copysign(0, -1), math.Copysign(0, -1)), false}, - } - - for _, tt := range tests { - t.Run("", func(t *testing.T) { - got := IsZero(reflect.ValueOf(tt.in)) - if got != tt.want { - t.Errorf("IsZero(%v) = %v, want %v", tt.in, got, tt.want) - } - }) - } -} diff --git a/cmp/report_compare.go b/cmp/report_compare.go index 1ef65ac..498c57a 100644 --- a/cmp/report_compare.go +++ b/cmp/report_compare.go @@ -7,8 +7,6 @@ package cmp import ( "fmt" "reflect" - - "github.com/google/go-cmp/cmp/internal/value" ) // numContextRecords is the number of surrounding equal records to print. @@ -248,11 +246,11 @@ func (opts formatOptions) formatDiffList(recs []reportRecord, k reflect.Kind, pt var isZero bool switch opts.DiffMode { case diffIdentical: - isZero = value.IsZero(r.Value.ValueX) || value.IsZero(r.Value.ValueY) + isZero = r.Value.ValueX.IsZero() || r.Value.ValueY.IsZero() case diffRemoved: - isZero = value.IsZero(r.Value.ValueX) + isZero = r.Value.ValueX.IsZero() case diffInserted: - isZero = value.IsZero(r.Value.ValueY) + isZero = r.Value.ValueY.IsZero() } if isZero { continue diff --git a/cmp/report_reflect.go b/cmp/report_reflect.go index 287b893..381fd31 100644 --- a/cmp/report_reflect.go +++ b/cmp/report_reflect.go @@ -184,7 +184,7 @@ func (opts formatOptions) FormatValue(v reflect.Value, parentKind reflect.Kind, } for i := 0; i < v.NumField(); i++ { vv := v.Field(i) - if value.IsZero(vv) { + if vv.IsZero() { continue // Elide fields with zero values } if len(list) == maxLen { -- cgit v1.2.3 From 14ad8a02f30ba66e7e19f9814e69daab44219cb8 Mon Sep 17 00:00:00 2001 From: Joe Tsai Date: Wed, 13 Jul 2022 11:10:07 -0700 Subject: Format with Go 1.19 formatter (#304) This allows the GoDoc to take advantage of new markup syntax introduced in Go 1.19. This does not require that our minimum supported version be bumped to Go 1.19 since the pkgsite renders our godoc regardless of supported Go version. --- cmp/cmpopts/equate.go | 1 + cmp/cmpopts/sort.go | 14 +++++----- cmp/cmpopts/struct_filter.go | 2 ++ cmp/cmpopts/xform.go | 1 + cmp/compare.go | 62 +++++++++++++++++++++++--------------------- cmp/compare_test.go | 2 +- cmp/internal/diff/diff.go | 44 +++++++++++++++++-------------- cmp/options.go | 8 +++--- cmp/path.go | 16 +++++++----- cmp/report_slices.go | 12 ++++----- cmp/report_text.go | 1 + 11 files changed, 88 insertions(+), 75 deletions(-) diff --git a/cmp/cmpopts/equate.go b/cmp/cmpopts/equate.go index c49a756..e54a76c 100644 --- a/cmp/cmpopts/equate.go +++ b/cmp/cmpopts/equate.go @@ -42,6 +42,7 @@ func isEmpty(x, y interface{}) bool { // The fraction and margin must be non-negative. // // The mathematical expression used is equivalent to: +// // |x-y| ≤ max(fraction*min(|x|, |y|), margin) // // EquateApprox can be used in conjunction with EquateNaNs. diff --git a/cmp/cmpopts/sort.go b/cmp/cmpopts/sort.go index a646d74..0eb2a75 100644 --- a/cmp/cmpopts/sort.go +++ b/cmp/cmpopts/sort.go @@ -18,9 +18,9 @@ import ( // sort any slice with element type V that is assignable to T. // // The less function must be: -// • Deterministic: less(x, y) == less(x, y) -// • Irreflexive: !less(x, x) -// • Transitive: if !less(x, y) and !less(y, z), then !less(x, z) +// - Deterministic: less(x, y) == less(x, y) +// - Irreflexive: !less(x, x) +// - Transitive: if !less(x, y) and !less(y, z), then !less(x, z) // // The less function does not have to be "total". That is, if !less(x, y) and // !less(y, x) for two elements x and y, their relative order is maintained. @@ -91,10 +91,10 @@ func (ss sliceSorter) less(v reflect.Value, i, j int) bool { // use Comparers on K or the K.Equal method if it exists. // // The less function must be: -// • Deterministic: less(x, y) == less(x, y) -// • Irreflexive: !less(x, x) -// • Transitive: if !less(x, y) and !less(y, z), then !less(x, z) -// • Total: if x != y, then either less(x, y) or less(y, x) +// - Deterministic: less(x, y) == less(x, y) +// - Irreflexive: !less(x, x) +// - Transitive: if !less(x, y) and !less(y, z), then !less(x, z) +// - Total: if x != y, then either less(x, y) or less(y, x) // // SortMaps can be used in conjunction with EquateEmpty. func SortMaps(lessFunc interface{}) cmp.Option { diff --git a/cmp/cmpopts/struct_filter.go b/cmp/cmpopts/struct_filter.go index a09829c..ca11a40 100644 --- a/cmp/cmpopts/struct_filter.go +++ b/cmp/cmpopts/struct_filter.go @@ -67,12 +67,14 @@ func (sf structFilter) filter(p cmp.Path) bool { // fieldTree represents a set of dot-separated identifiers. // // For example, inserting the following selectors: +// // Foo // Foo.Bar.Baz // Foo.Buzz // Nuka.Cola.Quantum // // Results in a tree of the form: +// // {sub: { // "Foo": {ok: true, sub: { // "Bar": {sub: { diff --git a/cmp/cmpopts/xform.go b/cmp/cmpopts/xform.go index 4eb49d6..8812443 100644 --- a/cmp/cmpopts/xform.go +++ b/cmp/cmpopts/xform.go @@ -23,6 +23,7 @@ func (xf xformFilter) filter(p cmp.Path) bool { // that the transformer cannot be recursively applied upon its own output. // // An example use case is a transformer that splits a string by lines: +// // AcyclicTransformer("SplitLines", func(s string) []string{ // return strings.Split(s, "\n") // }) diff --git a/cmp/compare.go b/cmp/compare.go index fd2b3a4..caf75ad 100644 --- a/cmp/compare.go +++ b/cmp/compare.go @@ -13,21 +13,21 @@ // // The primary features of cmp are: // -// • When the default behavior of equality does not suit the needs of the test, -// custom equality functions can override the equality operation. -// For example, an equality function may report floats as equal so long as they -// are within some tolerance of each other. +// - When the default behavior of equality does not suit the test's needs, +// custom equality functions can override the equality operation. +// For example, an equality function may report floats as equal so long as +// they are within some tolerance of each other. // -// • Types that have an Equal method may use that method to determine equality. -// This allows package authors to determine the equality operation for the types -// that they define. +// - Types with an Equal method may use that method to determine equality. +// This allows package authors to determine the equality operation +// for the types that they define. // -// • If no custom equality functions are used and no Equal method is defined, -// equality is determined by recursively comparing the primitive kinds on both -// values, much like reflect.DeepEqual. Unlike reflect.DeepEqual, unexported -// fields are not compared by default; they result in panics unless suppressed -// by using an Ignore option (see cmpopts.IgnoreUnexported) or explicitly -// compared using the Exporter option. +// - If no custom equality functions are used and no Equal method is defined, +// equality is determined by recursively comparing the primitive kinds on +// both values, much like reflect.DeepEqual. Unlike reflect.DeepEqual, +// unexported fields are not compared by default; they result in panics +// unless suppressed by using an Ignore option (see cmpopts.IgnoreUnexported) +// or explicitly compared using the Exporter option. package cmp import ( @@ -45,25 +45,25 @@ import ( // Equal reports whether x and y are equal by recursively applying the // following rules in the given order to x and y and all of their sub-values: // -// • Let S be the set of all Ignore, Transformer, and Comparer options that -// remain after applying all path filters, value filters, and type filters. -// If at least one Ignore exists in S, then the comparison is ignored. -// If the number of Transformer and Comparer options in S is greater than one, -// then Equal panics because it is ambiguous which option to use. -// If S contains a single Transformer, then use that to transform the current -// values and recursively call Equal on the output values. -// If S contains a single Comparer, then use that to compare the current values. -// Otherwise, evaluation proceeds to the next rule. +// - Let S be the set of all Ignore, Transformer, and Comparer options that +// remain after applying all path filters, value filters, and type filters. +// If at least one Ignore exists in S, then the comparison is ignored. +// If the number of Transformer and Comparer options in S is non-zero, +// then Equal panics because it is ambiguous which option to use. +// If S contains a single Transformer, then use that to transform +// the current values and recursively call Equal on the output values. +// If S contains a single Comparer, then use that to compare the current values. +// Otherwise, evaluation proceeds to the next rule. // -// • If the values have an Equal method of the form "(T) Equal(T) bool" or -// "(T) Equal(I) bool" where T is assignable to I, then use the result of -// x.Equal(y) even if x or y is nil. Otherwise, no such method exists and -// evaluation proceeds to the next rule. +// - If the values have an Equal method of the form "(T) Equal(T) bool" or +// "(T) Equal(I) bool" where T is assignable to I, then use the result of +// x.Equal(y) even if x or y is nil. Otherwise, no such method exists and +// evaluation proceeds to the next rule. // -// • Lastly, try to compare x and y based on their basic kinds. -// Simple kinds like booleans, integers, floats, complex numbers, strings, and -// channels are compared using the equivalent of the == operator in Go. -// Functions are only equal if they are both nil, otherwise they are unequal. +// - Lastly, try to compare x and y based on their basic kinds. +// Simple kinds like booleans, integers, floats, complex numbers, strings, +// and channels are compared using the equivalent of the == operator in Go. +// Functions are only equal if they are both nil, otherwise they are unequal. // // Structs are equal if recursively calling Equal on all fields report equal. // If a struct contains unexported fields, Equal panics unless an Ignore option @@ -639,7 +639,9 @@ type dynChecker struct{ curr, next int } // Next increments the state and reports whether a check should be performed. // // Checks occur every Nth function call, where N is a triangular number: +// // 0 1 3 6 10 15 21 28 36 45 55 66 78 91 105 120 136 153 171 190 ... +// // See https://en.wikipedia.org/wiki/Triangular_number // // This sequence ensures that the cost of checks drops significantly as diff --git a/cmp/compare_test.go b/cmp/compare_test.go index aafe441..dc86f01 100644 --- a/cmp/compare_test.go +++ b/cmp/compare_test.go @@ -43,7 +43,7 @@ var update = flag.Bool("update", false, "update golden test files") const goldenHeaderPrefix = "<<< " const goldenFooterPrefix = ">>> " -/// mustParseGolden parses a file as a set of key-value pairs. +// mustParseGolden parses a file as a set of key-value pairs. // // The syntax is simple and looks something like: // diff --git a/cmp/internal/diff/diff.go b/cmp/internal/diff/diff.go index bc196b1..a248e54 100644 --- a/cmp/internal/diff/diff.go +++ b/cmp/internal/diff/diff.go @@ -127,9 +127,9 @@ var randBool = rand.New(rand.NewSource(time.Now().Unix())).Intn(2) == 0 // This function returns an edit-script, which is a sequence of operations // needed to convert one list into the other. The following invariants for // the edit-script are maintained: -// • eq == (es.Dist()==0) -// • nx == es.LenX() -// • ny == es.LenY() +// - eq == (es.Dist()==0) +// - nx == es.LenX() +// - ny == es.LenY() // // This algorithm is not guaranteed to be an optimal solution (i.e., one that // produces an edit-script with a minimal Levenshtein distance). This algorithm @@ -169,12 +169,13 @@ func Difference(nx, ny int, f EqualFunc) (es EditScript) { // A diagonal edge is equivalent to a matching symbol between both X and Y. // Invariants: - // • 0 ≤ fwdPath.X ≤ (fwdFrontier.X, revFrontier.X) ≤ revPath.X ≤ nx - // • 0 ≤ fwdPath.Y ≤ (fwdFrontier.Y, revFrontier.Y) ≤ revPath.Y ≤ ny + // - 0 ≤ fwdPath.X ≤ (fwdFrontier.X, revFrontier.X) ≤ revPath.X ≤ nx + // - 0 ≤ fwdPath.Y ≤ (fwdFrontier.Y, revFrontier.Y) ≤ revPath.Y ≤ ny // // In general: - // • fwdFrontier.X < revFrontier.X - // • fwdFrontier.Y < revFrontier.Y + // - fwdFrontier.X < revFrontier.X + // - fwdFrontier.Y < revFrontier.Y + // // Unless, it is time for the algorithm to terminate. fwdPath := path{+1, point{0, 0}, make(EditScript, 0, (nx+ny)/2)} revPath := path{-1, point{nx, ny}, make(EditScript, 0)} @@ -195,19 +196,21 @@ func Difference(nx, ny int, f EqualFunc) (es EditScript) { // computing sub-optimal edit-scripts between two lists. // // The algorithm is approximately as follows: - // • Searching for differences switches back-and-forth between - // a search that starts at the beginning (the top-left corner), and - // a search that starts at the end (the bottom-right corner). The goal of - // the search is connect with the search from the opposite corner. - // • As we search, we build a path in a greedy manner, where the first - // match seen is added to the path (this is sub-optimal, but provides a - // decent result in practice). When matches are found, we try the next pair - // of symbols in the lists and follow all matches as far as possible. - // • When searching for matches, we search along a diagonal going through - // through the "frontier" point. If no matches are found, we advance the - // frontier towards the opposite corner. - // • This algorithm terminates when either the X coordinates or the - // Y coordinates of the forward and reverse frontier points ever intersect. + // - Searching for differences switches back-and-forth between + // a search that starts at the beginning (the top-left corner), and + // a search that starts at the end (the bottom-right corner). + // The goal of the search is connect with the search + // from the opposite corner. + // - As we search, we build a path in a greedy manner, + // where the first match seen is added to the path (this is sub-optimal, + // but provides a decent result in practice). When matches are found, + // we try the next pair of symbols in the lists and follow all matches + // as far as possible. + // - When searching for matches, we search along a diagonal going through + // through the "frontier" point. If no matches are found, + // we advance the frontier towards the opposite corner. + // - This algorithm terminates when either the X coordinates or the + // Y coordinates of the forward and reverse frontier points ever intersect. // This algorithm is correct even if searching only in the forward direction // or in the reverse direction. We do both because it is commonly observed @@ -389,6 +392,7 @@ type point struct{ X, Y int } func (p *point) add(dx, dy int) { p.X += dx; p.Y += dy } // zigzag maps a consecutive sequence of integers to a zig-zag sequence. +// // [0 1 2 3 4 5 ...] => [0 -1 +1 -2 +2 ...] func zigzag(x int) int { if x&1 != 0 { diff --git a/cmp/options.go b/cmp/options.go index e57b9eb..e254ffd 100644 --- a/cmp/options.go +++ b/cmp/options.go @@ -33,6 +33,7 @@ type Option interface { } // applicableOption represents the following types: +// // Fundamental: ignore | validator | *comparer | *transformer // Grouping: Options type applicableOption interface { @@ -43,6 +44,7 @@ type applicableOption interface { } // coreOption represents the following types: +// // Fundamental: ignore | validator | *comparer | *transformer // Filters: *pathFilter | *valuesFilter type coreOption interface { @@ -336,9 +338,9 @@ func (tr transformer) String() string { // both implement T. // // The equality function must be: -// • Symmetric: equal(x, y) == equal(y, x) -// • Deterministic: equal(x, y) == equal(x, y) -// • Pure: equal(x, y) does not modify x or y +// - Symmetric: equal(x, y) == equal(y, x) +// - Deterministic: equal(x, y) == equal(x, y) +// - Pure: equal(x, y) does not modify x or y func Comparer(f interface{}) Option { v := reflect.ValueOf(f) if !function.IsType(v.Type(), function.Equal) || v.IsNil() { diff --git a/cmp/path.go b/cmp/path.go index c710034..557c7f2 100644 --- a/cmp/path.go +++ b/cmp/path.go @@ -41,13 +41,13 @@ type PathStep interface { // The type of each valid value is guaranteed to be identical to Type. // // In some cases, one or both may be invalid or have restrictions: - // • For StructField, both are not interface-able if the current field - // is unexported and the struct type is not explicitly permitted by - // an Exporter to traverse unexported fields. - // • For SliceIndex, one may be invalid if an element is missing from - // either the x or y slice. - // • For MapIndex, one may be invalid if an entry is missing from - // either the x or y map. + // - For StructField, both are not interface-able if the current field + // is unexported and the struct type is not explicitly permitted by + // an Exporter to traverse unexported fields. + // - For SliceIndex, one may be invalid if an element is missing from + // either the x or y slice. + // - For MapIndex, one may be invalid if an entry is missing from + // either the x or y map. // // The provided values must not be mutated. Values() (vx, vy reflect.Value) @@ -94,6 +94,7 @@ func (pa Path) Index(i int) PathStep { // The simplified path only contains struct field accesses. // // For example: +// // MyMap.MySlices.MyField func (pa Path) String() string { var ss []string @@ -108,6 +109,7 @@ func (pa Path) String() string { // GoString returns the path to a specific node using Go syntax. // // For example: +// // (*root.MyMap["key"].(*mypkg.MyStruct).MySlices)[2][3].MyField func (pa Path) GoString() string { var ssPre, ssPost []string diff --git a/cmp/report_slices.go b/cmp/report_slices.go index 68b5c1a..21aecd9 100644 --- a/cmp/report_slices.go +++ b/cmp/report_slices.go @@ -171,12 +171,13 @@ func (opts formatOptions) FormatDiffSlice(v *valueNode) textNode { // differences in a string literal. This format is more readable, // but has edge-cases where differences are visually indistinguishable. // This format is avoided under the following conditions: - // • A line starts with `"""` - // • A line starts with "..." - // • A line contains non-printable characters - // • Adjacent different lines differ only by whitespace + // - A line starts with `"""` + // - A line starts with "..." + // - A line contains non-printable characters + // - Adjacent different lines differ only by whitespace // // For example: + // // """ // ... // 3 identical lines // foo @@ -446,7 +447,6 @@ func (opts formatOptions) formatDiffSlice( // {NumIdentical: 3}, // {NumInserted: 1}, // ] -// func coalesceAdjacentEdits(name string, es diff.EditScript) (groups []diffStats) { var prevMode byte lastStats := func(mode byte) *diffStats { @@ -503,7 +503,6 @@ func coalesceAdjacentEdits(name string, es diff.EditScript) (groups []diffStats) // {NumIdentical: 8, NumRemoved: 12, NumInserted: 3}, // {NumIdentical: 63}, // ] -// func coalesceInterveningIdentical(groups []diffStats, windowSize int) []diffStats { groups, groupsOrig := groups[:0], groups for i, ds := range groupsOrig { @@ -548,7 +547,6 @@ func coalesceInterveningIdentical(groups []diffStats, windowSize int) []diffStat // {NumRemoved: 9}, // {NumIdentical: 64}, // incremented by 10 // ] -// func cleanupSurroundingIdentical(groups []diffStats, eq func(i, j int) bool) []diffStats { var ix, iy int // indexes into sequence x and y for i, ds := range groups { diff --git a/cmp/report_text.go b/cmp/report_text.go index 0fd46d7..388fcf5 100644 --- a/cmp/report_text.go +++ b/cmp/report_text.go @@ -393,6 +393,7 @@ func (s diffStats) Append(ds diffStats) diffStats { // String prints a humanly-readable summary of coalesced records. // // Example: +// // diffStats{Name: "Field", NumIgnored: 5}.String() => "5 ignored fields" func (s diffStats) String() string { var ss []string -- cgit v1.2.3 From 5dac6aa44b75666a956f67df1b5bd4e2e044e1f8 Mon Sep 17 00:00:00 2001 From: Joe Tsai Date: Tue, 30 Aug 2022 15:55:16 -0700 Subject: Fix typo in Result documentation (#300) --- cmp/options.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmp/options.go b/cmp/options.go index e254ffd..1f9ca9c 100644 --- a/cmp/options.go +++ b/cmp/options.go @@ -432,7 +432,7 @@ func AllowUnexported(types ...interface{}) Option { } // Result represents the comparison result for a single node and -// is provided by cmp when calling Result (see Reporter). +// is provided by cmp when calling Report (see Reporter). type Result struct { _ [0]func() // Make Result incomparable flags resultFlags -- cgit v1.2.3 From f36a68d19a9bca43e070954ab9170a8305662d15 Mon Sep 17 00:00:00 2001 From: Joe Tsai Date: Tue, 30 Aug 2022 16:06:02 -0700 Subject: Pre-declare global type variables (#302) Co-authored-by: Damien Neil --- cmp/compare.go | 2 +- cmp/report_compare.go | 2 +- cmp/report_reflect.go | 9 ++++++++- cmp/report_slices.go | 8 ++++---- 4 files changed, 14 insertions(+), 7 deletions(-) diff --git a/cmp/compare.go b/cmp/compare.go index caf75ad..087320d 100644 --- a/cmp/compare.go +++ b/cmp/compare.go @@ -144,7 +144,7 @@ func rootStep(x, y interface{}) PathStep { // so that they have the same parent type. var t reflect.Type if !vx.IsValid() || !vy.IsValid() || vx.Type() != vy.Type() { - t = reflect.TypeOf((*interface{})(nil)).Elem() + t = anyType if vx.IsValid() { vvx := reflect.New(t).Elem() vvx.Set(vx) diff --git a/cmp/report_compare.go b/cmp/report_compare.go index 498c57a..2050bf6 100644 --- a/cmp/report_compare.go +++ b/cmp/report_compare.go @@ -115,7 +115,7 @@ func (opts formatOptions) FormatDiff(v *valueNode, ptrs *pointerReferences) (out // For leaf nodes, format the value based on the reflect.Values alone. // As a special case, treat equal []byte as a leaf nodes. - isBytes := v.Type.Kind() == reflect.Slice && v.Type.Elem() == reflect.TypeOf(byte(0)) + isBytes := v.Type.Kind() == reflect.Slice && v.Type.Elem() == byteType isEqualBytes := isBytes && v.NumDiff+v.NumIgnored+v.NumTransformed == 0 if v.MaxDepth == 0 || isEqualBytes { switch opts.DiffMode { diff --git a/cmp/report_reflect.go b/cmp/report_reflect.go index 381fd31..2ab41fa 100644 --- a/cmp/report_reflect.go +++ b/cmp/report_reflect.go @@ -16,6 +16,13 @@ import ( "github.com/google/go-cmp/cmp/internal/value" ) +var ( + anyType = reflect.TypeOf((*interface{})(nil)).Elem() + stringType = reflect.TypeOf((*string)(nil)).Elem() + bytesType = reflect.TypeOf((*[]byte)(nil)).Elem() + byteType = reflect.TypeOf((*byte)(nil)).Elem() +) + type formatValueOptions struct { // AvoidStringer controls whether to avoid calling custom stringer // methods like error.Error or fmt.Stringer.String. @@ -205,7 +212,7 @@ func (opts formatOptions) FormatValue(v reflect.Value, parentKind reflect.Kind, } // Check whether this is a []byte of text data. - if t.Elem() == reflect.TypeOf(byte(0)) { + if t.Elem() == byteType { b := v.Bytes() isPrintSpace := func(r rune) bool { return unicode.IsPrint(r) || unicode.IsSpace(r) } if len(b) > 0 && utf8.Valid(b) && len(bytes.TrimFunc(b, isPrintSpace)) == 0 { diff --git a/cmp/report_slices.go b/cmp/report_slices.go index 21aecd9..b38ed68 100644 --- a/cmp/report_slices.go +++ b/cmp/report_slices.go @@ -104,7 +104,7 @@ func (opts formatOptions) FormatDiffSlice(v *valueNode) textNode { case t.Kind() == reflect.String: sx, sy = vx.String(), vy.String() isString = true - case t.Kind() == reflect.Slice && t.Elem() == reflect.TypeOf(byte(0)): + case t.Kind() == reflect.Slice && t.Elem() == byteType: sx, sy = string(vx.Bytes()), string(vy.Bytes()) isString = true case t.Kind() == reflect.Array: @@ -232,7 +232,7 @@ func (opts formatOptions) FormatDiffSlice(v *valueNode) textNode { var out textNode = &textWrap{Prefix: "(", Value: list2, Suffix: ")"} switch t.Kind() { case reflect.String: - if t != reflect.TypeOf(string("")) { + if t != stringType { out = opts.FormatType(t, out) } case reflect.Slice: @@ -327,12 +327,12 @@ func (opts formatOptions) FormatDiffSlice(v *valueNode) textNode { switch t.Kind() { case reflect.String: out = &textWrap{Prefix: "strings.Join(", Value: out, Suffix: fmt.Sprintf(", %q)", delim)} - if t != reflect.TypeOf(string("")) { + if t != stringType { out = opts.FormatType(t, out) } case reflect.Slice: out = &textWrap{Prefix: "bytes.Join(", Value: out, Suffix: fmt.Sprintf(", %q)", delim)} - if t != reflect.TypeOf([]byte(nil)) { + if t != bytesType { out = opts.FormatType(t, out) } } -- cgit v1.2.3 From 6606d4d51e3239f038565f525940ac6043aff53e Mon Sep 17 00:00:00 2001 From: Joe Tsai Date: Tue, 30 Aug 2022 16:39:27 -0700 Subject: Use value.TypeString in PathStep.String (#306) The value.TypeString function is what the rest of the package uses and is slightly cleaner than using reflect.Type.String. Updates #305 Co-authored-by: Damien Neil --- cmp/path.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cmp/path.go b/cmp/path.go index 557c7f2..a0a5885 100644 --- a/cmp/path.go +++ b/cmp/path.go @@ -161,7 +161,7 @@ func (ps pathStep) String() string { if ps.typ == nil { return "" } - s := ps.typ.String() + s := value.TypeString(ps.typ, false) if s == "" || strings.ContainsAny(s, "{}\n") { return "root" // Type too simple or complex to print } @@ -284,7 +284,7 @@ type typeAssertion struct { func (ta TypeAssertion) Type() reflect.Type { return ta.typ } func (ta TypeAssertion) Values() (vx, vy reflect.Value) { return ta.vx, ta.vy } -func (ta TypeAssertion) String() string { return fmt.Sprintf(".(%v)", ta.typ) } +func (ta TypeAssertion) String() string { return fmt.Sprintf(".(%v)", value.TypeString(ta.typ, false)) } // Transform is a transformation from the parent type to the current type. type Transform struct{ *transform } -- cgit v1.2.3 From 377d28384c85781079e04aab3937170479da8cd6 Mon Sep 17 00:00:00 2001 From: Aoang Date: Wed, 31 Aug 2022 09:25:27 +0800 Subject: Run tests on Go 1.19 (#309) * Run tests on Go 1.19 * Format comment Finish the rest of the work for https://github.com/google/go-cmp/pull/304 Co-authored-by: Damien Neil --- .github/workflows/test.yml | 4 ++-- cmp/cmpopts/util_test.go | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index a93f058..b49573d 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -4,7 +4,7 @@ jobs: test: strategy: matrix: - go-version: [1.13.x, 1.14.x, 1.15.x, 1.16.x, 1.17.x, 1.18.x] + go-version: [1.13.x, 1.14.x, 1.15.x, 1.16.x, 1.17.x, 1.18.x, 1.19.x] os: [ubuntu-latest, macos-latest] runs-on: ${{ matrix.os }} steps: @@ -17,5 +17,5 @@ jobs: - name: Test run: go test -v -race ./... - name: Format - if: matrix.go-version == '1.18.x' + if: matrix.go-version == '1.19.x' run: diff -u <(echo -n) <(gofmt -d .) diff --git a/cmp/cmpopts/util_test.go b/cmp/cmpopts/util_test.go index c2eaf0b..7adeb9b 100644 --- a/cmp/cmpopts/util_test.go +++ b/cmp/cmpopts/util_test.go @@ -1072,7 +1072,7 @@ func TestOptions(t *testing.T) { }, { label: "AcyclicTransformer", x: "this is a sentence", - y: "this is a sentence", + y: "this is a sentence", opts: []cmp.Option{ AcyclicTransformer("", strings.Fields), }, -- cgit v1.2.3 From a97318bf6562f2ed2632c5f985db51b1bc5bdcd0 Mon Sep 17 00:00:00 2001 From: Joe Tsai Date: Fri, 2 Sep 2022 15:41:56 -0700 Subject: Adjust heuristic for line-based versus byte-based diffing (#299) If the string has many characters that require escape sequences to print, then we need to take that into consideration and avoid byte-by-byte diffing. Co-authored-by: Damien Neil --- cmp/compare_test.go | 17 +++++++++++++++++ cmp/report_slices.go | 5 ++++- cmp/testdata/diffs | 12 ++++++++++++ 3 files changed, 33 insertions(+), 1 deletion(-) diff --git a/cmp/compare_test.go b/cmp/compare_test.go index dc86f01..88b7d45 100644 --- a/cmp/compare_test.go +++ b/cmp/compare_test.go @@ -1403,6 +1403,23 @@ using the AllowUnexported option.`, "\n"), []byte("\xffoo"), []byte("foo"), []byte("barbaz"), []byte("added"), []byte("here"), []byte("hrmph\xff"), }, reason: "should print text byte slices as strings except those with binary", + }, { + label: label + "/ManyEscapeCharacters", + x: `[ + {"Base32": "NA======"}, + {"Base32": "NBSQ===="}, + {"Base32": "NBSWY==="}, + {"Base32": "NBSWY3A="}, + {"Base32": "NBSWY3DP"} +]`, + y: `[ + {"Base32": "NB======"}, + {"Base32": "NBSQ===="}, + {"Base32": "NBSWY==="}, + {"Base32": "NBSWY3A="}, + {"Base32": "NBSWY3DP"} +]`, + reason: "should use line-based diffing since byte-based diffing is unreadable due to heavy amounts of escaping", }} } diff --git a/cmp/report_slices.go b/cmp/report_slices.go index b38ed68..23e444f 100644 --- a/cmp/report_slices.go +++ b/cmp/report_slices.go @@ -147,7 +147,10 @@ func (opts formatOptions) FormatDiffSlice(v *valueNode) textNode { }) efficiencyLines := float64(esLines.Dist()) / float64(len(esLines)) efficiencyBytes := float64(esBytes.Dist()) / float64(len(esBytes)) - isPureLinedText = efficiencyLines < 4*efficiencyBytes + quotedLength := len(strconv.Quote(sx + sy)) + unquotedLength := len(sx) + len(sy) + escapeExpansionRatio := float64(quotedLength) / float64(unquotedLength) + isPureLinedText = efficiencyLines < 4*efficiencyBytes || escapeExpansionRatio > 1.1 } } diff --git a/cmp/testdata/diffs b/cmp/testdata/diffs index 8bff76f..be77b95 100644 --- a/cmp/testdata/diffs +++ b/cmp/testdata/diffs @@ -1182,6 +1182,18 @@ + {0x68, 0x72, 0x6d, 0x70, 0x68, 0xff}, } >>> TestDiff/Reporter/SliceOfBytesBinary +<<< TestDiff/Reporter/ManyEscapeCharacters + ( + """ + [ +- {"Base32": "NA======"}, ++ {"Base32": "NB======"}, + {"Base32": "NBSQ===="}, + {"Base32": "NBSWY==="}, + ... // 3 identical lines + """ + ) +>>> TestDiff/Reporter/ManyEscapeCharacters <<< TestDiff/EmbeddedStruct/ParentStructA/Inequal teststructs.ParentStructA{ privateStruct: teststructs.privateStruct{ -- cgit v1.2.3