Skip to content

Commit 1d73c69

Browse files
committed
Limit verbosity of reporter output
A common complaint is that the reporter it prints out too much irrelevant information, resulting in a low signal-to-noise ratio. Improve this metric by imposing a verbosity limit. For nodes that are equal, we set the verbosity level is a lower value than when the nodes are inequal. Other minor changes: * Adjust heuristic for triple-quote usage to operate on more cases. * Elide type more aggressively for equal nodes. * Printing the address for a slice includes the length and capacity. * The pointed-at value for map keys are printed.
1 parent 0d296f9 commit 1d73c69

File tree

6 files changed

+287
-197
lines changed

6 files changed

+287
-197
lines changed

cmp/compare_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -159,8 +159,8 @@ func TestDiff(t *testing.T) {
159159
}
160160
} else {
161161
wantDiff := wantDiffs[t.Name()]
162-
if gotDiff != wantDiff {
163-
t.Fatalf("Diff:\ngot:\n%s\nwant:\n%s\nreason: %v", gotDiff, wantDiff, tt.reason)
162+
if diff := cmp.Diff(wantDiff, gotDiff); diff != "" {
163+
t.Fatalf("Diff:\ngot:\n%s\nwant:\n%s\ndiff: (-want +got)\n%s\nreason: %v", gotDiff, wantDiff, diff, tt.reason)
164164
}
165165
}
166166
gotEqual := gotDiff == ""

cmp/example_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ func ExampleDiff_testing() {
3737
// SSID: "CoffeeShopWiFi",
3838
// - IPAddress: s"192.168.0.2",
3939
// + IPAddress: s"192.168.0.1",
40-
// NetMask: net.IPMask{0xff, 0xff, 0x00, 0x00},
40+
// NetMask: {0xff, 0xff, 0x00, 0x00},
4141
// Clients: []cmp_test.Client{
4242
// ... // 2 identical elements
4343
// {Hostname: "macchiato", IPAddress: s"192.168.0.153", LastSeen: s"2009-11-10 23:39:43 +0000 UTC"},

cmp/report_compare.go

Lines changed: 41 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -11,10 +11,6 @@ import (
1111
"github.com/google/go-cmp/cmp/internal/value"
1212
)
1313

14-
// TODO: Enforce limits?
15-
// * Enforce maximum number of records to print per node?
16-
// * Enforce maximum size in bytes allowed?
17-
// * As a heuristic, use less verbosity for equal nodes than unequal nodes.
1814
// TODO: Enforce unique outputs?
1915
// * Avoid Stringer methods if it results in same output?
2016
// * Print pointer address if outputs still equal?
@@ -71,10 +67,31 @@ func (opts formatOptions) WithTypeMode(t typeMode) formatOptions {
7167
opts.TypeMode = t
7268
return opts
7369
}
70+
func (opts formatOptions) WithVerbosity(level int) formatOptions {
71+
opts.VerbosityLevel = level
72+
opts.LimitVerbosity = true
73+
return opts
74+
}
75+
func (opts formatOptions) verbosity() uint {
76+
switch {
77+
case opts.VerbosityLevel < 0:
78+
return 0
79+
case opts.VerbosityLevel > 16:
80+
return 16 // some reasonable maximum to avoid shift overflow
81+
default:
82+
return uint(opts.VerbosityLevel)
83+
}
84+
}
7485

7586
// FormatDiff converts a valueNode tree into a textNode tree, where the later
7687
// is a textual representation of the differences detected in the former.
7788
func (opts formatOptions) FormatDiff(v *valueNode) textNode {
89+
if opts.DiffMode == diffIdentical {
90+
opts = opts.WithVerbosity(1)
91+
} else {
92+
opts = opts.WithVerbosity(3)
93+
}
94+
7895
// Check whether we have specialized formatting for this node.
7996
// This is not necessary, but helpful for producing more readable outputs.
8097
if opts.CanFormatDiffSlice(v) {
@@ -124,6 +141,8 @@ func (opts formatOptions) FormatDiff(v *valueNode) textNode {
124141
}
125142
}
126143

144+
// TODO: Print cycle reference for pointers, maps, and elements of a slice.
145+
127146
// Descend into the child value node.
128147
if v.TransformerName != "" {
129148
out := opts.WithTypeMode(emitType).FormatDiff(v.Value)
@@ -162,12 +181,27 @@ func (opts formatOptions) formatDiffList(recs []reportRecord, k reflect.Kind) te
162181
formatKey = formatMapKey
163182
}
164183

184+
maxLen := -1
185+
if opts.LimitVerbosity {
186+
if opts.DiffMode == diffIdentical {
187+
maxLen = ((1 << opts.verbosity()) >> 1) << 2 // 0, 4, 8, 16, 32, etc...
188+
} else {
189+
maxLen = (1 << opts.verbosity()) << 1 // 2, 4, 8, 16, 32, 64, etc...
190+
}
191+
opts.VerbosityLevel--
192+
}
193+
165194
// Handle unification.
166195
switch opts.DiffMode {
167196
case diffIdentical, diffRemoved, diffInserted:
168197
var list textList
169198
var deferredEllipsis bool // Add final "..." to indicate records were dropped
170199
for _, r := range recs {
200+
if len(list) == maxLen {
201+
deferredEllipsis = true
202+
break
203+
}
204+
171205
// Elide struct fields that are zero value.
172206
if k == reflect.Struct {
173207
var isZero bool
@@ -205,11 +239,12 @@ func (opts formatOptions) formatDiffList(recs []reportRecord, k reflect.Kind) te
205239
}
206240

207241
// Handle differencing.
242+
var numDiffs int
208243
var list textList
209244
groups := coalesceAdjacentRecords(name, recs)
210245
maxGroup := diffStats{Name: name}
211246
for i, ds := range groups {
212-
if len(list) >= maxDiffElements {
247+
if maxLen >= 0 && numDiffs >= maxLen {
213248
maxGroup = maxGroup.Append(ds)
214249
continue
215250
}
@@ -273,6 +308,7 @@ func (opts formatOptions) formatDiffList(recs []reportRecord, k reflect.Kind) te
273308
}
274309
}
275310
recs = recs[ds.NumDiff():]
311+
numDiffs += ds.NumDiff()
276312
}
277313
if maxGroup.IsZero() {
278314
assert(len(recs) == 0)

cmp/report_reflect.go

Lines changed: 70 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -21,14 +21,23 @@ type formatValueOptions struct {
2121
// methods like error.Error or fmt.Stringer.String.
2222
AvoidStringer bool
2323

24-
// ShallowPointers controls whether to avoid descending into pointers.
24+
// PrintShallowPointer controls whether to print the next pointer.
2525
// Useful when printing map keys, where pointer comparison is performed
2626
// on the pointer address rather than the pointed-at value.
27-
ShallowPointers bool
27+
PrintShallowPointer bool
2828

2929
// PrintAddresses controls whether to print the address of all pointers,
3030
// slice elements, and maps.
3131
PrintAddresses bool
32+
33+
// VerbosityLevel controls the amount of output to produce.
34+
// A higher value produces more output. A value of zero or lower produces
35+
// no output (represented using an ellipsis).
36+
// If LimitVerbosity is false, then the level is treated as infinite.
37+
VerbosityLevel int
38+
39+
// LimitVerbosity specifies that formatting should respect VerbosityLevel.
40+
LimitVerbosity bool
3241
}
3342

3443
// FormatType prints the type as if it were wrapping s.
@@ -45,6 +54,9 @@ func (opts formatOptions) FormatType(t reflect.Type, s textNode) textNode {
4554
default:
4655
return s
4756
}
57+
if opts.DiffMode == diffIdentical {
58+
return s // elide type for identical nodes
59+
}
4860
case elideType:
4961
return s
5062
}
@@ -86,11 +98,22 @@ func (opts formatOptions) FormatValue(v reflect.Value, withinSlice bool, m visit
8698
// Avoid calling Error or String methods on nil receivers since many
8799
// implementations crash when doing so.
88100
if (t.Kind() != reflect.Ptr && t.Kind() != reflect.Interface) || !v.IsNil() {
101+
var prefix, strVal string
89102
switch v := v.Interface().(type) {
90103
case error:
91-
return textLine("e" + formatString(v.Error()))
104+
prefix, strVal = "e", v.Error()
92105
case fmt.Stringer:
93-
return textLine("s" + formatString(v.String()))
106+
prefix, strVal = "s", v.String()
107+
}
108+
if prefix != "" {
109+
maxLen := len(strVal)
110+
if opts.LimitVerbosity {
111+
maxLen = (1 << opts.verbosity()) << 5 // 32, 64, 128, 256, etc...
112+
}
113+
if len(strVal) > maxLen+len(textEllipsis) {
114+
return textLine(prefix + formatString(strVal[:maxLen]) + string(textEllipsis))
115+
}
116+
return textLine(prefix + formatString(strVal))
94117
}
95118
}
96119
}
@@ -123,17 +146,33 @@ func (opts formatOptions) FormatValue(v reflect.Value, withinSlice bool, m visit
123146
case reflect.Complex64, reflect.Complex128:
124147
return textLine(fmt.Sprint(v.Complex()))
125148
case reflect.String:
149+
maxLen := v.Len()
150+
if opts.LimitVerbosity {
151+
maxLen = (1 << opts.verbosity()) << 5 // 32, 64, 128, 256, etc...
152+
}
153+
if v.Len() > maxLen+len(textEllipsis) {
154+
return textLine(formatString(v.String()[:maxLen]) + string(textEllipsis))
155+
}
126156
return textLine(formatString(v.String()))
127157
case reflect.UnsafePointer, reflect.Chan, reflect.Func:
128158
return textLine(formatPointer(v))
129159
case reflect.Struct:
130160
var list textList
131161
v := makeAddressable(v) // needed for retrieveUnexportedField
162+
maxLen := v.NumField()
163+
if opts.LimitVerbosity {
164+
maxLen = ((1 << opts.verbosity()) >> 1) << 2 // 0, 4, 8, 16, 32, etc...
165+
opts.VerbosityLevel--
166+
}
132167
for i := 0; i < v.NumField(); i++ {
133168
vv := v.Field(i)
134169
if value.IsZero(vv) {
135170
continue // Elide fields with zero values
136171
}
172+
if len(list) == maxLen {
173+
list.AppendEllipsis(diffStats{})
174+
break
175+
}
137176
sf := t.Field(i)
138177
if supportExporters && !isExported(sf.Name) {
139178
vv = retrieveUnexportedField(v, sf, true)
@@ -147,12 +186,21 @@ func (opts formatOptions) FormatValue(v reflect.Value, withinSlice bool, m visit
147186
return textNil
148187
}
149188
if opts.PrintAddresses {
150-
ptr = formatPointer(v)
189+
ptr = fmt.Sprintf("⟪ptr:0x%x, len:%d, cap:%d⟫", pointerValue(v), v.Len(), v.Cap())
151190
}
152191
fallthrough
153192
case reflect.Array:
193+
maxLen := v.Len()
194+
if opts.LimitVerbosity {
195+
maxLen = ((1 << opts.verbosity()) >> 1) << 2 // 0, 4, 8, 16, 32, etc...
196+
opts.VerbosityLevel--
197+
}
154198
var list textList
155199
for i := 0; i < v.Len(); i++ {
200+
if len(list) == maxLen {
201+
list.AppendEllipsis(diffStats{})
202+
break
203+
}
156204
vi := v.Index(i)
157205
if vi.CanAddr() { // Check for cyclic elements
158206
p := vi.Addr()
@@ -177,8 +225,17 @@ func (opts formatOptions) FormatValue(v reflect.Value, withinSlice bool, m visit
177225
return textLine(formatPointer(v))
178226
}
179227

228+
maxLen := v.Len()
229+
if opts.LimitVerbosity {
230+
maxLen = ((1 << opts.verbosity()) >> 1) << 2 // 0, 4, 8, 16, 32, etc...
231+
opts.VerbosityLevel--
232+
}
180233
var list textList
181234
for _, k := range value.SortKeys(v.MapKeys()) {
235+
if len(list) == maxLen {
236+
list.AppendEllipsis(diffStats{})
237+
break
238+
}
182239
sk := formatMapKey(k)
183240
sv := opts.WithTypeMode(elideType).FormatValue(v.MapIndex(k), false, m)
184241
list = append(list, textRecord{Key: sk, Value: sv})
@@ -191,11 +248,12 @@ func (opts formatOptions) FormatValue(v reflect.Value, withinSlice bool, m visit
191248
if v.IsNil() {
192249
return textNil
193250
}
194-
if m.Visit(v) || opts.ShallowPointers {
251+
if m.Visit(v) {
195252
return textLine(formatPointer(v))
196253
}
197-
if opts.PrintAddresses {
254+
if opts.PrintAddresses || opts.PrintShallowPointer {
198255
ptr = formatPointer(v)
256+
opts.PrintShallowPointer = false
199257
}
200258
skipType = true // Let the underlying value print the type instead
201259
return textWrap{"&" + ptr, opts.FormatValue(v.Elem(), false, m), ""}
@@ -217,7 +275,7 @@ func (opts formatOptions) FormatValue(v reflect.Value, withinSlice bool, m visit
217275
func formatMapKey(v reflect.Value) string {
218276
var opts formatOptions
219277
opts.TypeMode = elideType
220-
opts.ShallowPointers = true
278+
opts.PrintShallowPointer = true
221279
s := opts.FormatValue(v, false, visitedPointers{}).String()
222280
return strings.TrimSpace(s)
223281
}
@@ -268,11 +326,14 @@ func formatHex(u uint64) string {
268326

269327
// formatPointer prints the address of the pointer.
270328
func formatPointer(v reflect.Value) string {
329+
return fmt.Sprintf("⟪0x%x⟫", pointerValue(v))
330+
}
331+
func pointerValue(v reflect.Value) uintptr {
271332
p := v.Pointer()
272333
if flags.Deterministic {
273334
p = 0xdeadf00f // Only used for stable testing purposes
274335
}
275-
return fmt.Sprintf("⟪0x%x⟫", p)
336+
return p
276337
}
277338

278339
type visitedPointers map[value.Pointer]struct{}

cmp/report_slices.go

Lines changed: 24 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -16,10 +16,6 @@ import (
1616
"github.com/google/go-cmp/cmp/internal/diff"
1717
)
1818

19-
// maxDiffElements is the maximum number of difference elements to format
20-
// before the remaining differences are coalesced together.
21-
const maxDiffElements = 32
22-
2319
// CanFormatDiffSlice reports whether we support custom formatting for nodes
2420
// that are slices of primitive kinds or strings.
2521
func (opts formatOptions) CanFormatDiffSlice(v *valueNode) bool {
@@ -155,7 +151,8 @@ func (opts formatOptions) FormatDiffSlice(v *valueNode) textNode {
155151
// + BAZ
156152
// """
157153
isTripleQuoted := true
158-
prevDiffLines := map[string]bool{}
154+
prevRemoveLines := map[string]bool{}
155+
prevInsertLines := map[string]bool{}
159156
var list2 textList
160157
list2 = append(list2, textRecord{Value: textLine(`"""`), ElideComma: true})
161158
for _, r := range list {
@@ -171,20 +168,24 @@ func (opts formatOptions) FormatDiffSlice(v *valueNode) textNode {
171168
isPrintable := func(r rune) bool {
172169
return unicode.IsPrint(r) || r == '\t' // specially treat tab as printable
173170
}
174-
isTripleQuoted = isTripleQuoted &&
175-
!strings.HasPrefix(line, `"""`) &&
176-
!strings.HasPrefix(line, "...") &&
177-
strings.TrimFunc(line, isPrintable) == "" &&
178-
(r.Diff == 0 || !prevDiffLines[normLine])
171+
isTripleQuoted = !strings.HasPrefix(line, `"""`) && !strings.HasPrefix(line, "...") && strings.TrimFunc(line, isPrintable) == ""
172+
switch r.Diff {
173+
case diffRemoved:
174+
isTripleQuoted = isTripleQuoted && !prevInsertLines[normLine]
175+
prevRemoveLines[normLine] = true
176+
case diffInserted:
177+
isTripleQuoted = isTripleQuoted && !prevRemoveLines[normLine]
178+
prevInsertLines[normLine] = true
179+
}
179180
if !isTripleQuoted {
180181
break
181182
}
182183
r.Value = textLine(line)
183184
r.ElideComma = true
184-
prevDiffLines[normLine] = true
185185
}
186-
if r.Diff == 0 {
187-
prevDiffLines = map[string]bool{} // start a new non-adjacent difference group
186+
if !(r.Diff == diffRemoved || r.Diff == diffInserted) { // start a new non-adjacent difference group
187+
prevRemoveLines = map[string]bool{}
188+
prevInsertLines = map[string]bool{}
188189
}
189190
list2 = append(list2, r)
190191
}
@@ -337,11 +338,18 @@ func (opts formatOptions) formatDiffSlice(
337338
return n0 - v.Len()
338339
}
339340

341+
var numDiffs int
342+
maxLen := -1
343+
if opts.LimitVerbosity {
344+
maxLen = (1 << opts.verbosity()) << 2 // 4, 8, 16, 32, 64, etc...
345+
opts.VerbosityLevel--
346+
}
347+
340348
groups := coalesceAdjacentEdits(name, es)
341349
groups = coalesceInterveningIdentical(groups, chunkSize/4)
342350
maxGroup := diffStats{Name: name}
343351
for i, ds := range groups {
344-
if len(list) >= maxDiffElements {
352+
if maxLen >= 0 && numDiffs >= maxLen {
345353
maxGroup = maxGroup.Append(ds)
346354
continue
347355
}
@@ -374,10 +382,12 @@ func (opts formatOptions) formatDiffSlice(
374382
}
375383

376384
// Print unequal.
385+
len0 := len(list)
377386
nx := appendChunks(vx.Slice(0, ds.NumIdentical+ds.NumRemoved+ds.NumModified), diffRemoved)
378387
vx = vx.Slice(nx, vx.Len())
379388
ny := appendChunks(vy.Slice(0, ds.NumIdentical+ds.NumInserted+ds.NumModified), diffInserted)
380389
vy = vy.Slice(ny, vy.Len())
390+
numDiffs += len(list) - len0
381391
}
382392
if maxGroup.IsZero() {
383393
assert(vx.Len() == 0 && vy.Len() == 0)

0 commit comments

Comments
 (0)