Skip to content

Commit

Permalink
Limit number of printed differences for variable-length composites (#213
Browse files Browse the repository at this point in the history
)

For large slices, arrays, and maps, the reporter can be unreadable
if there are many differences. Limit the number of results to some
reasonable maximum.
  • Loading branch information
dsnet authored Jun 10, 2020
1 parent 7c9a834 commit 0d296f9
Show file tree
Hide file tree
Showing 5 changed files with 173 additions and 3 deletions.
28 changes: 28 additions & 0 deletions cmp/compare_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -974,6 +974,34 @@ func reporterTests() []test {
y: "aaa\nbbb\nccc \nddd\neee\nfff\nggg\nhhh\niii\njjj\nkkk\nlll\nmmm\nnnn\nooo\nppp\nqqq\nrrr\nsss\nttt\nuuu\nvvv\nwww\nxxx\nyyy\nzzz\n",
wantEqual: false,
reason: "avoid triple-quote syntax due to visual equivalence of differences",
}, {
label: label + "/LimitMaximumBytesDiffs",
x: []byte("\xcd====\x06\x1f\xc2\xcc\xc2-S=====\x1d\xdfa\xae\x98\x9fH======ǰ\xb7=======\xef====:\\\x94\xe6J\xc7=====\xb4======\n\n\xf7\x94===========\xf2\x9c\xc0f=====4\xf6\xf1\xc3\x17\x82======n\x16`\x91D\xc6\x06=======\x1cE====.===========\xc4\x18=======\x8a\x8d\x0e====\x87\xb1\xa5\x8e\xc3=====z\x0f1\xaeU======G,=======5\xe75\xee\x82\xf4\xce====\x11r===========\xaf]=======z\x05\xb3\x91\x88%\xd2====\n1\x89=====i\xb7\x055\xe6\x81\xd2=============\x883=@̾====\x14\x05\x96%^t\x04=====\xe7Ȉ\x90\x1d============="),
y: []byte("\\====|\x96\xe7SB\xa0\xab=====\xf0\xbd\xa5q\xab\x17;======\xabP\x00=======\xeb====\xa5\x14\xe6O(\xe4=====(======/c@?===========\xd9x\xed\x13=====J\xfc\x918B\x8d======a8A\xebs\x04\xae=======\aC====\x1c===========\x91\"=======uؾ====s\xec\x845\a=====;\xabS9t======\x1f\x1b=======\x80\xab/\xed+:;====\xeaI===========\xabl=======\xb9\xe9\xfdH\x93\x8e\u007f====ח\xe5=====Ig\x88m\xf5\x01V=============\xf7+4\xb0\x92E====\x9fj\xf8&\xd0h\xf9=====\xeeΨ\r\xbf============="),
wantEqual: false,
reason: "total bytes difference output is truncated due to excessive number of differences",
}, {
label: label + "/LimitMaximumStringDiffs",
x: "a\nb\nc\nd\ne\nf\ng\nh\ni\nj\nk\nl\nm\nn\no\np\nq\nr\ns\nt\nu\nv\nw\nx\ny\nz\nA\nB\nC\nD\nE\nF\nG\nH\nI\nJ\nK\nL\nM\nN\nO\nP\nQ\nR\nS\nT\nU\nV\nW\nX\nY\nZ\n",
y: "aa\nb\ncc\nd\nee\nf\ngg\nh\nii\nj\nkk\nl\nmm\nn\noo\np\nqq\nr\nss\nt\nuu\nv\nww\nx\nyy\nz\nAA\nB\nCC\nD\nEE\nF\nGG\nH\nII\nJ\nKK\nL\nMM\nN\nOO\nP\nQQ\nR\nSS\nT\nUU\nV\nWW\nX\nYY\nZ\n",
wantEqual: false,
reason: "total string difference output is truncated due to excessive number of differences",
}, {
label: label + "/LimitMaximumSliceDiffs",
x: func() (out []struct{ S string }) {
for _, s := range strings.Split("a\nb\nc\nd\ne\nf\ng\nh\ni\nj\nk\nl\nm\nn\no\np\nq\nr\ns\nt\nu\nv\nw\nx\ny\nz\nA\nB\nC\nD\nE\nF\nG\nH\nI\nJ\nK\nL\nM\nN\nO\nP\nQ\nR\nS\nT\nU\nV\nW\nX\nY\nZ\n", "\n") {
out = append(out, struct{ S string }{s})
}
return out
}(),
y: func() (out []struct{ S string }) {
for _, s := range strings.Split("aa\nb\ncc\nd\nee\nf\ngg\nh\nii\nj\nkk\nl\nmm\nn\noo\np\nqq\nr\nss\nt\nuu\nv\nww\nx\nyy\nz\nAA\nB\nCC\nD\nEE\nF\nGG\nH\nII\nJ\nKK\nL\nMM\nN\nOO\nP\nQQ\nR\nSS\nT\nUU\nV\nWW\nX\nYY\nZ\n", "\n") {
out = append(out, struct{ S string }{s})
}
return out
}(),
wantEqual: false,
reason: "total slice difference output is truncated due to excessive number of differences",
}, {
label: label,
x: MyComposite{
Expand Down
12 changes: 11 additions & 1 deletion cmp/report_compare.go
Original file line number Diff line number Diff line change
Expand Up @@ -207,7 +207,13 @@ func (opts formatOptions) formatDiffList(recs []reportRecord, k reflect.Kind) te
// Handle differencing.
var list textList
groups := coalesceAdjacentRecords(name, recs)
maxGroup := diffStats{Name: name}
for i, ds := range groups {
if len(list) >= maxDiffElements {
maxGroup = maxGroup.Append(ds)
continue
}

// Handle equal records.
if ds.NumDiff() == 0 {
// Compute the number of leading and trailing records to print.
Expand Down Expand Up @@ -268,7 +274,11 @@ func (opts formatOptions) formatDiffList(recs []reportRecord, k reflect.Kind) te
}
recs = recs[ds.NumDiff():]
}
assert(len(recs) == 0)
if maxGroup.IsZero() {
assert(len(recs) == 0)
} else {
list.AppendEllipsis(maxGroup)
}
return textWrap{"{", list, "}"}
}

Expand Down
16 changes: 15 additions & 1 deletion cmp/report_slices.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,10 @@ 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 @@ -335,7 +339,13 @@ func (opts formatOptions) formatDiffSlice(

groups := coalesceAdjacentEdits(name, es)
groups = coalesceInterveningIdentical(groups, chunkSize/4)
maxGroup := diffStats{Name: name}
for i, ds := range groups {
if len(list) >= maxDiffElements {
maxGroup = maxGroup.Append(ds)
continue
}

// Print equal.
if ds.NumDiff() == 0 {
// Compute the number of leading and trailing equal bytes to print.
Expand Down Expand Up @@ -369,7 +379,11 @@ func (opts formatOptions) formatDiffSlice(
ny := appendChunks(vy.Slice(0, ds.NumIdentical+ds.NumInserted+ds.NumModified), diffInserted)
vy = vy.Slice(ny, vy.Len())
}
assert(vx.Len() == 0 && vy.Len() == 0)
if maxGroup.IsZero() {
assert(vx.Len() == 0 && vy.Len() == 0)
} else {
list.AppendEllipsis(maxGroup)
}
return list
}

Expand Down
7 changes: 6 additions & 1 deletion cmp/report_text.go
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ type textRecord struct {
// exists at the end. If cs is non-zero it coalesces the statistics with the
// previous diffStats.
func (s *textList) AppendEllipsis(ds diffStats) {
hasStats := ds != diffStats{}
hasStats := !ds.IsZero()
if len(*s) == 0 || !(*s)[len(*s)-1].Value.Equal(textEllipsis) {
if hasStats {
*s = append(*s, textRecord{Value: textEllipsis, ElideComma: true, Comment: ds})
Expand Down Expand Up @@ -369,6 +369,11 @@ type diffStats struct {
NumModified int
}

func (s diffStats) IsZero() bool {
s.Name = ""
return s == diffStats{}
}

func (s diffStats) NumDiff() int {
return s.NumRemoved + s.NumInserted + s.NumModified
}
Expand Down
113 changes: 113 additions & 0 deletions cmp/testdata/diffs
Original file line number Diff line number Diff line change
Expand Up @@ -659,6 +659,119 @@
... // 7 identical lines
}, "\n")
>>> TestDiff/Reporter/AvoidTripleQuoteIdenticalWhitespace
<<< TestDiff/Reporter/LimitMaximumBytesDiffs
[]uint8{
- 0xcd, 0x3d, 0x3d, 0x3d, 0x3d, 0x06, 0x1f, 0xc2, 0xcc, 0xc2, 0x2d, 0x53, // -|.====.....-S|
+ 0x5c, 0x3d, 0x3d, 0x3d, 0x3d, 0x7c, 0x96, 0xe7, 0x53, 0x42, 0xa0, 0xab, // +|\====|..SB..|
0x3d, 0x3d, 0x3d, 0x3d, 0x3d, // |=====|
- 0x1d, 0xdf, 0x61, 0xae, 0x98, 0x9f, 0x48, // -|..a...H|
+ 0xf0, 0xbd, 0xa5, 0x71, 0xab, 0x17, 0x3b, // +|...q..;|
0x3d, 0x3d, 0x3d, 0x3d, 0x3d, 0x3d, // |======|
- 0xc7, 0xb0, 0xb7, // -|...|
+ 0xab, 0x50, 0x00, // +|.P.|
0x3d, 0x3d, 0x3d, 0x3d, 0x3d, 0x3d, 0x3d, // |=======|
- 0xef, 0x3d, 0x3d, 0x3d, 0x3d, 0x3a, 0x5c, 0x94, 0xe6, 0x4a, 0xc7, // -|.====:\..J.|
+ 0xeb, 0x3d, 0x3d, 0x3d, 0x3d, 0xa5, 0x14, 0xe6, 0x4f, 0x28, 0xe4, // +|.====...O(.|
0x3d, 0x3d, 0x3d, 0x3d, 0x3d, // |=====|
- 0xb4, // -|.|
+ 0x28, // +|(|
0x3d, 0x3d, 0x3d, 0x3d, 0x3d, 0x3d, // |======|
- 0x0a, 0x0a, 0xf7, 0x94, // -|....|
+ 0x2f, 0x63, 0x40, 0x3f, // +|/c@?|
0x3d, 0x3d, 0x3d, 0x3d, 0x3d, 0x3d, 0x3d, 0x3d, 0x3d, 0x3d, 0x3d, // |===========|
- 0xf2, 0x9c, 0xc0, 0x66, // -|...f|
+ 0xd9, 0x78, 0xed, 0x13, // +|.x..|
0x3d, 0x3d, 0x3d, 0x3d, 0x3d, // |=====|
- 0x34, 0xf6, 0xf1, 0xc3, 0x17, 0x82, // -|4.....|
+ 0x4a, 0xfc, 0x91, 0x38, 0x42, 0x8d, // +|J..8B.|
0x3d, 0x3d, 0x3d, 0x3d, 0x3d, 0x3d, // |======|
- 0x6e, 0x16, 0x60, 0x91, 0x44, 0xc6, 0x06, // -|n.`.D..|
+ 0x61, 0x38, 0x41, 0xeb, 0x73, 0x04, 0xae, // +|a8A.s..|
0x3d, 0x3d, 0x3d, 0x3d, 0x3d, 0x3d, 0x3d, // |=======|
- 0x1c, 0x45, 0x3d, 0x3d, 0x3d, 0x3d, 0x2e, // -|.E====.|
+ 0x07, 0x43, 0x3d, 0x3d, 0x3d, 0x3d, 0x1c, // +|.C====.|
0x3d, 0x3d, 0x3d, 0x3d, 0x3d, 0x3d, 0x3d, 0x3d, 0x3d, 0x3d, 0x3d, // |===========|
- 0xc4, 0x18, // -|..|
+ 0x91, 0x22, // +|."|
... // 95 identical, 61 removed, and 61 inserted bytes
}
>>> TestDiff/Reporter/LimitMaximumBytesDiffs
<<< TestDiff/Reporter/LimitMaximumStringDiffs
(
"""
- a
+ aa
b
- c
+ cc
d
- e
+ ee
f
- g
+ gg
h
- i
+ ii
j
- k
+ kk
l
- m
+ mm
n
- o
+ oo
p
- q
+ qq
r
- s
+ ss
t
- u
+ uu
... // 17 identical, 15 removed, and 15 inserted lines
"""
)
>>> TestDiff/Reporter/LimitMaximumStringDiffs
<<< TestDiff/Reporter/LimitMaximumSliceDiffs
[]struct{ S string }{
- {S: "a"},
+ {S: "aa"},
{S: "b"},
- {S: "c"},
+ {S: "cc"},
{S: "d"},
- {S: "e"},
+ {S: "ee"},
{S: "f"},
- {S: "g"},
+ {S: "gg"},
{S: "h"},
- {S: "i"},
+ {S: "ii"},
{S: "j"},
- {S: "k"},
+ {S: "kk"},
{S: "l"},
- {S: "m"},
+ {S: "mm"},
{S: "n"},
- {S: "o"},
+ {S: "oo"},
{S: "p"},
- {S: "q"},
+ {S: "qq"},
{S: "r"},
- {S: "s"},
+ {S: "ss"},
{S: "t"},
- {S: "u"},
+ {S: "uu"},
... // 17 identical and 15 modified elements
}
>>> TestDiff/Reporter/LimitMaximumSliceDiffs
<<< TestDiff/Reporter#06
cmp_test.MyComposite{
StringA: strings.Join({
Expand Down

0 comments on commit 0d296f9

Please sign in to comment.