Standardize validation errors to use InvalidArgument

Since Praefect throws an InvalidArgument error when it sees an empty
target repository, Gitaly should throw an InvalidArgument as well to be
consistent. This commit also modifies some of the tests to bypass the
gRPC server for testing validation, so we can bypass Praefect all
together. Praefect as a proxy will not have the same error string as the
gitaly server when an empty repository is detected since praefect will
return right away beore the request reaches Gitaly.

5 files changed, 13 insertions, 41 deletions

diff --git a/changelogs/unreleased/jc-praefect-test-changes.yml b/changelogs/unreleased/jc-praefect-test-changes.yml
new file mode 100644
index 000000000..f27c2e687
--- /dev/null
+++ b/changelogs/unreleased/jc-praefect-test-changes.yml
@@ -0,0 +1,5 @@
+---
+title: Bypass praefect server in tests that check the error message
+merge_request: 1799
+author:
+type: other
diff --git a/internal/service/commit/filter_shas_with_signatures_test.go b/internal/service/commit/filter_shas_with_signatures_test.go
index f5715f225..877a58f5b 100644
--- a/internal/service/commit/filter_shas_with_signatures_test.go
+++ b/internal/service/commit/filter_shas_with_signatures_test.go
@@ -7,7 +7,6 @@ import (
 	"github.com/stretchr/testify/require"
 	"gitlab.com/gitlab-org/gitaly/internal/testhelper"
 	"gitlab.com/gitlab-org/gitaly/proto/go/gitalypb"
-	"google.golang.org/grpc/codes"
 )
 
 func TestFilterShasWithSignaturesSuccessful(t *testing.T) {
@@ -66,23 +65,7 @@ func TestFilterShasWithSignaturesSuccessful(t *testing.T) {
 }
 
 func TestFilterShasWithSignaturesValidationError(t *testing.T) {
-	ctx, cancel := testhelper.Context()
-	defer cancel()
-
-	server, serverSocketPath := startTestServices(t)
-	defer server.Stop()
-
-	client, conn := newCommitServiceClient(t, serverSocketPath)
-	defer conn.Close()
-
-	stream, err := client.FilterShasWithSignatures(ctx)
-	require.NoError(t, err)
-
-	require.NoError(t, stream.Send(&gitalypb.FilterShasWithSignaturesRequest{}))
-	require.NoError(t, stream.CloseSend())
-
-	_, err = recvFSWS(stream)
-	testhelper.RequireGrpcError(t, err, codes.InvalidArgument)
+	err := validateFirstFilterShasWithSignaturesRequest(&gitalypb.FilterShasWithSignaturesRequest{})
 	require.Contains(t, err.Error(), "no repository given")
 }
 
diff --git a/internal/service/objectpool/fetch_into_object_pool_test.go b/internal/service/objectpool/fetch_into_object_pool_test.go
index c2e409e6b..5f31a4159 100644
--- a/internal/service/objectpool/fetch_into_object_pool_test.go
+++ b/internal/service/objectpool/fetch_into_object_pool_test.go
@@ -116,11 +116,7 @@ func TestFetchIntoObjectPool_CollectLogStatistics(t *testing.T) {
 }
 
 func TestFetchIntoObjectPool_Failure(t *testing.T) {
-	server, serverSocketPath := runObjectPoolServer(t)
-	defer server.Stop()
-
-	client, conn := newObjectPoolClient(t, serverSocketPath)
-	defer conn.Close()
+	server := NewServer()
 
 	ctx, cancel := testhelper.Context()
 	defer cancel()
@@ -169,7 +165,7 @@ func TestFetchIntoObjectPool_Failure(t *testing.T) {
 	}
 	for _, tc := range testCases {
 		t.Run(tc.description, func(t *testing.T) {
-			_, err := client.FetchIntoObjectPool(ctx, tc.request)
+			_, err := server.FetchIntoObjectPool(ctx, tc.request)
 			require.Error(t, err)
 			testhelper.RequireGrpcError(t, err, tc.code)
 			assert.Contains(t, err.Error(), tc.errMsg)
diff --git a/internal/service/repository/fetch_remote_test.go b/internal/service/repository/fetch_remote_test.go
index be309b7c7..56947c5d3 100644
--- a/internal/service/repository/fetch_remote_test.go
+++ b/internal/service/repository/fetch_remote_test.go
@@ -67,10 +67,7 @@ func TestFetchRemoteSuccess(t *testing.T) {
 }
 
 func TestFetchRemoteFailure(t *testing.T) {
-	server, serverSocketPath := runRepoServer(t)
-	defer server.Stop()
-
-	client, _ := newRepositoryClient(t, serverSocketPath)
+	server := NewServer(RubyServer)
 
 	tests := []struct {
 		desc string
@@ -91,7 +88,7 @@ func TestFetchRemoteFailure(t *testing.T) {
 			ctx, cancel := testhelper.Context()
 			defer cancel()
 
-			resp, err := client.FetchRemote(ctx, tc.req)
+			resp, err := server.FetchRemote(ctx, tc.req)
 			testhelper.RequireGrpcError(t, err, tc.code)
 			require.Contains(t, err.Error(), tc.err)
 			assert.Nil(t, resp)
diff --git a/internal/service/repository/search_files_test.go b/internal/service/repository/search_files_test.go
index 2b320aced..528aaa7ed 100644
--- a/internal/service/repository/search_files_test.go
+++ b/internal/service/repository/search_files_test.go
@@ -208,14 +208,7 @@ func TestSearchFilesByContentLargeFile(t *testing.T) {
 }
 
 func TestSearchFilesByContentFailure(t *testing.T) {
-	ctx, cancel := testhelper.Context()
-	defer cancel()
-
-	server, serverSocketPath := runRepoServer(t)
-	defer server.Stop()
-
-	client, conn := newRepositoryClient(t, serverSocketPath)
-	defer conn.Close()
+	server := NewServer(RubyServer)
 
 	testRepo, _, cleanupRepo := testhelper.NewTestRepo(t)
 	defer cleanupRepo()
@@ -258,14 +251,12 @@ func TestSearchFilesByContentFailure(t *testing.T) {
 
 	for _, tc := range testCases {
 		t.Run(tc.desc, func(t *testing.T) {
-			stream, err := client.SearchFilesByContent(ctx, &gitalypb.SearchFilesByContentRequest{
+			err := server.SearchFilesByContent(&gitalypb.SearchFilesByContentRequest{
 				Repository: tc.repo,
 				Query:      tc.query,
 				Ref:        []byte(tc.ref),
-			})
-			require.NoError(t, err)
+			}, nil)
 
-			_, err = consumeFilenameByContentChunked(stream)
 			testhelper.RequireGrpcError(t, err, tc.code)
 			require.Contains(t, err.Error(), tc.msg)
 		})