From a4a8b24b817429cb74656feb57ceabdd472f37d9 Mon Sep 17 00:00:00 2001 From: John Cai Date: Sat, 1 Feb 2020 22:44:07 -0800 Subject: 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. --- .../commit/filter_shas_with_signatures_test.go | 19 +------------------ .../service/objectpool/fetch_into_object_pool_test.go | 8 ++------ internal/service/repository/fetch_remote_test.go | 7 ++----- internal/service/repository/search_files_test.go | 15 +++------------ 4 files changed, 8 insertions(+), 41 deletions(-) (limited to 'internal') 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) }) -- cgit v1.2.3