assert: detect cycles in EqualExportedValues

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,15 +140,18 @@ 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))
 			}
 		}
 		return result.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...)

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()