From 0d296f9f534978cc25de69216b23b74bbc10fad9 Mon Sep 17 00:00:00 2001 From: Joe Tsai Date: Wed, 10 Jun 2020 16:51:30 -0700 Subject: [PATCH] Limit number of printed differences for variable-length composites (#213) 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. --- cmp/compare_test.go | 28 +++++++++++ cmp/report_compare.go | 12 ++++- cmp/report_slices.go | 16 +++++- cmp/report_text.go | 7 ++- cmp/testdata/diffs | 113 ++++++++++++++++++++++++++++++++++++++++++ 5 files changed, 173 insertions(+), 3 deletions(-) diff --git a/cmp/compare_test.go b/cmp/compare_test.go index 7a21455..d23c31f 100644 --- a/cmp/compare_test.go +++ b/cmp/compare_test.go @@ -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{ diff --git a/cmp/report_compare.go b/cmp/report_compare.go index d3fa154..8ecdd28 100644 --- a/cmp/report_compare.go +++ b/cmp/report_compare.go @@ -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. @@ -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, "}"} } diff --git a/cmp/report_slices.go b/cmp/report_slices.go index 3da92bc..7a35f39 100644 --- a/cmp/report_slices.go +++ b/cmp/report_slices.go @@ -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 { @@ -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. @@ -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 } diff --git a/cmp/report_text.go b/cmp/report_text.go index d3a3084..17a376e 100644 --- a/cmp/report_text.go +++ b/cmp/report_text.go @@ -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}) @@ -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 } diff --git a/cmp/testdata/diffs b/cmp/testdata/diffs index 60fcbb8..703ea5f 100644 --- a/cmp/testdata/diffs +++ b/cmp/testdata/diffs @@ -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({