ssh: Verify we get expected errors instead of only asserting error codes

The tests verifying that request validation works as expected for
SSHUploadPack are currently only asserting the error code. This is
fragile and may easily lead to failures that we assume to be what we
expected, but which are actually caused by something different.

Refactor the test to instead verify the full error to tighten it.

1 files changed, 45 insertions, 28 deletions

diff --git a/internal/gitaly/service/ssh/upload_pack_test.go b/internal/gitaly/service/ssh/upload_pack_test.go
index 6b4363d92..9e3a721d6 100644
--- a/internal/gitaly/service/ssh/upload_pack_test.go
+++ b/internal/gitaly/service/ssh/upload_pack_test.go
@@ -17,6 +17,7 @@ import (
 	"gitlab.com/gitlab-org/gitaly/v15/internal/git"
 	"gitlab.com/gitlab-org/gitaly/v15/internal/git/gittest"
 	"gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/config"
+	"gitlab.com/gitlab-org/gitaly/v15/internal/helper"
 	"gitlab.com/gitlab-org/gitaly/v15/internal/helper/text"
 	"gitlab.com/gitlab-org/gitaly/v15/internal/metadata/featureflag"
 	"gitlab.com/gitlab-org/gitaly/v15/internal/testhelper"
@@ -156,49 +157,65 @@ func TestFailedUploadPackRequestDueToValidationError(t *testing.T) {
 	client, conn := newSSHClient(t, serverSocketPath)
 	defer conn.Close()
 
-	tests := []struct {
-		Desc string
-		Req  *gitalypb.SSHUploadPackRequest
-		Code codes.Code
+	for _, tc := range []struct {
+		desc        string
+		request     *gitalypb.SSHUploadPackRequest
+		expectedErr error
 	}{
 		{
-			Desc: "Repository.RelativePath is empty",
-			Req:  &gitalypb.SSHUploadPackRequest{Repository: &gitalypb.Repository{StorageName: cfg.Storages[0].Name, RelativePath: ""}},
-			Code: codes.InvalidArgument,
+			desc: "missing relative path",
+			request: &gitalypb.SSHUploadPackRequest{
+				Repository: &gitalypb.Repository{
+					StorageName:  cfg.Storages[0].Name,
+					RelativePath: "",
+				},
+			},
+			expectedErr: func() error {
+				if testhelper.IsPraefectEnabled() {
+					return helper.ErrInvalidArgumentf("repo scoped: invalid Repository")
+				}
+				return helper.ErrInvalidArgumentf("GetPath: relative path missing from storage_name:%q", "default")
+			}(),
 		},
 		{
-			Desc: "Repository is nil",
-			Req:  &gitalypb.SSHUploadPackRequest{Repository: nil},
-			Code: codes.InvalidArgument,
+			desc: "missing repository",
+			request: &gitalypb.SSHUploadPackRequest{
+				Repository: nil,
+			},
+			expectedErr: func() error {
+				if testhelper.IsPraefectEnabled() {
+					return helper.ErrInvalidArgumentf("repo scoped: empty Repository")
+				}
+				return helper.ErrInvalidArgumentf("GetStorageByName: no such storage: \"\"")
+			}(),
 		},
 		{
-			Desc: "Data exists on first request",
-			Req:  &gitalypb.SSHUploadPackRequest{Repository: &gitalypb.Repository{StorageName: cfg.Storages[0].Name, RelativePath: "path/to/repo"}, Stdin: []byte("Fail")},
-			Code: func() codes.Code {
+			desc: "data in first request",
+			request: &gitalypb.SSHUploadPackRequest{
+				Repository: &gitalypb.Repository{
+					StorageName:  cfg.Storages[0].Name,
+					RelativePath: "path/to/repo",
+				},
+				Stdin: []byte("Fail"),
+			},
+			expectedErr: func() error {
 				if testhelper.IsPraefectEnabled() {
-					return codes.NotFound
+					return helper.ErrNotFoundf("accessor call: route repository accessor: consistent storages: repository %q/%q not found", cfg.Storages[0].Name, "path/to/repo")
 				}
-
-				return codes.InvalidArgument
+				return helper.ErrInvalidArgumentf("non-empty stdin in first request")
 			}(),
 		},
-	}
-
-	for _, test := range tests {
-		t.Run(test.Desc, func(t *testing.T) {
+	} {
+		t.Run(tc.desc, func(t *testing.T) {
 			ctx := testhelper.Context(t)
-			stream, err := client.SSHUploadPack(ctx)
-			if err != nil {
-				t.Fatal(err)
-			}
 
-			if err = stream.Send(test.Req); err != nil {
-				t.Fatal(err)
-			}
+			stream, err := client.SSHUploadPack(ctx)
+			require.NoError(t, err)
+			require.NoError(t, stream.Send(tc.request))
 			require.NoError(t, stream.CloseSend())
 
 			err = testPostUploadPackFailedResponse(t, stream)
-			testhelper.RequireGrpcCode(t, err, test.Code)
+			testhelper.RequireGrpcError(t, tc.expectedErr, err)
 		})
 	}
 }