Tests: only match error strings we create

15 files changed, 82 insertions, 67 deletions

diff --git a/changelogs/unreleased/grpc-error-dont-match-text-2.yml b/changelogs/unreleased/grpc-error-dont-match-text-2.yml
new file mode 100644
index 000000000..982f6a640
--- /dev/null
+++ b/changelogs/unreleased/grpc-error-dont-match-text-2.yml
@@ -0,0 +1,5 @@
+---
+title: 'Tests: only match error strings we create'
+merge_request: 743
+author:
+type: other
diff --git a/internal/service/blob/lfs_pointers_test.go b/internal/service/blob/lfs_pointers_test.go
index cd76e2d2c..c873edc0c 100644
--- a/internal/service/blob/lfs_pointers_test.go
+++ b/internal/service/blob/lfs_pointers_test.go
@@ -304,7 +304,8 @@ func TestFailedGetNewLFSPointersRequestDueToValidations(t *testing.T) {
 			require.NoError(t, err)
 
 			err = drainNewPointers(c)
-			testhelper.AssertGrpcError(t, err, codes.InvalidArgument, tc.desc)
+			testhelper.AssertGrpcError(t, err, codes.InvalidArgument, "")
+			require.Contains(t, err.Error(), tc.desc)
 		})
 	}
 }
@@ -420,7 +421,8 @@ func TestFailedGetAllLFSPointersRequestDueToValidations(t *testing.T) {
 			require.NoError(t, err)
 
 			err = drainAllPointers(c)
-			testhelper.AssertGrpcError(t, err, codes.InvalidArgument, tc.desc)
+			testhelper.AssertGrpcError(t, err, codes.InvalidArgument, "")
+			require.Contains(t, err.Error(), tc.desc)
 		})
 	}
 }
diff --git a/internal/service/commit/filter_shas_with_signatures_test.go b/internal/service/commit/filter_shas_with_signatures_test.go
index 1e57d7092..c400cc99d 100644
--- a/internal/service/commit/filter_shas_with_signatures_test.go
+++ b/internal/service/commit/filter_shas_with_signatures_test.go
@@ -82,7 +82,8 @@ func TestFilterShasWithSignaturesValidationError(t *testing.T) {
 	require.NoError(t, stream.CloseSend())
 
 	_, err = recvFSWS(stream)
-	testhelper.AssertGrpcError(t, err, codes.InvalidArgument, "no repo")
+	testhelper.AssertGrpcError(t, err, codes.InvalidArgument, "")
+	require.Contains(t, err.Error(), "no repository given")
 }
 
 func recvFSWS(stream pb.CommitService_FilterShasWithSignaturesClient) ([][]byte, error) {
diff --git a/internal/service/operations/commit_files_test.go b/internal/service/operations/commit_files_test.go
index ed6919fe7..41143a7d1 100644
--- a/internal/service/operations/commit_files_test.go
+++ b/internal/service/operations/commit_files_test.go
@@ -232,7 +232,8 @@ func TestFailedUserCommitFilesRequest(t *testing.T) {
 			require.NoError(t, stream.Send(tc.req))
 
 			_, err = stream.CloseAndRecv()
-			testhelper.AssertGrpcError(t, err, codes.InvalidArgument, tc.desc)
+			testhelper.AssertGrpcError(t, err, codes.InvalidArgument, "")
+			require.Contains(t, err.Error(), tc.desc)
 		})
 	}
 }
diff --git a/internal/service/operations/squash_test.go b/internal/service/operations/squash_test.go
index f256cdb6a..88e2fdb07 100644
--- a/internal/service/operations/squash_test.go
+++ b/internal/service/operations/squash_test.go
@@ -229,7 +229,8 @@ func TestFailedUserSquashRequestDueToValidations(t *testing.T) {
 			defer cancel()
 
 			_, err := client.UserSquash(ctx, testCase.request)
-			testhelper.AssertGrpcError(t, err, testCase.code, testCase.desc)
+			testhelper.AssertGrpcError(t, err, testCase.code, "")
+			require.Contains(t, err.Error(), testCase.desc)
 		})
 	}
 }
diff --git a/internal/service/remote/fetch_internal_remote_test.go b/internal/service/remote/fetch_internal_remote_test.go
index fbcd706e0..f50b76dca 100644
--- a/internal/service/remote/fetch_internal_remote_test.go
+++ b/internal/service/remote/fetch_internal_remote_test.go
@@ -110,7 +110,8 @@ func TestFailedFetchInternalRemoteDueToValidations(t *testing.T) {
 		t.Run(tc.desc, func(t *testing.T) {
 			_, err := client.FetchInternalRemote(ctx, tc.request)
 
-			testhelper.AssertGrpcError(t, err, codes.InvalidArgument, tc.desc)
+			testhelper.AssertGrpcError(t, err, codes.InvalidArgument, "")
+			require.Contains(t, err.Error(), tc.desc)
 		})
 	}
 }
diff --git a/internal/service/remote/update_remote_mirror_test.go b/internal/service/remote/update_remote_mirror_test.go
index 07ed5eee4..908787ca9 100644
--- a/internal/service/remote/update_remote_mirror_test.go
+++ b/internal/service/remote/update_remote_mirror_test.go
@@ -119,7 +119,8 @@ func TestFailedUpdateRemoteMirrorRequestDueToValidation(t *testing.T) {
 			require.NoError(t, stream.Send(tc.request))
 
 			_, err = stream.CloseAndRecv()
-			testhelper.AssertGrpcError(t, err, codes.InvalidArgument, tc.desc)
+			testhelper.AssertGrpcError(t, err, codes.InvalidArgument, "")
+			require.Contains(t, err.Error(), tc.desc)
 		})
 	}
 }
diff --git a/internal/service/repository/calculate_checksum_test.go b/internal/service/repository/calculate_checksum_test.go
index a5c2f029a..c17d07df0 100644
--- a/internal/service/repository/calculate_checksum_test.go
+++ b/internal/service/repository/calculate_checksum_test.go
@@ -75,7 +75,7 @@ func TestBrokenRepositoryCalculateChecksum(t *testing.T) {
 	defer cancelCtx()
 
 	_, err := client.CalculateChecksum(testCtx, request)
-	testhelper.AssertGrpcError(t, err, codes.DataLoss, "not a git repository")
+	testhelper.AssertGrpcError(t, err, codes.DataLoss, "")
 }
 
 func TestFailedCalculateChecksum(t *testing.T) {
diff --git a/internal/service/repository/create_from_snapshot_test.go b/internal/service/repository/create_from_snapshot_test.go
index c44630810..d5b97faf7 100644
--- a/internal/service/repository/create_from_snapshot_test.go
+++ b/internal/service/repository/create_from_snapshot_test.go
@@ -118,7 +118,8 @@ func TestCreateRepositoryFromSnapshotFailsIfRepositoryExists(t *testing.T) {
 
 	req := &pb.CreateRepositoryFromSnapshotRequest{Repository: testRepo}
 	rsp, err := createFromSnapshot(t, req)
-	testhelper.AssertGrpcError(t, err, codes.InvalidArgument, "destination directory exists")
+	testhelper.AssertGrpcError(t, err, codes.InvalidArgument, "")
+	require.Contains(t, err.Error(), "destination directory exists")
 	require.Nil(t, rsp)
 }
 
@@ -132,65 +133,63 @@ func TestCreateRepositoryFromSnapshotFailsIfBadURL(t *testing.T) {
 	}
 
 	rsp, err := createFromSnapshot(t, req)
-	testhelper.AssertGrpcError(t, err, codes.InvalidArgument, "Bad HTTP URL")
+	testhelper.AssertGrpcError(t, err, codes.InvalidArgument, "")
+	require.Contains(t, err.Error(), "Bad HTTP URL")
 	require.Nil(t, rsp)
 }
 
-func TestCreateRepositoryFromSnapshotFailsIfBadAuth(t *testing.T) {
+func TestCreateRepositoryFromSnapshotBadRequests(t *testing.T) {
 	testRepo, _, cleanupFn := testhelper.NewTestRepo(t)
 	cleanupFn() // free up the destination dir for use
 
-	// Create a HTTP server that serves a given tar file
-	srv := httptest.NewServer(&testhandler{})
-	defer srv.Close()
-
-	req := &pb.CreateRepositoryFromSnapshotRequest{
-		Repository: testRepo,
-		HttpUrl:    srv.URL + tarPath,
-		HttpAuth:   "Bad authentication",
+	testCases := []struct {
+		desc        string
+		url         string
+		auth        string
+		code        codes.Code
+		errContains string
+	}{
+		{
+			desc:        "http bad auth",
+			url:         tarPath,
+			auth:        "Bad authentication",
+			code:        codes.Internal,
+			errContains: "HTTP server: 401 ",
+		},
+		{
+			desc:        "http not found",
+			url:         tarPath + ".does-not-exist",
+			auth:        secret,
+			code:        codes.Internal,
+			errContains: "HTTP server: 404 ",
+		},
+		{
+			desc:        "http do not follow redirects",
+			url:         redirectPath,
+			auth:        secret,
+			code:        codes.Internal,
+			errContains: "HTTP server: 302 ",
+		},
 	}
 
-	rsp, err := createFromSnapshot(t, req)
-	testhelper.AssertGrpcError(t, err, codes.Internal, "HTTP server: 401 Unauthorized")
-	require.Nil(t, rsp)
-}
-
-func TestCreateRepositoryFromSnapshotFailsIfHttp404(t *testing.T) {
-	testRepo, _, cleanupFn := testhelper.NewTestRepo(t)
-	cleanupFn() // free up the destination dir for use
-
-	// Create a HTTP server that serves a given tar file
 	srv := httptest.NewServer(&testhandler{})
 	defer srv.Close()
 
-	req := &pb.CreateRepositoryFromSnapshotRequest{
-		Repository: testRepo,
-		HttpUrl:    srv.URL + tarPath + ".does-not-exist",
-		HttpAuth:   secret,
-	}
+	for _, tc := range testCases {
+		t.Run(tc.desc, func(t *testing.T) {
+			req := &pb.CreateRepositoryFromSnapshotRequest{
+				Repository: testRepo,
+				HttpUrl:    srv.URL + tc.url,
+				HttpAuth:   tc.auth,
+			}
 
-	rsp, err := createFromSnapshot(t, req)
-	testhelper.AssertGrpcError(t, err, codes.Internal, "HTTP server: 404 Not Found")
-	require.Nil(t, rsp)
-}
+			rsp, err := createFromSnapshot(t, req)
+			testhelper.AssertGrpcError(t, err, tc.code, "")
+			require.Nil(t, rsp)
 
-func TestCreateRepositoryFromSnapshotDoesNotFollowRedirects(t *testing.T) {
-	testRepo, _, cleanupFn := testhelper.NewTestRepo(t)
-	cleanupFn() // free up the destination dir for use
-
-	// Create a HTTP server that serves a given tar file
-	srv := httptest.NewServer(&testhandler{})
-	defer srv.Close()
-
-	req := &pb.CreateRepositoryFromSnapshotRequest{
-		Repository: testRepo,
-		HttpUrl:    srv.URL + redirectPath,
-		HttpAuth:   secret,
+			require.Contains(t, err.Error(), tc.errContains)
+		})
 	}
-
-	rsp, err := createFromSnapshot(t, req)
-	testhelper.AssertGrpcError(t, err, codes.Internal, "HTTP server: 302 Found")
-	require.Nil(t, rsp)
 }
 
 func TestCreateRepositoryFromSnapshotHandlesMalformedResponse(t *testing.T) {
diff --git a/internal/service/repository/fetch_remote_test.go b/internal/service/repository/fetch_remote_test.go
index 8f18d4af3..f1306ae21 100644
--- a/internal/service/repository/fetch_remote_test.go
+++ b/internal/service/repository/fetch_remote_test.go
@@ -88,8 +88,8 @@ func TestFetchRemoteFailure(t *testing.T) {
 			defer cancel()
 
 			resp, err := client.FetchRemote(ctx, tc.req)
-			testhelper.AssertGrpcError(t, err, tc.code, tc.err)
-			assert.Error(t, err)
+			testhelper.AssertGrpcError(t, err, tc.code, "")
+			require.Contains(t, err.Error(), tc.err)
 			assert.Nil(t, resp)
 		})
 	}
diff --git a/internal/service/repository/gc_test.go b/internal/service/repository/gc_test.go
index 951f983c4..d305f59bf 100644
--- a/internal/service/repository/gc_test.go
+++ b/internal/service/repository/gc_test.go
@@ -13,6 +13,7 @@ import (
 	"google.golang.org/grpc/codes"
 
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 	"gitlab.com/gitlab-org/gitaly/internal/testhelper"
 
 	pb "gitlab.com/gitlab-org/gitaly-proto/go"
@@ -116,7 +117,8 @@ func TestGarbageCollectDeletesRefsLocks(t *testing.T) {
 	createFileWithTimes(deleteLockPath, oldTime)
 
 	c, err := client.GarbageCollect(ctx, req)
-	testhelper.AssertGrpcError(t, err, codes.Internal, "GarbageCollect: cmd wait")
+	testhelper.AssertGrpcError(t, err, codes.Internal, "")
+	require.Contains(t, err.Error(), "GarbageCollect: cmd wait")
 	assert.Nil(t, c)
 
 	// Sanity checks
diff --git a/internal/service/repository/search_files_test.go b/internal/service/repository/search_files_test.go
index 2679e4715..52010a7d8 100644
--- a/internal/service/repository/search_files_test.go
+++ b/internal/service/repository/search_files_test.go
@@ -181,7 +181,8 @@ func TestSearchFilesByContentFailure(t *testing.T) {
 			require.NoError(t, err)
 
 			_, err = consumeFilenameByContent(stream)
-			testhelper.AssertGrpcError(t, err, tc.code, tc.msg)
+			testhelper.AssertGrpcError(t, err, tc.code, "")
+			require.Contains(t, err.Error(), tc.msg)
 		})
 	}
 }
@@ -285,7 +286,8 @@ func TestSearchFilesByNameFailure(t *testing.T) {
 			require.NoError(t, err)
 
 			_, err = consumeFilenameByName(stream)
-			testhelper.AssertGrpcError(t, err, tc.code, tc.msg)
+			testhelper.AssertGrpcError(t, err, tc.code, "")
+			require.Contains(t, err.Error(), tc.msg)
 		})
 	}
 }
diff --git a/internal/service/repository/snapshot_test.go b/internal/service/repository/snapshot_test.go
index ce0f4e926..309e301ec 100644
--- a/internal/service/repository/snapshot_test.go
+++ b/internal/service/repository/snapshot_test.go
@@ -91,7 +91,7 @@ func TestGetSnapshotFailsIfRepositoryMissing(t *testing.T) {
 
 	req := &pb.GetSnapshotRequest{Repository: testRepo}
 	data, err := getSnapshot(t, req)
-	testhelper.AssertGrpcError(t, err, codes.NotFound, "not a git repository")
+	testhelper.AssertGrpcError(t, err, codes.NotFound, "")
 	require.Empty(t, data)
 }
 
@@ -106,7 +106,8 @@ func TestGetSnapshotFailsIfRepositoryContainsSymlink(t *testing.T) {
 
 	req := &pb.GetSnapshotRequest{Repository: testRepo}
 	data, err := getSnapshot(t, req)
-	testhelper.AssertGrpcError(t, err, codes.Internal, "Building snapshot failed")
+	testhelper.AssertGrpcError(t, err, codes.Internal, "")
+	require.Contains(t, err.Error(), "Building snapshot failed")
 
 	// At least some of the tar file should have been written so far
 	require.NotEmpty(t, data)
diff --git a/internal/service/smarthttp/inforefs_test.go b/internal/service/smarthttp/inforefs_test.go
index dbb65e2dc..899d3dfaa 100644
--- a/internal/service/smarthttp/inforefs_test.go
+++ b/internal/service/smarthttp/inforefs_test.go
@@ -120,7 +120,7 @@ func TestFailureRepoNotFoundInfoRefsReceivePack(t *testing.T) {
 	for err == nil {
 		_, err = c.Recv()
 	}
-	testhelper.AssertGrpcError(t, err, codes.NotFound, "not a git repository")
+	testhelper.AssertGrpcError(t, err, codes.NotFound, "")
 }
 
 func TestFailureRepoNotSetInfoRefsReceivePack(t *testing.T) {
diff --git a/internal/testhelper/testhelper.go b/internal/testhelper/testhelper.go
index 892c11b78..d856fada2 100644
--- a/internal/testhelper/testhelper.go
+++ b/internal/testhelper/testhelper.go
@@ -124,9 +124,8 @@ func TestRepository() *pb.Repository {
 	return repo
 }
 
-// AssertGrpcError asserts the passed err is of the same code as expectedCode. Optionally, it can
-// assert the error contains the text of containsText if the latter is not an empty string.
-func AssertGrpcError(t *testing.T, err error, expectedCode codes.Code, containsText string) {
+// AssertGrpcError asserts the passed err is of the same code as expectedCode.
+func AssertGrpcError(t *testing.T, err error, expectedCode codes.Code, _deprecated string) {
 	if err == nil {
 		t.Fatal("Expected an error, got nil")
 	}
@@ -137,8 +136,8 @@ func AssertGrpcError(t *testing.T, err error, expectedCode codes.Code, containsT
 		t.Fatalf("Expected an error with code %v, got %v. The error was %v", expectedCode, code, err)
 	}
 
-	if containsText != "" && !strings.Contains(err.Error(), containsText) {
-		t.Fatalf("Expected an error message containing %v, got %v", containsText, err.Error())
+	if _deprecated != "" && !strings.Contains(err.Error(), _deprecated) {
+		t.Fatalf("Expected an error message containing %v, got %v", _deprecated, err.Error())
 	}
 }