fix: deterministic order of keys when diffing maps (#31)

Adapted from stretchr#1822

The SpewKeys setting looks sensible.

test: refactored the diff test to make it a table-driven, iterable test.

Signed-off-by: Frederic BIDON <[email protected]>

Author: fredbi

internal/assertions/helpers_impl_test.go

@@ -44,7 +44,60 @@ func testDiff() func(*testing.T) {
 	return func(t *testing.T) {
 		t.Parallel()
 
-		expected := `
+		for tt := range diffCases() {
+			t.Run(tt.name, func(t *testing.T) {
+				t.Parallel()
+
+				for range min(1, tt.repeat) { // for tests on maps, need to verify the ordering is stable
+					actual := diff(
+						tt.valueA,
+						tt.valueB,
+					)
+					Equal(t, tt.expected, actual)
+				}
+			})
+		}
+	}
+}
+
+func testDiffList() func(*testing.T) {
+	return func(t *testing.T) {
+		t.Parallel()
+
+		for test := range compareDiffListCases() {
+			t.Run(test.name, func(t *testing.T) {
+				t.Parallel()
+
+				actualExtraA, actualExtraB := diffLists(test.listA, test.listB)
+				Equal(t, test.extraA, actualExtraA, "extra A does not match for listA=%v listB=%v",
+					test.listA, test.listB)
+				Equal(t, test.extraB, actualExtraB, "extra B does not match for listA=%v listB=%v",
+					test.listA, test.listB)
+			})
+		}
+	}
+}
+
+type diffCase struct {
+	name     string
+	repeat   int
+	valueA   any
+	valueB   any
+	expected string
+}
+
+func diffCases() iter.Seq[diffCase] {
+	const n = 5
+	type Key struct {
+		x int
+	}
+
+	return slices.Values([]diffCase{
+		{
+			name:   "with diff struct",
+			valueA: struct{ foo string }{"hello"},
+			valueB: struct{ foo string }{"bar"},
+			expected: `
 
 Diff:
 --- Expected
@@ -54,14 +107,13 @@ Diff:
 - foo: (string) (len=5) "hello"
 + foo: (string) (len=3) "bar"
  }
-`
-		actual := diff(
-			struct{ foo string }{"hello"},
-			struct{ foo string }{"bar"},
-		)
-		Equal(t, expected, actual)
-
-		expected = `
+`,
+		},
+		{
+			name:   "with diff slice",
+			valueA: []int{1, 2, 3, 4},
+			valueB: []int{1, 3, 5, 7},
+			expected: `
 
 Diff:
 --- Expected
@@ -74,14 +126,13 @@ Diff:
 + (int) 5,
 + (int) 7
  }
-`
-		actual = diff(
-			[]int{1, 2, 3, 4},
-			[]int{1, 3, 5, 7},
-		)
-		Equal(t, expected, actual)
-
-		expected = `
+`,
+		},
+		{
+			name:   "with diff slice (sliced)",
+			valueA: []int{1, 2, 3, 4}[0:3],
+			valueB: []int{1, 3, 5, 7}[0:3],
+			expected: `
 
 Diff:
 --- Expected
@@ -93,14 +144,14 @@ Diff:
 + (int) 3,
 + (int) 5
  }
-`
-		actual = diff(
-			[]int{1, 2, 3, 4}[0:3],
-			[]int{1, 3, 5, 7}[0:3],
-		)
-		Equal(t, expected, actual)
-
-		expected = `
+`,
+		},
+		{
+			name:   "with string keys map keys should be rendered deterministically in diffs",
+			repeat: n,
+			valueA: map[string]int{"one": 1, "two": 2, "three": 3, "four": 4},
+			valueB: map[string]int{"one": 1, "three": 3, "five": 5, "seven": 7},
+			expected: `
 
 Diff:
 --- Expected
@@ -115,15 +166,13 @@ Diff:
 + (string) (len=5) "seven": (int) 7,
 + (string) (len=5) "three": (int) 3
  }
-`
-
-		actual = diff(
-			map[string]int{"one": 1, "two": 2, "three": 3, "four": 4},
-			map[string]int{"one": 1, "three": 3, "five": 5, "seven": 7},
-		)
-		Equal(t, expected, actual)
-
-		expected = `
+`,
+		},
+		{
+			name:   "with diff error",
+			valueA: errors.New("some expected error"),
+			valueB: errors.New("actual error"),
+			expected: `
 
 Diff:
 --- Expected
@@ -133,15 +182,30 @@ Diff:
 - s: (string) (len=19) "some expected error"
 + s: (string) (len=12) "actual error"
  })
-`
-
-		actual = diff(
-			errors.New("some expected error"),
-			errors.New("actual error"),
-		)
-		Equal(t, expected, actual)
+`,
+		},
+		{
+			name:   "with arbitrary comparable keys map keys should be rendered deterministically in diffs",
+			repeat: n,
+			valueA: map[Key]int{{1}: 1, {2}: 2, {3}: 3, {4}: 4},
+			valueB: map[Key]int{{1}: 1, {2}: 2, {3}: 3, {4}: 999},
+			expected: `
 
-		expected = `
+Diff:
+--- Expected
++++ Actual
+@@ -12,3 +12,3 @@
+   x: (int) 4
+- }: (int) 4
++ }: (int) 999
+ }
+`,
+		},
+		{
+			name:   "with diff unexported struct",
+			valueA: diffTestingStruct{A: "some string", B: 10},
+			valueB: diffTestingStruct{A: "some string", B: 15},
+			expected: `
 
 Diff:
 --- Expected
@@ -151,15 +215,13 @@ Diff:
 - B: (int) 10
 + B: (int) 15
  }
-`
-
-		actual = diff(
-			diffTestingStruct{A: "some string", B: 10},
-			diffTestingStruct{A: "some string", B: 15},
-		)
-		Equal(t, expected, actual)
-
-		expected = `
+`,
+		},
+		{
+			name:   "with diff date",
+			valueA: time.Date(2020, 9, 24, 0, 0, 0, 0, time.UTC),
+			valueB: time.Date(2020, 9, 25, 0, 0, 0, 0, time.UTC),
+			expected: `
 
 Diff:
 --- Expected
@@ -168,32 +230,9 @@ Diff:
 -(time.Time) 2020-09-24 00:00:00 +0000 UTC
 +(time.Time) 2020-09-25 00:00:00 +0000 UTC
  
-`
-
-		actual = diff(
-			time.Date(2020, 9, 24, 0, 0, 0, 0, time.UTC),
-			time.Date(2020, 9, 25, 0, 0, 0, 0, time.UTC),
-		)
-		Equal(t, expected, actual)
-	}
-}
-
-func testDiffList() func(*testing.T) {
-	return func(t *testing.T) {
-		t.Parallel()
-
-		for test := range compareDiffListCases() {
-			t.Run(test.name, func(t *testing.T) {
-				t.Parallel()
-
-				actualExtraA, actualExtraB := diffLists(test.listA, test.listB)
-				Equal(t, test.extraA, actualExtraA, "extra A does not match for listA=%v listB=%v",
-					test.listA, test.listB)
-				Equal(t, test.extraB, actualExtraB, "extra B does not match for listA=%v listB=%v",
-					test.listA, test.listB)
-			})
-		}
-	}
+`,
+		},
+	})
 }
 
 type compareDiffListCase struct {

internal/assertions/spew.go

@@ -14,6 +14,7 @@ var (
 		DisablePointerAddresses: true,
 		DisableCapacities:       true,
 		SortKeys:                true,
+		SpewKeys:                true,
 		DisableMethods:          true,
 		MaxDepth:                spewMaxDepth,
 	}
@@ -23,6 +24,7 @@ var (
 		DisablePointerAddresses: true,
 		DisableCapacities:       true,
 		SortKeys:                true,
+		SpewKeys:                true,
 		MaxDepth:                spewMaxDepth,
 	}
 )