diff --git a/assert/assert.go b/assert/assert.go index 83610aa8..e75d1f38 100644 --- a/assert/assert.go +++ b/assert/assert.go @@ -119,12 +119,8 @@ func assert( return true case error: - // Handle nil structs which implement error as a nil error - if reflect.ValueOf(check).IsNil() { - return true - } - msg := "error is not nil: " - t.Log(format.WithCustomMessage(failureMessage+msg+check.Error(), msgAndArgs...)) + msg := failureMsgFromError(check) + t.Log(format.WithCustomMessage(failureMessage+msg, msgAndArgs...)) case cmp.Comparison: success = runComparison(t, argSelector, check, msgAndArgs...) @@ -179,6 +175,15 @@ func logFailureFromBool(t TestingT, msgAndArgs ...interface{}) { t.Log(format.WithCustomMessage(failureMessage+msg, msgAndArgs...)) } +func failureMsgFromError(err error) string { + // 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) + } + return "error is not nil: " + err.Error() +} + func boolFailureMessage(expr ast.Expr) (string, error) { if binaryExpr, ok := expr.(*ast.BinaryExpr); ok && binaryExpr.Op == token.NEQ { x, err := source.FormatNode(binaryExpr.X) diff --git a/assert/assert_test.go b/assert/assert_test.go index b703f400..c9758cb4 100644 --- a/assert/assert_test.go +++ b/assert/assert_test.go @@ -115,32 +115,70 @@ func TestAssertWithComparisonAndExtraMessage(t *testing.T) { expectFailNowed(t, fakeT, "assertion failed: oops, not good: extra stuff true") } -type customError struct{} +type customError struct { + field bool +} func (e *customError) Error() string { + // access a field of the receiver to simulate the behaviour of most + // implementations, and test handling of non-nil typed errors. + e.field = true return "custom error" } -func TestNilErrorSuccess(t *testing.T) { - fakeT := &fakeTestingT{} +func TestNilError(t *testing.T) { + t.Run("nil interface", func(t *testing.T) { + fakeT := &fakeTestingT{} + var err error + NilError(fakeT, err) + expectSuccess(t, fakeT) + }) - var err error - NilError(fakeT, err) - expectSuccess(t, fakeT) + t.Run("nil literal", func(t *testing.T) { + fakeT := &fakeTestingT{} + NilError(fakeT, nil) + expectSuccess(t, fakeT) + }) - NilError(fakeT, nil) - expectSuccess(t, fakeT) + t.Run("interface with non-nil type", func(t *testing.T) { + fakeT := &fakeTestingT{} + var customErr *customError + NilError(fakeT, customErr) + expected := "assertion failed: error is not nil: error has type *assert.customError" + expectFailNowed(t, fakeT, expected) + }) - var customErr *customError - NilError(fakeT, customErr) - expectSuccess(t, fakeT) + t.Run("non-nil error", func(t *testing.T) { + fakeT := &fakeTestingT{} + NilError(fakeT, fmt.Errorf("this is the error")) + expectFailNowed(t, fakeT, "assertion failed: error is not nil: this is the error") + }) + + t.Run("non-nil error with struct type", func(t *testing.T) { + fakeT := &fakeTestingT{} + err := structError{} + NilError(fakeT, err) + expectFailNowed(t, fakeT, "assertion failed: error is not nil: this is a struct") + }) + + t.Run("non-nil error with map type", func(t *testing.T) { + fakeT := &fakeTestingT{} + var err mapError + NilError(fakeT, err) + expectFailNowed(t, fakeT, "assertion failed: error is not nil: ") + }) } -func TestNilErrorFailure(t *testing.T) { - fakeT := &fakeTestingT{} +type structError struct{} + +func (structError) Error() string { + return "this is a struct" +} + +type mapError map[int]string - NilError(fakeT, fmt.Errorf("this is the error")) - expectFailNowed(t, fakeT, "assertion failed: error is not nil: this is the error") +func (m mapError) Error() string { + return m[0] } func TestCheckFailure(t *testing.T) {