feat: ignore context.Canceled by default in slogtest #207
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Signed-off-by: Spike Curtis <[email protected]>
Pull Request Test Coverage Report for Build 051f52451600cfdd55b8c1c1cc95f2abf43965ff-PR-207
💛 - Coveralls |
coadler
approved these changes
Jan 25, 2024
sloggers/slogtest/t.go
Outdated
| @@ -36,13 +38,27 @@ type Options struct { | |||
| // conditions exist when t.Log is called concurrently of a test exiting. Set | |||
| // to true if you don't need this behavior. | |||
| SkipCleanup bool | |||
| // IgnoredErrorValues causes the test logger not to error the test on Error | |||
There was a problem hiding this comment.
Suggested change
| // IgnoredErrorValues causes the test logger not to error the test on Error | |
| // IgnoredErrorIs causes the test logger not to error the test on Error |
sloggers/slogtest/t.go
Outdated
Comment on lines
57
to
60
| opts.IgnoredErrorIs = []error{ | ||
| context.Canceled, | ||
| context.DeadlineExceeded, | ||
| } |
There was a problem hiding this comment.
It might be nice to include this as an exported slice or have some function that produces an IgnoredErrorIs slice with these included, so you don't have to manually specify them every time you want to add an extra error instead of overriding them.
Signed-off-by: Spike Curtis <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
I feel like we spend a lot of time chasing flakes due to logging
context.CanceledatErrorin product code, because a lot of our tests shut down by canceling contexts. This is not a good use of our time, so we should, by default, ignore these errors vis a vis erroring the test case inslogtest