assert: Fix NilError, error non-nil type #187
Conversation
Codecov Report
@@ Coverage Diff @@
## master #187 +/- ##
==========================================
+ Coverage 84.04% 84.07% +0.02%
==========================================
Files 29 29
Lines 2050 2053 +3
==========================================
+ Hits 1723 1726 +3
Misses 224 224
Partials 103 103
Continue to review full report at Codecov.
|
dnephin
force-pushed
the
non-nil-type-errors
branch
from
da3a789
to
9f9ce80
Compare
The previous fix incorrectly reported non-nil types as nil errors. Instead this commit handles the case by using a different error message.
dnephin
force-pushed
the
non-nil-type-errors
branch
from
9f9ce80
to
0a11a57
Compare
| // Handle errors with non-nil types | ||
| v := reflect.ValueOf(err) | ||
| if v.Kind() == reflect.Ptr && v.IsNil() { | ||
| return fmt.Sprintf("error is not nil: error has type %T", err) |
There was a problem hiding this comment.
I still think that this could include the message if the call doesn't fail (#185 (comment)), since calls on nil are valid so long as the method doesn't try and access its receiver as though it's not nil. That's a further improvement though, and not strictly necessary to fix the other issues. Up to you.
There was a problem hiding this comment.
That is true. I suspect that most error implementations which use a pointer receiver will require it to be non-nil. I would generally like to avoid the panic/recover, so would prefer to not add it for such a rare case, but it could be done as a future improvement.
|
tagged v3.0.2 with this fix |
The previous fix in #171 incorrectly reported non-nil types as nil errors.
Instead this commit handles the case by using a different error message.
Fixes #185
Fixes #186