Skip to content
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: confusing error message #1343

Open
craig65535 opened this issue Feb 10, 2023 · 2 comments · May be fixed by #1345
Open

assert.ErrorAs: confusing error message #1343

craig65535 opened this issue Feb 10, 2023 · 2 comments · May be fixed by #1345
Labels
pkg-assert Change related to package testify/assert type-error Issue related to comparing values implementing the error interface

Comments

@craig65535
Copy link
Contributor

Typically, the target passed to errors.As has a zero value. But if the errors.As call is wrapped by assert.ErrorAs, this can lead to a confusing error message if the assertion fails:

--- FAIL: TestCustomErr (0.00s)
    main_test.go:32: 
                Error Trace:    /.../main_test.go:32
                Error:          Should be in error chain:
                                expected: %!q(PANIC=Error method: runtime error: invalid memory address or nil pointer dereference)
                                in chain: "file does not exist"
                Test:           TestCustomErr
                Messages:       index 1
FAIL

For expected:, it's printing the target. But in my case the target can't be printed until it's assigned a value, and I think printing the target is not useful anyway - I think what it should be doing instead is printing the type of the target:

--- FAIL: TestCustomErr (0.00s)
    main_test.go:32: 
                Error Trace:    /.../main_test.go:32
                Error:          Should be in error chain:
                                expected: "*main.customErr"
                                in chain: "file does not exist"
                Test:           TestCustomErr
                Messages:       index 1
FAIL

Here's the code I used:

package main

import (
	"errors"
	"fmt"
	"io/fs"
	"testing"

	"github.com/stretchr/testify/assert"
)

type customErr struct {
	err error
}

func (ce customErr) Error() string {
	return ce.err.Error()
}

func getErrs() []error {
	return []error{
		customErr{
			err: errors.New(`something`),
		},
		fs.ErrNotExist,
	}
}

func TestCustomErr(t *testing.T) {
	for i, err := range getErrs() {
		var aCustomErr customErr
		assert.ErrorAs(t, err, &aCustomErr, fmt.Sprintf(`index %v`, i))
	}
}

I am happy to submit a PR to address this.

@brackendawson
Copy link
Collaborator

For assert.ErrorAs() only, I agree printing the type of error is always more useful. assert.ErrorIs() would keep printing the value.

@craig65535 craig65535 linked a pull request Feb 10, 2023 that will close this issue
@craig65535
Copy link
Contributor Author

@brackendawson I made a PR: #1345

@dolmen dolmen added pkg-assert Change related to package testify/assert type-error Issue related to comparing values implementing the error interface labels Jul 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pkg-assert Change related to package testify/assert type-error Issue related to comparing values implementing the error interface
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants