Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Limit verbosity of reporter output #215

Merged
merged 1 commit into from
Jun 12, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions cmp/compare_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -159,8 +159,8 @@ func TestDiff(t *testing.T) {
}
} else {
wantDiff := wantDiffs[t.Name()]
if gotDiff != wantDiff {
t.Fatalf("Diff:\ngot:\n%s\nwant:\n%s\nreason: %v", gotDiff, wantDiff, tt.reason)
if diff := cmp.Diff(wantDiff, gotDiff); diff != "" {
t.Fatalf("Diff:\ngot:\n%s\nwant:\n%s\ndiff: (-want +got)\n%s\nreason: %v", gotDiff, wantDiff, diff, tt.reason)
}
}
gotEqual := gotDiff == ""
Expand Down
2 changes: 1 addition & 1 deletion cmp/example_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ func ExampleDiff_testing() {
// SSID: "CoffeeShopWiFi",
// - IPAddress: s"192.168.0.2",
// + IPAddress: s"192.168.0.1",
// NetMask: net.IPMask{0xff, 0xff, 0x00, 0x00},
// NetMask: {0xff, 0xff, 0x00, 0x00},
// Clients: []cmp_test.Client{
// ... // 2 identical elements
// {Hostname: "macchiato", IPAddress: s"192.168.0.153", LastSeen: s"2009-11-10 23:39:43 +0000 UTC"},
Expand Down
46 changes: 41 additions & 5 deletions cmp/report_compare.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,6 @@ import (
"github.com/google/go-cmp/cmp/internal/value"
)

// TODO: Enforce limits?
// * Enforce maximum number of records to print per node?
// * Enforce maximum size in bytes allowed?
// * As a heuristic, use less verbosity for equal nodes than unequal nodes.
// TODO: Enforce unique outputs?
// * Avoid Stringer methods if it results in same output?
// * Print pointer address if outputs still equal?
Expand Down Expand Up @@ -71,10 +67,31 @@ func (opts formatOptions) WithTypeMode(t typeMode) formatOptions {
opts.TypeMode = t
return opts
}
func (opts formatOptions) WithVerbosity(level int) formatOptions {
opts.VerbosityLevel = level
opts.LimitVerbosity = true
return opts
}
func (opts formatOptions) verbosity() uint {
switch {
case opts.VerbosityLevel < 0:
return 0
case opts.VerbosityLevel > 16:
return 16 // some reasonable maximum to avoid shift overflow
default:
return uint(opts.VerbosityLevel)
}
}

// FormatDiff converts a valueNode tree into a textNode tree, where the later
// is a textual representation of the differences detected in the former.
func (opts formatOptions) FormatDiff(v *valueNode) textNode {
if opts.DiffMode == diffIdentical {
opts = opts.WithVerbosity(1)
} else {
opts = opts.WithVerbosity(3)
}

// Check whether we have specialized formatting for this node.
// This is not necessary, but helpful for producing more readable outputs.
if opts.CanFormatDiffSlice(v) {
Expand Down Expand Up @@ -124,6 +141,8 @@ func (opts formatOptions) FormatDiff(v *valueNode) textNode {
}
}

// TODO: Print cycle reference for pointers, maps, and elements of a slice.

// Descend into the child value node.
if v.TransformerName != "" {
out := opts.WithTypeMode(emitType).FormatDiff(v.Value)
Expand Down Expand Up @@ -162,12 +181,27 @@ func (opts formatOptions) formatDiffList(recs []reportRecord, k reflect.Kind) te
formatKey = formatMapKey
}

maxLen := -1
if opts.LimitVerbosity {
if opts.DiffMode == diffIdentical {
maxLen = ((1 << opts.verbosity()) >> 1) << 2 // 0, 4, 8, 16, 32, etc...
} else {
maxLen = (1 << opts.verbosity()) << 1 // 2, 4, 8, 16, 32, 64, etc...
}
opts.VerbosityLevel--
}

// Handle unification.
switch opts.DiffMode {
case diffIdentical, diffRemoved, diffInserted:
var list textList
var deferredEllipsis bool // Add final "..." to indicate records were dropped
for _, r := range recs {
if len(list) == maxLen {
deferredEllipsis = true
break
}

// Elide struct fields that are zero value.
if k == reflect.Struct {
var isZero bool
Expand Down Expand Up @@ -205,11 +239,12 @@ func (opts formatOptions) formatDiffList(recs []reportRecord, k reflect.Kind) te
}

// Handle differencing.
var numDiffs int
var list textList
groups := coalesceAdjacentRecords(name, recs)
maxGroup := diffStats{Name: name}
for i, ds := range groups {
if len(list) >= maxDiffElements {
if maxLen >= 0 && numDiffs >= maxLen {
maxGroup = maxGroup.Append(ds)
continue
}
Expand Down Expand Up @@ -273,6 +308,7 @@ func (opts formatOptions) formatDiffList(recs []reportRecord, k reflect.Kind) te
}
}
recs = recs[ds.NumDiff():]
numDiffs += ds.NumDiff()
}
if maxGroup.IsZero() {
assert(len(recs) == 0)
Expand Down
79 changes: 70 additions & 9 deletions cmp/report_reflect.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,14 +21,23 @@ type formatValueOptions struct {
// methods like error.Error or fmt.Stringer.String.
AvoidStringer bool

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

// PrintAddresses controls whether to print the address of all pointers,
// slice elements, and maps.
PrintAddresses bool

// VerbosityLevel controls the amount of output to produce.
// A higher value produces more output. A value of zero or lower produces
// no output (represented using an ellipsis).
// If LimitVerbosity is false, then the level is treated as infinite.
VerbosityLevel int

// LimitVerbosity specifies that formatting should respect VerbosityLevel.
LimitVerbosity bool
}

// FormatType prints the type as if it were wrapping s.
Expand All @@ -45,6 +54,9 @@ func (opts formatOptions) FormatType(t reflect.Type, s textNode) textNode {
default:
return s
}
if opts.DiffMode == diffIdentical {
return s // elide type for identical nodes
}
case elideType:
return s
}
Expand Down Expand Up @@ -86,11 +98,22 @@ func (opts formatOptions) FormatValue(v reflect.Value, withinSlice bool, m visit
// Avoid calling Error or String methods on nil receivers since many
// implementations crash when doing so.
if (t.Kind() != reflect.Ptr && t.Kind() != reflect.Interface) || !v.IsNil() {
var prefix, strVal string
switch v := v.Interface().(type) {
case error:
return textLine("e" + formatString(v.Error()))
prefix, strVal = "e", v.Error()
case fmt.Stringer:
return textLine("s" + formatString(v.String()))
prefix, strVal = "s", v.String()
}
if prefix != "" {
maxLen := len(strVal)
if opts.LimitVerbosity {
maxLen = (1 << opts.verbosity()) << 5 // 32, 64, 128, 256, etc...
}
if len(strVal) > maxLen+len(textEllipsis) {
return textLine(prefix + formatString(strVal[:maxLen]) + string(textEllipsis))
}
return textLine(prefix + formatString(strVal))
}
}
}
Expand Down Expand Up @@ -123,17 +146,33 @@ func (opts formatOptions) FormatValue(v reflect.Value, withinSlice bool, m visit
case reflect.Complex64, reflect.Complex128:
return textLine(fmt.Sprint(v.Complex()))
case reflect.String:
maxLen := v.Len()
if opts.LimitVerbosity {
maxLen = (1 << opts.verbosity()) << 5 // 32, 64, 128, 256, etc...
}
if v.Len() > maxLen+len(textEllipsis) {
return textLine(formatString(v.String()[:maxLen]) + string(textEllipsis))
}
return textLine(formatString(v.String()))
case reflect.UnsafePointer, reflect.Chan, reflect.Func:
return textLine(formatPointer(v))
case reflect.Struct:
var list textList
v := makeAddressable(v) // needed for retrieveUnexportedField
maxLen := v.NumField()
if opts.LimitVerbosity {
maxLen = ((1 << opts.verbosity()) >> 1) << 2 // 0, 4, 8, 16, 32, etc...
opts.VerbosityLevel--
}
for i := 0; i < v.NumField(); i++ {
vv := v.Field(i)
if value.IsZero(vv) {
continue // Elide fields with zero values
}
if len(list) == maxLen {
list.AppendEllipsis(diffStats{})
break
}
sf := t.Field(i)
if supportExporters && !isExported(sf.Name) {
vv = retrieveUnexportedField(v, sf, true)
Expand All @@ -147,12 +186,21 @@ func (opts formatOptions) FormatValue(v reflect.Value, withinSlice bool, m visit
return textNil
}
if opts.PrintAddresses {
ptr = formatPointer(v)
ptr = fmt.Sprintf("⟪ptr:0x%x, len:%d, cap:%d⟫", pointerValue(v), v.Len(), v.Cap())
}
fallthrough
case reflect.Array:
maxLen := v.Len()
if opts.LimitVerbosity {
maxLen = ((1 << opts.verbosity()) >> 1) << 2 // 0, 4, 8, 16, 32, etc...
opts.VerbosityLevel--
}
var list textList
for i := 0; i < v.Len(); i++ {
if len(list) == maxLen {
list.AppendEllipsis(diffStats{})
break
}
vi := v.Index(i)
if vi.CanAddr() { // Check for cyclic elements
p := vi.Addr()
Expand All @@ -177,8 +225,17 @@ func (opts formatOptions) FormatValue(v reflect.Value, withinSlice bool, m visit
return textLine(formatPointer(v))
}

maxLen := v.Len()
if opts.LimitVerbosity {
maxLen = ((1 << opts.verbosity()) >> 1) << 2 // 0, 4, 8, 16, 32, etc...
opts.VerbosityLevel--
}
var list textList
for _, k := range value.SortKeys(v.MapKeys()) {
if len(list) == maxLen {
list.AppendEllipsis(diffStats{})
break
}
sk := formatMapKey(k)
sv := opts.WithTypeMode(elideType).FormatValue(v.MapIndex(k), false, m)
list = append(list, textRecord{Key: sk, Value: sv})
Expand All @@ -191,11 +248,12 @@ func (opts formatOptions) FormatValue(v reflect.Value, withinSlice bool, m visit
if v.IsNil() {
return textNil
}
if m.Visit(v) || opts.ShallowPointers {
if m.Visit(v) {
return textLine(formatPointer(v))
}
if opts.PrintAddresses {
if opts.PrintAddresses || opts.PrintShallowPointer {
ptr = formatPointer(v)
opts.PrintShallowPointer = false
}
skipType = true // Let the underlying value print the type instead
return textWrap{"&" + ptr, opts.FormatValue(v.Elem(), false, m), ""}
Expand All @@ -217,7 +275,7 @@ func (opts formatOptions) FormatValue(v reflect.Value, withinSlice bool, m visit
func formatMapKey(v reflect.Value) string {
var opts formatOptions
opts.TypeMode = elideType
opts.ShallowPointers = true
opts.PrintShallowPointer = true
s := opts.FormatValue(v, false, visitedPointers{}).String()
return strings.TrimSpace(s)
}
Expand Down Expand Up @@ -268,11 +326,14 @@ func formatHex(u uint64) string {

// formatPointer prints the address of the pointer.
func formatPointer(v reflect.Value) string {
return fmt.Sprintf("⟪0x%x⟫", pointerValue(v))
}
func pointerValue(v reflect.Value) uintptr {
p := v.Pointer()
if flags.Deterministic {
p = 0xdeadf00f // Only used for stable testing purposes
}
return fmt.Sprintf("⟪0x%x⟫", p)
return p
}

type visitedPointers map[value.Pointer]struct{}
Expand Down
38 changes: 24 additions & 14 deletions cmp/report_slices.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,6 @@ import (
"github.com/google/go-cmp/cmp/internal/diff"
)

// maxDiffElements is the maximum number of difference elements to format
// before the remaining differences are coalesced together.
const maxDiffElements = 32

// CanFormatDiffSlice reports whether we support custom formatting for nodes
// that are slices of primitive kinds or strings.
func (opts formatOptions) CanFormatDiffSlice(v *valueNode) bool {
Expand Down Expand Up @@ -155,7 +151,8 @@ func (opts formatOptions) FormatDiffSlice(v *valueNode) textNode {
// + BAZ
// """
isTripleQuoted := true
prevDiffLines := map[string]bool{}
prevRemoveLines := map[string]bool{}
prevInsertLines := map[string]bool{}
var list2 textList
list2 = append(list2, textRecord{Value: textLine(`"""`), ElideComma: true})
for _, r := range list {
Expand All @@ -171,20 +168,24 @@ func (opts formatOptions) FormatDiffSlice(v *valueNode) textNode {
isPrintable := func(r rune) bool {
return unicode.IsPrint(r) || r == '\t' // specially treat tab as printable
}
isTripleQuoted = isTripleQuoted &&
!strings.HasPrefix(line, `"""`) &&
!strings.HasPrefix(line, "...") &&
strings.TrimFunc(line, isPrintable) == "" &&
(r.Diff == 0 || !prevDiffLines[normLine])
isTripleQuoted = !strings.HasPrefix(line, `"""`) && !strings.HasPrefix(line, "...") && strings.TrimFunc(line, isPrintable) == ""
switch r.Diff {
case diffRemoved:
isTripleQuoted = isTripleQuoted && !prevInsertLines[normLine]
prevRemoveLines[normLine] = true
case diffInserted:
isTripleQuoted = isTripleQuoted && !prevRemoveLines[normLine]
prevInsertLines[normLine] = true
}
if !isTripleQuoted {
break
}
r.Value = textLine(line)
r.ElideComma = true
prevDiffLines[normLine] = true
}
if r.Diff == 0 {
prevDiffLines = map[string]bool{} // start a new non-adjacent difference group
if !(r.Diff == diffRemoved || r.Diff == diffInserted) { // start a new non-adjacent difference group
prevRemoveLines = map[string]bool{}
prevInsertLines = map[string]bool{}
}
list2 = append(list2, r)
}
Expand Down Expand Up @@ -337,11 +338,18 @@ func (opts formatOptions) formatDiffSlice(
return n0 - v.Len()
}

var numDiffs int
maxLen := -1
if opts.LimitVerbosity {
maxLen = (1 << opts.verbosity()) << 2 // 4, 8, 16, 32, 64, etc...
opts.VerbosityLevel--
}

groups := coalesceAdjacentEdits(name, es)
groups = coalesceInterveningIdentical(groups, chunkSize/4)
maxGroup := diffStats{Name: name}
for i, ds := range groups {
if len(list) >= maxDiffElements {
if maxLen >= 0 && numDiffs >= maxLen {
maxGroup = maxGroup.Append(ds)
continue
}
Expand Down Expand Up @@ -374,10 +382,12 @@ func (opts formatOptions) formatDiffSlice(
}

// Print unequal.
len0 := len(list)
nx := appendChunks(vx.Slice(0, ds.NumIdentical+ds.NumRemoved+ds.NumModified), diffRemoved)
vx = vx.Slice(nx, vx.Len())
ny := appendChunks(vy.Slice(0, ds.NumIdentical+ds.NumInserted+ds.NumModified), diffInserted)
vy = vy.Slice(ny, vy.Len())
numDiffs += len(list) - len0
}
if maxGroup.IsZero() {
assert(vx.Len() == 0 && vy.Len() == 0)
Expand Down
Loading