Repository navigation
assert.NoError have unexpected behavior #1298
Description
Hi !
I just had issue with a custom made error struct because of a nil pointer.
If I use assert.NoError passing a nil pointer, it will fail. error being an interface, you should check for nil underlying values.
Here is the sandbox to explain the issue : https://go.dev/play/p/03adogriDwB
References : https://go.dev/tour/methods/12
Solution : in the file assertion.go, line #1381-1396
// NoError asserts that a function returned no error (i.e. `nil`).
//
// actualObj, err := SomeFunction()
// if assert.NoError(t, err) {
// assert.Equal(t, expectedObj, actualObj)
// }
func NoError(t TestingT, err error, msgAndArgs ...interface{}) bool {
if err != nil && !reflect.ValueOf(err).IsNil() {
if h, ok := t.(tHelper); ok {
h.Helper()
}
return Fail(t, fmt.Sprintf("Received unexpected error:\n%+v", err), msgAndArgs...)
}
return true
}If you want me to open a pull-request for this quick fix, just let me know.
Thanks !
Activity
brackendawson commentedon Nov 11, 2022
I'm not able to reproduce your problem: https://go.dev/play/p/V2YQ2_ozEKc
Can you share a minimal recreate?
mario-gravel commentedon Nov 11, 2022
Here it is : https://go.dev/play/p/sciEHYfC7dv
Is the nil assignation is done to an interface variable, it's fine. But when the nil assignation is done to an object that fits the error interface, there is a problem. It is still nil, you can't call .Error() on it.
mario-gravel commentedon Nov 11, 2022
This one better explain my point
https://go.dev/play/p/sX_e79lX2H7
Thanks for your time ! I appreciate.
brackendawson commentedon Nov 13, 2022
Apologies if I've not understood you properly, I could really use an example of a test that fails but you expect to pass.
You are expecting a non-nil error that contains a nil pointer to indicate "no error", according to the specification that is not correct.
The panic when you call
err.Error()is a bug in your custom error which can be fixed as the documentation you link to describes: https://go.dev/play/p/0jwANOfMIMGmario-gravel commentedon Nov 14, 2022
We both understand what's going on. I think there is two way of seeing it. As you said, and I agree, an interface pointing to a struct that is nil should not be equal to nil because it isn't. I was expecting assert.Nil to take care of this situation. About the NoError, I was expecting it to return false if there is no Error in the chain.
If you consider it right, I'm pretty fine with that and I thank you for your time and patience. My mindset is build on the way that I learned Go : If you are checking an interface{} for nil value, make sure you look at its value with reflect.ValueOf('interface').IsNil().
Thank you again and you may close this issue as you please.
dolmen commentedon Jul 12, 2023
I agree with @brackendawson.
nilmight be a valid value for some custom error types.assert.NoErrorcan't reject them even if it is an unusual practice.