From f0eb3c28307fcb4233fb10ba2a8fe19659c73b87 Mon Sep 17 00:00:00 2001 From: Jordan Liggitt Date: Wed, 2 May 2018 16:31:27 -0400 Subject: [PATCH] ensure diff output includes the portion that differs --- .../k8s.io/apimachinery/pkg/util/diff/diff.go | 44 ++++++++++++++--- .../apimachinery/pkg/util/diff/diff_test.go | 47 +++++++++++++++++++ 2 files changed, 85 insertions(+), 6 deletions(-) diff --git a/staging/src/k8s.io/apimachinery/pkg/util/diff/diff.go b/staging/src/k8s.io/apimachinery/pkg/util/diff/diff.go index 3d5ec14bfa9..bce95baf1c1 100644 --- a/staging/src/k8s.io/apimachinery/pkg/util/diff/diff.go +++ b/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 { diff --git a/staging/src/k8s.io/apimachinery/pkg/util/diff/diff_test.go b/staging/src/k8s.io/apimachinery/pkg/util/diff/diff_test.go index 2b72c2f531a..d26dba81823 100644 --- a/staging/src/k8s.io/apimachinery/pkg/util/diff/diff_test.go +++ b/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) + } + } +}