assert NoError behaves unexpectedly with nil struct

[dominicbarnes]

We have some internal functions that return a struct dedicated to wrapping customer-facing errors, and thus have more to it than the standard error interface. To enforce that we use these structs everywhere, as we want to, we're returning our custom *errors.Error type instead of error.

However, when passing the returned value to assert.NoError(t, err) it will always indicate an error, even when the value is nil. Our type implements Error(), so it can be passed places that error is expected, so testify's behavior here feels like a bug.

func TestSomething(t *testing.T) {
  require.NoError(t, succeed())  // Received unexpected error: <nil>
}

type CustomError struct {}
func (e *CustomError) Error() string { return "boom" }

func succeed() *CustomError {
  return nil
}

This looks like an edge-case in the way that nil values are being detected, and the workaround for now is to use require.Nil(t, err) but I have been using NoError() for a long time elsewhere and will probably forget at some point. 😛

[devdinu]

@dominicbarnes Since you're returning *CustomError from the succeed method succeed().Error() returns "boom" since method call is allowed on nil. Changing the return type to error would not fail the test.

[ernesto-jimenez]

@dominicbarnes as @devdinu mentions (thanks!), the issue is a bug in the example. That is a common gotcha from Go.

Closing.

[mohanraj-r]

I am running into the same issue as well and forced to use require.Nil(), even in cases where require.NoError(), reads better

Changing the return type to error would not fail the test

But then it requires type casting to the custom error type to take advantage it e.g to access fields in the custom error struct.

[devdinu]

Yes, if you wanna propagate information through custom error type, the you've to typecast it in the caller side. Idiomatic way suggested to use errors as values.

[AlekSi]

That common gotcha is explained there: https://golang.org/doc/faq#nil_error Adding this link since people still ask about it.