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

Add ErrorContains #1022

Merged
merged 1 commit into from
Jan 16, 2021
Merged

Add ErrorContains #1022

merged 1 commit into from
Jan 16, 2021

Conversation

alxn
Copy link
Contributor

@alxn alxn commented Nov 9, 2020

Summary

It's common practice to wrap errors, so that you do not have a transitive dependency on the error format of a third party library, but also so that you are able to exactly match you are testing a specific error case correctly.

This change adds an ErrorContains() method to check that both an error is reurned, and it contains a provided string.

Changes

Added ErrorContains() and ErrorContainsf().

Motivation

When I wrap errors returned from third party modules:

func method(f foo, b bar) error {
        if err := f(); err != nil {
                return fmt.Errorf("foo failed: %w", err)
        }
        if err := b(); err != nil {
                return fmt.Errorf("bar failed: %w", err)
        }
        return nil
}

This then forces me to write:

f := func() error { return assert.AnError }
b := func() error { return nil }
err := method(f, b)
require.Error(t, err)
assert.Contains(t, err.Error(), "foo failed")

f = func() error { return nil }
b = func() error { return assert.AnError }
err = method(f, b)
require.Error(t, err)
assert.Contains(t, err.Error(), "bar failed")

This change removes the require and Error() unpacking:

f := func() error { return assert.AnError }
b := func() error { return nil }
err := method(f, b)
assert.ErrorContains(t, err, "foo failed")

f = func() error { return nil }
b = func() error { return assert.AnError }
err = method(f, b)
assert.ErrorContains(t, err, "bar failed")

Related issues

@boyan-soubachov
Copy link
Collaborator

I'd love to get community feedback on this. I'm not sure the value this extra function adds is greater than the confusion/complexity it creates by having yet another function in the assert/require packages.

In fact, how is this different from the EqualError assertions?

@alxn
Copy link
Contributor Author

alxn commented Jan 14, 2021

EqualError is different because it requires an exact error match. Here, I have the problem where I have a decorated error. I don't know the underlying error, only the wrapping.

@boyan-soubachov
Copy link
Collaborator

I see. Would it not make sense to rename this function to ContainsError to be consistent with EqualError then?

@alxn
Copy link
Contributor Author

alxn commented Jan 15, 2021

assert.ErrorContains(t, err, "bar failed")

Semantically I'm asserting that the Error err Contains "bar failed".

ContainsError scans more like:

assert.ContainsError(t, something, err)

I'd written ErrorContains to be a variant of ErrorIs:

func EqualError(t TestingT, theError error, errString string, msgAndArgs ...interface{}) bool
func Error(t TestingT, err error, msgAndArgs ...interface{}) bool
func ErrorAs(t TestingT, err error, target interface{}, msgAndArgs ...interface{}) bool
func ErrorIs(t TestingT, err, target error, msgAndArgs ...interface{}) bool
func NoError(t TestingT, err error, msgAndArgs ...interface{}) bool
func NotErrorIs(t TestingT, err, target error, msgAndArgs ...interface{}) bool
func PanicsWithError(t TestingT, errString string, f PanicTestFunc, msgAndArgs ...interface{}) bool

@boyan-soubachov
Copy link
Collaborator

Fair point, that makes sense :)

@boyan-soubachov boyan-soubachov added this to the v1.8.0 milestone Jan 16, 2021
@boyan-soubachov boyan-soubachov merged commit 6990a05 into stretchr:master Jan 16, 2021
@alxn alxn deleted the errorContains branch January 16, 2021 01:37
@yarikk
Copy link
Contributor

yarikk commented Jun 21, 2021

This was merged 6 months ago but still unavailable in @latest which currently resolves to v1.7.0. When to expect it version-tagged?

@webmaster128
Copy link

1.7.1 is realeased and seems to include this feature. Thank you everyone.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants