Skip to content

Commit

Permalink
improve error formatting and remove duplication of error message in E…
Browse files Browse the repository at this point in the history
…ventually/Consistently
  • Loading branch information
onsi committed Mar 13, 2023
1 parent ccebd9b commit 854f075
Show file tree
Hide file tree
Showing 6 changed files with 28 additions and 18 deletions.
4 changes: 2 additions & 2 deletions format/format.go
Original file line number Diff line number Diff line change
Expand Up @@ -260,9 +260,9 @@ func Object(object interface{}, indentation uint) string {
value := reflect.ValueOf(object)
commonRepresentation := ""
if err, ok := object.(error); ok {
commonRepresentation += "\n" + IndentString(err.Error(), indentation)
commonRepresentation += "\n" + IndentString(err.Error(), indentation) + "\n" + indent
}
return fmt.Sprintf("%s<%s>: %s%s", indent, formatType(value), formatValue(value, indentation), commonRepresentation)
return fmt.Sprintf("%s<%s>: %s%s", indent, formatType(value), commonRepresentation, formatValue(value, indentation))
}

/*
Expand Down
7 changes: 4 additions & 3 deletions format/format_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -616,14 +616,15 @@ var _ = Describe("Format", func() {
Describe("formatting errors", func() {
It("should include the error() representation", func() {
err := fmt.Errorf("whoops: %w", fmt.Errorf("welp: %w", fmt.Errorf("ruh roh")))
Expect(Object(err, 1)).Should(MatchRegexp(` \<\*fmt\.wrapError \| 0x[0-9a-f]*\>\: \{
Expect(Object(err, 1)).Should(MatchRegexp(` \<\*fmt\.wrapError \| 0x[0-9a-f]*\>\:
whoops\: welp\: ruh roh
\{
msg\: "whoops\: welp\: ruh roh",
err\: \<\*fmt.wrapError \| 0x[0-9a-f]*\>\{
msg\: "welp\: ruh roh",
err\: \<\*errors.errorString \| 0x[0-9a-f]*\>\{s\: "ruh roh"\},
\},
\}
whoops\: welp\: ruh roh`))
\}`))
})
})
})
Expand Down
2 changes: 1 addition & 1 deletion internal/async_assertion.go
Original file line number Diff line number Diff line change
Expand Up @@ -412,7 +412,7 @@ func (assertion *AsyncAssertion) match(matcher types.GomegaMatcher, desiredMatch
message += format.Object(attachment.Object, 1)
}
} else {
message = preamble + "\n" + err.Error() + "\n" + format.Object(err, 1)
message = preamble + "\n" + format.Object(err, 1)
}
return message
}
Expand Down
27 changes: 18 additions & 9 deletions internal/async_assertion_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,8 @@ var _ = Describe("Asynchronous Assertions", func() {
It("renders the matcher's error if an error occured", func() {
ig.G.Eventually(ERR_MATCH).WithTimeout(50 * time.Millisecond).WithPolling(10 * time.Millisecond).Should(SpecMatch())
Ω(ig.FailureMessage).Should(ContainSubstring("Timed out after"))
Ω(ig.FailureMessage).Should(ContainSubstring("The matcher passed to Eventually returned the following error:\nspec matcher error"))
Ω(ig.FailureMessage).Should(ContainSubstring("The matcher passed to Eventually returned the following error:"))
Ω(ig.FailureMessage).Should(ContainSubstring("spec matcher error"))
})

It("renders the optional description", func() {
Expand Down Expand Up @@ -372,7 +373,8 @@ var _ = Describe("Asynchronous Assertions", func() {
}).WithTimeout(50 * time.Millisecond).WithPolling(10 * time.Millisecond).Should(SpecMatch())
Ω(counter).Should(Equal(3))
Ω(ig.FailureMessage).Should(ContainSubstring("Failed after"))
Ω(ig.FailureMessage).Should(ContainSubstring("The matcher passed to Consistently returned the following error:\nspec matcher error"))
Ω(ig.FailureMessage).Should(ContainSubstring("The matcher passed to Consistently returned the following error:"))
Ω(ig.FailureMessage).Should(ContainSubstring("spec matcher error"))
})

It("fails if the matcher doesn't match at any point", func() {
Expand Down Expand Up @@ -444,7 +446,8 @@ var _ = Describe("Asynchronous Assertions", func() {
It("renders the matcher's error if an error occured", func() {
ig.G.Consistently(ERR_MATCH).Should(SpecMatch())
Ω(ig.FailureMessage).Should(ContainSubstring("Failed after"))
Ω(ig.FailureMessage).Should(ContainSubstring("The matcher passed to Consistently returned the following error:\nspec matcher error"))
Ω(ig.FailureMessage).Should(ContainSubstring("The matcher passed to Consistently returned the following error:"))
Ω(ig.FailureMessage).Should(ContainSubstring("spec matcher error"))
})

It("renders the optional description", func() {
Expand Down Expand Up @@ -588,7 +591,8 @@ var _ = Describe("Asynchronous Assertions", func() {
ig.G.Eventually(func() (int, string, Foo, error) {
return 1, "", Foo{}, errors.New("welp!")
}).WithTimeout(50 * time.Millisecond).WithPolling(10 * time.Millisecond).Should(BeNumerically("<", 100))
Ω(ig.FailureMessage).Should(ContainSubstring("The function passed to Eventually returned the following error:\nwelp!"))
Ω(ig.FailureMessage).Should(ContainSubstring("The function passed to Eventually returned the following error:"))
Ω(ig.FailureMessage).Should(ContainSubstring("welp!"))
})

Context("when making a ShouldNot assertion", func() {
Expand Down Expand Up @@ -1529,7 +1533,8 @@ sprocket:
}).WithTimeout(time.Millisecond*10).Should(QuickMatcher(func(actual any) (bool, error) {
return false, fmt.Errorf("matcher-error")
}), "My Description")
Ω(ig.FailureMessage).Should(ContainSubstring("My Description\nThe matcher passed to Eventually returned the following error:\nmatcher-error\n <*errors.errorString"))
Ω(ig.FailureMessage).Should(ContainSubstring("My Description\nThe matcher passed to Eventually returned the following error:\n <*errors.errorString"))
Ω(ig.FailureMessage).Should(ContainSubstring("matcher-error"))
})

When("the matcher error is a StopTrying with attachments", func() {
Expand All @@ -1553,7 +1558,8 @@ sprocket:
}).WithTimeout(time.Millisecond*10).Should(QuickMatcher(func(actual any) (bool, error) {
return true, nil
}), "My Description")
Ω(ig.FailureMessage).Should(ContainSubstring("My Description\nThe function passed to Eventually returned the following error:\nactual-err\n <*errors.errorString"))
Ω(ig.FailureMessage).Should(ContainSubstring("My Description\nThe function passed to Eventually returned the following error:\n <*errors.errorString"))
Ω(ig.FailureMessage).Should(ContainSubstring("actual-err"))
})
})

Expand Down Expand Up @@ -1608,8 +1614,10 @@ sprocket:
}
return false, nil
}), "My Description")
Ω(ig.FailureMessage).Should(ContainSubstring("My Description\nThe function passed to Eventually returned the following error:\nactual-err\n <*errors.errorString"))
Ω(ig.FailureMessage).Should(ContainSubstring("At one point, however, the function did return successfully.\nYet, Eventually failed because the matcher returned the following error:\nmatcher-err"))
Ω(ig.FailureMessage).Should(ContainSubstring("My Description\nThe function passed to Eventually returned the following error:\n <*errors.errorString"))
Ω(ig.FailureMessage).Should(ContainSubstring("actual-err"))
Ω(ig.FailureMessage).Should(ContainSubstring("At one point, however, the function did return successfully.\nYet, Eventually failed because the matcher returned the following error:"))
Ω(ig.FailureMessage).Should(ContainSubstring("matcher-err"))
})
})

Expand All @@ -1627,7 +1635,8 @@ sprocket:
actualInt := actual.(int)
return actualInt > 3, nil
}), "My Description")
Ω(ig.FailureMessage).Should(ContainSubstring("My Description\nThe function passed to Eventually returned the following error:\nactual-err\n <*errors.errorString"))
Ω(ig.FailureMessage).Should(ContainSubstring("My Description\nThe function passed to Eventually returned the following error:\n <*errors.errorString"))
Ω(ig.FailureMessage).Should(ContainSubstring("actual-err"))
Ω(ig.FailureMessage).Should(ContainSubstring("At one point, however, the function did return successfully.\nYet, Eventually failed because the matcher was not satisfied:\nQM failure message: 3"))

})
Expand Down
4 changes: 2 additions & 2 deletions matchers/match_error_matcher_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -135,14 +135,14 @@ var _ = Describe("MatchErrorMatcher", func() {
failuresMessages := InterceptGomegaFailures(func() {
Expect(errors.New("foo")).To(MatchError("bar"))
})
Expect(failuresMessages[0]).To(ContainSubstring("{s: \"foo\"}\n foo\nto match error\n <string>: bar"))
Expect(failuresMessages[0]).To(ContainSubstring("foo\n {s: \"foo\"}\nto match error\n <string>: bar"))
})

It("shows negated failure message", func() {
failuresMessages := InterceptGomegaFailures(func() {
Expect(errors.New("foo")).ToNot(MatchError("foo"))
})
Expect(failuresMessages[0]).To(ContainSubstring("{s: \"foo\"}\n foo\nnot to match error\n <string>: foo"))
Expect(failuresMessages[0]).To(ContainSubstring("foo\n {s: \"foo\"}\nnot to match error\n <string>: foo"))
})

})
Expand Down
2 changes: 1 addition & 1 deletion matchers/succeed_matcher_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ var _ = Describe("Succeed", func() {
It("builds failure message", func() {
actual := Succeed().FailureMessage(errors.New("oops"))
actual = regexp.MustCompile(" 0x.*>").ReplaceAllString(actual, " 0x00000000>")
Expect(actual).To(Equal("Expected success, but got an error:\n <*errors.errorString | 0x00000000>: {s: \"oops\"}\n oops"))
Expect(actual).To(Equal("Expected success, but got an error:\n <*errors.errorString | 0x00000000>: \n oops\n {s: \"oops\"}"))
})

It("simply returns .Error() for the failure message if the error is an AsyncPolledActualError", func() {
Expand Down

0 comments on commit 854f075

Please sign in to comment.