Merge pull request #63375 from liggitt/diff-limit

Automatic merge from submit-queue (batch tested with PRs 62657, 63278, 62903, 63375). If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>.

ensure diff output includes the portion that differs

When using ObjectReflectDiff() on objects with long string fields, the
80 character limit on diffs will commonly hide the actual difference
between the fields and require that the dev change which diff function
is used to see what the issue was. This defeats the purpose of printing
the diff between objects.

staging/src/k8s.io/apimachinery/pkg/util/diff/diff.go

@@ -89,20 +89,52 @@ func ObjectReflectDiff(a, b interface{}) string {
 	}
 	out := []string{""}
 	for _, d := range diffs {
+		elidedA, elidedB := limit(d.a, d.b, 80)
 		out = append(out,
 			fmt.Sprintf("%s:", d.path),
-			limit(fmt.Sprintf("  a: %#v", d.a), 80),
-			limit(fmt.Sprintf("  b: %#v", d.b), 80),
+			fmt.Sprintf("  a: %s", elidedA),
+			fmt.Sprintf("  b: %s", elidedB),
 		)
 	}
 	return strings.Join(out, "\n")
 }
 
-func limit(s string, max int) string {
-	if len(s) > max {
-		return s[:max]
+// limit:
+// 1. stringifies aObj and bObj
+// 2. elides identical prefixes if either is too long
+// 3. elides remaining content from the end if either is too long
+func limit(aObj, bObj interface{}, max int) (string, string) {
+	elidedPrefix := ""
+	elidedASuffix := ""
+	elidedBSuffix := ""
+	a, b := fmt.Sprintf("%#v", aObj), fmt.Sprintf("%#v", bObj)
+	for {
+		switch {
+		case len(a) > max && len(a) > 4 && len(b) > 4 && a[:4] == b[:4]:
+			// a is too long, b has data, and the first several characters are the same
+			elidedPrefix = "..."
+			a = a[2:]
+			b = b[2:]
+
+		case len(b) > max && len(b) > 4 && len(a) > 4 && a[:4] == b[:4]:
+			// b is too long, a has data, and the first several characters are the same
+			elidedPrefix = "..."
+			a = a[2:]
+			b = b[2:]
+
+		case len(a) > max:
+			a = a[:max]
+			elidedASuffix = "..."
+
+		case len(b) > max:
+			b = b[:max]
+			elidedBSuffix = "..."
+
+		default:
+			// both are short enough
+			return elidedPrefix + a + elidedASuffix, elidedPrefix + b + elidedBSuffix
+		}
 	}
-	return s
 }
 
 func public(s string) bool {

staging/src/k8s.io/apimachinery/pkg/util/diff/diff_test.go

@@ -94,3 +94,50 @@ func TestStringDiff(t *testing.T) {
 		t.Errorf("diff returned %v", diff)
 	}
 }
+
+func TestLimit(t *testing.T) {
+	testcases := []struct {
+		a       interface{}
+		b       interface{}
+		expectA string
+		expectB string
+	}{
+		{
+			a:       `short a`,
+			b:       `short b`,
+			expectA: `"short a"`,
+			expectB: `"short b"`,
+		},
+		{
+			a:       `short a`,
+			b:       `long b needs truncating`,
+			expectA: `"short a"`,
+			expectB: `"long b ne...`,
+		},
+		{
+			a:       `long a needs truncating`,
+			b:       `long b needs truncating`,
+			expectA: `...g a needs ...`,
+			expectB: `...g b needs ...`,
+		},
+		{
+			a:       `long common prefix with different stuff at the end of a`,
+			b:       `long common prefix with different stuff at the end of b`,
+			expectA: `...end of a"`,
+			expectB: `...end of b"`,
+		},
+		{
+			a:       `long common prefix with different stuff at the end of a`,
+			b:       `long common prefix with different stuff at the end of b which continues`,
+			expectA: `...of a"`,
+			expectB: `...of b which...`,
+		},
+	}
+
+	for _, tc := range testcases {
+		a, b := limit(tc.a, tc.b, 10)
+		if a != tc.expectA || b != tc.expectB {
+			t.Errorf("limit(%q, %q)\n\texpected: %s, %s\n\tgot:      %s, %s", tc.a, tc.b, tc.expectA, tc.expectB, a, b)
+		}
+	}
+}