Use `go-cmp` instead of `reflect.DeepEqual` to compare unit test results
/kind user-story /area refactoring /area testing
User Story
As an odo developer writing unit tests or inspecting non-passing tests, I want to be able to compare test results, so that I can iterate and fix tests faster.
This was discussed during one of our Cabal meetings.
We currently have a lot of places in the unit tests where we write:
if !reflect.DeepEqual(tt.want, got) {
t.Errorf("expected: %v, got %v", tt.want, got)
}
When there is a mismatch causing a test failure, it is quite difficult to figure out what the diff is by reading the output, e.g:
--- FAIL: TestGetInputEnvVarsFromStrings/Test_case_1:_with_valid_two_key_value_pairs (0.00s)
utils_test.go:99: expected: [{key value nil} {key1 value12 nil}], got [{key value nil} {key1 value1 nil}]
In some other places, we rely on reflect.DeepEqual to compare objects and on pretty.Compare to display the diff:
if !reflect.DeepEqual(got, tt.want) {
t.Errorf("Got: %+v \n\n Want: %+v", got, tt.want)
t.Errorf("Comparison: %v", pretty.Compare(got, tt.want))
t.Logf("Error message is: %v", err)
}
go-cmp allows to write the condition in a simpler (and apparently more common) way and returns a detailed diff of what does not match, making it quicker to investigate. For example, the condition below would generate the output that follows:
if diff := cmp.Diff(tt.want, got); diff != "" {
t.Error(diff)
}
--- FAIL: TestGetInputEnvVarsFromStrings/Test_case_1:_with_valid_two_key_value_pairs (0.00s)
utils_test.go:96: []v1.EnvVar{
{Name: "key", Value: "value"},
{
Name: "key1",
- Value: "value12",
+ Value: "value1",
ValueFrom: nil,
},
}
Acceptance Criteria
- [ ] Replace occurrences of all our
reflect.DeepEqualandpretty.Compareusage withcmp.Diff. Replacement should be done only in test sources, asgo-cmpis recommended in tests only. - [ ] Replacement should be backwards-compatible, i.e., no test should be broken or skipped. Bear in mind that
go-cmpallows flexible options (or even defining custom Comparers) such as:- the ability to change its behavior w.r.t. unexported fields:
IgnoreUnexported(typs ...interface{}) - the ability to compare floating-point numbers with an acceptable margin:
EquateApprox(fraction, margin float64) - the ability to sort maps prior to comparing them:
SortMaps(lessFunc interface{})
- the ability to change its behavior w.r.t. unexported fields:
- [ ] If not used anymore, remove the dependency to the
github.com/kylelemons/godebugpackage (whichpretty.Comparecomes from) - [ ] Document this in the Coding Conventions wiki page
Links
- Related Epic (mandatory):
- go-cmp: https://github.com/google/go-cmp
/kind user-story