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

Allow batched diffing of slices with a custom comparer #210

Merged
merged 1 commit into from
Jun 10, 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
9 changes: 9 additions & 0 deletions cmp/compare_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -833,6 +833,15 @@ func reporterTests() []test {
y: MyComposite{IntsA: []int8{12, 29, 13, 27, 22, 23, 17, 18, 19, 20, 21, 10, 26, 16, 25, 28, 11, 15, 24, 14}},
wantEqual: false,
reason: "batched diffing desired since many elements differ",
}, {
label: label + "/BatchedWithComparer",
x: MyComposite{BytesA: []byte{10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22, 23, 24, 25, 26, 27, 28, 29}},
y: MyComposite{BytesA: []byte{12, 29, 13, 27, 22, 23, 17, 18, 19, 20, 21, 10, 26, 16, 25, 28, 11, 15, 24, 14}},
wantEqual: false,
opts: []cmp.Option{
cmp.Comparer(bytes.Equal),
},
reason: "batched diffing desired since many elements differ",
}, {
label: label + "/BatchedLong",
x: MyComposite{IntsA: []int8{0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22, 23, 24, 25, 26, 27, 28, 29, 30, 31, 32, 33, 34, 35, 36, 37, 38, 39, 40, 41, 42, 43, 44, 45, 46, 47, 48, 49, 50, 51, 52, 53, 54, 55, 56, 57, 58, 59, 60, 61, 62, 63, 64, 65, 66, 67, 68, 69, 70, 71, 72, 73, 74, 75, 76, 77, 78, 79, 80, 81, 82, 83, 84, 85, 86, 87, 88, 89, 90, 91, 92, 93, 94, 95, 96, 97, 98, 99, 100, 101, 102, 103, 104, 105, 106, 107, 108, 109, 110, 111, 112, 113, 114, 115, 116, 117, 118, 119, 120, 121, 122, 123, 124, 125, 126, 127}},
Expand Down
18 changes: 15 additions & 3 deletions cmp/report_slices.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,13 +23,25 @@ func (opts formatOptions) CanFormatDiffSlice(v *valueNode) bool {
return false // Must be formatting in diff mode
case v.NumDiff == 0:
return false // No differences detected
case v.NumIgnored+v.NumCompared+v.NumTransformed > 0:
// TODO: Handle the case where someone uses bytes.Equal on a large slice.
return false // Some custom option was used to determined equality
case !v.ValueX.IsValid() || !v.ValueY.IsValid():
return false // Both values must be valid
case v.Type.Kind() == reflect.Slice && (v.ValueX.IsNil() || v.ValueY.IsNil()):
return false // Both of values have to be non-nil
case v.NumIgnored > 0:
return false // Some ignore option was used
case v.NumTransformed > 0:
return false // Some transform option was used
case v.NumCompared > 1:
return false // More than one comparison was used
case v.NumCompared == 1 && v.Type.Name() != "":
// The need for cmp to check applicability of options on every element
// in a slice is a significant performance detriment for large []byte.
// The workaround is to specify Comparer(bytes.Equal),
// which enables cmp to compare []byte more efficiently.
// If they differ, we still want to provide batched diffing.
// The logic disallows named types since they tend to have their own
// String method, with nicer formatting than what this provides.
return false
}

switch t := v.Type; t.Kind() {
Expand Down
16 changes: 16 additions & 0 deletions cmp/testdata/diffs
Original file line number Diff line number Diff line change
Expand Up @@ -294,6 +294,22 @@
... // 6 identical fields
}
>>> TestDiff/Reporter#01
<<< TestDiff/Reporter/BatchedWithComparer
cmp_test.MyComposite{
StringA: "",
StringB: "",
BytesA: []uint8{
- 0x0a, 0x0b, 0x0c, 0x0d, 0x0e, 0x0f, 0x10, // -|.......|
+ 0x0c, 0x1d, 0x0d, 0x1b, 0x16, 0x17, // +|......|
0x11, 0x12, 0x13, 0x14, 0x15, // |.....|
- 0x16, 0x17, 0x18, 0x19, 0x1a, 0x1b, 0x1c, 0x1d, // -|........|
+ 0x0a, 0x1a, 0x10, 0x19, 0x1c, 0x0b, 0x0f, 0x18, 0x0e, // +|.........|
},
BytesB: nil,
BytesC: nil,
... // 9 identical fields
}
>>> TestDiff/Reporter/BatchedWithComparer
<<< TestDiff/Reporter/BatchedLong
interface{}(
- cmp_test.MyComposite{
Expand Down