Skip to content

Commit

Permalink
review suggestion, always print the full diff first, followed by mini…
Browse files Browse the repository at this point in the history
…mized diff

this way we always have all the info when test fails, but it's still not difficult
to find the minimized diff for initial diagnostics, because it's the most obvious thing at the bottom
  • Loading branch information
jiridanek committed Jan 15, 2025
1 parent a20d65e commit b8de303
Showing 1 changed file with 12 additions and 9 deletions.
21 changes: 12 additions & 9 deletions components/odh-notebook-controller/controllers/matchers_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -107,14 +107,17 @@ func (m *beMatchingK8sResource[T, PT]) Match(actual interface{}) (success bool,
}

func (m *beMatchingK8sResource[T, PT]) FailureMessage(actual interface{}) (message string) {
//m.expected
diff := m.computeMinimizedDiff(actual.(T))
return fmt.Sprintf("Expected\n\t%#v\nto compare identical to\n\t%#v\nbut it differs in\n%s", actual, m.expected, diff)
fullDiff := cmp.Diff(actual, m.expected)
minimalDiff := m.computeMinimizedDiff(actual.(T))
return fmt.Sprintf("Expected\n\t%#v\nto compare identical to\n\t%#v\nbut it differs in\n%s\nMinimized diff is\n%s",
actual, m.expected, fullDiff, minimalDiff)
}

func (m *beMatchingK8sResource[T, PT]) NegatedFailureMessage(actual interface{}) (message string) {
diff := m.computeMinimizedDiff(actual.(T))
return fmt.Sprintf("Expected\n\t%#v\nto not compare identical to\n\t%#v\nit differs in\n%s", actual, m.expected, diff)
fullDiff := cmp.Diff(actual, m.expected)
minimalDiff := m.computeMinimizedDiff(actual.(T))
return fmt.Sprintf("Expected\n\t%#v\nto not compare identical to\n\t%#v\nit differs in\n%s\nMinimized diff is\n%s",
actual, m.expected, fullDiff, minimalDiff)
}

// diffReporter is the basis of a custom [cmp.Reporter] that records differences detected during comparison.
Expand Down Expand Up @@ -202,9 +205,8 @@ func (m *beMatchingK8sResource[T, PT]) computeMinimizedDiff(actual T) (message s
defer func() {
// in case things later go wrong with diff minimization
if r := recover(); r != nil {
// return the un-minimized diff so that we have at least something
message = cmp.Diff(actual, m.expected) + "\n" +
"while computing the diff, there was a crash\n\t" +
// we print the un-minimized diff always just before this, so no info is lost
message = "while computing the diff, there was a crash\n\t" +
fmt.Sprint(r)
}
}()
Expand Down Expand Up @@ -318,7 +320,8 @@ func Test_BeMatchingK8sResource_DiffRoute(t *testing.T) {
assert.Regexp(t, regexp.MustCompile(`^[\pZ\pC]+$`), `  `)

// go-cmp diff output is not guaranteed to remain unchanged, so this may need revisiting
msg := matcher.FailureMessage(someRoute)
// the full `matcher.FailureMessage(someRoute)` contains both full and minimized diffs, assert on minimized only
msg := matcher.(*beMatchingK8sResource[routev1.Route, *routev1.Route]).computeMinimizedDiff(someRoute)
assert.Regexp(t, regexp.MustCompile(`-[\pZ\pC]+Host:\s+"someRouteHost",`), msg)
assert.Regexp(t, regexp.MustCompile(`\+[\pZ\pC]+Host:\s+"someOtherRouteHost",`), msg)

Expand Down

0 comments on commit b8de303

Please sign in to comment.