-
Notifications
You must be signed in to change notification settings - Fork 3.2k
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
refactor(tests): assert.Error
->require.Error
(remainder)
#13462
Conversation
assert.Error
->require.Error
(remainder)
This is part of a series of test tidies started by #13365. The aim is to enable the testifylint golangci-lint checker. This commit converts assert.Error checks into require.Error for the rest of the code base. In some places checks have been coaleced - in particular the pattern ```go if assert.Error() { assert.Contains(..., "message") } ``` is now ```go require.ErrorContains(..., "message") ``` Getting this wrong and missing the Contains is still valid go, so that's a mistake I may have made. Signed-off-by: Alan Clucas <[email protected]>
84ddac5
to
886146b
Compare
assert.Error
->require.Error
(remainder)assert.Error
->require.Error
(remainder)
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.
I didn't realize there were still some leftover. One question and two small consistency suggestions below, otherwise LGTM
Would you like to tackle the remaining if assert.*
-> require.*
changes as mentioned in #13401 (comment) etc? the vast majority in my searches were actually Error
/NoError
, so there should only be a small number remaining
Signed-off-by: Anton Gilgur <[email protected]>
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.
Made the changes myself above, LGTM now
This is the first bullet point although there are some here outside
I might do, my main target right now around this is getting this linter enabled in CI. I'll track this as an issue. |
Issue #13465 raised. |
This is part of a series of test tidies started by #13365.
The aim is to enable the testifylint golangci-lint checker.
This commit converts assert.Error checks into require.Error for the rest of the code base.
In some places checks have been coaleced - in particular the pattern
is now
Getting this wrong and missing the Contains is still valid go, so that's a mistake I may have made.