Open pohly opened 1 year ago
hey there - obviously I'm open to this but want to point out what currently exists first. You can do all this with:
Expect(err).To(MatchError(SatisfyAll(
ContainSubstring(`Invalid value: "foo-"`),
ContainSubstring(`Invalid value: "bar.."`),
Not(ContainSubstring(`safe-and-unsafe`)),
Not(ContainSubstring("kernel.shmmax")),
))
(note you don't need the HaveOccurred
as MatchError
performs a nil
check for you).
I could imagine expandingMatchError
to take multiple matchers that it implicitly And
s together with SatisfyAll
- but nothing else does that right now so it would end up being the odd one out. Also the output you get for a failure given a composite matcher like SatisfyAll
might not be as helpful as we'd like - but I'd be happy to invest effort fixing that.
That looks promising. I think it can satisfy all cases that I had in mind for this issue, so we can (almost) close it.
The output could be a bit better, though:
err := fmt.Errorf("foo")
[FAILED] Expected
<*errors.errorString | 0xc001517520>: {s: "foo"}
to match error
<*matchers.AndMatcher | 0xc00401f7d0>: {
Matchers: [
<*matchers.ContainSubstringMatcher | 0xc00401f710>{
Substr: "Invalid value: \"foo-\"",
Args: nil,
},
<*matchers.ContainSubstringMatcher | 0xc00401f740>{
Substr: "Invalid value: \"bar..\"",
Args: nil,
},
<*matchers.NotMatcher | 0xc001517550>{
Matcher: <*matchers.ContainSubstringMatcher | 0xc00401f770>{
Substr: "safe-and-unsafe",
Args: nil,
},
},
<*matchers.NotMatcher | 0xc001517580>{
Matcher: <*matchers.ContainSubstringMatcher | 0xc00401f7a0>{
Substr: "kernel.shmmax",
Args: nil,
},
},
],
firstFailedMatcher: <*matchers.ContainSubstringMatcher | 0xc00401f710>{
Substr: "Invalid value: \"foo-\"",
Args: nil,
},
}
Dumping the error as struct makes sense because it may contain additional details. However, I would also expect to see the result of Error()
because (depending on the type of error), the actual error string might not be in the struct at all.
you can even avoid the "broken pavement" _, _, _, err := foo()
boilerplate (in case you needed it before) using Expect(foo()).Error().To(...)
woof - yeah that failure message could use some work. will need to think about it - one areA i've been bumping into the last couple of days is the general problem of consistent and useful reporting when a (hierarchy of)submatcher(s) is involved.
right now most composite matchers just print the format.Object
of the submatcher(s) but we can surely do better.
i'm knee deep in another project right now but will add this to my todo list for when i free up.
It's not just the matchers: the Expected <*errors.errorString | 0xc001517520>: {s: "foo"}
would also be more informative when it included the Error()
result.
I'm thinking of this case:
package test
import (
"testing"
"github.com/onsi/gomega"
)
type myError struct{}
func (m myError) Error() string {
return "some error"
}
func TestSomething(t *testing.T) {
g := gomega.NewGomegaWithT(t)
err := error(myError{})
// g.Expect(err).ToNot(gomega.HaveOccurred())
g.Expect(err).To(gomega.MatchError(gomega.ContainSubstring("foo")))
}
I found that HaveOccurred
includes the Error
output:
Unexpected error:
<test.myError>: {}
some error
occurred
MatchError
does not:
Expected
<test.myError>: {}
to match error
<*matchers.ContainSubstringMatcher | 0xc0000c3620>: {Substr: "foo", Args: nil}
This seems to be solved as a special case in HaveOccurred
. Would it perhaps be better to let format.Object
always include the Error
result when it encounters an object that implements error
?
Yes that makes sense - doing it at the format.Object
level in particular.
hey sorry for the delay - i havent worked on the submatchers yet but the error output seemed like a higher priority anyway. it's now fixed on v1.27.3 - now anytime format.Object
sees an error
it will add on the Error()
message.
Note that gomega.Eventually
now prints the error string twice, once before the format.Object()
result and once inside it:
[FAILED] Timed out after <after>.
The function passed to Eventually returned the following error:
pods "no-such-pod" not found
<framework.transientError>: {
error: <*errors.StatusError>{
ErrStatus: {
TypeMeta: {Kind: "", APIVersion: ""},
ListMeta: {
SelfLink: "",
ResourceVersion: "",
Continue: "",
RemainingItemCount: nil,
},
Status: "Failure",
Message: "pods \"no-such-pod\" not found",
Reason: "NotFound",
Details: {Name: "no-such-pod", Group: "", Kind: "pods", UID: "", Causes: nil, RetryAfterSeconds: 0},
Code: 404,
},
},
}
pods "no-such-pod" not found
In [It] at: wait_test.go:66 <time>
hey good catch. i've fixed this on master and rearranged things so format.Object()
prints the err.Error()
representation first and then the struct. please take a look and i'll cut a release if it looks better to you.
That looks better, please cut a release.
thanks for taking a look - 1.27.4 is out now with this change
Looks like the godoc documentation for MatchError
https://github.com/onsi/gomega/blob/5129b5cf7d54245b130b241a4ace8a0d12442ab1/matchers.go#LL90C4-L90C4 needs an update: it only mentions matching an error or a sting, but not using other matchers. Will work on this.
❤️ thank you!On Mar 24, 2023, at 1:28 PM, TheDiveO @.***> wrote: Looks like the godoc documentation for MatchError https://github.com/onsi/gomega/blob/5129b5cf7d54245b130b241a4ace8a0d12442ab1/matchers.go#LL90C4-L90C4 needs an update: it only mentions matching an error or a sting, but not using other matchers. Will work on this.
—Reply to this email directly, view it on GitHub, or unsubscribe.You are receiving this because you commented.Message ID: @.***>
In Kubernetes, we (not often) check for specific errors:
I can currently write this as two assertions:
I would like to reduce this to one assertion:
This currently fails:
I think would be consistent with the gomega design to allow
error
as an alternative for afmt.Stringer
and to callError()
to obtain a string when given anerror
.The advantages are:
err
instead of many)