-
Notifications
You must be signed in to change notification settings - Fork 1.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add assertion for NotErrorAs #1129
Conversation
bb0fb09
to
5a5b06e
Compare
+1 for this PR. |
assert/assertion_forward.go
Outdated
} | ||
|
||
// NotErrorAsf asserts that at none of the errors in err's chain matches target. | ||
// This is the inverse of the ErrorAs function. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
// This is the inverse of the ErrorAs function. | |
// This is the opposite of the ErrorAs function. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
None of the other negative assertions refer to their counterpart in this way. The description of their function is sufficient. Just drop this line.
Hi @dolmen , |
Commented on @mmorel-35 's review, I'm happy merging once resolved. This also needs a re-base. This also needs coordinating with #1646. if @palsivertsen is no longer around then feel free to open a new PR based on their commit @mmorel-35. |
5849ec5
to
90d5d50
Compare
Review feedback: stretchr#1129 (comment)
Review feedback: stretchr#1129 (comment)
90d5d50
to
1d575dd
Compare
@brackendawson Rebased on master and addressed review feedback. Also fixed a typo. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry for the tennis, one more change to that doc-string. I think this slightly left field behaviour needs writing down.
assert/assertions.go
Outdated
@@ -2149,6 +2149,23 @@ func ErrorAs(t TestingT, err error, target interface{}, msgAndArgs ...interface{ | |||
), msgAndArgs...) | |||
} | |||
|
|||
// NotErrorAs asserts that none of the errors in err's chain matches target. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
// NotErrorAs asserts that none of the errors in err's chain matches target. | |
// NotErrorAs asserts that none of the errors in err's chain matches target, but if so, sets target to that error value. |
The library already had assertions for `ErrorIs`, `NotErrorIs` and `ErrorAs`. This commit adds the `NotErrorAs` assertion which is the inverse of `ErrorAs`.
Checks that the assertion result matches what's set in `testing.T`.
Review feedback: stretchr#1129 (comment)
b1df1ea
to
f844b26
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thank you for your contribution.
Summary
The library had implementations for ErrorIs, ErrorAs and NotErrorIs but
was missing a NotErrorAs implementation. This PR adds a NotErrorAs
function, which is the inverse of the ErrorAs function.
Changes
NotErrorAs
function and testTestErrorIs
,TestNotErrorIs
andTestErrorAs
) by comparing assertion result andtesting.T
flag.NotErrorIs
functionMotivation
There's already an inverse of
ErrorIs
,ErrorAs
should have an inverse as well.Related issues
Closes #1066