Merge branch 'gitaly-test-refactor' into 'master'

Refactor ad-hoc protobuf marshaling in gitaly test server

See merge request gitlab-org/gitlab-workhorse!287

gitaly_test.go

@@ -87,14 +87,26 @@ func TestGetInfoRefsProxiedToGitalySuccessfully(t *testing.T) {
 
 			gitProtocol := "fake git protocol"
 			resource := "/gitlab-org/gitlab-test.git/info/refs?service=" + tc.gitRpc
-			_, body := httpGet(t, ws.URL+resource, map[string]string{"Git-Protocol": gitProtocol})
+			resp, body := httpGet(t, ws.URL+resource, map[string]string{"Git-Protocol": gitProtocol})
 
-			expectedContent := fmt.Sprintf("\n\000%s\000%s\000%s\000", gitProtocol, tc.gitRpc, testhelper.GitalyInfoRefsResponseMock)
+			require.Equal(t, 200, resp.StatusCode)
+
+			bodySplit := strings.SplitN(body, "\000", 3)
+			require.Len(t, bodySplit, 3)
+
+			gitalyRequest := &pb.InfoRefsRequest{}
+			require.NoError(t, jsonpb.UnmarshalString(bodySplit[0], gitalyRequest))
+
+			require.Equal(t, gitProtocol, gitalyRequest.GitProtocol)
 			if tc.showAllRefs {
-				expectedContent = git.GitConfigShowAllRefs + expectedContent
+				require.Equal(t, []string{git.GitConfigShowAllRefs}, gitalyRequest.GitConfigOptions)
+			} else {
+				require.Empty(t, gitalyRequest.GitConfigOptions)
 			}
 
-			require.Equal(t, expectedContent, body, "GET %q: response body", resource)
+			require.Equal(t, tc.gitRpc, bodySplit[1])
+
+			require.Equal(t, string(testhelper.GitalyInfoRefsResponseMock), bodySplit[2], "GET %q: response body", resource)
 		})
 	}
 }
@@ -248,23 +260,26 @@ func TestPostUploadPackProxiedToGitalySuccessfully(t *testing.T) {
 				testhelper.GitalyUploadPackResponseMock,
 			)
 
-			expectedBodyParts := []string{
-				apiResponse.Repository.StorageName,
-				apiResponse.Repository.RelativePath,
-				gitProtocol,
-			}
+			require.Equal(t, 200, resp.StatusCode, "POST %q", resource)
+			testhelper.AssertResponseHeader(t, resp, "Content-Type", "application/x-git-upload-pack-result")
+
+			bodySplit := strings.SplitN(body, "\000", 2)
+			require.Len(t, bodySplit, 2)
+
+			gitalyRequest := &pb.PostUploadPackRequest{}
+			require.NoError(t, jsonpb.UnmarshalString(bodySplit[0], gitalyRequest))
+
+			require.Equal(t, apiResponse.Repository.StorageName, gitalyRequest.Repository.StorageName)
+			require.Equal(t, apiResponse.Repository.RelativePath, gitalyRequest.Repository.RelativePath)
+			require.Equal(t, gitProtocol, gitalyRequest.GitProtocol)
+
 			if tc.showAllRefs {
-				expectedBodyParts = append(expectedBodyParts, git.GitConfigShowAllRefs+"\n")
+				require.Equal(t, []string{git.GitConfigShowAllRefs}, gitalyRequest.GitConfigOptions)
 			} else {
-				expectedBodyParts = append(expectedBodyParts, "\n")
+				require.Empty(t, gitalyRequest.GitConfigOptions)
 			}
 
-			expectedBodyParts = append(expectedBodyParts, string(testhelper.GitalyUploadPackResponseMock))
-			expectedBody := strings.Join(expectedBodyParts, "\000")
-
-			assert.Equal(t, 200, resp.StatusCode, "POST %q", resource)
-			assert.Equal(t, expectedBody, body, "POST %q: response body", resource)
-			testhelper.AssertResponseHeader(t, resp, "Content-Type", "application/x-git-upload-pack-result")
+			require.Equal(t, string(testhelper.GitalyUploadPackResponseMock), bodySplit[1], "POST %q: response body", resource)
 		})
 	}
 }

internal/testhelper/gitaly.go

@@ -8,6 +8,7 @@ import (
 	"strings"
 	"sync"
 
+	"github.com/golang/protobuf/jsonpb"
 	log "github.com/sirupsen/logrus"
 
 	pb "gitlab.com/gitlab-org/gitaly-proto/go"
@@ -59,12 +60,17 @@ func (s *GitalyTestServer) InfoRefsUploadPack(in *pb.InfoRefsRequest, stream pb.
 
 	fmt.Printf("Result: %+v\n", in)
 
+	marshaler := &jsonpb.Marshaler{}
+	jsonString, err := marshaler.MarshalToString(in)
+	if err != nil {
+		return err
+	}
+
 	data := []byte(strings.Join([]string{
-		strings.Join(in.GitConfigOptions, "\n") + "\n",
-		in.GitProtocol,
+		jsonString,
 		"git-upload-pack",
 		GitalyInfoRefsResponseMock,
-	}, "\000") + "\000")
+	}, "\000"))
 
 	nSends, err := sendBytes(data, 100, func(p []byte) error {
 		return stream.Send(&pb.InfoRefsResponse{Data: p})
@@ -89,12 +95,17 @@ func (s *GitalyTestServer) InfoRefsReceivePack(in *pb.InfoRefsRequest, stream pb
 
 	fmt.Printf("Result: %+v\n", in)
 
+	marshaler := &jsonpb.Marshaler{}
+	jsonString, err := marshaler.MarshalToString(in)
+	if err != nil {
+		return err
+	}
+
 	data := []byte(strings.Join([]string{
-		strings.Join(in.GitConfigOptions, "\n") + "\n",
-		in.GitProtocol,
+		jsonString,
 		"git-receive-pack",
 		GitalyInfoRefsResponseMock,
-	}, "\000") + "\000")
+	}, "\000"))
 
 	nSends, err := sendBytes(data, 100, func(p []byte) error {
 		return stream.Send(&pb.InfoRefsResponse{Data: p})
@@ -165,16 +176,18 @@ func (s *GitalyTestServer) PostUploadPack(stream pb.SmartHTTPService_PostUploadP
 		return err
 	}
 
-	repo := req.GetRepository()
 	if err := validateRepository(req.GetRepository()); err != nil {
 		return err
 	}
 
+	marshaler := &jsonpb.Marshaler{}
+	jsonString, err := marshaler.MarshalToString(req)
+	if err != nil {
+		return err
+	}
+
 	data := []byte(strings.Join([]string{
-		repo.GetStorageName(),
-		repo.GetRelativePath(),
-		req.GitProtocol,
-		strings.Join(req.GitConfigOptions, "\n") + "\n",
+		jsonString,
 	}, "\000") + "\000")
 
 	// The body of the request starts in the second message