Error helper tests: refactor recently added tests

Refactor the tests added in the last commit to duplicate fewer test
conditions. We also made the hard assumption that all the
ErrorWhateverf functions did not wrap errors.

Let's add an otherwise unused ErrNotFound() which doesn't have the bug
introduced in 777a12cfd (wrap errors with grpc context, 2020-05-07)
purely to stress the tests themselves in this scenario.

Since I don't like this whole package and intend these test changes as
leading changes to try to get rid of it let's not add it to the main
package and encourage its use, but rather just to the tests. Note that
it uses DecorateError() unlike the non-format functions.

1 files changed, 38 insertions, 9 deletions

diff --git a/internal/helper/errors_test.go b/internal/helper/errors_test.go
index b43bde0e5..ea4ba1d6b 100644
--- a/internal/helper/errors_test.go
+++ b/internal/helper/errors_test.go
@@ -10,12 +10,18 @@ import (
 	"google.golang.org/grpc/status"
 )
 
+// MockErrNotFoundf wraps error with codes.NotFound, unless err is already a grpc error
+func MockErrNotFoundf(format string, a ...interface{}) error {
+	return DecorateError(codes.NotFound, fmt.Errorf(format, a...))
+}
+
 func TestError(t *testing.T) {
 	errorFormat := "expected %s"
 	errorMessage := "sentinel error"
 	input := errors.New(errorMessage)
-	inputGRPC := status.Error(codes.Unauthenticated, errorMessage)
-	inputGRPCFmt := status.Errorf(codes.Unauthenticated, errorFormat, errorMessage)
+	inputGRPCCode := codes.Unauthenticated
+	inputGRPC := status.Error(inputGRPCCode, errorMessage)
+	inputGRPCFmt := status.Errorf(inputGRPCCode, errorFormat, errorMessage)
 
 	for _, tc := range []struct {
 		desc      string
@@ -77,31 +83,54 @@ func TestError(t *testing.T) {
 			format:    true,
 			wrapped:   false,
 		},
+		{
+			desc:      "MockErrNotFoundf",
+			functionf: MockErrNotFoundf,
+			code:      codes.NotFound,
+			format:    true,
+			wrapped:   true,
+		},
 	} {
 		t.Run(tc.desc, func(t *testing.T) {
+			// assert: tc.code and our test code must not
+			// clash.
+			require.NotEqual(t, tc.code, inputGRPCCode)
+
 			var err error
+			errorMessageFormatted := errorMessage
 			if tc.format {
+				errorMessageFormatted = fmt.Sprintf(errorFormat, errorMessage)
 				err = tc.functionf(errorFormat, input)
-				require.EqualError(t, err, fmt.Sprintf(errorFormat, errorMessage))
-				require.False(t, errors.Is(err, input))
 			} else {
 				err = tc.function(input)
-				require.EqualError(t, err, errorMessage)
-				require.True(t, errors.Is(err, input))
+				require.True(t, tc.wrapped)
+			}
+			require.EqualError(t, err, errorMessageFormatted)
+			if tc.wrapped {
+				require.False(t, errors.Is(err, inputGRPC))
+			} else {
+				require.Equal(t, errors.Is(err, input), tc.wrapped)
 			}
 			require.Equal(t, tc.code, status.Code(err))
 
 			// Does an existing GRPC's error's code get
 			// preserved?
+			expectedCode := inputGRPCCode
 			if tc.format {
 				err = tc.functionf(errorFormat, inputGRPCFmt)
-				require.Equal(t, tc.code, status.Code(err))
+				expectedCode = tc.code
 			} else {
 				err = tc.function(inputGRPC)
 				require.True(t, errors.Is(err, inputGRPC))
-				require.NotEqual(t, tc.code, status.Code(inputGRPC))
-				require.Equal(t, status.Code(inputGRPC), status.Code(err))
+				require.True(t, tc.wrapped)
+			}
+			if tc.wrapped {
+				require.NotEqual(t, errors.Is(err, input), tc.wrapped)
+			} else {
+				require.Equal(t, errors.Is(err, input), tc.wrapped)
 			}
+			require.Equal(t, expectedCode, status.Code(err))
+			require.NotEqual(t, tc.code, status.Code(inputGRPC))
 		})
 	}
 }