odo icon indicating copy to clipboard operation
odo copied to clipboard

Use `go-cmp` instead of `reflect.DeepEqual` to compare unit test results

Open rm3l opened this issue 3 years ago • 0 comments

/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.DeepEqual and pretty.Compare usage with cmp.Diff. Replacement should be done only in test sources, as go-cmp is recommended in tests only.
  • [ ] Replacement should be backwards-compatible, i.e., no test should be broken or skipped. Bear in mind that go-cmp allows flexible options (or even defining custom Comparers) such as:
  • [ ] If not used anymore, remove the dependency to the github.com/kylelemons/godebug package (which pretty.Compare comes from)
  • [ ] Document this in the Coding Conventions wiki page

Links

  • Related Epic (mandatory):
  • go-cmp: https://github.com/google/go-cmp

/kind user-story

rm3l avatar Jul 08 '22 07:07 rm3l