Proposal: add testing for errors when the expected error is optional

[bkielbasa]

In many cases, I have a code similar to the above

if testCase.expectedError != nil {
    assert.EqualError(t, err, testCase.expectedError.Error())
} else {
     assert.NoError(t, err)
}

where the testCase.expectedError contains an instance of the error. I want to write a new function which will combine those 5 lines of code into 1. For example

assert.EqualNilOrError(t, testCase.expectedError, err)

WDYT about the change? We can discuss the name or parameters it takes but it would help me a lot.

[dennypenta]

Seems like you need your own implementation of Suite.

type YourSuite struct {
    suite.Suite
    expectedError bool
}

func (s * YourSuite) Error(err error, argsAndMsg ...interface{}) {
    if s.expectedError {
        s.Suite.Error(err, argsAndMsg)
    } else {
        s.Suite.NoError(err, argsAndMsg)
    }
}

[bkielbasa]

thanks for your response! What If I don't want to use suites? :)

[brackendawson]

I frequently bump into this desire too.

In your example Equal actually works pretty well:

assert.Equal(t, testCase.expectedError, err)

Sometimes expected is a bool and then you can do this:

assert.Equal(t, testCase.expectedError, err != nil)

But then the failure is distinctly unhelpful:

    /Users/alandaws/src/github.com/brackendawson/kata/kata_test.go:30: 
            Error Trace:    kata_test.go:30
            Error:          Not equal: 
                            expected: false
                            actual  : true
            Test:           TestU

So perhaps it would be nice to have a function for testing the error in tabled tests. What would the signature be? Getting back to basics; as a developer I should be testing my API for state[^1], so I should be testing the error is the error I expect, including nil which is a real value. The way we should check errors in modern Go is with errors.Is(), because what our errors wrap is a part of our API. Testify has ErrorIs which works quite well and does even say that (error) nil does wrap nil:

assert.ErrorIs(t, err, testCase.expectedError)

This would make a cost effective test, because you can change the implementation and even the text of your error freely without breaking the test, so long as you adhere to your promise to wrap the expected error. The failure message is okay-ish:

    /Users/alandaws/src/github.com/brackendawson/kata/kata_test.go:37: 
            Error Trace:    kata_test.go:37
            Error:          Target error should be in err chain:
                            expected: ""
                            in chain: "my hovercraft is full of eels"
            Test:           TestV

in chain: isn't a great label and "" is a poor way to show a nil error. There's also #1115 for the ErrorAs version. So maybe cleaning up the error output from these would address the issue? This wasn't meant to be a blog post, oops. Would also love to hear what you think, and what function you'd like to see added if any?

tl;dr: Test for what you promise rather than what you do. Use assert.ErrorIs and assert.ErrorAs if you can.

[^1]: Sandi Metz, Practical Object-Oriented Design in Ruby: An Agile Primer, ISBN-13: 978-0-321-72133-4, Chapter 9

[brackendawson]

Thinking about it more, ErrorIs isn't a complete story either, It allows you to expect either no error or an error wrapping some error. But a lot of great APIs return a mix of nil errors, errors that wrap some error, or just non-nil errors that don't.

I wonder if we could do something to move the different cases up into the test definitions like this maybe:

func ExpectedError(t TestingT, err error, expected ExpectErrFn, msgAndArgs ...interface{})

So then you could do anything, like:

func TestExpectedErr(t *testing.T) {
    for name, test := range map[string]struct {
        expectedErr assert.ExpectErrFn
    }{
        "no error":           {expectedErr: nil},
        "any error":          {expectedErr: assert.ExpectAnyError},
        "error wrapping EOF": {expectedErr: assert.ExpectErrorIs(io.EOF)},
        "some error string":  {expectedErr: assert.ExpectError("my hovercraft is full of eels")},
        "some of the string": {expectedErr: assert.ExpectErrorContains("failed to set thing: ")},
        "a type with a value": {expectedErr: assert.ExpectErrorAs(func(m MyErrType) bool {
            return m.Count == 4
        })},
    } {
        t.Run(name, func(t *testing.T) {
            err := MyAPI()
            assert.ExpectedError(t, err, test.expectedErr)
        })
    }
}