diff --git a/assert/assertions.go b/assert/assertions.go index 166f63726..49f6643ef 100644 --- a/assert/assertions.go +++ b/assert/assertions.go @@ -83,6 +83,37 @@ func ObjectsAreEqual(expected, actual interface{}) bool { // copyExportedFields iterates downward through nested data structures and creates a copy // that only contains the exported struct fields. func copyExportedFields(expected interface{}) interface{} { + return copyExportedFieldsWithVisited(expected, make(map[copyExportedFieldsVisit]reflect.Value)) +} + +// copyExportedFieldsVisit identifies pointer, slice, and map values by identity so +// copyExportedFields can detect cycles (as encoding/json does) and reuse in-progress copies. +type copyExportedFieldsVisit struct { + typ reflect.Type + ptr uintptr + length int +} + +func copyExportedFieldsVisitKey(v reflect.Value) (copyExportedFieldsVisit, bool) { + switch v.Kind() { + case reflect.Ptr, reflect.Map: + if v.IsNil() { + return copyExportedFieldsVisit{}, false + } + return copyExportedFieldsVisit{typ: v.Type(), ptr: v.Pointer()}, true + case reflect.Slice: + if v.IsNil() { + return copyExportedFieldsVisit{}, false + } + // Include length so reslices of a shared backing array are distinct, + // matching encoding/json's slice cycle key. + return copyExportedFieldsVisit{typ: v.Type(), ptr: v.Pointer(), length: v.Len()}, true + default: + return copyExportedFieldsVisit{}, false + } +} + +func copyExportedFieldsWithVisited(expected interface{}, visited map[copyExportedFieldsVisit]reflect.Value) interface{} { if isNil(expected) { return expected } @@ -91,6 +122,13 @@ func copyExportedFields(expected interface{}) interface{} { expectedKind := expectedType.Kind() expectedValue := reflect.ValueOf(expected) + visit, ok := copyExportedFieldsVisitKey(expectedValue) + if ok { + if result, seen := visited[visit]; seen { + return result.Interface() + } + } + switch expectedKind { case reflect.Struct: result := reflect.New(expectedType).Elem() @@ -102,7 +140,7 @@ func copyExportedFields(expected interface{}) interface{} { if isNil(fieldValue) || isNil(fieldValue.Interface()) { continue } - newValue := copyExportedFields(fieldValue.Interface()) + newValue := copyExportedFieldsWithVisited(fieldValue.Interface(), visited) result.Field(i).Set(reflect.ValueOf(newValue)) } } @@ -110,7 +148,10 @@ func copyExportedFields(expected interface{}) interface{} { case reflect.Ptr: result := reflect.New(expectedType.Elem()) - unexportedRemoved := copyExportedFields(expectedValue.Elem().Interface()) + if ok { + visited[visit] = result + } + unexportedRemoved := copyExportedFieldsWithVisited(expectedValue.Elem().Interface(), visited) result.Elem().Set(reflect.ValueOf(unexportedRemoved)) return result.Interface() @@ -120,22 +161,28 @@ func copyExportedFields(expected interface{}) interface{} { result = reflect.New(reflect.ArrayOf(expectedValue.Len(), expectedType.Elem())).Elem() } else { result = reflect.MakeSlice(expectedType, expectedValue.Len(), expectedValue.Len()) + if ok { + visited[visit] = result + } } for i := 0; i < expectedValue.Len(); i++ { index := expectedValue.Index(i) if isNil(index) { continue } - unexportedRemoved := copyExportedFields(index.Interface()) + unexportedRemoved := copyExportedFieldsWithVisited(index.Interface(), visited) result.Index(i).Set(reflect.ValueOf(unexportedRemoved)) } return result.Interface() case reflect.Map: result := reflect.MakeMap(expectedType) + if ok { + visited[visit] = result + } for _, k := range expectedValue.MapKeys() { index := expectedValue.MapIndex(k) - unexportedRemoved := copyExportedFields(index.Interface()) + unexportedRemoved := copyExportedFieldsWithVisited(index.Interface(), visited) result.SetMapIndex(k, reflect.ValueOf(unexportedRemoved)) } return result.Interface() @@ -675,7 +722,9 @@ func EqualExportedValues(t TestingT, expected, actual interface{}, msgAndArgs .. if !ObjectsAreEqualValues(expected, actual) { diff := diff(expected, actual) - expected, actual = formatUnequalValues(expected, actual) + // spew handles pointer/slice/map cycles; fmt's %#v does not and can overflow. + expected = truncatingFormat("%s", strings.TrimSuffix(spewConfig.Sdump(expected), "\n")) + actual = truncatingFormat("%s", strings.TrimSuffix(spewConfig.Sdump(actual), "\n")) return Fail(t, fmt.Sprintf("Not equal (comparing only exported fields): \n"+ "expected: %s\n"+ "actual : %s%s", expected, actual, diff), msgAndArgs...) diff --git a/assert/assertions_test.go b/assert/assertions_test.go index 11642e096..82b5dd78d 100644 --- a/assert/assertions_test.go +++ b/assert/assertions_test.go @@ -527,6 +527,207 @@ func TestEqualExportedValues(t *testing.T) { } } +func TestCopyExportedFieldsCycles(t *testing.T) { + t.Parallel() + + type Node struct { + Value int + Self *Node + unexported string + } + + n := &Node{Value: 1, unexported: "hidden"} + n.Self = n + + copied, ok := copyExportedFields(n).(*Node) + if !ok { + t.Fatalf("copyExportedFields returned %T, want *Node", copyExportedFields(n)) + } + if copied == n { + t.Fatal("copy should not be the original pointer") + } + if copied.Value != 1 { + t.Fatalf("Value = %d, want 1", copied.Value) + } + if copied.unexported != "" { + t.Fatalf("unexported field = %q, want empty", copied.unexported) + } + if copied.Self != copied { + t.Fatal("pointer cycle should be preserved on the copy") + } + + m := map[string]interface{}{"v": 1} + m["self"] = m + copiedMap, ok := copyExportedFields(m).(map[string]interface{}) + if !ok { + t.Fatalf("copyExportedFields returned %T, want map[string]interface{}", copyExportedFields(m)) + } + selfMap, ok := copiedMap["self"].(map[string]interface{}) + if !ok { + t.Fatalf("copied map self = %T, want map[string]interface{}", copiedMap["self"]) + } + if reflect.ValueOf(selfMap).Pointer() != reflect.ValueOf(copiedMap).Pointer() { + t.Fatal("map cycle should be preserved on the copy") + } + + s := make([]interface{}, 1) + s[0] = s + copiedSlice, ok := copyExportedFields(s).([]interface{}) + if !ok { + t.Fatalf("copyExportedFields returned %T, want []interface{}", copyExportedFields(s)) + } + selfSlice, ok := copiedSlice[0].([]interface{}) + if !ok { + t.Fatalf("copied slice [0] = %T, want []interface{}", copiedSlice[0]) + } + if reflect.ValueOf(selfSlice).Pointer() != reflect.ValueOf(copiedSlice).Pointer() { + t.Fatal("slice cycle should be preserved on the copy") + } +} + +func TestEqualExportedValuesCycles(t *testing.T) { + t.Parallel() + + type Node struct { + Value int + Next *Node + unexported string + } + + cyclicNode := func(value int, unexported string) *Node { + n := &Node{Value: value, unexported: unexported} + n.Next = n + return n + } + + ring := func(values []int, unexported string) *Node { + nodes := make([]*Node, len(values)) + for i, v := range values { + nodes[i] = &Node{Value: v, unexported: unexported} + } + for i := range nodes { + nodes[i].Next = nodes[(i+1)%len(nodes)] + } + return nodes[0] + } + + type ArrNode struct { + Value int + Children [1]*ArrNode + unexported string + } + cyclicArray := func(value int, unexported string) *ArrNode { + n := &ArrNode{Value: value, unexported: unexported} + n.Children[0] = n + return n + } + + cyclicMap := func(value int) map[string]interface{} { + m := map[string]interface{}{"v": value} + m["self"] = m + return m + } + + cyclicSlice := func(value int) []interface{} { + s := []interface{}{value, nil} + s[1] = s + return s + } + + cases := []struct { + name string + value1 interface{} + value2 interface{} + expectedEqual bool + }{ + { + name: "self pointer equal ignoring unexported", + value1: cyclicNode(1, "expected"), + value2: cyclicNode(1, "actual"), + expectedEqual: true, + }, + { + name: "self pointer different exported", + value1: cyclicNode(1, "expected"), + value2: cyclicNode(2, "actual"), + expectedEqual: false, + }, + { + name: "cyclic vs non-cyclic", + value1: cyclicNode(1, "expected"), + value2: &Node{Value: 1, unexported: "actual"}, + expectedEqual: false, + }, + { + name: "finite chain equal", + value1: &Node{Value: 1, Next: &Node{Value: 2, Next: &Node{Value: 3}}}, + value2: &Node{Value: 1, Next: &Node{Value: 2, Next: &Node{Value: 3, unexported: "x"}}}, + expectedEqual: true, + }, + { + name: "mutual pointers equal", + value1: ring([]int{1, 2}, "a"), + value2: ring([]int{1, 2}, "b"), + expectedEqual: true, + }, + { + name: "mutual pointers different exported", + value1: ring([]int{1, 2}, "a"), + value2: ring([]int{1, 3}, "b"), + expectedEqual: false, + }, + { + name: "map cycle equal", + value1: cyclicMap(1), + value2: cyclicMap(1), + expectedEqual: true, + }, + { + name: "map cycle different values", + value1: cyclicMap(1), + value2: cyclicMap(2), + expectedEqual: false, + }, + { + name: "slice cycle equal", + value1: cyclicSlice(1), + value2: cyclicSlice(1), + expectedEqual: true, + }, + { + name: "slice cycle different values", + value1: cyclicSlice(1), + value2: cyclicSlice(2), + expectedEqual: false, + }, + { + name: "array of cyclic pointers equal ignoring unexported", + value1: cyclicArray(1, "expected"), + value2: cyclicArray(1, "actual"), + expectedEqual: true, + }, + { + name: "array of cyclic pointers different exported", + value1: cyclicArray(1, "expected"), + value2: cyclicArray(2, "actual"), + expectedEqual: false, + }, + } + + for _, c := range cases { + c := c + t.Run(c.name, func(t *testing.T) { + t.Parallel() + + mockT := new(mockTestingT) + actual := EqualExportedValues(mockT, c.value1, c.value2) + if actual != c.expectedEqual { + t.Errorf("EqualExportedValues() = %t, want %t\nfailure: %s", actual, c.expectedEqual, mockT.errorString()) + } + }) + } +} + func TestImplements(t *testing.T) { t.Parallel()