This repository has been archived by the owner on Oct 21, 2024. It is now read-only.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What are you trying to accomplish?
Better UX in case of test failures:
Moreover: Make code related to test assertions a bit more concise.
What approach did you choose and why?
Use
assert.NoError
instead ofassert.Nil
This gives nicer error messages.
Swap arguments to
assert.Equal
The expectation comes first. Otherwise, error messages will be misleading.
Use
assert.Error
instead ofassert.NotNil
This gives nicer error messages.
Use
assert.ErrorAs
andassert.ErrorContains
This simplifies the assertions and potentially gives better error messages.
Use
require
instead ofassert
and useassert.NotNil
as guardThis is to prevent panics in tests, when things get accessed which shouldn't be
nil
.Anything you want to highlight for special attention from reviewers?
An example of a misleading assertion message (expected and actual are flipped):