feat: IsError ErrorAssertionFunc

[robdefeo]

Summary

gotest -template=testify has an assert.ErrorAssertionFunc as one of the parameters. Testify has assert.NoError and assert.Error which asserts the functions returns an error or not. Usually this is enough but sometimes checking for a specific error is more appropriate.

Changes

Add IsError that takes an expectedErr as a parameter and returns an assert.ErrorAssertionFunc

Motivation

While assert.EqualError checks for specific errors, it is not an assert.ErrorAssertionFunc so can not be used in the template table tests. A new function that takes an expectedErr and returns an assert.ErrorAssertionFunc makes testing for specific errors easier and clearer.

func Example(input int) (bool, error) {
	switch input {
	case 1:
		return false, errors.New("1 not allowed")
	case 2:
		return false, errors.New("2 not allowed")
	default:
		return true, nil
	}
}

Example test

func TestExample(t *testing.T) {
	type args struct {
		input int
	}
	tests := []struct {
		name      string
		args      args
		want      bool
		assertion assert.ErrorAssertionFunc
	}{
		{
			"condition-1",
			args{1},
			true,
			assert.IsError(errors.New("2 not allowed")),
		},
		{
			"condition-2",
			args{2},
			true,
			assert.IsError(errors.New("2 not allowed")),
		},
		{
			"condition-other",
			args{99},
			true,
			assert.NoError,
		},
	}
	for _, tt := range tests {
		t.Run(tt.name, func(t *testing.T) {
			got, err := Example(tt.args.input)
			tt.assertion(t, err)
			assert.Equal(t, tt.want, got)
		})
	}
}

[robdefeo]

Let me know @glesica, @boyan-soubachov, or @mvdkleijn if you have any feedback on this PR. At least one thing I wasn't sure about is the most appropriate name for this function. 😄

[Antonboom]

@robdefeo why is assert.ErrorIs not enough?

[robdefeo]

ErrorIs is useful, I didn't see it as I routinely used v1.6.1 😊 but it solves the problem in a different place. The naming is confusing immediately making the name I chose a poor choice.

Let me explain the intended case again and try to clean up the confusion between the two. Most my tests are written with a wantErr and they checking for that error in the tests case.

If you want to use the assertion assert.ErrorAssertionFunc because it expects a function, you only have the option of assert.NoError and assert.Error. You can check for specific errors with ErrorIs, however you need to call the function which means it can only be done in test body not in the test case definition. Using ErrorIs would mean adding another param to the tests case like wantError which is a reasonable solution but it doesn't fit the pattern of assert.ErrorAssertionFunc. I've started writing tests with assert.ErrorAssertionFunc and I find the tests are clearer to read as the assertion is the the test case definition, there is not way to specify the error through without a function that returns a function, hence this PR.

If accepted, it will need a new name so open to suggests. Thanks

[Antonboom]

I see.

Your tests written in bad pattern: it's better to just check for an error (how was before) than to use its text for comparison. EqualError, ErrorContains are examples of an API that should not be used universally and is only useful in very rare cases.

What your tests should look like in modern Go without changes in testify:

var (
	ErrNotAllowed1 = errors.New("1 not allowed") // can be private `errNotAllowed`
	ErrNotAllowed2 = errors.New("2 not allowed")
)

func Example(input int) (bool, error) {
	switch input {
	case 1:
		return false, ErrNotAllowed1
	case 2:
		return false, fmt.Errorf("do operation: %w", ErrNotAllowed2)
	default:
		return true, nil
	}
}

func TestExample(t *testing.T) {
	type args struct {
		input int
	}
	tests := []struct {
		name     string
		args     args
		want     bool
		expError error
	}{
		{
			"condition-1",
			args{1},
			false,
			ErrNotAllowed1,
		},
		{
			"condition-2",
			args{2},
			false,
			ErrNotAllowed2,
		},
		{
			"condition-other",
			args{99},
			true,
			nil,
		},
	}
	for _, tt := range tests {
		t.Run(tt.name, func(t *testing.T) {
			got, err := Example(tt.args.input)
			require.ErrorIs(t, err, tt.expError)
			assert.Equal(t, tt.want, got)
		})
	}
}

[boyan-soubachov]

I'm with @Antonboom on this one. It's a simpler, more commonly used pattern that achieves the same legibility (maybe even better), IMO.

[dolmen]

I agree with @Antonboom (https://github.com/stretchr/testify/pull/1174#issuecomment-1104725016).

I'm closing this PR as "wont fix".