-
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
assert.ErrorAs: log target type #1345
Open
craig65535
wants to merge
5
commits into
stretchr:master
Choose a base branch
from
craig65535:fix-assert-erroras
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
5 commits
Select commit
Hold shift + click to select a range
1747ed5
assert.ErrorAs: log target type
craig65535 49400c1
buildErrorChainString: log types, and support go 1.20
craig65535 6759c5a
TestErrorAs: check error contents
craig65535 ed3d1bc
Use checkResultAndErrMsg in TestErrorIs/TestNotErrorIs
craig65535 972befb
Review comments
craig65535 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -1997,7 +1997,7 @@ func ErrorIs(t TestingT, err, target error, msgAndArgs ...interface{}) bool { | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
expectedText = target.Error() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
chain := buildErrorChainString(err) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
chain := buildErrorChainString(err, false) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
return Fail(t, fmt.Sprintf("Target error should be in err chain:\n"+ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
"expected: %q\n"+ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
@@ -2020,7 +2020,7 @@ func NotErrorIs(t TestingT, err, target error, msgAndArgs ...interface{}) bool { | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
expectedText = target.Error() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
chain := buildErrorChainString(err) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
chain := buildErrorChainString(err, false) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
return Fail(t, fmt.Sprintf("Target error should not be in err chain:\n"+ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
"found: %q\n"+ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
@@ -2038,24 +2038,50 @@ func ErrorAs(t TestingT, err error, target interface{}, msgAndArgs ...interface{ | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
return true | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
chain := buildErrorChainString(err) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
chain := buildErrorChainString(err, true) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
return Fail(t, fmt.Sprintf("Should be in error chain:\n"+ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
"expected: %q\n"+ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
"expected: %T\n"+ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
"in chain: %s", target, chain, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
), msgAndArgs...) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
func buildErrorChainString(err error) string { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
func unwrapAll(err error) (errs []error) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
errs = append(errs, err) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
for { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
switch x := err.(type) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
case interface{ Unwrap() error }: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
err = x.Unwrap() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
if err == nil { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
return | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
errs = append(errs, err) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
case interface{ Unwrap() []error }: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
for _, err := range x.Unwrap() { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
errs = append(errs, unwrapAll(err)...) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
return | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
default: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
return | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Comment on lines
+2051
to
+2067
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This can be simpler if all the switch cases recurse, but the above implementation is fine.
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
func buildErrorChainString(err error, withType bool) string { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
if err == nil { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
return "" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
e := errors.Unwrap(err) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
chain := fmt.Sprintf("%q", err.Error()) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
for e != nil { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
chain += fmt.Sprintf("\n\t%q", e.Error()) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
e = errors.Unwrap(e) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
var chain string | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
errs := unwrapAll(err) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
craig65535 marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
for i := range errs { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
if i != 0 { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
chain += "\n\t" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
chain += fmt.Sprintf("%q", errs[i].Error()) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
if withType { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
chain += fmt.Sprintf(" (%T)", errs[i]) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
return chain | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} |
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
Oops, something went wrong.
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.
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.
Do you mind explaining this one? I believe this should only be the default cause since you're going to have recursive definitions of error if you explore each value in the slice.
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 basically "flattens" the provided error (
err
) into a slice so we can access the type andError()
string of each constituent error. We start by appendingerr
to the slice because it itself has a type andError()
string.errors.As
failing for a given error means that the target error type is not present in any of these constituent errors. So, we need to exhaustively log all of the types in the error message.I'm not 100% sure if this answers your question.
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.
Aaa, so the usecase I am thinking of is the following:
This would mean the resulting values would be:
Which I don't believe you want to repeat those values
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.
In this case, I do want to repeat those values - I want 3 lines in the error message, and 3 types.