diff options
author | John Cai <jcai@gitlab.com> | 2020-05-18 23:15:35 +0300 |
---|---|---|
committer | John Cai <jcai@gitlab.com> | 2020-05-22 07:31:11 +0300 |
commit | d4b4122abc7c7a8240aac762aee5bb5d20ee0d76 (patch) | |
tree | bb6237bc01372c61380af825d48d3a04833e4f2c | |
parent | 100d1926b1daeb12459ffd692f833c839cc19da9 (diff) |
Refactor operations service tests
21 files changed, 567 insertions, 746 deletions
diff --git a/cmd/gitaly-hooks/hooks_test.go b/cmd/gitaly-hooks/hooks_test.go index 5df748688..f22489a1f 100644 --- a/cmd/gitaly-hooks/hooks_test.go +++ b/cmd/gitaly-hooks/hooks_test.go @@ -88,7 +88,7 @@ func TestHooksPrePostReceive(t *testing.T) { RepoPath: testRepoPath, } - ts := testhelper.NewGitlabTestServer(t, c) + ts := testhelper.NewGitlabTestServer(c) defer ts.Close() config.Config.GitlabShell.Dir = tempGitlabShellDir @@ -122,12 +122,10 @@ func TestHooksPrePostReceive(t *testing.T) { customHookOutputPath, cleanup := testhelper.WriteEnvToCustomHook(t, testRepoPath, hookName) defer cleanup() - if featureSet.IsEnabled("use_gitaly_gitlabshell_config") { - config.Config.Gitlab.URL = ts.URL - config.Config.Gitlab.SecretFile = filepath.Join(tempGitlabShellDir, ".gitlab_shell_secret") - config.Config.Gitlab.HTTPSettings.User = gitlabUser - config.Config.Gitlab.HTTPSettings.Password = gitlabPassword - } + config.Config.Gitlab.URL = ts.URL + config.Config.Gitlab.SecretFile = filepath.Join(tempGitlabShellDir, ".gitlab_shell_secret") + config.Config.Gitlab.HTTPSettings.User = gitlabUser + config.Config.Gitlab.HTTPSettings.Password = gitlabPassword var stderr, stdout bytes.Buffer stdin := bytes.NewBuffer([]byte(changes)) @@ -326,7 +324,7 @@ func TestHooksPostReceiveFailed(t *testing.T) { PostReceiveCounterDecreased: false, Protocol: "ssh", } - ts := testhelper.NewGitlabTestServer(t, c) + ts := testhelper.NewGitlabTestServer(c) defer ts.Close() testhelper.WriteShellSecretFile(t, tempGitlabShellDir, secretToken) @@ -409,7 +407,7 @@ func TestHooksNotAllowed(t *testing.T) { PostReceiveCounterDecreased: true, Protocol: "ssh", } - ts := testhelper.NewGitlabTestServer(t, c) + ts := testhelper.NewGitlabTestServer(c) defer ts.Close() testhelper.WriteTemporaryGitlabShellConfigFile(t, tempGitlabShellDir, testhelper.GitlabShellConfig{GitlabURL: ts.URL}) @@ -461,7 +459,7 @@ func TestCheckOK(t *testing.T) { PostReceiveCounterDecreased: false, Protocol: "ssh", } - ts := testhelper.NewGitlabTestServer(t, c) + ts := testhelper.NewGitlabTestServer(c) defer ts.Close() tempDir, err := ioutil.TempDir("", t.Name()) @@ -510,7 +508,7 @@ func TestCheckBadCreds(t *testing.T) { Protocol: "ssh", GitPushOptions: nil, } - ts := testhelper.NewGitlabTestServer(t, c) + ts := testhelper.NewGitlabTestServer(c) defer ts.Close() tempDir, err := ioutil.TempDir("", t.Name()) diff --git a/internal/praefect/helper_test.go b/internal/praefect/helper_test.go index 3d6f18f65..a38fabc09 100644 --- a/internal/praefect/helper_test.go +++ b/internal/praefect/helper_test.go @@ -109,7 +109,10 @@ func noopBackoffFunc() (backoff, backoffReset) { type nullNodeMgr struct{} -func (nullNodeMgr) GetShard(virtualStorageName string) (nodes.Shard, error) { return nodes.Shard{}, nil } +func (nullNodeMgr) GetShard(virtualStorageName string) (nodes.Shard, error) { + return nodes.Shard{}, nil +} + func (nullNodeMgr) EnableWrites(ctx context.Context, virtualStorageName string) error { return nil } func (nullNodeMgr) GetSyncedNode(ctx context.Context, virtualStorageName, repoPath string) (nodes.Node, error) { return nil, nil diff --git a/internal/praefect/metadata/server.go b/internal/praefect/metadata/server.go index ff06a2a95..7b1adba6c 100644 --- a/internal/praefect/metadata/server.go +++ b/internal/praefect/metadata/server.go @@ -8,7 +8,7 @@ import ( "fmt" "strings" - "gitlab.com/gitlab-org/gitaly/auth" + gitalyauth "gitlab.com/gitlab-org/gitaly/auth" "gitlab.com/gitlab-org/gitaly/client" "gitlab.com/gitlab-org/gitaly/internal/praefect/config" "google.golang.org/grpc" diff --git a/internal/service/hooks/api/access_test.go b/internal/service/hooks/api/access_test.go index 198896898..a197fe5af 100644 --- a/internal/service/hooks/api/access_test.go +++ b/internal/service/hooks/api/access_test.go @@ -39,7 +39,7 @@ func TestAllowedVerifyParams(t *testing.T) { gitAlternateObjectDirsFull = append(gitAlternateObjectDirsFull, filepath.Join(testRepoPath, gitAlternateObjectDirRel)) } - server := testhelper.NewGitlabTestServer(t, testhelper.GitlabTestServerOptions{ + server := testhelper.NewGitlabTestServer(testhelper.GitlabTestServerOptions{ User: user, Password: password, SecretToken: secretToken, diff --git a/internal/service/operations/apply_patch.go b/internal/service/operations/apply_patch.go index dea6152c4..99e990ff0 100644 --- a/internal/service/operations/apply_patch.go +++ b/internal/service/operations/apply_patch.go @@ -72,7 +72,7 @@ func validateUserApplyPatchHeader(header *gitalypb.UserApplyPatchRequest_Header) return fmt.Errorf("missing User") } - if header.GetTargetBranch() == nil { + if len(header.GetTargetBranch()) == 0 { return fmt.Errorf("missing Branch") } diff --git a/internal/service/operations/apply_patch_test.go b/internal/service/operations/apply_patch_test.go index 9ce2ad15d..32e36af1c 100644 --- a/internal/service/operations/apply_patch_test.go +++ b/internal/service/operations/apply_patch_test.go @@ -1,4 +1,4 @@ -package operations_test +package operations import ( "context" @@ -13,7 +13,6 @@ import ( "github.com/stretchr/testify/require" "gitlab.com/gitlab-org/gitaly/internal/git/log" "gitlab.com/gitlab-org/gitaly/internal/metadata/featureflag" - "gitlab.com/gitlab-org/gitaly/internal/service/operations" "gitlab.com/gitlab-org/gitaly/internal/testhelper" "gitlab.com/gitlab-org/gitaly/proto/go/gitalypb" "gitlab.com/gitlab-org/gitaly/streamio" @@ -35,24 +34,15 @@ func TestSuccessfulUserApplyPatch(t *testing.T) { } func testSuccessfulUserApplyPatch(t *testing.T, ctx context.Context) { - serverSocketPath, stop := operations.RunOperationServiceServer(t) + serverSocketPath, stop := runOperationServiceServer(t) defer stop() - client, conn := operations.NewOperationClient(t, serverSocketPath) + client, conn := newOperationClient(t, serverSocketPath) defer conn.Close() testRepo, testRepoPath, cleanupFn := testhelper.NewTestRepo(t) defer cleanupFn() - cleanupSrv := operations.SetupAndStartGitlabServer(t, user.GlId, testRepo.GlRepository) - defer cleanupSrv() - - user := &gitalypb.User{ - Name: []byte("Jane Doe"), - Email: []byte("janedoe@gitlab.com"), - GlId: "user-1", - } - testPatchReadme := "testdata/0001-A-commit-from-a-patch.patch" testPatchFeature := "testdata/0001-This-does-not-apply-to-the-feature-branch.patch" @@ -91,7 +81,7 @@ func testSuccessfulUserApplyPatch(t *testing.T, ctx context.Context) { stream, err := client.UserApplyPatch(ctx) require.NoError(t, err) - headerRequest := applyPatchHeaderRequest(testRepo, user, testCase.branchName) + headerRequest := applyPatchHeaderRequest(testRepo, testhelper.TestUser, testCase.branchName) require.NoError(t, stream.Send(headerRequest)) writer := streamio.NewWriter(func(p []byte) error { @@ -148,41 +138,32 @@ func testSuccessfulUserApplyPatch(t *testing.T, ctx context.Context) { require.NotNil(t, commit) require.Equal(t, string(commit.Subject), testCase.commitMessages[index]) require.Equal(t, string(commit.Author.Email), "patchuser@gitlab.org") - require.Equal(t, string(commit.Committer.Email), string(user.Email)) + require.Equal(t, string(commit.Committer.Email), string(testhelper.TestUser.Email)) } }) } } func TestFailedPatchApplyPatch(t *testing.T) { - server, serverSocketPath := runFullServerWithHooks(t) - defer server.Stop() + serverSocketPath, stop := runOperationServiceServer(t) + defer stop() - client, conn := operations.NewOperationClient(t, serverSocketPath) + client, conn := newOperationClient(t, serverSocketPath) defer conn.Close() testRepo, _, cleanupFn := testhelper.NewTestRepo(t) defer cleanupFn() - cleanupSrv := operations.SetupAndStartGitlabServer(t, user.GlId, testRepo.GlRepository, "") - defer cleanupSrv() - ctx, cancel := testhelper.Context() defer cancel() - user := &gitalypb.User{ - Name: []byte("Jane Doe"), - Email: []byte("janedoe@gitlab.com"), - GlId: "user-1", - } - testPatch, err := ioutil.ReadFile("testdata/0001-This-does-not-apply-to-the-feature-branch.patch") require.NoError(t, err) stream, err := client.UserApplyPatch(ctx) require.NoError(t, err) - headerRequest := applyPatchHeaderRequest(testRepo, user, "feature") + headerRequest := applyPatchHeaderRequest(testRepo, testhelper.TestUser, "feature") require.NoError(t, stream.Send(headerRequest)) patchRequest := applyPatchPatchesRequest(testPatch) @@ -193,24 +174,9 @@ func TestFailedPatchApplyPatch(t *testing.T) { } func TestFailedValidationUserApplyPatch(t *testing.T) { - server, serverSocketPath := runFullServerWithHooks(t) - defer server.Stop() - - client, conn := operations.NewOperationClient(t, serverSocketPath) - defer conn.Close() - testRepo, _, cleanupFn := testhelper.NewTestRepo(t) defer cleanupFn() - ctx, cancel := testhelper.Context() - defer cancel() - - user := &gitalypb.User{ - Name: []byte("Jane Doe"), - Email: []byte("janedoe@gitlab.com"), - GlId: "user-1", - } - testCases := []struct { desc string errorMessage string @@ -222,19 +188,20 @@ func TestFailedValidationUserApplyPatch(t *testing.T) { desc: "missing Repository", errorMessage: "missing Repository", branchName: "new-branch", - user: user, + user: testhelper.TestUser, }, + { desc: "missing Branch", errorMessage: "missing Branch", repo: testRepo, - user: user, + user: testhelper.TestUser, }, { desc: "empty BranchName", errorMessage: "missing Branch", repo: testRepo, - user: user, + user: testhelper.TestUser, branchName: "", }, { @@ -247,15 +214,9 @@ func TestFailedValidationUserApplyPatch(t *testing.T) { for _, testCase := range testCases { t.Run(testCase.desc, func(t *testing.T) { - stream, err := client.UserApplyPatch(ctx) - require.NoError(t, err) - request := applyPatchHeaderRequest(testCase.repo, testCase.user, testCase.branchName) - require.NoError(t, stream.Send(request)) - - _, err = stream.CloseAndRecv() + err := validateUserApplyPatchHeader(request.GetHeader()) - testhelper.RequireGrpcError(t, err, codes.InvalidArgument) require.Contains(t, err.Error(), testCase.errorMessage) }) } diff --git a/internal/service/operations/branches_test.go b/internal/service/operations/branches_test.go index 1b9e68777..8c06d3791 100644 --- a/internal/service/operations/branches_test.go +++ b/internal/service/operations/branches_test.go @@ -30,9 +30,6 @@ func TestSuccessfulCreateBranchRequest(t *testing.T) { startPointCommit, err := log.GetCommit(ctx, testRepo, startPoint) require.NoError(t, err) - cleanupSrv := SetupAndStartGitlabServer(t, user.GlId, testRepo.GlRepository) - defer cleanupSrv() - testCases := []struct { desc string branchName string @@ -57,7 +54,7 @@ func TestSuccessfulCreateBranchRequest(t *testing.T) { Repository: testRepo, BranchName: []byte(branchName), StartPoint: []byte(testCase.startPoint), - User: user, + User: testhelper.TestUser, } ctx, cancel := testhelper.Context() @@ -96,9 +93,6 @@ func testSuccessfulGitHooksForUserCreateBranchRequest(t *testing.T, ctx context. testRepo, testRepoPath, cleanupFn := testhelper.NewTestRepo(t) defer cleanupFn() - cleanupSrv := SetupAndStartGitlabServer(t, user.GlId, testRepo.GlRepository) - defer cleanupSrv() - serverSocketPath, stop := runOperationServiceServer(t) defer stop() @@ -110,7 +104,7 @@ func testSuccessfulGitHooksForUserCreateBranchRequest(t *testing.T, ctx context. Repository: testRepo, BranchName: []byte(branchName), StartPoint: []byte("c7fbe50c7c7419d9701eebe64b1fdacc3df5b9dd"), - User: user, + User: testhelper.TestUser, } for _, hookName := range GitlabHooks { @@ -125,7 +119,7 @@ func testSuccessfulGitHooksForUserCreateBranchRequest(t *testing.T, ctx context. require.Empty(t, response.PreReceiveError) output := string(testhelper.MustReadFile(t, hookOutputTempPath)) - require.Contains(t, output, "GL_USERNAME="+user.GlUsername) + require.Contains(t, output, "GL_USERNAME="+testhelper.TestUser.GlUsername) }) } } @@ -134,9 +128,6 @@ func TestFailedUserCreateBranchDueToHooks(t *testing.T) { testRepo, testRepoPath, cleanupFn := testhelper.NewTestRepo(t) defer cleanupFn() - cleanupSrv := SetupAndStartGitlabServer(t, user.GlId, testRepo.GlRepository) - defer cleanupSrv() - serverSocketPath, stop := runOperationServiceServer(t) defer stop() @@ -147,7 +138,7 @@ func TestFailedUserCreateBranchDueToHooks(t *testing.T) { Repository: testRepo, BranchName: []byte("new-branch"), StartPoint: []byte("c7fbe50c7c7419d9701eebe64b1fdacc3df5b9dd"), - User: user, + User: testhelper.TestUser, } // Write a hook that will fail with the environment as the error message // so we can check that string for our env variables. @@ -163,7 +154,7 @@ func TestFailedUserCreateBranchDueToHooks(t *testing.T) { response, err := client.UserCreateBranch(ctx, request) require.Nil(t, err) - require.Contains(t, response.PreReceiveError, "GL_USERNAME="+user.GlUsername) + require.Contains(t, response.PreReceiveError, "GL_USERNAME="+testhelper.TestUser.GlUsername) } } @@ -177,9 +168,6 @@ func TestFailedUserCreateBranchRequest(t *testing.T) { testRepo, _, cleanupFn := testhelper.NewTestRepo(t) defer cleanupFn() - cleanupSrv := SetupAndStartGitlabServer(t, user.GlId, testRepo.GlRepository) - defer cleanupSrv() - testCases := []struct { desc string branchName string @@ -191,7 +179,7 @@ func TestFailedUserCreateBranchRequest(t *testing.T) { desc: "empty start_point", branchName: "shiny-new-branch", startPoint: "", - user: user, + user: testhelper.TestUser, code: codes.InvalidArgument, }, { @@ -205,7 +193,7 @@ func TestFailedUserCreateBranchRequest(t *testing.T) { desc: "non-existing starting point", branchName: "new-branch", startPoint: "i-dont-exist", - user: user, + user: testhelper.TestUser, code: codes.FailedPrecondition, }, @@ -213,7 +201,7 @@ func TestFailedUserCreateBranchRequest(t *testing.T) { desc: "branch exists", branchName: "master", startPoint: "master", - user: user, + user: testhelper.TestUser, code: codes.FailedPrecondition, }, } @@ -253,21 +241,12 @@ func TestSuccessfulUserDeleteBranchRequest(t *testing.T) { defer exec.Command("git", "-C", testRepoPath, "branch", "-d", branchNameInput).Run() - user := &gitalypb.User{ - Name: []byte("Alejandro Rodríguez"), - Email: []byte("alejandro@gitlab.com"), - GlId: "user-123", - } - - cleanupSrv := SetupAndStartGitlabServer(t, user.GlId, testRepo.GlRepository) - defer cleanupSrv() - testhelper.MustRunCommand(t, nil, "git", "-C", testRepoPath, "branch", branchNameInput) request := &gitalypb.UserDeleteBranchRequest{ Repository: testRepo, BranchName: []byte(branchNameInput), - User: user, + User: testhelper.TestUser, } _, err := client.UserDeleteBranch(ctx, request) @@ -290,20 +269,10 @@ func TestSuccessfulGitHooksForUserDeleteBranchRequest(t *testing.T) { branchNameInput := "to-be-deleted-soon-branch" defer exec.Command("git", "-C", testRepoPath, "branch", "-d", branchNameInput).Run() - user := &gitalypb.User{ - Name: []byte("Alejandro Rodríguez"), - Email: []byte("alejandro@gitlab.com"), - GlId: "user-123", - GlUsername: "johndoe", - } - - cleanupSrv := SetupAndStartGitlabServer(t, user.GlId, testRepo.GlRepository) - defer cleanupSrv() - request := &gitalypb.UserDeleteBranchRequest{ Repository: testRepo, BranchName: []byte(branchNameInput), - User: user, + User: testhelper.TestUser, } for _, hookName := range GitlabHooks { @@ -320,7 +289,7 @@ func TestSuccessfulGitHooksForUserDeleteBranchRequest(t *testing.T) { require.NoError(t, err) output := testhelper.MustReadFile(t, hookOutputTempPath) - require.Contains(t, string(output), "GL_USERNAME=johndoe") + require.Contains(t, string(output), "GL_USERNAME="+testhelper.TestUser.GlUsername) }) } } @@ -335,12 +304,6 @@ func TestFailedUserDeleteBranchDueToValidation(t *testing.T) { testRepo, _, cleanupFn := testhelper.NewTestRepo(t) defer cleanupFn() - user := &gitalypb.User{ - Name: []byte("Alejandro Rodríguez"), - Email: []byte("alejandro@gitlab.com"), - GlId: "user-123", - } - testCases := []struct { desc string request *gitalypb.UserDeleteBranchRequest @@ -358,7 +321,7 @@ func TestFailedUserDeleteBranchDueToValidation(t *testing.T) { desc: "empty branch name", request: &gitalypb.UserDeleteBranchRequest{ Repository: testRepo, - User: user, + User: testhelper.TestUser, }, code: codes.InvalidArgument, }, @@ -366,7 +329,7 @@ func TestFailedUserDeleteBranchDueToValidation(t *testing.T) { desc: "non-existent branch name", request: &gitalypb.UserDeleteBranchRequest{ Repository: testRepo, - User: user, + User: testhelper.TestUser, BranchName: []byte("i-do-not-exist"), }, code: codes.FailedPrecondition, @@ -398,19 +361,10 @@ func TestFailedUserDeleteBranchDueToHooks(t *testing.T) { testhelper.MustRunCommand(t, nil, "git", "-C", testRepoPath, "branch", branchNameInput) defer exec.Command("git", "-C", testRepoPath, "branch", "-d", branchNameInput).Run() - user := &gitalypb.User{ - Name: []byte("Alejandro Rodríguez"), - Email: []byte("alejandro@gitlab.com"), - GlId: "user-123", - } - - cleanupSrv := SetupAndStartGitlabServer(t, user.GlId, testRepo.GlRepository) - defer cleanupSrv() - request := &gitalypb.UserDeleteBranchRequest{ Repository: testRepo, BranchName: []byte(branchNameInput), - User: user, + User: testhelper.TestUser, } hookContent := []byte("#!/bin/sh\necho GL_ID=$GL_ID\nexit 1") @@ -426,7 +380,7 @@ func TestFailedUserDeleteBranchDueToHooks(t *testing.T) { response, err := client.UserDeleteBranch(ctx, request) require.Nil(t, err) - require.Contains(t, response.PreReceiveError, "GL_ID="+user.GlId) + require.Contains(t, response.PreReceiveError, "GL_ID="+testhelper.TestUser.GlId) branches := testhelper.MustRunCommand(t, nil, "git", "-C", testRepoPath, "branch") require.Contains(t, string(branches), branchNameInput, "branch name does not exist in branches list") diff --git a/internal/service/operations/cherry_pick_test.go b/internal/service/operations/cherry_pick_test.go index b1f86d43b..0b1431c7a 100644 --- a/internal/service/operations/cherry_pick_test.go +++ b/internal/service/operations/cherry_pick_test.go @@ -1,19 +1,14 @@ -package operations_test +package operations import ( "context" - "net" "testing" "github.com/stretchr/testify/require" - "gitlab.com/gitlab-org/gitaly/internal/config" "gitlab.com/gitlab-org/gitaly/internal/git/log" "gitlab.com/gitlab-org/gitaly/internal/metadata/featureflag" - serverPkg "gitlab.com/gitlab-org/gitaly/internal/server" - "gitlab.com/gitlab-org/gitaly/internal/service/operations" "gitlab.com/gitlab-org/gitaly/internal/testhelper" "gitlab.com/gitlab-org/gitaly/proto/go/gitalypb" - "google.golang.org/grpc" "google.golang.org/grpc/codes" "google.golang.org/grpc/metadata" ) @@ -22,10 +17,10 @@ func TestSuccessfulUserCherryPickRequest(t *testing.T) { ctxOuter, cancel := testhelper.Context() defer cancel() - serverSocketPath, stop := operations.RunOperationServiceServer(t) + serverSocketPath, stop := runOperationServiceServer(t) defer stop() - client, conn := operations.NewOperationClient(t, serverSocketPath) + client, conn := newOperationClient(t, serverSocketPath) defer conn.Close() testRepo, testRepoPath, cleanup := testhelper.NewTestRepo(t) @@ -37,21 +32,12 @@ func TestSuccessfulUserCherryPickRequest(t *testing.T) { masterHeadCommit, err := log.GetCommit(ctxOuter, testRepo, "master") require.NoError(t, err) - user := &gitalypb.User{ - Name: []byte("Ahmad Sherif"), - Email: []byte("ahmad@gitlab.com"), - GlId: "user-123", - } - cherryPickedCommit, err := log.GetCommit(ctxOuter, testRepo, "8a0f2ee90d940bfb0ba1e14e8214b0649056e4ab") require.NoError(t, err) testRepoCopy, _, cleanup := testhelper.NewTestRepo(t) defer cleanup() - cleanupSrv := operations.SetupAndStartGitlabServer(t, user.GlId, testRepo.GlRepository) - defer cleanupSrv() - testCases := []struct { desc string request *gitalypb.UserCherryPickRequest @@ -61,7 +47,7 @@ func TestSuccessfulUserCherryPickRequest(t *testing.T) { desc: "branch exists", request: &gitalypb.UserCherryPickRequest{ Repository: testRepo, - User: user, + User: testhelper.TestUser, Commit: cherryPickedCommit, BranchName: []byte(destinationBranch), Message: []byte("Cherry-picking " + cherryPickedCommit.Id), @@ -72,7 +58,7 @@ func TestSuccessfulUserCherryPickRequest(t *testing.T) { desc: "nonexistent branch + start_repository == repository", request: &gitalypb.UserCherryPickRequest{ Repository: testRepo, - User: user, + User: testhelper.TestUser, Commit: cherryPickedCommit, BranchName: []byte("to-be-cherry-picked-into-1"), Message: []byte("Cherry-picking " + cherryPickedCommit.Id), @@ -84,7 +70,7 @@ func TestSuccessfulUserCherryPickRequest(t *testing.T) { desc: "nonexistent branch + start_repository != repository", request: &gitalypb.UserCherryPickRequest{ Repository: testRepo, - User: user, + User: testhelper.TestUser, Commit: cherryPickedCommit, BranchName: []byte("to-be-cherry-picked-into-2"), Message: []byte("Cherry-picking " + cherryPickedCommit.Id), @@ -97,7 +83,7 @@ func TestSuccessfulUserCherryPickRequest(t *testing.T) { desc: "nonexistent branch + empty start_repository", request: &gitalypb.UserCherryPickRequest{ Repository: testRepo, - User: user, + User: testhelper.TestUser, Commit: cherryPickedCommit, BranchName: []byte("to-be-cherry-picked-into-3"), Message: []byte("Cherry-picking " + cherryPickedCommit.Id), @@ -145,10 +131,10 @@ func TestSuccessfulGitHooksForUserCherryPickRequest(t *testing.T) { } func testSuccessfulGitHooksForUserCherryPickRequest(t *testing.T, ctxOuter context.Context) { - serverSocketPath, stop := operations.RunOperationServiceServer(t) + serverSocketPath, stop := runOperationServiceServer(t) defer stop() - client, conn := operations.NewOperationClient(t, serverSocketPath) + client, conn := newOperationClient(t, serverSocketPath) defer conn.Close() testRepo, testRepoPath, cleanup := testhelper.NewTestRepo(t) @@ -157,28 +143,19 @@ func testSuccessfulGitHooksForUserCherryPickRequest(t *testing.T, ctxOuter conte destinationBranch := "cherry-picking-dst" testhelper.MustRunCommand(t, nil, "git", "-C", testRepoPath, "branch", destinationBranch, "master") - user := &gitalypb.User{ - Name: []byte("Ahmad Sherif"), - Email: []byte("ahmad@gitlab.com"), - GlId: "user-123", - } - - cleanupSrv := operations.SetupAndStartGitlabServer(t, user.GlId, testRepo.GlRepository) - defer cleanupSrv() - cherryPickedCommit, err := log.GetCommit(ctxOuter, testRepo, "8a0f2ee90d940bfb0ba1e14e8214b0649056e4ab") require.NoError(t, err) request := &gitalypb.UserCherryPickRequest{ Repository: testRepo, - User: user, + User: testhelper.TestUser, Commit: cherryPickedCommit, BranchName: []byte(destinationBranch), Message: []byte("Cherry-picking " + cherryPickedCommit.Id), } var hookOutputFiles []string - for _, hookName := range operations.GitlabHooks { + for _, hookName := range GitlabHooks { hookOutputTempPath, cleanup := testhelper.WriteEnvToCustomHook(t, testRepoPath, hookName) defer cleanup() hookOutputFiles = append(hookOutputFiles, hookOutputTempPath) @@ -193,7 +170,7 @@ func testSuccessfulGitHooksForUserCherryPickRequest(t *testing.T, ctxOuter conte for _, file := range hookOutputFiles { output := string(testhelper.MustReadFile(t, file)) - require.Contains(t, output, "GL_USERNAME="+user.GlUsername) + require.Contains(t, output, "GL_USERNAME="+testhelper.TestUser.GlUsername) } } @@ -201,10 +178,10 @@ func TestFailedUserCherryPickRequestDueToValidations(t *testing.T) { ctxOuter, cancel := testhelper.Context() defer cancel() - serverSocketPath, stop := operations.RunOperationServiceServer(t) + serverSocketPath, stop := runOperationServiceServer(t) defer stop() - client, conn := operations.NewOperationClient(t, serverSocketPath) + client, conn := newOperationClient(t, serverSocketPath) defer conn.Close() testRepo, _, cleanup := testhelper.NewTestRepo(t) @@ -215,12 +192,6 @@ func TestFailedUserCherryPickRequestDueToValidations(t *testing.T) { destinationBranch := "cherry-picking-dst" - user := &gitalypb.User{ - Name: []byte("Ahmad Sherif"), - Email: []byte("ahmad@gitlab.com"), - GlId: "user-123", - } - testCases := []struct { desc string request *gitalypb.UserCherryPickRequest @@ -241,7 +212,7 @@ func TestFailedUserCherryPickRequestDueToValidations(t *testing.T) { desc: "empty commit", request: &gitalypb.UserCherryPickRequest{ Repository: testRepo, - User: user, + User: testhelper.TestUser, Commit: nil, BranchName: []byte(destinationBranch), Message: []byte("Cherry-picking " + cherryPickedCommit.Id), @@ -252,7 +223,7 @@ func TestFailedUserCherryPickRequestDueToValidations(t *testing.T) { desc: "empty branch name", request: &gitalypb.UserCherryPickRequest{ Repository: testRepo, - User: user, + User: testhelper.TestUser, Commit: cherryPickedCommit, BranchName: nil, Message: []byte("Cherry-picking " + cherryPickedCommit.Id), @@ -263,7 +234,7 @@ func TestFailedUserCherryPickRequestDueToValidations(t *testing.T) { desc: "empty message", request: &gitalypb.UserCherryPickRequest{ Repository: testRepo, - User: user, + User: testhelper.TestUser, Commit: cherryPickedCommit, BranchName: []byte(destinationBranch), Message: nil, @@ -287,10 +258,10 @@ func TestFailedUserCherryPickRequestDueToPreReceiveError(t *testing.T) { ctxOuter, cancel := testhelper.Context() defer cancel() - serverSocketPath, stop := operations.RunOperationServiceServer(t) + serverSocketPath, stop := runOperationServiceServer(t) defer stop() - client, conn := operations.NewOperationClient(t, serverSocketPath) + client, conn := newOperationClient(t, serverSocketPath) defer conn.Close() testRepo, testRepoPath, cleanup := testhelper.NewTestRepo(t) @@ -299,21 +270,12 @@ func TestFailedUserCherryPickRequestDueToPreReceiveError(t *testing.T) { destinationBranch := "cherry-picking-dst" testhelper.MustRunCommand(t, nil, "git", "-C", testRepoPath, "branch", destinationBranch, "master") - user := &gitalypb.User{ - Name: []byte("Ahmad Sherif"), - Email: []byte("ahmad@gitlab.com"), - GlId: "user-123", - } - - cleanupSrv := operations.SetupAndStartGitlabServer(t, user.GlId, testRepo.GlRepository) - defer cleanupSrv() - cherryPickedCommit, err := log.GetCommit(ctxOuter, testRepo, "8a0f2ee90d940bfb0ba1e14e8214b0649056e4ab") require.NoError(t, err) request := &gitalypb.UserCherryPickRequest{ Repository: testRepo, - User: user, + User: testhelper.TestUser, Commit: cherryPickedCommit, BranchName: []byte(destinationBranch), Message: []byte("Cherry-picking " + cherryPickedCommit.Id), @@ -321,7 +283,7 @@ func TestFailedUserCherryPickRequestDueToPreReceiveError(t *testing.T) { hookContent := []byte("#!/bin/sh\necho GL_ID=$GL_ID\nexit 1") - for _, hookName := range operations.GitlabPreHooks { + for _, hookName := range GitlabPreHooks { t.Run(hookName, func(t *testing.T) { remove, err := testhelper.WriteCustomHook(testRepoPath, hookName, hookContent) require.NoError(t, err) @@ -332,7 +294,7 @@ func TestFailedUserCherryPickRequestDueToPreReceiveError(t *testing.T) { response, err := client.UserCherryPick(ctx, request) require.NoError(t, err) - require.Contains(t, response.PreReceiveError, "GL_ID="+user.GlId) + require.Contains(t, response.PreReceiveError, "GL_ID="+testhelper.TestUser.GlId) }) } } @@ -341,10 +303,10 @@ func TestFailedUserCherryPickRequestDueToCreateTreeError(t *testing.T) { ctxOuter, cancel := testhelper.Context() defer cancel() - serverSocketPath, stop := operations.RunOperationServiceServer(t) + serverSocketPath, stop := runOperationServiceServer(t) defer stop() - client, conn := operations.NewOperationClient(t, serverSocketPath) + client, conn := newOperationClient(t, serverSocketPath) defer conn.Close() testRepo, testRepoPath, cleanup := testhelper.NewTestRepo(t) @@ -353,19 +315,13 @@ func TestFailedUserCherryPickRequestDueToCreateTreeError(t *testing.T) { destinationBranch := "cherry-picking-dst" testhelper.MustRunCommand(t, nil, "git", "-C", testRepoPath, "branch", destinationBranch, "master") - user := &gitalypb.User{ - Name: []byte("Ahmad Sherif"), - Email: []byte("ahmad@gitlab.com"), - GlId: "user-123", - } - // This commit already exists in master cherryPickedCommit, err := log.GetCommit(ctxOuter, testRepo, "4a24d82dbca5c11c61556f3b35ca472b7463187e") require.NoError(t, err) request := &gitalypb.UserCherryPickRequest{ Repository: testRepo, - User: user, + User: testhelper.TestUser, Commit: cherryPickedCommit, BranchName: []byte(destinationBranch), Message: []byte("Cherry-picking " + cherryPickedCommit.Id), @@ -384,10 +340,10 @@ func TestFailedUserCherryPickRequestDueToCommitError(t *testing.T) { ctxOuter, cancel := testhelper.Context() defer cancel() - serverSocketPath, stop := operations.RunOperationServiceServer(t) + serverSocketPath, stop := runOperationServiceServer(t) defer stop() - client, conn := operations.NewOperationClient(t, serverSocketPath) + client, conn := newOperationClient(t, serverSocketPath) defer conn.Close() testRepo, testRepoPath, cleanup := testhelper.NewTestRepo(t) @@ -398,18 +354,12 @@ func TestFailedUserCherryPickRequestDueToCommitError(t *testing.T) { testhelper.MustRunCommand(t, nil, "git", "-C", testRepoPath, "branch", destinationBranch, "master") testhelper.MustRunCommand(t, nil, "git", "-C", testRepoPath, "branch", sourceBranch, "8a0f2ee90d940bfb0ba1e14e8214b0649056e4ab") - user := &gitalypb.User{ - Name: []byte("Ahmad Sherif"), - Email: []byte("ahmad@gitlab.com"), - GlId: "user-123", - } - cherryPickedCommit, err := log.GetCommit(ctxOuter, testRepo, sourceBranch) require.NoError(t, err) request := &gitalypb.UserCherryPickRequest{ Repository: testRepo, - User: user, + User: testhelper.TestUser, Commit: cherryPickedCommit, BranchName: []byte(sourceBranch), Message: []byte("Cherry-picking " + cherryPickedCommit.Id), @@ -423,17 +373,3 @@ func TestFailedUserCherryPickRequestDueToCommitError(t *testing.T) { require.NoError(t, err) require.Equal(t, "Branch diverged", response.CommitError) } - -func runFullServerWithHooks(t *testing.T) (*grpc.Server, string) { - server := serverPkg.NewInsecure(operations.RubyServer, config.Config) - serverSocketPath := testhelper.GetTemporaryGitalySocketFileName() - - listener, err := net.Listen("unix", serverSocketPath) - if err != nil { - t.Fatal(err) - } - - go server.Serve(listener) - - return server, "unix://" + serverSocketPath -} diff --git a/internal/service/operations/commit_files_test.go b/internal/service/operations/commit_files_test.go index 9b1d147a9..cc292041a 100644 --- a/internal/service/operations/commit_files_test.go +++ b/internal/service/operations/commit_files_test.go @@ -1,4 +1,4 @@ -package operations_test +package operations import ( "context" @@ -10,7 +10,6 @@ import ( "gitlab.com/gitlab-org/gitaly/internal/git/log" "gitlab.com/gitlab-org/gitaly/internal/helper/text" "gitlab.com/gitlab-org/gitaly/internal/metadata/featureflag" - "gitlab.com/gitlab-org/gitaly/internal/service/operations" "gitlab.com/gitlab-org/gitaly/internal/testhelper" "gitlab.com/gitlab-org/gitaly/proto/go/gitalypb" "google.golang.org/grpc/codes" @@ -18,11 +17,6 @@ import ( ) var ( - user = &gitalypb.User{ - Name: []byte("John Doe"), - Email: []byte("johndoe@gitlab.com"), - GlId: "user-1", - } commitFilesMessage = []byte("Change files") ) @@ -30,13 +24,10 @@ func testSuccessfulUserCommitFilesRequest(t *testing.T, ctxWithFeatureFlags cont testRepo, testRepoPath, cleanup := testhelper.NewTestRepo(t) defer cleanup() - cleanupSrv := operations.SetupAndStartGitlabServer(t, user.GlId, testRepo.GlRepository) - defer cleanupSrv() - - serverSocketPath, stop := operations.RunOperationServiceServer(t) + serverSocketPath, stop := runOperationServiceServer(t) defer stop() - client, conn := operations.NewOperationClient(t, serverSocketPath) + client, conn := newOperationClient(t, serverSocketPath) defer conn.Close() newRepo, newRepoPath, newRepoCleanupFn := testhelper.InitBareRepo(t) @@ -93,7 +84,7 @@ func testSuccessfulUserCommitFilesRequest(t *testing.T, ctxWithFeatureFlags cont for _, tc := range testCases { t.Run(tc.desc, func(t *testing.T) { ctx := metadata.NewOutgoingContext(ctxWithFeatureFlags, md) - headerRequest := headerRequest(tc.repo, user, tc.branchName, commitFilesMessage) + headerRequest := headerRequest(tc.repo, testhelper.TestUser, tc.branchName, commitFilesMessage) setAuthorAndEmail(headerRequest, authorName, authorEmail) actionsRequest1 := createFileHeaderRequest(filePath) @@ -117,9 +108,9 @@ func testSuccessfulUserCommitFilesRequest(t *testing.T, ctxWithFeatureFlags cont headCommit, err := log.GetCommit(ctxWithFeatureFlags, tc.repo, tc.branchName) require.NoError(t, err) require.Equal(t, authorName, headCommit.Author.Name) - require.Equal(t, user.Name, headCommit.Committer.Name) + require.Equal(t, testhelper.TestUser.Name, headCommit.Committer.Name) require.Equal(t, authorEmail, headCommit.Author.Email) - require.Equal(t, user.Email, headCommit.Committer.Email) + require.Equal(t, testhelper.TestUser.Email, headCommit.Committer.Email) require.Equal(t, commitFilesMessage, headCommit.Subject) fileContent := testhelper.MustRunCommand(t, nil, "git", "-C", tc.repoPath, "show", headCommit.GetId()+":"+filePath) @@ -150,10 +141,10 @@ func TestSuccessfulUserCommitFilesRequest(t *testing.T) { } func TestSuccessfulUserCommitFilesRequestMove(t *testing.T) { - serverSocketPath, stop := operations.RunOperationServiceServer(t) + serverSocketPath, stop := runOperationServiceServer(t) defer stop() - client, conn := operations.NewOperationClient(t, serverSocketPath) + client, conn := newOperationClient(t, serverSocketPath) defer conn.Close() ctxOuter, cancel := testhelper.Context() @@ -178,13 +169,10 @@ func TestSuccessfulUserCommitFilesRequestMove(t *testing.T) { testRepo, testRepoPath, cleanupFn := testhelper.NewTestRepo(t) defer cleanupFn() - cleanupSrv := operations.SetupAndStartGitlabServer(t, user.GlId, testRepo.GlRepository) - defer cleanupSrv() - origFileContent := testhelper.MustRunCommand(t, nil, "git", "-C", testRepoPath, "show", branchName+":"+previousFilePath) md := testhelper.GitalyServersMetadata(t, serverSocketPath) ctx := metadata.NewOutgoingContext(ctxOuter, md) - headerRequest := headerRequest(testRepo, user, branchName, commitFilesMessage) + headerRequest := headerRequest(testRepo, testhelper.TestUser, branchName, commitFilesMessage) setAuthorAndEmail(headerRequest, authorName, authorEmail) actionsRequest1 := moveFileHeaderRequest(previousFilePath, filePath, tc.infer) @@ -216,10 +204,10 @@ func TestSuccessfulUserCommitFilesRequestMove(t *testing.T) { } func TestSuccessfulUserCommitFilesRequestForceCommit(t *testing.T) { - serverSocketPath, stop := operations.RunOperationServiceServer(t) + serverSocketPath, stop := runOperationServiceServer(t) defer stop() - client, conn := operations.NewOperationClient(t, serverSocketPath) + client, conn := newOperationClient(t, serverSocketPath) defer conn.Close() testRepo, testRepoPath, cleanupFn := testhelper.NewTestRepo(t) @@ -235,9 +223,6 @@ func TestSuccessfulUserCommitFilesRequestForceCommit(t *testing.T) { targetBranchName := "feature" startBranchName := []byte("master") - cleanupSrv := operations.SetupAndStartGitlabServer(t, user.GlId, testRepo.GlRepository) - defer cleanupSrv() - startBranchCommit, err := log.GetCommit(ctxOuter, testRepo, string(startBranchName)) require.NoError(t, err) @@ -248,7 +233,7 @@ func TestSuccessfulUserCommitFilesRequestForceCommit(t *testing.T) { mergeBaseID := text.ChompBytes(mergeBaseOut) require.NotEqual(t, mergeBaseID, targetBranchCommit.Id, "expected %s not to be an ancestor of %s", targetBranchCommit.Id, startBranchCommit.Id) - headerRequest := headerRequest(testRepo, user, targetBranchName, commitFilesMessage) + headerRequest := headerRequest(testRepo, testhelper.TestUser, targetBranchName, commitFilesMessage) setAuthorAndEmail(headerRequest, authorName, authorEmail) setStartBranchName(headerRequest, startBranchName) setForce(headerRequest, true) @@ -271,10 +256,10 @@ func TestSuccessfulUserCommitFilesRequestForceCommit(t *testing.T) { } func TestSuccessfulUserCommitFilesRequestStartSha(t *testing.T) { - serverSocketPath, stop := operations.RunOperationServiceServer(t) + serverSocketPath, stop := runOperationServiceServer(t) defer stop() - client, conn := operations.NewOperationClient(t, serverSocketPath) + client, conn := newOperationClient(t, serverSocketPath) defer conn.Close() testRepo, _, cleanupFn := testhelper.NewTestRepo(t) @@ -290,12 +275,9 @@ func TestSuccessfulUserCommitFilesRequestStartSha(t *testing.T) { startCommit, err := log.GetCommit(ctxOuter, testRepo, "master") require.NoError(t, err) - headerRequest := headerRequest(testRepo, user, targetBranchName, commitFilesMessage) + headerRequest := headerRequest(testRepo, testhelper.TestUser, targetBranchName, commitFilesMessage) setStartSha(headerRequest, startCommit.Id) - cleanupSrv := operations.SetupAndStartGitlabServer(t, user.GlId, testRepo.GlRepository) - defer cleanupSrv() - stream, err := client.UserCommitFiles(ctx) require.NoError(t, err) require.NoError(t, stream.Send(headerRequest)) @@ -314,10 +296,10 @@ func TestSuccessfulUserCommitFilesRequestStartSha(t *testing.T) { } func TestSuccessfulUserCommitFilesRequestStartShaRemoteRepository(t *testing.T) { - serverSocketPath, stop := operations.RunOperationServiceServer(t) + serverSocketPath, stop := runOperationServiceServer(t) defer stop() - client, conn := operations.NewOperationClient(t, serverSocketPath) + client, conn := newOperationClient(t, serverSocketPath) defer conn.Close() testRepo, _, cleanupFn := testhelper.NewTestRepo(t) @@ -336,13 +318,10 @@ func TestSuccessfulUserCommitFilesRequestStartShaRemoteRepository(t *testing.T) startCommit, err := log.GetCommit(ctxOuter, testRepo, "master") require.NoError(t, err) - headerRequest := headerRequest(newRepo, user, targetBranchName, commitFilesMessage) + headerRequest := headerRequest(newRepo, testhelper.TestUser, targetBranchName, commitFilesMessage) setStartSha(headerRequest, startCommit.Id) setStartRepository(headerRequest, testRepo) - cleanupSrv := operations.SetupAndStartGitlabServer(t, user.GlId, testRepo.GlRepository) - defer cleanupSrv() - stream, err := client.UserCommitFiles(ctx) require.NoError(t, err) require.NoError(t, stream.Send(headerRequest)) @@ -361,26 +340,21 @@ func TestSuccessfulUserCommitFilesRequestStartShaRemoteRepository(t *testing.T) } func TestSuccessfulUserCommitFilesRequestWithSpecialCharactersInSignature(t *testing.T) { - serverSocketPath, stop := operations.RunOperationServiceServer(t) + serverSocketPath, stop := runOperationServiceServer(t) defer stop() - client, conn := operations.NewOperationClient(t, serverSocketPath) + client, conn := newOperationClient(t, serverSocketPath) defer conn.Close() testRepo, _, cleanupFn := testhelper.InitBareRepo(t) defer cleanupFn() - glID := "key-123" - ctxOuter, cancel := testhelper.Context() defer cancel() md := testhelper.GitalyServersMetadata(t, serverSocketPath) targetBranchName := "master" - cleanupSrv := operations.SetupAndStartGitlabServer(t, glID, testRepo.GlRepository) - defer cleanupSrv() - testCases := []struct { desc string user *gitalypb.User @@ -388,12 +362,12 @@ func TestSuccessfulUserCommitFilesRequestWithSpecialCharactersInSignature(t *tes }{ { desc: "special characters at start and end", - user: &gitalypb.User{Name: []byte(".,:;<>\"'\nJane Doe.,:;<>'\"\n"), Email: []byte(".,:;<>'\"\njanedoe@gitlab.com.,:;<>'\"\n"), GlId: glID}, + user: &gitalypb.User{Name: []byte(".,:;<>\"'\nJane Doe.,:;<>'\"\n"), Email: []byte(".,:;<>'\"\njanedoe@gitlab.com.,:;<>'\"\n"), GlId: testhelper.GlID}, author: &gitalypb.CommitAuthor{Name: []byte("Jane Doe"), Email: []byte("janedoe@gitlab.com")}, }, { desc: "special characters in the middle", - user: &gitalypb.User{Name: []byte("Ja<ne\n D>oe"), Email: []byte("ja<ne\ndoe>@gitlab.com"), GlId: glID}, + user: &gitalypb.User{Name: []byte("Ja<ne\n D>oe"), Email: []byte("ja<ne\ndoe>@gitlab.com"), GlId: testhelper.GlID}, author: &gitalypb.CommitAuthor{Name: []byte("Jane Doe"), Email: []byte("janedoe@gitlab.com")}, }, } @@ -423,10 +397,10 @@ func TestSuccessfulUserCommitFilesRequestWithSpecialCharactersInSignature(t *tes } func TestFailedUserCommitFilesRequestDueToHooks(t *testing.T) { - serverSocketPath, stop := operations.RunOperationServiceServer(t) + serverSocketPath, stop := runOperationServiceServer(t) defer stop() - client, conn := operations.NewOperationClient(t, serverSocketPath) + client, conn := newOperationClient(t, serverSocketPath) defer conn.Close() testRepo, testRepoPath, cleanupFn := testhelper.NewTestRepo(t) @@ -437,15 +411,12 @@ func TestFailedUserCommitFilesRequestDueToHooks(t *testing.T) { branchName := "feature" filePath := "my/file.txt" - headerRequest := headerRequest(testRepo, user, branchName, commitFilesMessage) + headerRequest := headerRequest(testRepo, testhelper.TestUser, branchName, commitFilesMessage) actionsRequest1 := createFileHeaderRequest(filePath) actionsRequest2 := actionContentRequest("My content") hookContent := []byte("#!/bin/sh\nprintenv | paste -sd ' ' -\nexit 1") - cleanupSrv := operations.SetupAndStartGitlabServer(t, user.GlId, testRepo.GlRepository) - defer cleanupSrv() - - for _, hookName := range operations.GitlabPreHooks { + for _, hookName := range GitlabPreHooks { t.Run(hookName, func(t *testing.T) { remove, err := testhelper.WriteCustomHook(testRepoPath, hookName, hookContent) require.NoError(t, err) @@ -461,25 +432,22 @@ func TestFailedUserCommitFilesRequestDueToHooks(t *testing.T) { resp, err := stream.CloseAndRecv() require.NoError(t, err) - require.Contains(t, resp.PreReceiveError, "GL_ID="+user.GlId) - require.Contains(t, resp.PreReceiveError, "GL_USERNAME="+user.GlUsername) + require.Contains(t, resp.PreReceiveError, "GL_ID="+testhelper.TestUser.GlId) + require.Contains(t, resp.PreReceiveError, "GL_USERNAME="+testhelper.TestUser.GlUsername) }) } } func TestFailedUserCommitFilesRequestDueToIndexError(t *testing.T) { - serverSocketPath, stop := operations.RunOperationServiceServer(t) + serverSocketPath, stop := runOperationServiceServer(t) defer stop() - client, conn := operations.NewOperationClient(t, serverSocketPath) + client, conn := newOperationClient(t, serverSocketPath) defer conn.Close() testRepo, _, cleanupFn := testhelper.NewTestRepo(t) defer cleanupFn() - cleanupSrv := operations.SetupAndStartGitlabServer(t, user.GlId, testRepo.GlRepository) - defer cleanupSrv() - ctxOuter, cancel := testhelper.Context() defer cancel() @@ -493,7 +461,7 @@ func TestFailedUserCommitFilesRequestDueToIndexError(t *testing.T) { { desc: "file already exists", requests: []*gitalypb.UserCommitFilesRequest{ - headerRequest(testRepo, user, "feature", commitFilesMessage), + headerRequest(testRepo, testhelper.TestUser, "feature", commitFilesMessage), createFileHeaderRequest("README.md"), actionContentRequest("This file already exists"), }, @@ -502,7 +470,7 @@ func TestFailedUserCommitFilesRequestDueToIndexError(t *testing.T) { { desc: "file doesn't exists", requests: []*gitalypb.UserCommitFilesRequest{ - headerRequest(testRepo, user, "feature", commitFilesMessage), + headerRequest(testRepo, testhelper.TestUser, "feature", commitFilesMessage), chmodFileHeaderRequest("documents/story.txt", true), }, indexError: "A file with this name doesn't exist", @@ -510,7 +478,7 @@ func TestFailedUserCommitFilesRequestDueToIndexError(t *testing.T) { { desc: "dir already exists", requests: []*gitalypb.UserCommitFilesRequest{ - headerRequest(testRepo, user, "utf-dir", commitFilesMessage), + headerRequest(testRepo, testhelper.TestUser, "utf-dir", commitFilesMessage), actionRequest(&gitalypb.UserCommitFilesAction{ UserCommitFilesActionPayload: &gitalypb.UserCommitFilesAction_Header{ Header: &gitalypb.UserCommitFilesActionHeader{ @@ -543,10 +511,10 @@ func TestFailedUserCommitFilesRequestDueToIndexError(t *testing.T) { } func TestFailedUserCommitFilesRequest(t *testing.T) { - serverSocketPath, stop := operations.RunOperationServiceServer(t) + serverSocketPath, stop := runOperationServiceServer(t) defer stop() - client, conn := operations.NewOperationClient(t, serverSocketPath) + client, conn := newOperationClient(t, serverSocketPath) defer conn.Close() testRepo, _, cleanupFn := testhelper.NewTestRepo(t) @@ -559,16 +527,13 @@ func TestFailedUserCommitFilesRequest(t *testing.T) { ctx := metadata.NewOutgoingContext(ctxOuter, md) branchName := "feature" - cleanupSrv := operations.SetupAndStartGitlabServer(t, user.GlId, testRepo.GlRepository) - defer cleanupSrv() - testCases := []struct { desc string req *gitalypb.UserCommitFilesRequest }{ { desc: "empty Repository", - req: headerRequest(nil, user, branchName, commitFilesMessage), + req: headerRequest(nil, testhelper.TestUser, branchName, commitFilesMessage), }, { desc: "empty User", @@ -576,15 +541,15 @@ func TestFailedUserCommitFilesRequest(t *testing.T) { }, { desc: "empty BranchName", - req: headerRequest(testRepo, user, "", commitFilesMessage), + req: headerRequest(testRepo, testhelper.TestUser, "", commitFilesMessage), }, { desc: "empty CommitMessage", - req: headerRequest(testRepo, user, branchName, nil), + req: headerRequest(testRepo, testhelper.TestUser, branchName, nil), }, { desc: "invalid commit ID: \"foobar\"", - req: setStartSha(headerRequest(testRepo, user, branchName, commitFilesMessage), "foobar"), + req: setStartSha(headerRequest(testRepo, testhelper.TestUser, branchName, commitFilesMessage), "foobar"), }, { desc: "failed to parse signature - Signature cannot have an empty name or email", diff --git a/internal/service/operations/merge_test.go b/internal/service/operations/merge_test.go index faf6bf0d9..869a7a441 100644 --- a/internal/service/operations/merge_test.go +++ b/internal/service/operations/merge_test.go @@ -19,12 +19,6 @@ import ( ) var ( - mergeUser = &gitalypb.User{ - Name: []byte("Jane Doe"), - Email: []byte("janedoe@example.com"), - GlId: "user-1", - } - commitToMerge = "e63f41fe459e62e1228fcef60d7189127aeba95a" mergeBranchName = "gitaly-merge-test-branch" mergeBranchHeadBefore = "281d3a76f31c812dbf48abce82ccf6860adedd81" @@ -67,13 +61,10 @@ func testSuccessfulMerge(t *testing.T, ctx context.Context) { defer cleanup() } - cleanupSrv := SetupAndStartGitlabServer(t, mergeUser.GlId, testRepo.GlRepository) - defer cleanupSrv() - mergeCommitMessage := "Merged by Gitaly" firstRequest := &gitalypb.UserMergeBranchRequest{ Repository: testRepo, - User: mergeUser, + User: testhelper.TestUser, CommitId: commitToMerge, Branch: []byte(mergeBranchName), Message: []byte(mergeCommitMessage), @@ -109,10 +100,10 @@ func testSuccessfulMerge(t *testing.T, ctx context.Context) { require.True(t, strings.HasPrefix(string(commit.Body), mergeCommitMessage), "expected %q to start with %q", commit.Body, mergeCommitMessage) author := commit.Author - require.Equal(t, mergeUser.Name, author.Name) - require.Equal(t, mergeUser.Email, author.Email) + require.Equal(t, testhelper.TestUser.Name, author.Name) + require.Equal(t, testhelper.TestUser.Email, author.Email) - expectedGlID := "GL_ID=" + mergeUser.GlId + expectedGlID := "GL_ID=" + testhelper.TestUser.GlId for i, h := range hooks { hookEnv, err := ioutil.ReadFile(hookTempfiles[i]) require.NoError(t, err) @@ -137,7 +128,7 @@ func TestAbortedMerge(t *testing.T) { firstRequest := &gitalypb.UserMergeBranchRequest{ Repository: testRepo, - User: mergeUser, + User: testhelper.TestUser, CommitId: commitToMerge, Branch: []byte(mergeBranchName), Message: []byte("foobar"), @@ -209,7 +200,7 @@ func TestFailedMergeConcurrentUpdate(t *testing.T) { mergeCommitMessage := "Merged by Gitaly" firstRequest := &gitalypb.UserMergeBranchRequest{ Repository: testRepo, - User: mergeUser, + User: testhelper.TestUser, CommitId: commitToMerge, Branch: []byte(mergeBranchName), Message: []byte(mergeCommitMessage), @@ -247,9 +238,6 @@ func TestFailedMergeDueToHooks(t *testing.T) { prepareMergeBranch(t, testRepoPath) - cleanupSrv := SetupAndStartGitlabServer(t, mergeUser.GlId, testRepo.GlRepository) - defer cleanupSrv() - hookContent := []byte("#!/bin/sh\necho 'failure'\nexit 1") for _, hookName := range gitlabPreHooks { @@ -267,7 +255,7 @@ func TestFailedMergeDueToHooks(t *testing.T) { mergeCommitMessage := "Merged by Gitaly" firstRequest := &gitalypb.UserMergeBranchRequest{ Repository: testRepo, - User: mergeUser, + User: testhelper.TestUser, CommitId: commitToMerge, Branch: []byte(mergeBranchName), Message: []byte(mergeCommitMessage), @@ -316,7 +304,7 @@ func TestSuccessfulUserFFBranchRequest(t *testing.T) { Repository: testRepo, CommitId: commitID, Branch: []byte(branchName), - User: mergeUser, + User: testhelper.TestUser, } expectedResponse := &gitalypb.UserFFBranchResponse{ BranchUpdate: &gitalypb.OperationBranchUpdate{ @@ -329,9 +317,6 @@ func TestSuccessfulUserFFBranchRequest(t *testing.T) { testhelper.MustRunCommand(t, nil, "git", "-C", testRepoPath, "branch", "-f", branchName, "6d394385cf567f80a8fd85055db1ab4c5295806f") defer exec.Command("git", "-C", testRepoPath, "branch", "-d", branchName).Run() - cleanupSrv := SetupAndStartGitlabServer(t, mergeUser.GlId, testRepo.GlRepository) - defer cleanupSrv() - resp, err := client.UserFFBranch(ctx, request) require.NoError(t, err) require.Equal(t, expectedResponse, resp) @@ -365,7 +350,7 @@ func TestFailedUserFFBranchRequest(t *testing.T) { }{ { desc: "empty repository", - user: mergeUser, + user: testhelper.TestUser, branch: []byte(branchName), commitID: commitID, code: codes.InvalidArgument, @@ -380,14 +365,14 @@ func TestFailedUserFFBranchRequest(t *testing.T) { { desc: "empty commit", repo: testRepo, - user: mergeUser, + user: testhelper.TestUser, branch: []byte(branchName), code: codes.InvalidArgument, }, { desc: "non-existing commit", repo: testRepo, - user: mergeUser, + user: testhelper.TestUser, branch: []byte(branchName), commitID: "f001", code: codes.InvalidArgument, @@ -395,14 +380,14 @@ func TestFailedUserFFBranchRequest(t *testing.T) { { desc: "empty branch", repo: testRepo, - user: mergeUser, + user: testhelper.TestUser, commitID: commitID, code: codes.InvalidArgument, }, { desc: "non-existing branch", repo: testRepo, - user: mergeUser, + user: testhelper.TestUser, branch: []byte("this-isnt-real"), commitID: commitID, code: codes.InvalidArgument, @@ -410,7 +395,7 @@ func TestFailedUserFFBranchRequest(t *testing.T) { { desc: "commit is not a descendant of branch head", repo: testRepo, - user: mergeUser, + user: testhelper.TestUser, branch: []byte(branchName), commitID: "1a0b36b3cdad1d2ee32457c102a8c0b7056fa863", code: codes.FailedPrecondition, @@ -450,15 +435,12 @@ func TestFailedUserFFBranchDueToHooks(t *testing.T) { Repository: testRepo, CommitId: commitID, Branch: []byte(branchName), - User: mergeUser, + User: testhelper.TestUser, } testhelper.MustRunCommand(t, nil, "git", "-C", testRepoPath, "branch", "-f", branchName, "6d394385cf567f80a8fd85055db1ab4c5295806f") defer exec.Command("git", "-C", testRepoPath, "branch", "-d", branchName).Run() - cleanupSrv := SetupAndStartGitlabServer(t, mergeUser.GlId, testRepo.GlRepository) - defer cleanupSrv() - hookContent := []byte("#!/bin/sh\necho 'failure'\nexit 1") for _, hookName := range gitlabPreHooks { @@ -510,7 +492,7 @@ func TestSuccessfulUserMergeToRefRequest(t *testing.T) { }{ { desc: "empty target ref merge", - user: mergeUser, + user: testhelper.TestUser, targetRef: emptyTargetRef, emptyRef: true, sourceSha: commitToMerge, @@ -519,7 +501,7 @@ func TestSuccessfulUserMergeToRefRequest(t *testing.T) { }, { desc: "existing target ref", - user: mergeUser, + user: testhelper.TestUser, targetRef: existingTargetRef, emptyRef: false, sourceSha: commitToMerge, @@ -528,7 +510,7 @@ func TestSuccessfulUserMergeToRefRequest(t *testing.T) { }, { desc: "branch is specified and firstParentRef is empty", - user: mergeUser, + user: testhelper.TestUser, branch: []byte(mergeBranchName), targetRef: existingTargetRef, emptyRef: false, @@ -572,8 +554,8 @@ func TestSuccessfulUserMergeToRefRequest(t *testing.T) { // Asserts author author := commit.Author - require.Equal(t, mergeUser.Name, author.Name) - require.Equal(t, mergeUser.Email, author.Email) + require.Equal(t, testhelper.TestUser.Name, author.Name) + require.Equal(t, testhelper.TestUser.Email, author.Email) require.Equal(t, resp.CommitId, commit.Id) @@ -611,7 +593,7 @@ func TestFailedUserMergeToRefRequest(t *testing.T) { }{ { desc: "empty repository", - user: mergeUser, + user: testhelper.TestUser, branch: []byte(branchName), sourceSha: commitToMerge, targetRef: validTargetRef, @@ -628,7 +610,7 @@ func TestFailedUserMergeToRefRequest(t *testing.T) { { desc: "empty source SHA", repo: testRepo, - user: mergeUser, + user: testhelper.TestUser, branch: []byte(branchName), targetRef: validTargetRef, code: codes.InvalidArgument, @@ -636,7 +618,7 @@ func TestFailedUserMergeToRefRequest(t *testing.T) { { desc: "non-existing commit", repo: testRepo, - user: mergeUser, + user: testhelper.TestUser, branch: []byte(branchName), sourceSha: "f001", targetRef: validTargetRef, @@ -645,7 +627,7 @@ func TestFailedUserMergeToRefRequest(t *testing.T) { { desc: "empty branch and first parent ref", repo: testRepo, - user: mergeUser, + user: testhelper.TestUser, sourceSha: commitToMerge, targetRef: validTargetRef, code: codes.InvalidArgument, @@ -653,7 +635,7 @@ func TestFailedUserMergeToRefRequest(t *testing.T) { { desc: "invalid target ref", repo: testRepo, - user: mergeUser, + user: testhelper.TestUser, branch: []byte(branchName), sourceSha: commitToMerge, targetRef: []byte("refs/heads/branch"), @@ -662,7 +644,7 @@ func TestFailedUserMergeToRefRequest(t *testing.T) { { desc: "non-existing branch", repo: testRepo, - user: mergeUser, + user: testhelper.TestUser, branch: []byte("this-isnt-real"), sourceSha: commitToMerge, targetRef: validTargetRef, @@ -708,7 +690,7 @@ func TestUserMergeToRefIgnoreHooksRequest(t *testing.T) { SourceSha: commitToMerge, Branch: []byte(mergeBranchName), TargetRef: targetRef, - User: mergeUser, + User: testhelper.TestUser, Message: []byte(mergeCommitMessage), } diff --git a/internal/service/operations/rebase_test.go b/internal/service/operations/rebase_test.go index e793b6d7d..73e137e22 100644 --- a/internal/service/operations/rebase_test.go +++ b/internal/service/operations/rebase_test.go @@ -1,10 +1,9 @@ -package operations_test +package operations //lint:file-ignore SA1019 due to planned removal in issue https://gitlab.com/gitlab-org/gitaly/issues/1628 import ( "context" - "errors" "fmt" "strings" "testing" @@ -13,7 +12,7 @@ import ( "github.com/stretchr/testify/require" gitlog "gitlab.com/gitlab-org/gitaly/internal/git/log" "gitlab.com/gitlab-org/gitaly/internal/metadata/featureflag" - "gitlab.com/gitlab-org/gitaly/internal/service/operations" + "gitlab.com/gitlab-org/gitaly/internal/rubyserver" "gitlab.com/gitlab-org/gitaly/internal/testhelper" "gitlab.com/gitlab-org/gitaly/proto/go/gitalypb" "google.golang.org/grpc/codes" @@ -21,13 +20,7 @@ import ( ) var ( - rebaseUser = &gitalypb.User{ - Name: []byte("Ahmad Sherif"), - Email: []byte("ahmad@gitlab.com"), - GlId: "user-123", - } - - branchName = "many_files" + rebaseBranchName = "many_files" ) func TestSuccessfulUserRebaseConfirmableRequest(t *testing.T) { @@ -36,21 +29,28 @@ func TestSuccessfulUserRebaseConfirmableRequest(t *testing.T) { ctx, cancel := testhelper.Context() defer cancel() + var ruby rubyserver.Server + + pushOptions := []string{"ci.skip", "test=value"} + cleanupSrv := setupAndStartGitlabServer(t, testhelper.GlID, "project-1", pushOptions...) + defer cleanupSrv() + + require.NoError(t, ruby.Start()) + defer ruby.Stop() + + serverSocketPath, stop := runOperationServiceServerWithRubyServer(t, &ruby) + defer stop() + for _, features := range featureSet { t.Run(features.String(), func(t *testing.T) { ctx = features.WithParent(ctx) - testSuccessfulUserRebaseConfirmableRequest(t, ctx) + testSuccessfulUserRebaseConfirmableRequest(t, ctx, serverSocketPath, pushOptions) }) } } -func testSuccessfulUserRebaseConfirmableRequest(t *testing.T, ctxOuter context.Context) { - pushOptions := []string{"ci.skip", "test=value"} - - serverSocketPath, stop := operations.RunOperationServiceServer(t) - defer stop() - - client, conn := operations.NewOperationClient(t, serverSocketPath) +func testSuccessfulUserRebaseConfirmableRequest(t *testing.T, ctxOuter context.Context, serverSocketPath string, pushOptions []string) { + client, conn := newOperationClient(t, serverSocketPath) defer conn.Close() testRepo, testRepoPath, cleanup := testhelper.NewTestRepo(t) @@ -59,10 +59,7 @@ func testSuccessfulUserRebaseConfirmableRequest(t *testing.T, ctxOuter context.C testRepoCopy, _, cleanup := testhelper.NewTestRepo(t) defer cleanup() - cleanupSrv := operations.SetupAndStartGitlabServer(t, rebaseUser.GlId, testRepo.GlRepository, pushOptions...) - defer cleanupSrv() - - branchSha := getBranchSha(t, testRepoPath, branchName) + branchSha := getBranchSha(t, testRepoPath, rebaseBranchName) md := testhelper.GitalyServersMetadata(t, serverSocketPath) ctx := metadata.NewOutgoingContext(ctxOuter, md) @@ -75,7 +72,7 @@ func testSuccessfulUserRebaseConfirmableRequest(t *testing.T, ctxOuter context.C defer removePreReceive() defer removePostReceive() - headerRequest := buildHeaderRequest(testRepo, rebaseUser, "1", branchName, branchSha, testRepoCopy, "master") + headerRequest := buildHeaderRequest(testRepo, testhelper.TestUser, "1", rebaseBranchName, branchSha, testRepoCopy, "master") headerRequest.GetHeader().GitPushOptions = pushOptions require.NoError(t, rebaseStream.Send(headerRequest), "send header") @@ -97,7 +94,7 @@ func testSuccessfulUserRebaseConfirmableRequest(t *testing.T, ctxOuter context.C }) require.NoError(t, err, "consume EOF") - newBranchSha := getBranchSha(t, testRepoPath, branchName) + newBranchSha := getBranchSha(t, testRepoPath, rebaseBranchName) require.NotEqual(t, newBranchSha, branchSha) require.Equal(t, newBranchSha, firstResponse.GetRebaseSha()) @@ -116,22 +113,19 @@ func TestFailedRebaseUserRebaseConfirmableRequestDueToInvalidHeader(t *testing.T ctxOuter, cancel := testhelper.Context() defer cancel() - serverSocketPath, stop := operations.RunOperationServiceServer(t) + serverSocketPath, stop := runOperationServiceServer(t) defer stop() - client, conn := operations.NewOperationClient(t, serverSocketPath) + client, conn := newOperationClient(t, serverSocketPath) defer conn.Close() testRepo, testRepoPath, cleanup := testhelper.NewTestRepo(t) defer cleanup() - cleanupSrv := operations.SetupAndStartGitlabServer(t, rebaseUser.GlId, testRepo.GlRepository) - defer cleanupSrv() - testRepoCopy, _, cleanup := testhelper.NewTestRepo(t) defer cleanup() - branchSha := getBranchSha(t, testRepoPath, branchName) + branchSha := getBranchSha(t, testRepoPath, rebaseBranchName) md := testhelper.GitalyServersMetadata(t, serverSocketPath) ctx := metadata.NewOutgoingContext(ctxOuter, md) @@ -142,31 +136,31 @@ func TestFailedRebaseUserRebaseConfirmableRequestDueToInvalidHeader(t *testing.T }{ { desc: "empty Repository", - req: buildHeaderRequest(nil, rebaseUser, "1", branchName, branchSha, testRepoCopy, "master"), + req: buildHeaderRequest(nil, testhelper.TestUser, "1", rebaseBranchName, branchSha, testRepoCopy, "master"), }, { desc: "empty User", - req: buildHeaderRequest(testRepo, nil, "1", branchName, branchSha, testRepoCopy, "master"), + req: buildHeaderRequest(testRepo, nil, "1", rebaseBranchName, branchSha, testRepoCopy, "master"), }, { desc: "empty Branch", - req: buildHeaderRequest(testRepo, rebaseUser, "1", "", branchSha, testRepoCopy, "master"), + req: buildHeaderRequest(testRepo, testhelper.TestUser, "1", "", branchSha, testRepoCopy, "master"), }, { desc: "empty BranchSha", - req: buildHeaderRequest(testRepo, rebaseUser, "1", branchName, "", testRepoCopy, "master"), + req: buildHeaderRequest(testRepo, testhelper.TestUser, "1", rebaseBranchName, "", testRepoCopy, "master"), }, { desc: "empty RemoteRepository", - req: buildHeaderRequest(testRepo, rebaseUser, "1", branchName, branchSha, nil, "master"), + req: buildHeaderRequest(testRepo, testhelper.TestUser, "1", rebaseBranchName, branchSha, nil, "master"), }, { desc: "empty RemoteBranch", - req: buildHeaderRequest(testRepo, rebaseUser, "1", branchName, branchSha, testRepoCopy, ""), + req: buildHeaderRequest(testRepo, testhelper.TestUser, "1", rebaseBranchName, branchSha, testRepoCopy, ""), }, { desc: "invalid branch name", - req: buildHeaderRequest(testRepo, rebaseUser, "1", branchName, branchSha, testRepoCopy, "+dev:master"), + req: buildHeaderRequest(testRepo, testhelper.TestUser, "1", rebaseBranchName, branchSha, testRepoCopy, "+dev:master"), }, } @@ -189,10 +183,10 @@ func TestAbortedUserRebaseConfirmable(t *testing.T) { ctxOuter, cancel := testhelper.Context() defer cancel() - serverSocketPath, stop := operations.RunOperationServiceServer(t) + serverSocketPath, stop := runOperationServiceServer(t) defer stop() - client, conn := operations.NewOperationClient(t, serverSocketPath) + client, conn := newOperationClient(t, serverSocketPath) defer conn.Close() md := testhelper.GitalyServersMetadata(t, serverSocketPath) @@ -214,15 +208,12 @@ func TestAbortedUserRebaseConfirmable(t *testing.T) { testRepo, testRepoPath, cleanup := testhelper.NewTestRepo(t) defer cleanup() - cleanupSrv := operations.SetupAndStartGitlabServer(t, rebaseUser.GlId, testRepo.GlRepository) - defer cleanupSrv() - testRepoCopy, _, cleanup := testhelper.NewTestRepo(t) defer cleanup() - branchSha := getBranchSha(t, testRepoPath, branchName) + branchSha := getBranchSha(t, testRepoPath, rebaseBranchName) - headerRequest := buildHeaderRequest(testRepo, rebaseUser, fmt.Sprintf("%v", i), branchName, branchSha, testRepoCopy, "master") + headerRequest := buildHeaderRequest(testRepo, testhelper.TestUser, fmt.Sprintf("%v", i), rebaseBranchName, branchSha, testRepoCopy, "master") rebaseStream, err := client.UserRebaseConfirmable(ctx) require.NoError(t, err) @@ -241,7 +232,7 @@ func TestAbortedUserRebaseConfirmable(t *testing.T) { require.NoError(t, rebaseStream.CloseSend(), "close request stream from client") } - secondResponse, err := recvTimeout(rebaseStream, 1*time.Second) + secondResponse, err := rebaseRecvTimeout(rebaseStream, 1*time.Second) if err == errRecvTimeout { t.Fatal(err) } @@ -250,7 +241,7 @@ func TestAbortedUserRebaseConfirmable(t *testing.T) { require.Error(t, err) testhelper.RequireGrpcError(t, err, tc.code) - newBranchSha := getBranchSha(t, testRepoPath, branchName) + newBranchSha := getBranchSha(t, testRepoPath, rebaseBranchName) require.Equal(t, newBranchSha, branchSha, "branch should not change when the rebase is aborted") }) } @@ -260,10 +251,10 @@ func TestFailedUserRebaseConfirmableDueToApplyBeingFalse(t *testing.T) { ctxOuter, cancel := testhelper.Context() defer cancel() - serverSocketPath, stop := operations.RunOperationServiceServer(t) + serverSocketPath, stop := runOperationServiceServer(t) defer stop() - client, conn := operations.NewOperationClient(t, serverSocketPath) + client, conn := newOperationClient(t, serverSocketPath) defer conn.Close() testRepo, testRepoPath, cleanup := testhelper.NewTestRepo(t) @@ -272,10 +263,7 @@ func TestFailedUserRebaseConfirmableDueToApplyBeingFalse(t *testing.T) { testRepoCopy, _, cleanup := testhelper.NewTestRepo(t) defer cleanup() - cleanupSrv := operations.SetupAndStartGitlabServer(t, rebaseUser.GlId, testRepo.GlRepository) - defer cleanupSrv() - - branchSha := getBranchSha(t, testRepoPath, branchName) + branchSha := getBranchSha(t, testRepoPath, rebaseBranchName) md := testhelper.GitalyServersMetadata(t, serverSocketPath) ctx := metadata.NewOutgoingContext(ctxOuter, md) @@ -283,7 +271,7 @@ func TestFailedUserRebaseConfirmableDueToApplyBeingFalse(t *testing.T) { rebaseStream, err := client.UserRebaseConfirmable(ctx) require.NoError(t, err) - headerRequest := buildHeaderRequest(testRepo, rebaseUser, "1", branchName, branchSha, testRepoCopy, "master") + headerRequest := buildHeaderRequest(testRepo, testhelper.TestUser, "1", rebaseBranchName, branchSha, testRepoCopy, "master") require.NoError(t, rebaseStream.Send(headerRequest), "send header") firstResponse, err := rebaseStream.Recv() @@ -300,7 +288,7 @@ func TestFailedUserRebaseConfirmableDueToApplyBeingFalse(t *testing.T) { testhelper.RequireGrpcError(t, err, codes.FailedPrecondition) require.False(t, secondResponse.GetRebaseApplied(), "the second rebase is not applied") - newBranchSha := getBranchSha(t, testRepoPath, branchName) + newBranchSha := getBranchSha(t, testRepoPath, rebaseBranchName) require.Equal(t, branchSha, newBranchSha, "branch should not change when the rebase is not applied") require.NotEqual(t, newBranchSha, firstResponse.GetRebaseSha(), "branch should not be the sha returned when the rebase is not applied") } @@ -309,10 +297,10 @@ func TestFailedUserRebaseConfirmableRequestDueToPreReceiveError(t *testing.T) { ctxOuter, cancel := testhelper.Context() defer cancel() - serverSocketPath, stop := operations.RunOperationServiceServer(t) + serverSocketPath, stop := runOperationServiceServer(t) defer stop() - client, conn := operations.NewOperationClient(t, serverSocketPath) + client, conn := newOperationClient(t, serverSocketPath) defer conn.Close() testRepo, testRepoPath, cleanup := testhelper.NewTestRepo(t) @@ -321,14 +309,11 @@ func TestFailedUserRebaseConfirmableRequestDueToPreReceiveError(t *testing.T) { testRepoCopy, _, cleanup := testhelper.NewTestRepo(t) defer cleanup() - cleanupSrv := operations.SetupAndStartGitlabServer(t, rebaseUser.GlId, testRepo.GlRepository) - defer cleanupSrv() - - branchSha := getBranchSha(t, testRepoPath, branchName) + branchSha := getBranchSha(t, testRepoPath, rebaseBranchName) hookContent := []byte("#!/bin/sh\necho 'failure'\nexit 1") - for i, hookName := range operations.GitlabPreHooks { + for i, hookName := range GitlabPreHooks { t.Run(hookName, func(t *testing.T) { remove, err := testhelper.WriteCustomHook(testRepoPath, hookName, hookContent) require.NoError(t, err, "set up hooks override") @@ -340,7 +325,7 @@ func TestFailedUserRebaseConfirmableRequestDueToPreReceiveError(t *testing.T) { rebaseStream, err := client.UserRebaseConfirmable(ctx) require.NoError(t, err) - headerRequest := buildHeaderRequest(testRepo, rebaseUser, fmt.Sprintf("%v", i), branchName, branchSha, testRepoCopy, "master") + headerRequest := buildHeaderRequest(testRepo, testhelper.TestUser, fmt.Sprintf("%v", i), rebaseBranchName, branchSha, testRepoCopy, "master") require.NoError(t, rebaseStream.Send(headerRequest), "send header") firstResponse, err := rebaseStream.Recv() @@ -363,7 +348,7 @@ func TestFailedUserRebaseConfirmableRequestDueToPreReceiveError(t *testing.T) { }) require.NoError(t, err, "consume EOF") - newBranchSha := getBranchSha(t, testRepoPath, branchName) + newBranchSha := getBranchSha(t, testRepoPath, rebaseBranchName) require.Equal(t, branchSha, newBranchSha, "branch should not change when the rebase fails due to PreReceiveError") require.NotEqual(t, newBranchSha, firstResponse.GetRebaseSha(), "branch should not be the sha returned when the rebase fails due to PreReceiveError") }) @@ -374,10 +359,10 @@ func TestFailedUserRebaseConfirmableDueToGitError(t *testing.T) { ctxOuter, cancel := testhelper.Context() defer cancel() - serverSocketPath, stop := operations.RunOperationServiceServer(t) + serverSocketPath, stop := runOperationServiceServer(t) defer stop() - client, conn := operations.NewOperationClient(t, serverSocketPath) + client, conn := newOperationClient(t, serverSocketPath) defer conn.Close() testRepo, testRepoPath, cleanup := testhelper.NewTestRepo(t) @@ -395,7 +380,7 @@ func TestFailedUserRebaseConfirmableDueToGitError(t *testing.T) { rebaseStream, err := client.UserRebaseConfirmable(ctx) require.NoError(t, err) - headerRequest := buildHeaderRequest(testRepo, rebaseUser, "1", failedBranchName, branchSha, testRepoCopy, "master") + headerRequest := buildHeaderRequest(testRepo, testhelper.TestUser, "1", failedBranchName, branchSha, testRepoCopy, "master") require.NoError(t, rebaseStream.Send(headerRequest), "send header") firstResponse, err := rebaseStream.Recv() @@ -421,10 +406,10 @@ func TestRebaseRequestWithDeletedFile(t *testing.T) { ctxOuter, cancel := testhelper.Context() defer cancel() - serverSocketPath, stop := operations.RunOperationServiceServer(t) + serverSocketPath, stop := runOperationServiceServer(t) defer stop() - client, conn := operations.NewOperationClient(t, serverSocketPath) + client, conn := newOperationClient(t, serverSocketPath) defer conn.Close() testRepo, testRepoPath, cleanupFn := testhelper.NewTestRepoWithWorktree(t) @@ -433,16 +418,13 @@ func TestRebaseRequestWithDeletedFile(t *testing.T) { testRepoCopy, _, cleanup := testhelper.NewTestRepo(t) defer cleanup() - cleanupSrv := operations.SetupAndStartGitlabServer(t, rebaseUser.GlId, testRepo.GlRepository) - defer cleanupSrv() - md := testhelper.GitalyServersMetadata(t, serverSocketPath) ctx := metadata.NewOutgoingContext(ctxOuter, md) branch := "rebase-delete-test" - testhelper.MustRunCommand(t, nil, "git", "-C", testRepoPath, "config", "user.name", string(rebaseUser.Name)) - testhelper.MustRunCommand(t, nil, "git", "-C", testRepoPath, "config", "user.email", string(rebaseUser.Email)) + testhelper.MustRunCommand(t, nil, "git", "-C", testRepoPath, "config", "user.name", string(testhelper.TestUser.Name)) + testhelper.MustRunCommand(t, nil, "git", "-C", testRepoPath, "config", "user.email", string(testhelper.TestUser.Email)) testhelper.MustRunCommand(t, nil, "git", "-C", testRepoPath, "checkout", "-b", branch, "master~1") testhelper.MustRunCommand(t, nil, "git", "-C", testRepoPath, "rm", "README") testhelper.MustRunCommand(t, nil, "git", "-C", testRepoPath, "commit", "-a", "-m", "delete file") @@ -452,7 +434,7 @@ func TestRebaseRequestWithDeletedFile(t *testing.T) { rebaseStream, err := client.UserRebaseConfirmable(ctx) require.NoError(t, err) - headerRequest := buildHeaderRequest(testRepo, rebaseUser, "1", branch, branchSha, testRepoCopy, "master") + headerRequest := buildHeaderRequest(testRepo, testhelper.TestUser, "1", branch, branchSha, testRepoCopy, "master") require.NoError(t, rebaseStream.Send(headerRequest), "send header") firstResponse, err := rebaseStream.Recv() @@ -481,10 +463,7 @@ func TestRebaseRequestWithDeletedFile(t *testing.T) { require.True(t, secondResponse.GetRebaseApplied(), "the second rebase is applied") } -// This error is used as a sentinel value -var errRecvTimeout = errors.New("timeout waiting for response") - -func recvTimeout(bidi gitalypb.OperationService_UserRebaseConfirmableClient, timeout time.Duration) (*gitalypb.UserRebaseConfirmableResponse, error) { +func rebaseRecvTimeout(bidi gitalypb.OperationService_UserRebaseConfirmableClient, timeout time.Duration) (*gitalypb.UserRebaseConfirmableResponse, error) { type responseError struct { response *gitalypb.UserRebaseConfirmableResponse err error diff --git a/internal/service/operations/revert_test.go b/internal/service/operations/revert_test.go index 21adc720f..bb8845478 100644 --- a/internal/service/operations/revert_test.go +++ b/internal/service/operations/revert_test.go @@ -1,4 +1,4 @@ -package operations_test +package operations import ( "context" @@ -7,7 +7,6 @@ import ( "github.com/stretchr/testify/require" "gitlab.com/gitlab-org/gitaly/internal/git/log" "gitlab.com/gitlab-org/gitaly/internal/metadata/featureflag" - "gitlab.com/gitlab-org/gitaly/internal/service/operations" "gitlab.com/gitlab-org/gitaly/internal/testhelper" "gitlab.com/gitlab-org/gitaly/proto/go/gitalypb" "google.golang.org/grpc/codes" @@ -18,10 +17,10 @@ func TestSuccessfulUserRevertRequest(t *testing.T) { ctxOuter, cancel := testhelper.Context() defer cancel() - serverSocketPath, stop := operations.RunOperationServiceServer(t) + serverSocketPath, stop := runOperationServiceServer(t) defer stop() - client, conn := operations.NewOperationClient(t, serverSocketPath) + client, conn := newOperationClient(t, serverSocketPath) defer conn.Close() testRepo, testRepoPath, cleanup := testhelper.NewTestRepo(t) @@ -33,15 +32,6 @@ func TestSuccessfulUserRevertRequest(t *testing.T) { masterHeadCommit, err := log.GetCommit(ctxOuter, testRepo, "master") require.NoError(t, err) - user := &gitalypb.User{ - Name: []byte("Ahmad Sherif"), - Email: []byte("ahmad@gitlab.com"), - GlId: "user-123", - } - - cleanupSrv := operations.SetupAndStartGitlabServer(t, user.GlId, testRepo.GlRepository) - defer cleanupSrv() - revertedCommit, err := log.GetCommit(ctxOuter, testRepo, "d59c60028b053793cecfb4022de34602e1a9218e") require.NoError(t, err) @@ -57,7 +47,7 @@ func TestSuccessfulUserRevertRequest(t *testing.T) { desc: "branch exists", request: &gitalypb.UserRevertRequest{ Repository: testRepo, - User: user, + User: testhelper.TestUser, Commit: revertedCommit, BranchName: []byte(destinationBranch), Message: []byte("Reverting " + revertedCommit.Id), @@ -68,7 +58,7 @@ func TestSuccessfulUserRevertRequest(t *testing.T) { desc: "nonexistent branch + start_repository == repository", request: &gitalypb.UserRevertRequest{ Repository: testRepo, - User: user, + User: testhelper.TestUser, Commit: revertedCommit, BranchName: []byte("to-be-reverted-into-1"), Message: []byte("Reverting " + revertedCommit.Id), @@ -80,7 +70,7 @@ func TestSuccessfulUserRevertRequest(t *testing.T) { desc: "nonexistent branch + start_repository != repository", request: &gitalypb.UserRevertRequest{ Repository: testRepo, - User: user, + User: testhelper.TestUser, Commit: revertedCommit, BranchName: []byte("to-be-reverted-into-2"), Message: []byte("Reverting " + revertedCommit.Id), @@ -93,7 +83,7 @@ func TestSuccessfulUserRevertRequest(t *testing.T) { desc: "nonexistent branch + empty start_repository", request: &gitalypb.UserRevertRequest{ Repository: testRepo, - User: user, + User: testhelper.TestUser, Commit: revertedCommit, BranchName: []byte("to-be-reverted-into-3"), Message: []byte("Reverting " + revertedCommit.Id), @@ -141,10 +131,10 @@ func TestSuccessfulGitHooksForUserRevertRequest(t *testing.T) { } func testSuccessfulGitHooksForUserRevertRequest(t *testing.T, ctxOuter context.Context) { - serverSocketPath, stop := operations.RunOperationServiceServer(t) + serverSocketPath, stop := runOperationServiceServer(t) defer stop() - client, conn := operations.NewOperationClient(t, serverSocketPath) + client, conn := newOperationClient(t, serverSocketPath) defer conn.Close() testRepo, testRepoPath, cleanup := testhelper.NewTestRepo(t) @@ -153,29 +143,19 @@ func testSuccessfulGitHooksForUserRevertRequest(t *testing.T, ctxOuter context.C destinationBranch := "revert-dst" testhelper.MustRunCommand(t, nil, "git", "-C", testRepoPath, "branch", destinationBranch, "master") - user := &gitalypb.User{ - Name: []byte("Ahmad Sherif"), - Email: []byte("ahmad@gitlab.com"), - GlId: "user-123", - GlUsername: "username-223", - } - - cleanupSrv := operations.SetupAndStartGitlabServer(t, user.GlId, testRepo.GlRepository) - defer cleanupSrv() - revertedCommit, err := log.GetCommit(ctxOuter, testRepo, "d59c60028b053793cecfb4022de34602e1a9218e") require.NoError(t, err) request := &gitalypb.UserRevertRequest{ Repository: testRepo, - User: user, + User: testhelper.TestUser, Commit: revertedCommit, BranchName: []byte(destinationBranch), Message: []byte("Reverting " + revertedCommit.Id), } var hookOutputFiles []string - for _, hookName := range operations.GitlabHooks { + for _, hookName := range GitlabHooks { hookOutputTempPath, cleanup := testhelper.WriteEnvToCustomHook(t, testRepoPath, hookName) defer cleanup() hookOutputFiles = append(hookOutputFiles, hookOutputTempPath) @@ -190,7 +170,7 @@ func testSuccessfulGitHooksForUserRevertRequest(t *testing.T, ctxOuter context.C for _, file := range hookOutputFiles { output := string(testhelper.MustReadFile(t, file)) - require.Contains(t, output, "GL_USERNAME="+user.GlUsername) + require.Contains(t, output, "GL_USERNAME="+testhelper.TestUser.GlUsername) } } @@ -198,10 +178,10 @@ func TestFailedUserRevertRequestDueToValidations(t *testing.T) { ctxOuter, cancel := testhelper.Context() defer cancel() - serverSocketPath, stop := operations.RunOperationServiceServer(t) + serverSocketPath, stop := runOperationServiceServer(t) defer stop() - client, conn := operations.NewOperationClient(t, serverSocketPath) + client, conn := newOperationClient(t, serverSocketPath) defer conn.Close() testRepo, _, cleanup := testhelper.NewTestRepo(t) @@ -212,12 +192,6 @@ func TestFailedUserRevertRequestDueToValidations(t *testing.T) { destinationBranch := "revert-dst" - user := &gitalypb.User{ - Name: []byte("Ahmad Sherif"), - Email: []byte("ahmad@gitlab.com"), - GlId: "user-123", - } - testCases := []struct { desc string request *gitalypb.UserRevertRequest @@ -238,7 +212,7 @@ func TestFailedUserRevertRequestDueToValidations(t *testing.T) { desc: "empty commit", request: &gitalypb.UserRevertRequest{ Repository: testRepo, - User: user, + User: testhelper.TestUser, Commit: nil, BranchName: []byte(destinationBranch), Message: []byte("Reverting " + revertedCommit.Id), @@ -249,7 +223,7 @@ func TestFailedUserRevertRequestDueToValidations(t *testing.T) { desc: "empty branch name", request: &gitalypb.UserRevertRequest{ Repository: testRepo, - User: user, + User: testhelper.TestUser, Commit: revertedCommit, BranchName: nil, Message: []byte("Reverting " + revertedCommit.Id), @@ -260,7 +234,7 @@ func TestFailedUserRevertRequestDueToValidations(t *testing.T) { desc: "empty message", request: &gitalypb.UserRevertRequest{ Repository: testRepo, - User: user, + User: testhelper.TestUser, Commit: revertedCommit, BranchName: []byte(destinationBranch), Message: nil, @@ -284,10 +258,10 @@ func TestFailedUserRevertRequestDueToPreReceiveError(t *testing.T) { ctxOuter, cancel := testhelper.Context() defer cancel() - serverSocketPath, stop := operations.RunOperationServiceServer(t) + serverSocketPath, stop := runOperationServiceServer(t) defer stop() - client, conn := operations.NewOperationClient(t, serverSocketPath) + client, conn := newOperationClient(t, serverSocketPath) defer conn.Close() testRepo, testRepoPath, cleanup := testhelper.NewTestRepo(t) @@ -296,21 +270,12 @@ func TestFailedUserRevertRequestDueToPreReceiveError(t *testing.T) { destinationBranch := "revert-dst" testhelper.MustRunCommand(t, nil, "git", "-C", testRepoPath, "branch", destinationBranch, "master") - user := &gitalypb.User{ - Name: []byte("Ahmad Sherif"), - Email: []byte("ahmad@gitlab.com"), - GlId: "user-123", - } - - cleanupSrv := operations.SetupAndStartGitlabServer(t, user.GlId, testRepo.GlRepository) - defer cleanupSrv() - revertedCommit, err := log.GetCommit(ctxOuter, testRepo, "d59c60028b053793cecfb4022de34602e1a9218e") require.NoError(t, err) request := &gitalypb.UserRevertRequest{ Repository: testRepo, - User: user, + User: testhelper.TestUser, Commit: revertedCommit, BranchName: []byte(destinationBranch), Message: []byte("Reverting " + revertedCommit.Id), @@ -318,7 +283,7 @@ func TestFailedUserRevertRequestDueToPreReceiveError(t *testing.T) { hookContent := []byte("#!/bin/sh\necho GL_ID=$GL_ID\nexit 1") - for _, hookName := range operations.GitlabPreHooks { + for _, hookName := range GitlabPreHooks { t.Run(hookName, func(t *testing.T) { remove, err := testhelper.WriteCustomHook(testRepoPath, hookName, hookContent) require.NoError(t, err) @@ -329,7 +294,7 @@ func TestFailedUserRevertRequestDueToPreReceiveError(t *testing.T) { response, err := client.UserRevert(ctx, request) require.NoError(t, err) - require.Contains(t, response.PreReceiveError, "GL_ID="+user.GlId) + require.Contains(t, response.PreReceiveError, "GL_ID="+testhelper.TestUser.GlId) }) } } @@ -338,10 +303,10 @@ func TestFailedUserRevertRequestDueToCreateTreeError(t *testing.T) { ctxOuter, cancel := testhelper.Context() defer cancel() - serverSocketPath, stop := operations.RunOperationServiceServer(t) + serverSocketPath, stop := runOperationServiceServer(t) defer stop() - client, conn := operations.NewOperationClient(t, serverSocketPath) + client, conn := newOperationClient(t, serverSocketPath) defer conn.Close() testRepo, testRepoPath, cleanup := testhelper.NewTestRepo(t) @@ -350,19 +315,13 @@ func TestFailedUserRevertRequestDueToCreateTreeError(t *testing.T) { destinationBranch := "revert-dst" testhelper.MustRunCommand(t, nil, "git", "-C", testRepoPath, "branch", destinationBranch, "master") - user := &gitalypb.User{ - Name: []byte("Ahmad Sherif"), - Email: []byte("ahmad@gitlab.com"), - GlId: "user-123", - } - // This revert patch of the following commit cannot be applied to the destinationBranch above revertedCommit, err := log.GetCommit(ctxOuter, testRepo, "372ab6950519549b14d220271ee2322caa44d4eb") require.NoError(t, err) request := &gitalypb.UserRevertRequest{ Repository: testRepo, - User: user, + User: testhelper.TestUser, Commit: revertedCommit, BranchName: []byte(destinationBranch), Message: []byte("Reverting " + revertedCommit.Id), @@ -381,10 +340,10 @@ func TestFailedUserRevertRequestDueToCommitError(t *testing.T) { ctxOuter, cancel := testhelper.Context() defer cancel() - serverSocketPath, stop := operations.RunOperationServiceServer(t) + serverSocketPath, stop := runOperationServiceServer(t) defer stop() - client, conn := operations.NewOperationClient(t, serverSocketPath) + client, conn := newOperationClient(t, serverSocketPath) defer conn.Close() testRepo, testRepoPath, cleanup := testhelper.NewTestRepo(t) @@ -395,18 +354,12 @@ func TestFailedUserRevertRequestDueToCommitError(t *testing.T) { testhelper.MustRunCommand(t, nil, "git", "-C", testRepoPath, "branch", destinationBranch, "master") testhelper.MustRunCommand(t, nil, "git", "-C", testRepoPath, "branch", sourceBranch, "a5391128b0ef5d21df5dd23d98557f4ef12fae20") - user := &gitalypb.User{ - Name: []byte("Ahmad Sherif"), - Email: []byte("ahmad@gitlab.com"), - GlId: "user-123", - } - revertedCommit, err := log.GetCommit(ctxOuter, testRepo, sourceBranch) require.NoError(t, err) request := &gitalypb.UserRevertRequest{ Repository: testRepo, - User: user, + User: testhelper.TestUser, Commit: revertedCommit, BranchName: []byte(destinationBranch), Message: []byte("Reverting " + revertedCommit.Id), diff --git a/internal/service/operations/squash_test.go b/internal/service/operations/squash_test.go index b4e235e5a..6787bfe48 100644 --- a/internal/service/operations/squash_test.go +++ b/internal/service/operations/squash_test.go @@ -46,7 +46,7 @@ func testSuccessfulUserSquashRequest(t *testing.T, ctx context.Context) { serverSocketPath, stop := runOperationServiceServer(t) defer stop() - client, conn := NewOperationClient(t, serverSocketPath) + client, conn := newOperationClient(t, serverSocketPath) defer conn.Close() testRepo, _, cleanup := testhelper.NewTestRepo(t) @@ -54,7 +54,7 @@ func testSuccessfulUserSquashRequest(t *testing.T, ctx context.Context) { request := &gitalypb.UserSquashRequest{ Repository: testRepo, - User: user, + User: testhelper.TestUser, SquashId: "1", Author: author, CommitMessage: commitMessage, @@ -71,8 +71,8 @@ func testSuccessfulUserSquashRequest(t *testing.T, ctx context.Context) { require.Equal(t, []string{startSha}, commit.ParentIds) require.Equal(t, author.Name, commit.Author.Name) require.Equal(t, author.Email, commit.Author.Email) - require.Equal(t, user.Name, commit.Committer.Name) - require.Equal(t, user.Email, commit.Committer.Email) + require.Equal(t, testhelper.TestUser.Name, commit.Committer.Name) + require.Equal(t, testhelper.TestUser.Email, commit.Committer.Email) require.Equal(t, commitMessage, commit.Subject) } @@ -96,7 +96,7 @@ func TestSuccessfulUserSquashRequestWith3wayMerge(t *testing.T) { serverSocketPath, stop := runOperationServiceServer(t) defer stop() - client, conn := NewOperationClient(t, serverSocketPath) + client, conn := newOperationClient(t, serverSocketPath) defer conn.Close() testRepo, testRepoPath, cleanupFn := testhelper.NewTestRepo(t) @@ -104,7 +104,7 @@ func TestSuccessfulUserSquashRequestWith3wayMerge(t *testing.T) { request := &gitalypb.UserSquashRequest{ Repository: testRepo, - User: user, + User: testhelper.TestUser, SquashId: "1", Author: author, CommitMessage: commitMessage, @@ -122,8 +122,8 @@ func TestSuccessfulUserSquashRequestWith3wayMerge(t *testing.T) { require.Equal(t, []string{"6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9"}, commit.ParentIds) require.Equal(t, author.Name, commit.Author.Name) require.Equal(t, author.Email, commit.Author.Email) - require.Equal(t, user.Name, commit.Committer.Name) - require.Equal(t, user.Email, commit.Committer.Email) + require.Equal(t, testhelper.TestUser.Name, commit.Committer.Name) + require.Equal(t, testhelper.TestUser.Email, commit.Committer.Email) require.Equal(t, commitMessage, commit.Subject) // Ensure Git metadata is cleaned up @@ -144,7 +144,7 @@ func TestSplitIndex(t *testing.T) { serverSocketPath, stop := runOperationServiceServer(t) defer stop() - client, conn := NewOperationClient(t, serverSocketPath) + client, conn := newOperationClient(t, serverSocketPath) defer conn.Close() testRepo, testRepoPath, cleanup := testhelper.NewTestRepo(t) @@ -154,7 +154,7 @@ func TestSplitIndex(t *testing.T) { request := &gitalypb.UserSquashRequest{ Repository: testRepo, - User: user, + User: testhelper.TestUser, SquashId: "1", Author: author, CommitMessage: commitMessage, @@ -175,7 +175,7 @@ func TestSquashRequestWithRenamedFiles(t *testing.T) { serverSocketPath, stop := runOperationServiceServer(t) defer stop() - client, conn := NewOperationClient(t, serverSocketPath) + client, conn := newOperationClient(t, serverSocketPath) defer conn.Close() testRepo, testRepoPath, cleanupFn := testhelper.NewTestRepoWithWorktree(t) @@ -184,8 +184,8 @@ func TestSquashRequestWithRenamedFiles(t *testing.T) { originalFilename := "original-file.txt" renamedFilename := "renamed-file.txt" - testhelper.MustRunCommand(t, nil, "git", "-C", testRepoPath, "config", "user.name", string(author.Name)) - testhelper.MustRunCommand(t, nil, "git", "-C", testRepoPath, "config", "user.email", string(author.Email)) + testhelper.MustRunCommand(t, nil, "git", "-C", testRepoPath, "config", "testhelper.TestUser.name", string(author.Name)) + testhelper.MustRunCommand(t, nil, "git", "-C", testRepoPath, "config", "testhelper.TestUser.email", string(author.Email)) testhelper.MustRunCommand(t, nil, "git", "-C", testRepoPath, "checkout", "-b", "squash-rename-test", "master") require.NoError(t, ioutil.WriteFile(filepath.Join(testRepoPath, originalFilename), []byte("This is a test"), 0644)) testhelper.MustRunCommand(t, nil, "git", "-C", testRepoPath, "add", ".") @@ -208,7 +208,7 @@ func TestSquashRequestWithRenamedFiles(t *testing.T) { request := &gitalypb.UserSquashRequest{ Repository: testRepo, - User: user, + User: testhelper.TestUser, SquashId: "1", Author: author, CommitMessage: commitMessage, @@ -225,8 +225,8 @@ func TestSquashRequestWithRenamedFiles(t *testing.T) { require.Equal(t, []string{startCommitID}, commit.ParentIds) require.Equal(t, author.Name, commit.Author.Name) require.Equal(t, author.Email, commit.Author.Email) - require.Equal(t, user.Name, commit.Committer.Name) - require.Equal(t, user.Email, commit.Committer.Email) + require.Equal(t, testhelper.TestUser.Name, commit.Committer.Name) + require.Equal(t, testhelper.TestUser.Email, commit.Committer.Email) require.Equal(t, commitMessage, commit.Subject) } @@ -237,7 +237,7 @@ func TestSuccessfulUserSquashRequestWithMissingFileOnTargetBranch(t *testing.T) serverSocketPath, stop := runOperationServiceServer(t) defer stop() - client, conn := NewOperationClient(t, serverSocketPath) + client, conn := newOperationClient(t, serverSocketPath) defer conn.Close() testRepo, _, cleanup := testhelper.NewTestRepo(t) @@ -247,7 +247,7 @@ func TestSuccessfulUserSquashRequestWithMissingFileOnTargetBranch(t *testing.T) request := &gitalypb.UserSquashRequest{ Repository: testRepo, - User: user, + User: testhelper.TestUser, SquashId: "1", Author: author, CommitMessage: commitMessage, @@ -264,7 +264,7 @@ func TestFailedUserSquashRequestDueToValidations(t *testing.T) { serverSocketPath, stop := runOperationServiceServer(t) defer stop() - client, conn := NewOperationClient(t, serverSocketPath) + client, conn := newOperationClient(t, serverSocketPath) defer conn.Close() testRepo, _, cleanup := testhelper.NewTestRepo(t) @@ -279,9 +279,9 @@ func TestFailedUserSquashRequestDueToValidations(t *testing.T) { desc: "empty Repository", request: &gitalypb.UserSquashRequest{ Repository: nil, - User: user, + User: testhelper.TestUser, SquashId: "1", - Author: user, + Author: testhelper.TestUser, CommitMessage: commitMessage, StartSha: startSha, EndSha: endSha, @@ -294,7 +294,7 @@ func TestFailedUserSquashRequestDueToValidations(t *testing.T) { Repository: testRepo, User: nil, SquashId: "1", - Author: user, + Author: testhelper.TestUser, CommitMessage: commitMessage, StartSha: startSha, EndSha: endSha, @@ -305,9 +305,9 @@ func TestFailedUserSquashRequestDueToValidations(t *testing.T) { desc: "empty SquashId", request: &gitalypb.UserSquashRequest{ Repository: testRepo, - User: user, + User: testhelper.TestUser, SquashId: "", - Author: user, + Author: testhelper.TestUser, CommitMessage: commitMessage, StartSha: startSha, EndSha: endSha, @@ -318,9 +318,9 @@ func TestFailedUserSquashRequestDueToValidations(t *testing.T) { desc: "empty StartSha", request: &gitalypb.UserSquashRequest{ Repository: testRepo, - User: user, + User: testhelper.TestUser, SquashId: "1", - Author: user, + Author: testhelper.TestUser, CommitMessage: commitMessage, StartSha: "", EndSha: endSha, @@ -331,9 +331,9 @@ func TestFailedUserSquashRequestDueToValidations(t *testing.T) { desc: "empty EndSha", request: &gitalypb.UserSquashRequest{ Repository: testRepo, - User: user, + User: testhelper.TestUser, SquashId: "1", - Author: user, + Author: testhelper.TestUser, CommitMessage: commitMessage, StartSha: startSha, EndSha: "", @@ -344,7 +344,7 @@ func TestFailedUserSquashRequestDueToValidations(t *testing.T) { desc: "empty Author", request: &gitalypb.UserSquashRequest{ Repository: testRepo, - User: user, + User: testhelper.TestUser, SquashId: "1", Author: nil, CommitMessage: commitMessage, @@ -357,9 +357,9 @@ func TestFailedUserSquashRequestDueToValidations(t *testing.T) { desc: "empty CommitMessage", request: &gitalypb.UserSquashRequest{ Repository: testRepo, - User: user, + User: testhelper.TestUser, SquashId: "1", - Author: user, + Author: testhelper.TestUser, CommitMessage: nil, StartSha: startSha, EndSha: endSha, diff --git a/internal/service/operations/submodules_test.go b/internal/service/operations/submodules_test.go index c2a35b5ee..4553995f2 100644 --- a/internal/service/operations/submodules_test.go +++ b/internal/service/operations/submodules_test.go @@ -33,15 +33,12 @@ func testSuccessfulUserUpdateSubmoduleRequest(t *testing.T, ctx context.Context) serverSocketPath, stop := runOperationServiceServer(t) defer stop() - client, conn := NewOperationClient(t, serverSocketPath) + client, conn := newOperationClient(t, serverSocketPath) defer conn.Close() testRepo, testRepoPath, cleanup := testhelper.NewTestRepo(t) defer cleanup() - cleanupSrv := SetupAndStartGitlabServer(t, user.GlId, testRepo.GlRepository) - defer cleanupSrv() - commitMessage := []byte("Update Submodule message") testCases := []struct { @@ -68,7 +65,7 @@ func testSuccessfulUserUpdateSubmoduleRequest(t *testing.T, ctx context.Context) t.Run(testCase.desc, func(t *testing.T) { request := &gitalypb.UserUpdateSubmoduleRequest{ Repository: testRepo, - User: user, + User: testhelper.TestUser, Submodule: []byte(testCase.submodule), CommitSha: testCase.commitSha, Branch: []byte(testCase.branch), @@ -82,8 +79,8 @@ func testSuccessfulUserUpdateSubmoduleRequest(t *testing.T, ctx context.Context) commit, err := log.GetCommit(ctx, testRepo, response.BranchUpdate.CommitId) require.NoError(t, err) - require.Equal(t, commit.Author.Email, user.Email) - require.Equal(t, commit.Committer.Email, user.Email) + require.Equal(t, commit.Author.Email, testhelper.TestUser.Email) + require.Equal(t, commit.Committer.Email, testhelper.TestUser.Email) require.Equal(t, commit.Subject, commitMessage) entry := testhelper.MustRunCommand(t, nil, "git", "-C", testRepoPath, "ls-tree", "-z", fmt.Sprintf("%s^{tree}:", response.BranchUpdate.CommitId), testCase.submodule) @@ -100,7 +97,7 @@ func TestFailedUserUpdateSubmoduleRequestDueToValidations(t *testing.T) { serverSocketPath, stop := runOperationServiceServer(t) defer stop() - client, conn := NewOperationClient(t, serverSocketPath) + client, conn := newOperationClient(t, serverSocketPath) defer conn.Close() testRepo, _, cleanup := testhelper.NewTestRepo(t) @@ -115,7 +112,7 @@ func TestFailedUserUpdateSubmoduleRequestDueToValidations(t *testing.T) { desc: "empty Repository", request: &gitalypb.UserUpdateSubmoduleRequest{ Repository: nil, - User: user, + User: testhelper.TestUser, Submodule: []byte("six"), CommitSha: "db54006ff1c999fd485af44581dabe9b6c85a701", Branch: []byte("some-branch"), @@ -139,7 +136,7 @@ func TestFailedUserUpdateSubmoduleRequestDueToValidations(t *testing.T) { desc: "empty Submodule", request: &gitalypb.UserUpdateSubmoduleRequest{ Repository: testRepo, - User: user, + User: testhelper.TestUser, Submodule: nil, CommitSha: "db54006ff1c999fd485af44581dabe9b6c85a701", Branch: []byte("some-branch"), @@ -151,7 +148,7 @@ func TestFailedUserUpdateSubmoduleRequestDueToValidations(t *testing.T) { desc: "empty CommitSha", request: &gitalypb.UserUpdateSubmoduleRequest{ Repository: testRepo, - User: user, + User: testhelper.TestUser, Submodule: []byte("six"), CommitSha: "", Branch: []byte("some-branch"), @@ -163,7 +160,7 @@ func TestFailedUserUpdateSubmoduleRequestDueToValidations(t *testing.T) { desc: "invalid CommitSha", request: &gitalypb.UserUpdateSubmoduleRequest{ Repository: testRepo, - User: user, + User: testhelper.TestUser, Submodule: []byte("six"), CommitSha: "foobar", Branch: []byte("some-branch"), @@ -175,7 +172,7 @@ func TestFailedUserUpdateSubmoduleRequestDueToValidations(t *testing.T) { desc: "invalid CommitSha", request: &gitalypb.UserUpdateSubmoduleRequest{ Repository: testRepo, - User: user, + User: testhelper.TestUser, Submodule: []byte("six"), CommitSha: "db54006ff1c999fd485a", Branch: []byte("some-branch"), @@ -187,7 +184,7 @@ func TestFailedUserUpdateSubmoduleRequestDueToValidations(t *testing.T) { desc: "empty Branch", request: &gitalypb.UserUpdateSubmoduleRequest{ Repository: testRepo, - User: user, + User: testhelper.TestUser, Submodule: []byte("six"), CommitSha: "db54006ff1c999fd485af44581dabe9b6c85a701", Branch: nil, @@ -199,7 +196,7 @@ func TestFailedUserUpdateSubmoduleRequestDueToValidations(t *testing.T) { desc: "empty CommitMessage", request: &gitalypb.UserUpdateSubmoduleRequest{ Repository: testRepo, - User: user, + User: testhelper.TestUser, Submodule: []byte("six"), CommitSha: "db54006ff1c999fd485af44581dabe9b6c85a701", Branch: []byte("some-branch"), @@ -228,7 +225,7 @@ func TestFailedUserUpdateSubmoduleRequestDueToInvalidBranch(t *testing.T) { serverSocketPath, stop := runOperationServiceServer(t) defer stop() - client, conn := NewOperationClient(t, serverSocketPath) + client, conn := newOperationClient(t, serverSocketPath) defer conn.Close() testRepo, _, cleanup := testhelper.NewTestRepo(t) @@ -236,7 +233,7 @@ func TestFailedUserUpdateSubmoduleRequestDueToInvalidBranch(t *testing.T) { request := &gitalypb.UserUpdateSubmoduleRequest{ Repository: testRepo, - User: user, + User: testhelper.TestUser, Submodule: []byte("six"), CommitSha: "db54006ff1c999fd485af44581dabe9b6c85a701", Branch: []byte("non/existent"), @@ -255,7 +252,7 @@ func TestFailedUserUpdateSubmoduleRequestDueToInvalidSubmodule(t *testing.T) { serverSocketPath, stop := runOperationServiceServer(t) defer stop() - client, conn := NewOperationClient(t, serverSocketPath) + client, conn := newOperationClient(t, serverSocketPath) defer conn.Close() testRepo, _, cleanup := testhelper.NewTestRepo(t) @@ -263,7 +260,7 @@ func TestFailedUserUpdateSubmoduleRequestDueToInvalidSubmodule(t *testing.T) { request := &gitalypb.UserUpdateSubmoduleRequest{ Repository: testRepo, - User: user, + User: testhelper.TestUser, Submodule: []byte("non-existent-submodule"), CommitSha: "db54006ff1c999fd485af44581dabe9b6c85a701", Branch: []byte("master"), @@ -282,18 +279,15 @@ func TestFailedUserUpdateSubmoduleRequestDueToSameReference(t *testing.T) { serverSocketPath, stop := runOperationServiceServer(t) defer stop() - client, conn := NewOperationClient(t, serverSocketPath) + client, conn := newOperationClient(t, serverSocketPath) defer conn.Close() testRepo, _, cleanup := testhelper.NewTestRepo(t) defer cleanup() - cleanupSrv := SetupAndStartGitlabServer(t, user.GlId, testRepo.GlRepository) - defer cleanupSrv() - request := &gitalypb.UserUpdateSubmoduleRequest{ Repository: testRepo, - User: user, + User: testhelper.TestUser, Submodule: []byte("six"), CommitSha: "41fa1bc9e0f0630ced6a8a211d60c2af425ecc2d", Branch: []byte("master"), @@ -315,7 +309,7 @@ func TestFailedUserUpdateSubmoduleRequestDueToRepositoryEmpty(t *testing.T) { serverSocketPath, stop := runOperationServiceServer(t) defer stop() - client, conn := NewOperationClient(t, serverSocketPath) + client, conn := newOperationClient(t, serverSocketPath) defer conn.Close() testRepo, _, cleanup := testhelper.InitRepoWithWorktree(t) @@ -323,7 +317,7 @@ func TestFailedUserUpdateSubmoduleRequestDueToRepositoryEmpty(t *testing.T) { request := &gitalypb.UserUpdateSubmoduleRequest{ Repository: testRepo, - User: user, + User: testhelper.TestUser, Submodule: []byte("six"), CommitSha: "41fa1bc9e0f0630ced6a8a211d60c2af425ecc2d", Branch: []byte("master"), diff --git a/internal/service/operations/tags_test.go b/internal/service/operations/tags_test.go index 54d7f6523..6e0c64805 100644 --- a/internal/service/operations/tags_test.go +++ b/internal/service/operations/tags_test.go @@ -34,21 +34,12 @@ func TestSuccessfulUserDeleteTagRequest(t *testing.T) { defer exec.Command("git", "-C", testRepoPath, "tag", "-d", tagNameInput).Run() - user := &gitalypb.User{ - Name: []byte("Ahmad Sherif"), - Email: []byte("ahmad@gitlab.com"), - GlId: "user-123", - } - testhelper.MustRunCommand(t, nil, "git", "-C", testRepoPath, "tag", tagNameInput) - cleanupSrv := SetupAndStartGitlabServer(t, user.GlId, testRepo.GlRepository) - defer cleanupSrv() - request := &gitalypb.UserDeleteTagRequest{ Repository: testRepo, TagName: []byte(tagNameInput), - User: user, + User: testhelper.TestUser, } _, err := client.UserDeleteTag(ctx, request) @@ -82,23 +73,13 @@ func testSuccessfulGitHooksForUserDeleteTagRequest(t *testing.T, ctx context.Con testRepo, testRepoPath, cleanupFn := testhelper.NewTestRepo(t) defer cleanupFn() - cleanupSrv := SetupAndStartGitlabServer(t, user.GlId, testRepo.GlRepository) - defer cleanupSrv() - tagNameInput := "to-be-déleted-soon-tag" defer exec.Command("git", "-C", testRepoPath, "tag", "-d", tagNameInput).Run() - user := &gitalypb.User{ - Name: []byte("Ahmad Sherif"), - Email: []byte("ahmad@gitlab.com"), - GlId: "user-123", - GlUsername: "johndoe", - } - request := &gitalypb.UserDeleteTagRequest{ Repository: testRepo, TagName: []byte(tagNameInput), - User: user, + User: testhelper.TestUser, } for _, hookName := range GitlabHooks { @@ -112,7 +93,7 @@ func testSuccessfulGitHooksForUserDeleteTagRequest(t *testing.T, ctx context.Con require.NoError(t, err) output := testhelper.MustReadFile(t, hookOutputTempPath) - require.Contains(t, string(output), "GL_USERNAME=johndoe") + require.Contains(t, string(output), "GL_USERNAME="+testhelper.TestUser.GlUsername) }) } } @@ -134,11 +115,6 @@ func TestSuccessfulUserCreateTagRequest(t *testing.T) { targetRevisionCommit, err := log.GetCommit(ctx, testRepo, targetRevision) require.NoError(t, err) - user := &gitalypb.User{ - Name: []byte("Ahmad Sherif"), - Email: []byte("ahmad@gitlab.com"), - GlId: "user-123", - } inputTagName := "to-be-créated-soon" cwd, err := os.Getwd() @@ -194,13 +170,10 @@ func TestSuccessfulUserCreateTagRequest(t *testing.T) { Repository: testRepo, TagName: []byte(testCase.tagName), TargetRevision: []byte(testCase.targetRevision), - User: user, + User: testhelper.TestUser, Message: []byte(testCase.message), } - cleanupSrv := SetupAndStartGitlabServer(t, user.GlId, testRepo.GlRepository) - defer cleanupSrv() - ctx, cancel := testhelper.Context() defer cancel() @@ -246,21 +219,12 @@ func testSuccessfulGitHooksForUserCreateTagRequest(t *testing.T, ctx context.Con defer cleanupFn() tagName := "new-tag" - user := &gitalypb.User{ - Name: []byte("Ahmad Sherif"), - Email: []byte("ahmad@gitlab.com"), - GlId: "user-123", - GlUsername: "johndoe", - } - - cleanupSrv := SetupAndStartGitlabServer(t, user.GlId, testRepo.GlRepository) - defer cleanupSrv() request := &gitalypb.UserCreateTagRequest{ Repository: testRepo, TagName: []byte(tagName), TargetRevision: []byte("c7fbe50c7c7419d9701eebe64b1fdacc3df5b9dd"), - User: user, + User: testhelper.TestUser, } for _, hookName := range GitlabHooks { @@ -275,7 +239,7 @@ func testSuccessfulGitHooksForUserCreateTagRequest(t *testing.T, ctx context.Con require.Empty(t, response.PreReceiveError) output := string(testhelper.MustReadFile(t, hookOutputTempPath)) - require.Contains(t, output, "GL_USERNAME="+user.GlUsername) + require.Contains(t, output, "GL_USERNAME="+testhelper.TestUser.GlUsername) }) } } @@ -290,12 +254,6 @@ func TestFailedUserDeleteTagRequestDueToValidation(t *testing.T) { testRepo, _, cleanupFn := testhelper.NewTestRepo(t) defer cleanupFn() - user := &gitalypb.User{ - Name: []byte("Ahmad Sherif"), - Email: []byte("ahmad@gitlab.com"), - GlId: "user-123", - } - testCases := []struct { desc string request *gitalypb.UserDeleteTagRequest @@ -313,7 +271,7 @@ func TestFailedUserDeleteTagRequestDueToValidation(t *testing.T) { desc: "empty tag name", request: &gitalypb.UserDeleteTagRequest{ Repository: testRepo, - User: user, + User: testhelper.TestUser, }, code: codes.InvalidArgument, }, @@ -321,7 +279,7 @@ func TestFailedUserDeleteTagRequestDueToValidation(t *testing.T) { desc: "non-existent tag name", request: &gitalypb.UserDeleteTagRequest{ Repository: testRepo, - User: user, + User: testhelper.TestUser, TagName: []byte("i-do-not-exist"), }, code: codes.FailedPrecondition, @@ -367,19 +325,10 @@ func testFailedUserDeleteTagDueToHooks(t *testing.T, ctx context.Context) { testhelper.MustRunCommand(t, nil, "git", "-C", testRepoPath, "tag", tagNameInput) defer exec.Command("git", "-C", testRepoPath, "tag", "-d", tagNameInput).Run() - user := &gitalypb.User{ - Name: []byte("Ahmad Sherif"), - Email: []byte("ahmad@gitlab.com"), - GlId: "user-123", - } - - cleanupSrv := SetupAndStartGitlabServer(t, user.GlId, testRepo.GlRepository) - defer cleanupSrv() - request := &gitalypb.UserDeleteTagRequest{ Repository: testRepo, TagName: []byte(tagNameInput), - User: user, + User: testhelper.TestUser, } hookContent := []byte("#!/bin/sh\necho GL_ID=$GL_ID >&2\nexit 1") @@ -392,7 +341,7 @@ func testFailedUserDeleteTagDueToHooks(t *testing.T, ctx context.Context) { response, err := client.UserDeleteTag(ctx, request) require.Nil(t, err) - require.Contains(t, response.PreReceiveError, "GL_ID="+user.GlId) + require.Contains(t, response.PreReceiveError, "GL_ID="+testhelper.TestUser.GlId) tags := testhelper.MustRunCommand(t, nil, "git", "-C", testRepoPath, "tag") require.Contains(t, string(tags), tagNameInput, "tag name does not exist in tags list") @@ -410,21 +359,11 @@ func TestFailedUserCreateTagDueToHooks(t *testing.T) { testRepo, testRepoPath, cleanupFn := testhelper.NewTestRepo(t) defer cleanupFn() - user := &gitalypb.User{ - Name: []byte("Ahmad Sherif"), - Email: []byte("ahmad@gitlab.com"), - GlId: "user-123", - GlUsername: "johndoe", - } - - cleanupSrv := SetupAndStartGitlabServer(t, user.GlId, testRepo.GlRepository) - defer cleanupSrv() - request := &gitalypb.UserCreateTagRequest{ Repository: testRepo, TagName: []byte("new-tag"), TargetRevision: []byte("c7fbe50c7c7419d9701eebe64b1fdacc3df5b9dd"), - User: user, + User: testhelper.TestUser, } hookContent := []byte("#!/bin/sh\necho GL_ID=$GL_ID\nexit 1") @@ -439,7 +378,7 @@ func TestFailedUserCreateTagDueToHooks(t *testing.T) { response, err := client.UserCreateTag(ctx, request) require.Nil(t, err) - require.Contains(t, response.PreReceiveError, "GL_ID="+user.GlId) + require.Contains(t, response.PreReceiveError, "GL_ID="+testhelper.TestUser.GlId) } } @@ -453,15 +392,6 @@ func TestFailedUserCreateTagRequestDueToTagExistence(t *testing.T) { testRepo, _, cleanupFn := testhelper.NewTestRepo(t) defer cleanupFn() - user := &gitalypb.User{ - Name: []byte("Ahmad Sherif"), - Email: []byte("ahmad@gitlab.com"), - GlId: "user-123", - } - - cleanupSrv := SetupAndStartGitlabServer(t, user.GlId, testRepo.GlRepository) - defer cleanupSrv() - testCase := struct { tagName string targetRevision string @@ -469,7 +399,7 @@ func TestFailedUserCreateTagRequestDueToTagExistence(t *testing.T) { }{ tagName: "v1.1.0", targetRevision: "master", - user: user, + user: testhelper.TestUser, } request := &gitalypb.UserCreateTagRequest{ @@ -497,12 +427,6 @@ func TestFailedUserCreateTagRequestDueToValidation(t *testing.T) { testRepo, _, cleanupFn := testhelper.NewTestRepo(t) defer cleanupFn() - user := &gitalypb.User{ - Name: []byte("Ahmad Sherif"), - Email: []byte("ahmad@gitlab.com"), - GlId: "user-123", - } - testCases := []struct { desc string tagName string @@ -514,7 +438,7 @@ func TestFailedUserCreateTagRequestDueToValidation(t *testing.T) { desc: "empty target revision", tagName: "shiny-new-tag", targetRevision: "", - user: user, + user: testhelper.TestUser, code: codes.InvalidArgument, }, { @@ -528,7 +452,7 @@ func TestFailedUserCreateTagRequestDueToValidation(t *testing.T) { desc: "non-existing starting point", tagName: "new-tag", targetRevision: "i-dont-exist", - user: user, + user: testhelper.TestUser, code: codes.FailedPrecondition, }, } diff --git a/internal/service/operations/testhelper_test.go b/internal/service/operations/testhelper_test.go index ad52b4ae8..aae8c07ec 100644 --- a/internal/service/operations/testhelper_test.go +++ b/internal/service/operations/testhelper_test.go @@ -10,6 +10,7 @@ import ( "github.com/stretchr/testify/require" gitalyauth "gitlab.com/gitlab-org/gitaly/auth" "gitlab.com/gitlab-org/gitaly/internal/config" + gitalylog "gitlab.com/gitlab-org/gitaly/internal/log" "gitlab.com/gitlab-org/gitaly/internal/rubyserver" "gitlab.com/gitlab-org/gitaly/internal/service/commit" hook "gitlab.com/gitlab-org/gitaly/internal/service/hooks" @@ -28,12 +29,6 @@ var ( GitlabPreHooks = gitlabPreHooks GitlabHooks []string RubyServer = &rubyserver.Server{} - user = &gitalypb.User{ - Name: []byte("Jane Doe"), - Email: []byte("janedoe@gitlab.com"), - GlId: "user-123", - GlUsername: "janedoe", - } ) func init() { @@ -69,6 +64,9 @@ func testMain(m *testing.M) int { }(config.Config.Auth.Token) config.Config.Auth.Token = testhelper.RepositoryAuthToken + cleanupSrv := setupAndStartGitlabServer(gitalylog.Default(), testhelper.TestUser.GlId, testhelper.GlRepository) + defer cleanupSrv() + if err := RubyServer.Start(); err != nil { log.Fatal(err) } @@ -77,18 +75,20 @@ func testMain(m *testing.M) int { return m.Run() } -var RunOperationServiceServer = runOperationServiceServer - func runOperationServiceServer(t *testing.T) (string, func()) { + return runOperationServiceServerWithRubyServer(t, RubyServer) +} + +func runOperationServiceServerWithRubyServer(t *testing.T, ruby *rubyserver.Server) (string, func()) { srv := testhelper.NewServerWithAuth(t, nil, nil, config.Config.Auth.Token) internalSocket := config.GitalyInternalSocketPath() internalListener, err := net.Listen("unix", internalSocket) require.NoError(t, err) - gitalypb.RegisterOperationServiceServer(srv.GrpcServer(), &server{ruby: RubyServer}) + gitalypb.RegisterOperationServiceServer(srv.GrpcServer(), &server{ruby: ruby}) gitalypb.RegisterHookServiceServer(srv.GrpcServer(), hook.NewServer()) - gitalypb.RegisterRepositoryServiceServer(srv.GrpcServer(), repository.NewServer(RubyServer, internalSocket)) + gitalypb.RegisterRepositoryServiceServer(srv.GrpcServer(), repository.NewServer(ruby, internalSocket)) gitalypb.RegisterRefServiceServer(srv.GrpcServer(), ref.NewServer()) gitalypb.RegisterCommitServiceServer(srv.GrpcServer(), commit.NewServer()) gitalypb.RegisterSSHServiceServer(srv.GrpcServer(), ssh.NewServer()) @@ -116,10 +116,8 @@ func newOperationClient(t *testing.T, serverSocketPath string) (gitalypb.Operati return gitalypb.NewOperationServiceClient(conn), conn } -var NewOperationClient = newOperationClient - -func SetupAndStartGitlabServer(t *testing.T, glID, glRepository string, gitPushOptions ...string) func() { - return testhelper.SetupAndStartGitlabServer(t, &testhelper.GitlabTestServerOptions{ +func setupAndStartGitlabServer(t testhelper.FatalLogger, glID, glRepository string, gitPushOptions ...string) func() { + url, cleanup := testhelper.SetupAndStartGitlabServer(t, &testhelper.GitlabTestServerOptions{ SecretToken: "secretToken", GLID: glID, GLRepository: glRepository, @@ -127,4 +125,13 @@ func SetupAndStartGitlabServer(t *testing.T, glID, glRepository string, gitPushO Protocol: "web", GitPushOptions: gitPushOptions, }) + + gitlabURL := config.Config.Gitlab.URL + cleanupAll := func() { + cleanup() + config.Config.Gitlab.URL = gitlabURL + } + config.Config.Gitlab.URL = url + + return cleanupAll } diff --git a/internal/service/operations/update_branches_test.go b/internal/service/operations/update_branches_test.go index 9014c3a28..6602b11d9 100644 --- a/internal/service/operations/update_branches_test.go +++ b/internal/service/operations/update_branches_test.go @@ -27,9 +27,6 @@ func TestSuccessfulUserUpdateBranchRequest(t *testing.T) { testRepo, _, cleanupFn := testhelper.NewTestRepo(t) defer cleanupFn() - cleanupSrv := SetupAndStartGitlabServer(t, user.GlId, testRepo.GlRepository) - defer cleanupSrv() - serverSocketPath, stop := runOperationServiceServer(t) defer stop() @@ -41,7 +38,7 @@ func TestSuccessfulUserUpdateBranchRequest(t *testing.T) { BranchName: []byte(updateBranchName), Newrev: newrev, Oldrev: oldrev, - User: user, + User: testhelper.TestUser, } response, err := client.UserUpdateBranch(ctx, request) @@ -81,9 +78,6 @@ func testSuccessfulGitHooksForUserUpdateBranchRequest(t *testing.T, ctx context. testRepo, testRepoPath, cleanupFn := testhelper.NewTestRepo(t) defer cleanupFn() - cleanupSrv := SetupAndStartGitlabServer(t, user.GlId, testRepo.GlRepository) - defer cleanupSrv() - hookOutputTempPath, cleanup := testhelper.WriteEnvToCustomHook(t, testRepoPath, hookName) defer cleanup() @@ -92,7 +86,7 @@ func testSuccessfulGitHooksForUserUpdateBranchRequest(t *testing.T, ctx context. BranchName: []byte(updateBranchName), Newrev: newrev, Oldrev: oldrev, - User: user, + User: testhelper.TestUser, } response, err := client.UserUpdateBranch(ctx, request) @@ -100,7 +94,7 @@ func testSuccessfulGitHooksForUserUpdateBranchRequest(t *testing.T, ctx context. require.Empty(t, response.PreReceiveError) output := string(testhelper.MustReadFile(t, hookOutputTempPath)) - require.Contains(t, output, "GL_USERNAME="+user.GlUsername) + require.Contains(t, output, "GL_USERNAME="+testhelper.TestUser.GlUsername) }) } } @@ -123,9 +117,6 @@ func testFailedUserUpdateBranchDueToHooks(t *testing.T, ctx context.Context) { testRepo, testRepoPath, cleanupFn := testhelper.NewTestRepo(t) defer cleanupFn() - cleanupSrv := SetupAndStartGitlabServer(t, user.GlId, testRepo.GlRepository) - defer cleanupSrv() - serverSocketPath, stop := runOperationServiceServer(t) defer stop() @@ -137,7 +128,7 @@ func testFailedUserUpdateBranchDueToHooks(t *testing.T, ctx context.Context) { BranchName: []byte(updateBranchName), Newrev: newrev, Oldrev: oldrev, - User: user, + User: testhelper.TestUser, } // Write a hook that will fail with the environment as the error message // so we can check that string for our env variables. @@ -150,7 +141,7 @@ func testFailedUserUpdateBranchDueToHooks(t *testing.T, ctx context.Context) { response, err := client.UserUpdateBranch(ctx, request) require.Nil(t, err) - require.Contains(t, response.PreReceiveError, "GL_USERNAME="+user.GlUsername) + require.Contains(t, response.PreReceiveError, "GL_USERNAME="+testhelper.TestUser.GlUsername) require.Contains(t, response.PreReceiveError, "PWD="+testRepoPath) } } @@ -165,9 +156,6 @@ func TestFailedUserUpdateBranchRequest(t *testing.T) { testRepo, _, cleanupFn := testhelper.NewTestRepo(t) defer cleanupFn() - cleanupSrv := SetupAndStartGitlabServer(t, user.GlId, testRepo.GlRepository) - defer cleanupSrv() - revDoesntExist := fmt.Sprintf("%x", sha1.Sum([]byte("we need a non existent sha"))) testCases := []struct { @@ -183,7 +171,7 @@ func TestFailedUserUpdateBranchRequest(t *testing.T) { branchName: "", newrev: newrev, oldrev: oldrev, - user: user, + user: testhelper.TestUser, code: codes.InvalidArgument, }, { @@ -191,7 +179,7 @@ func TestFailedUserUpdateBranchRequest(t *testing.T) { branchName: updateBranchName, newrev: nil, oldrev: oldrev, - user: user, + user: testhelper.TestUser, code: codes.InvalidArgument, }, { @@ -199,7 +187,7 @@ func TestFailedUserUpdateBranchRequest(t *testing.T) { branchName: updateBranchName, newrev: newrev, oldrev: nil, - user: user, + user: testhelper.TestUser, code: codes.InvalidArgument, }, { @@ -215,7 +203,7 @@ func TestFailedUserUpdateBranchRequest(t *testing.T) { branchName: "i-dont-exist", newrev: newrev, oldrev: oldrev, - user: user, + user: testhelper.TestUser, code: codes.FailedPrecondition, }, { @@ -223,7 +211,7 @@ func TestFailedUserUpdateBranchRequest(t *testing.T) { branchName: updateBranchName, newrev: []byte(revDoesntExist), oldrev: oldrev, - user: user, + user: testhelper.TestUser, code: codes.FailedPrecondition, }, { @@ -231,7 +219,7 @@ func TestFailedUserUpdateBranchRequest(t *testing.T) { branchName: updateBranchName, newrev: newrev, oldrev: []byte(revDoesntExist), - user: user, + user: testhelper.TestUser, code: codes.FailedPrecondition, }, } diff --git a/internal/service/smarthttp/receive_pack_test.go b/internal/service/smarthttp/receive_pack_test.go index b8ae65a3a..1c61b626e 100644 --- a/internal/service/smarthttp/receive_pack_test.go +++ b/internal/service/smarthttp/receive_pack_test.go @@ -383,7 +383,7 @@ func testPostReceivePackToHooks(t *testing.T, callRPC bool) { changes := fmt.Sprintf("%s %s refs/heads/master\n", oldHead, push.newHead) - ts := testhelper.NewGitlabTestServer(t, testhelper.GitlabTestServerOptions{ + ts := testhelper.NewGitlabTestServer(testhelper.GitlabTestServerOptions{ User: "", Password: "", SecretToken: secretToken, @@ -491,7 +491,7 @@ func TestPostReceiveWithTransactions(t *testing.T) { RepoPath: repoPath, } - gitlabServer := testhelper.NewGitlabTestServer(t, opts) + gitlabServer := testhelper.NewGitlabTestServer(opts) defer gitlabServer.Close() gitlabShellDir, cleanup := testhelper.CreateTemporaryGitlabShellDir(t) diff --git a/internal/service/ssh/receive_pack_test.go b/internal/service/ssh/receive_pack_test.go index cbe2fd2be..a72cdbd5e 100644 --- a/internal/service/ssh/receive_pack_test.go +++ b/internal/service/ssh/receive_pack_test.go @@ -228,7 +228,7 @@ func TestSSHReceivePackToHooks(t *testing.T) { cloneDetails, cleanup := setupSSHClone(t) defer cleanup() - ts := testhelper.NewGitlabTestServer(t, testhelper.GitlabTestServerOptions{ + ts := testhelper.NewGitlabTestServer(testhelper.GitlabTestServerOptions{ User: "", Password: "", SecretToken: secretToken, diff --git a/internal/testhelper/testhelper.go b/internal/testhelper/testhelper.go index aabe1b220..0780d6ace 100644 --- a/internal/testhelper/testhelper.go +++ b/internal/testhelper/testhelper.go @@ -48,10 +48,21 @@ const ( RepositoryAuthToken = "the-secret-token" DefaultStorageName = "default" testGitEnv = "testdata/git-env" + GlRepository = "project-1" + GlID = "user-123" ) var configureOnce sync.Once +var ( + TestUser = &gitalypb.User{ + Name: []byte("Jane Doe"), + Email: []byte("janedoe@gitlab.com"), + GlId: GlID, + GlUsername: "janedoe", + } +) + // Configure sets up the global test configuration. On failure, // terminates the program. func Configure() { @@ -146,7 +157,7 @@ func TestRepository() *gitalypb.Repository { repo := &gitalypb.Repository{ StorageName: "default", RelativePath: TestRelativePath, - GlRepository: "project-1", + GlRepository: GlRepository, } storagePath, _ := config.Config.StoragePath(repo.GetStorageName()) @@ -430,7 +441,7 @@ func CreateRepo(t testing.TB, storagePath, relativePath string) *gitalypb.Reposi return &gitalypb.Repository{ StorageName: "default", RelativePath: relativePath, - GlRepository: "project-1", + GlRepository: GlRepository, } } diff --git a/internal/testhelper/testserver.go b/internal/testhelper/testserver.go index 017e2dbc0..e7672c5e1 100644 --- a/internal/testhelper/testserver.go +++ b/internal/testhelper/testserver.go @@ -282,7 +282,7 @@ var changeLineRegex = regexp.MustCompile("^[a-f0-9]{40} [a-f0-9]{40} refs/[^ ]+$ const secretHeaderName = "Gitlab-Shared-Secret" -func formToMap(u url.Values) map[string]string { +func preReceiveFormToMap(u url.Values) map[string]string { return map[string]string{ "action": u.Get("action"), "gl_repository": u.Get("gl_repository"), @@ -296,59 +296,118 @@ func formToMap(u url.Values) map[string]string { } } -func handleAllowed(t testing.TB, options GitlabTestServerOptions) func(w http.ResponseWriter, r *http.Request) { +func postReceiveFormToMap(u url.Values) map[string]interface{} { + return map[string]interface{}{ + "gl_repository": u.Get("gl_repository"), + "secret_token": u.Get("secret_token"), + "changes": u.Get("changes"), + "identifier": u.Get("identifier"), + "push_options": u["push_options[]"], + } +} + +func handleAllowed(options GitlabTestServerOptions) func(w http.ResponseWriter, r *http.Request) { return func(w http.ResponseWriter, r *http.Request) { - require.NoError(t, r.ParseForm()) - require.Equal(t, http.MethodPost, r.Method, "expected http post") + if err := r.ParseForm(); err != nil { + http.Error(w, "could not parse form", http.StatusBadRequest) + return + } + + if r.Method != http.MethodPost { + http.Error(w, "method not POST", http.StatusMethodNotAllowed) + return + } params := make(map[string]string) switch r.Header.Get("Content-Type") { case "application/x-www-form-urlencoded": - params = formToMap(r.Form) + params = preReceiveFormToMap(r.Form) case "application/json": - require.NoError(t, json.NewDecoder(r.Body).Decode(¶ms)) + if err := json.NewDecoder(r.Body).Decode(¶ms); err != nil { + http.Error(w, "could not unmarshal json body", http.StatusBadRequest) + return + } } user, password, _ := r.BasicAuth() - require.Equal(t, options.User, user) - require.Equal(t, options.Password, password) + if user != options.User || password != options.Password { + http.Error(w, "user or password incorrect", http.StatusUnauthorized) + return + } if options.GLID != "" { glidSplit := strings.SplitN(options.GLID, "-", 2) - require.Len(t, glidSplit, 2, "number of GLID components") + if len(glidSplit) != 2 { + http.Error(w, "gl_id invalid", http.StatusUnauthorized) + return + } + + glKey, glVal := glidSplit[0], glidSplit[1] - switch glidSplit[0] { + var glIDMatches bool + switch glKey { case "user": - require.Equal(t, glidSplit[1], params["user_id"]) + glIDMatches = glVal == params["user_id"] case "key": - require.Equal(t, glidSplit[1], params["key_id"]) + glIDMatches = glVal == params["key_id"] case "username": - require.Equal(t, glidSplit[1], params["username"]) + glIDMatches = glVal == params["username"] default: - t.Fatalf("invalid GLID: %q", options.GLID) + http.Error(w, "gl_id invalid", http.StatusUnauthorized) + return + } + + if !glIDMatches { + http.Error(w, "gl_id invalid", http.StatusUnauthorized) + return } } - require.NotEmpty(t, params["gl_repository"], "gl_repository should not be empty") + if params["gl_repository"] == "" { + http.Error(w, "gl_repository is empty", http.StatusUnauthorized) + return + } + if options.GLRepository != "" { - require.Equal(t, options.GLRepository, params["gl_repository"], "expected value of gl_repository should match form") + if params["gl_repository"] != options.GLRepository { + http.Error(w, "gl_repository is invalid", http.StatusUnauthorized) + return + } } - require.NotEmpty(t, params["protocol"], "protocol should not be empty") + + if params["protocol"] == "" { + http.Error(w, "protocol is empty", http.StatusUnauthorized) + return + } + if options.Protocol != "" { - require.Equal(t, options.Protocol, params["protocol"], "expected value of options.Protocol should match form") + if params["protocol"] != options.Protocol { + http.Error(w, "protocol is invalid", http.StatusUnauthorized) + return + } } if options.Changes != "" { - require.Equal(t, options.Changes, params["changes"], "expected value of options.Changes should match form") + if params["changes"] != options.Changes { + http.Error(w, "changes is invalid", http.StatusUnauthorized) + return + } } else { changeLines := strings.Split(strings.TrimSuffix(params["changes"], "\n"), "\n") for _, line := range changeLines { - require.Regexp(t, changeLineRegex, line) + if !changeLineRegex.MatchString(line) { + http.Error(w, "changes is invalid", http.StatusUnauthorized) + return + } } } + env := params["env"] - require.NotEmpty(t, env) + if len(env) == 0 { + http.Error(w, "env is empty", http.StatusUnauthorized) + return + } var gitVars struct { GitAlternateObjectDirsRel []string `json:"GIT_ALTERNATE_OBJECT_DIRECTORIES_RELATIVE"` @@ -357,11 +416,17 @@ func handleAllowed(t testing.TB, options GitlabTestServerOptions) func(w http.Re w.Header().Set("Content-Type", "application/json") - require.NoError(t, json.Unmarshal([]byte(env), &gitVars)) + if err := json.Unmarshal([]byte(env), &gitVars); err != nil { + http.Error(w, "could not unmarshal env", http.StatusUnauthorized) + return + } if options.GitObjectDir != "" { relObjectDir, err := filepath.Rel(options.RepoPath, options.GitObjectDir) - require.NoError(t, err) + if err != nil { + http.Error(w, "git object dirs is invalid", http.StatusUnauthorized) + return + } if relObjectDir != gitVars.GitObjectDirRel { w.Write([]byte(`{"status":false}`)) return @@ -369,10 +434,18 @@ func handleAllowed(t testing.TB, options GitlabTestServerOptions) func(w http.Re } if len(options.GitAlternateObjectDirs) > 0 { - require.Len(t, gitVars.GitAlternateObjectDirsRel, len(options.GitAlternateObjectDirs)) + if len(gitVars.GitAlternateObjectDirsRel) != len(options.GitAlternateObjectDirs) { + http.Error(w, "git alternate object dirs is invalid", http.StatusUnauthorized) + return + } + for i, gitAlterateObjectDir := range options.GitAlternateObjectDirs { relAltObjectDir, err := filepath.Rel(options.RepoPath, gitAlterateObjectDir) - require.NoError(t, err) + if err != nil { + http.Error(w, "git alternate object dirs is invalid", http.StatusUnauthorized) + return + } + if relAltObjectDir != gitVars.GitAlternateObjectDirsRel[i] { w.Write([]byte(`{"status":false}`)) return @@ -401,32 +474,60 @@ func handleAllowed(t testing.TB, options GitlabTestServerOptions) func(w http.Re } } -func handlePreReceive(t testing.TB, options GitlabTestServerOptions) func(w http.ResponseWriter, r *http.Request) { +func handlePreReceive(options GitlabTestServerOptions) func(w http.ResponseWriter, r *http.Request) { return func(w http.ResponseWriter, r *http.Request) { - require.NoError(t, r.ParseForm()) + if err := r.ParseForm(); err != nil { + http.Error(w, "could not parse form", http.StatusBadRequest) + return + } params := make(map[string]string) switch r.Header.Get("Content-Type") { case "application/x-www-form-urlencoded": b, err := json.Marshal(r.Form) - require.NoError(t, err) + if err != nil { + http.Error(w, "could not marshal form", http.StatusBadRequest) + return + } var reqForm struct { GLRepository []string `json:"gl_repository"` } - require.NoError(t, json.Unmarshal(b, &reqForm)) - require.Greater(t, len(reqForm.GLRepository), 0) + if err = json.Unmarshal(b, &reqForm); err != nil { + http.Error(w, "could not unmarshal form", http.StatusBadRequest) + return + } + + if len(reqForm.GLRepository) == 0 { + http.Error(w, "gl_repository is missing", http.StatusBadRequest) + return + } + params["gl_repository"] = reqForm.GLRepository[0] case "application/json": - require.NoError(t, json.NewDecoder(r.Body).Decode(¶ms)) + if err := json.NewDecoder(r.Body).Decode(¶ms); err != nil { + http.Error(w, "error when unmarshalling json body", http.StatusBadRequest) + return + } + } + + if r.Method != http.MethodPost { + http.Error(w, "method not POST", http.StatusMethodNotAllowed) + return + } + + if params["gl_repository"] == "" { + http.Error(w, "gl_repository is empty", http.StatusUnauthorized) + return } - require.Equal(t, http.MethodPost, r.Method) - require.NotEmpty(t, params["gl_repository"], "gl_repository should not be empty") if options.GLRepository != "" { - require.Equal(t, options.GLRepository, params["gl_repository"], "expected value of gl_repository should match form") + if params["gl_repository"] != options.GLRepository { + http.Error(w, "gl_repository is invalid", http.StatusUnauthorized) + return + } } var authenticated bool @@ -441,7 +542,10 @@ func handlePreReceive(t testing.TB, options GitlabTestServerOptions) func(w http } } - require.True(t, authenticated, "expected value of secret_token should request") + if !authenticated { + http.Error(w, "unauthorized", http.StatusUnauthorized) + return + } w.Header().Set("Content-Type", "application/json") w.WriteHeader(http.StatusOK) @@ -449,30 +553,82 @@ func handlePreReceive(t testing.TB, options GitlabTestServerOptions) func(w http } } -func handlePostReceive(t testing.TB, options GitlabTestServerOptions) func(w http.ResponseWriter, r *http.Request) { +func handlePostReceive(options GitlabTestServerOptions) func(w http.ResponseWriter, r *http.Request) { return func(w http.ResponseWriter, r *http.Request) { - defer r.Body.Close() - require.NoError(t, r.ParseForm()) - require.Equal(t, http.MethodPost, r.Method) - require.Equal(t, "application/x-www-form-urlencoded", r.Header.Get("Content-Type")) - require.NotEmpty(t, r.Form.Get("gl_repository")) + if err := r.ParseForm(); err != nil { + http.Error(w, "couldn't parse form", http.StatusBadRequest) + return + } + + if r.Method != http.MethodPost { + http.Error(w, "method not POST", http.StatusMethodNotAllowed) + return + } + + var params map[string]interface{} + + switch r.Header.Get("Content-Type") { + case "application/x-www-form-urlencoded": + params = postReceiveFormToMap(r.Form) + case "application/json": + if err := json.NewDecoder(r.Body).Decode(¶ms); err != nil { + http.Error(w, "could not parse json body", http.StatusBadRequest) + return + } + } + + if params["gl_repository"] == "" { + http.Error(w, "gl_repository is empty", http.StatusUnauthorized) + return + } + + if options.GLRepository != "" { + if params["gl_repository"] != options.GLRepository { + http.Error(w, "gl_repository is invalid", http.StatusUnauthorized) + return + } + } + + if params["secret_token"] != options.SecretToken { + http.Error(w, "secret_token is invalid", http.StatusUnauthorized) + return + } + if params["identifier"] == "" { + http.Error(w, "identifier is empty", http.StatusUnauthorized) + return + } + if options.GLRepository != "" { - require.Equal(t, options.GLRepository, r.Form.Get("gl_repository"), "expected value of gl_repository should match form") + if params["identifier"] != options.GLID { + http.Error(w, "identifier is invalid", http.StatusUnauthorized) + return + } } - require.Equal(t, options.SecretToken, r.Form.Get("secret_token"), "expected value of gl_repository should match form") - require.NotEmpty(t, r.Form.Get("identifier"), "identifier should exist") - if options.GLID != "" { - require.Equal(t, options.GLID, r.Form.Get("identifier"), "identifier should be GLID") + if params["changes"] == "" { + http.Error(w, "changes is empty", http.StatusUnauthorized) + return } - require.NotEmpty(t, r.Form.Get("changes"), "changes should exist") if options.Changes != "" { - require.Regexp(t, options.Changes, r.Form.Get("changes"), "expected value of changes should match form") + if params["changes"] != options.Changes { + http.Error(w, "changes is invalid", http.StatusUnauthorized) + return + } } - if len(options.GitPushOptions) > 0 { - require.Equal(t, options.GitPushOptions, r.Form["push_options[]"], "expected value of push_options should match form") + pushOptions := params["push_options"].([]string) + if len(pushOptions) != len(options.GitPushOptions) { + http.Error(w, "git push options is invalid", http.StatusUnauthorized) + return + } + + for i, pushOption := range pushOptions { + if pushOption != options.GitPushOptions[i] { + http.Error(w, "git push options is invalid", http.StatusUnauthorized) + return + } + } } w.WriteHeader(http.StatusOK) @@ -509,11 +665,11 @@ type GitlabTestServerOptions struct { } // NewGitlabTestServer returns a mock gitlab server that responds to the hook api endpoints -func NewGitlabTestServer(t testing.TB, options GitlabTestServerOptions) *httptest.Server { +func NewGitlabTestServer(options GitlabTestServerOptions) *httptest.Server { mux := http.NewServeMux() - mux.Handle("/api/v4/internal/allowed", http.HandlerFunc(handleAllowed(t, options))) - mux.Handle("/api/v4/internal/pre_receive", http.HandlerFunc(handlePreReceive(t, options))) - mux.Handle("/api/v4/internal/post_receive", http.HandlerFunc(handlePostReceive(t, options))) + mux.Handle("/api/v4/internal/allowed", http.HandlerFunc(handleAllowed(options))) + mux.Handle("/api/v4/internal/pre_receive", http.HandlerFunc(handlePreReceive(options))) + mux.Handle("/api/v4/internal/post_receive", http.HandlerFunc(handlePostReceive(options))) mux.Handle("/api/v4/internal/check", http.HandlerFunc(handleCheck(options))) return httptest.NewServer(mux) @@ -531,12 +687,16 @@ func CreateTemporaryGitlabShellDir(t testing.TB) (string, func()) { // WriteTemporaryGitlabShellConfigFile writes a gitlab shell config.yml in a temporary directory. It returns the path // and a cleanup function -func WriteTemporaryGitlabShellConfigFile(t testing.TB, dir string, config GitlabShellConfig) (string, func()) { +func WriteTemporaryGitlabShellConfigFile(t FatalLogger, dir string, config GitlabShellConfig) (string, func()) { out, err := yaml.Marshal(&config) - require.NoError(t, err) + if err != nil { + t.Fatalf("error marshalling config", err) + } path := filepath.Join(dir, "config.yml") - require.NoError(t, ioutil.WriteFile(path, out, 0644)) + if err = ioutil.WriteFile(path, out, 0644); err != nil { + t.Fatalf("error writing gitlab shell config", err) + } return path, func() { os.RemoveAll(path) @@ -555,8 +715,8 @@ func WriteTemporaryGitalyConfigFile(t testing.TB, tempDir, gitlabURL, user, pass user = %q password = %q `, tempDir, gitlabURL, user, password) - require.NoError(t, ioutil.WriteFile(path, []byte(contents), 0644)) + require.NoError(t, ioutil.WriteFile(path, []byte(contents), 0644)) return path, func() { os.RemoveAll(path) } @@ -605,9 +765,15 @@ func EnvForHooks(t testing.TB, gitlabShellDir, gitalySocket, gitalyToken string, return env } +type FatalLogger interface { + Fatalf(string, ...interface{}) +} + // WriteShellSecretFile writes a .gitlab_shell_secret file in the specified directory -func WriteShellSecretFile(t testing.TB, dir, secretToken string) { - require.NoError(t, ioutil.WriteFile(filepath.Join(dir, ".gitlab_shell_secret"), []byte(secretToken), 0644)) +func WriteShellSecretFile(t FatalLogger, dir, secretToken string) { + if err := ioutil.WriteFile(filepath.Join(dir, ".gitlab_shell_secret"), []byte(secretToken), 0644); err != nil { + t.Fatalf("writing shell secret file: %v", err) + } } // GitlabShellConfig contains a subset of gitlabshell's config.yml @@ -641,13 +807,13 @@ func NewHealthServerWithListener(t testing.TB, listener net.Listener) (*grpc.Ser return srv, healthSrvr } -func SetupAndStartGitlabServer(t testing.TB, c *GitlabTestServerOptions) func() { - ts := NewGitlabTestServer(t, *c) +func SetupAndStartGitlabServer(t FatalLogger, c *GitlabTestServerOptions) (string, func()) { + ts := NewGitlabTestServer(*c) WriteTemporaryGitlabShellConfigFile(t, config.Config.GitlabShell.Dir, GitlabShellConfig{GitlabURL: ts.URL}) WriteShellSecretFile(t, config.Config.GitlabShell.Dir, c.SecretToken) - return func() { + return ts.URL, func() { ts.Close() } } |