repository: Extend test coverage for PackRefs RPC

Make sure NotFound code and specific message is returned
in case requested repository doesn't exist on the disk.
To support it existing implementation was changed by
removing error wrapping that was not that useful anyway.

3 files changed, 55 insertions, 4 deletions

diff --git a/internal/gitaly/service/ref/pack_refs.go b/internal/gitaly/service/ref/pack_refs.go
index 3d09b8739..0b9ae588c 100644
--- a/internal/gitaly/service/ref/pack_refs.go
+++ b/internal/gitaly/service/ref/pack_refs.go
@@ -2,9 +2,8 @@ package ref
 
 import (
 	"context"
-	"errors"
-	"fmt"
 
+	gitalyerrors "gitlab.com/gitlab-org/gitaly/v14/internal/errors"
 	"gitlab.com/gitlab-org/gitaly/v14/internal/git"
 	"gitlab.com/gitlab-org/gitaly/v14/internal/git/repository"
 	"gitlab.com/gitlab-org/gitaly/v14/internal/helper"
@@ -25,7 +24,7 @@ func (s *server) PackRefs(ctx context.Context, in *gitalypb.PackRefsRequest) (*g
 
 func validatePackRefsRequest(in *gitalypb.PackRefsRequest) error {
 	if in.GetRepository() == nil {
-		return errors.New("empty repository")
+		return gitalyerrors.ErrEmptyRepository
 	}
 	return nil
 }
@@ -36,7 +35,7 @@ func (s *server) packRefs(ctx context.Context, repository repository.GitRepo) er
 		Flags: []git.Option{git.Flag{Name: "--all"}},
 	})
 	if err != nil {
-		return fmt.Errorf("initializing pack-refs command: %v", err)
+		return err
 	}
 
 	return cmd.Wait()
diff --git a/internal/gitaly/service/ref/pack_refs_test.go b/internal/gitaly/service/ref/pack_refs_test.go
index 8a7141d42..f16aa8175 100644
--- a/internal/gitaly/service/ref/pack_refs_test.go
+++ b/internal/gitaly/service/ref/pack_refs_test.go
@@ -15,6 +15,8 @@ import (
 	"gitlab.com/gitlab-org/gitaly/v14/internal/git/localrepo"
 	"gitlab.com/gitlab-org/gitaly/v14/internal/testhelper"
 	"gitlab.com/gitlab-org/gitaly/v14/proto/go/gitalypb"
+	"google.golang.org/grpc/codes"
+	"google.golang.org/grpc/status"
 )
 
 func TestPackRefsSuccessfulRequest(t *testing.T) {
@@ -62,3 +64,46 @@ func linesInPackfile(t *testing.T, repoPath string) int {
 	}
 	return refs
 }
+
+func TestPackRefs_invalidRequest(t *testing.T) {
+	t.Parallel()
+
+	cfg, client := setupRefServiceWithoutRepo(t)
+
+	tests := []struct {
+		repo *gitalypb.Repository
+		err  error
+		desc string
+	}{
+		{
+			desc: "nil repo",
+			repo: nil,
+			err:  status.Error(codes.InvalidArgument, gitalyOrPraefect("empty Repository", "repo scoped: empty Repository")),
+		},
+		{
+			desc: "invalid storage name",
+			repo: &gitalypb.Repository{StorageName: "foo"},
+			err:  status.Error(codes.InvalidArgument, gitalyOrPraefect(`GetStorageByName: no such storage: "foo"`, "repo scoped: invalid Repository")),
+		},
+		{
+			desc: "non-existing repo",
+			repo: &gitalypb.Repository{StorageName: cfg.Storages[0].Name, RelativePath: "bar"},
+			err: status.Error(
+				codes.NotFound,
+				gitalyOrPraefect(
+					fmt.Sprintf(`GetRepoPath: not a git repository: "%s/bar"`, cfg.Storages[0].Path),
+					`mutator call: route repository mutator: get repository id: repository "default"/"bar" not found`,
+				),
+			),
+		},
+	}
+
+	for _, tc := range tests {
+		t.Run(tc.desc, func(t *testing.T) {
+			ctx := testhelper.Context(t)
+			//nolint:staticcheck
+			_, err := client.PackRefs(ctx, &gitalypb.PackRefsRequest{Repository: tc.repo})
+			testhelper.RequireGrpcError(t, err, tc.err)
+		})
+	}
+}
diff --git a/internal/gitaly/service/ref/testhelper_test.go b/internal/gitaly/service/ref/testhelper_test.go
index 4231b9497..b21d5a61d 100644
--- a/internal/gitaly/service/ref/testhelper_test.go
+++ b/internal/gitaly/service/ref/testhelper_test.go
@@ -133,3 +133,10 @@ func assertContainsBranch(t *testing.T, branches []*gitalypb.FindAllBranchesResp
 
 	t.Errorf("Expected to find branch %q in branches %s", branch.Name, branchNames)
 }
+
+func gitalyOrPraefect(gitalyMsg, praefectMsg string) string {
+	if testhelper.IsPraefectEnabled() {
+		return praefectMsg
+	}
+	return gitalyMsg
+}