diff options
author | Patrick Steinhardt <psteinhardt@gitlab.com> | 2020-03-13 19:15:42 +0300 |
---|---|---|
committer | Patrick Steinhardt <psteinhardt@gitlab.com> | 2020-03-16 14:06:35 +0300 |
commit | 70776dc9196fde982daec2d7496ef57a081c04eb (patch) | |
tree | c4e6190d012d9723107ab7dcb44ec41b8dd915cf | |
parent | 985c411c97d764de3ec67953b07353de19857712 (diff) |
ssh: upload_pack: Refactor `cloneCommand.test()` to always assert success
The `cloneCommand.test()` function returns an error code in case any
error happened, but there is only a single caller that will _not_
immediately call `require.NoError()` on that error code. As such, let's
refactor that caller to use `cloneCommand.execute()` instead and then
convert `cloneCommand.test()` to assert for success itself to remove the
burden for callers. This lifts the burden for all other callers and
makes code easier to read.
-rw-r--r-- | internal/service/ssh/upload_pack_test.go | 26 |
1 files changed, 8 insertions, 18 deletions
diff --git a/internal/service/ssh/upload_pack_test.go b/internal/service/ssh/upload_pack_test.go index 999374e54..5f543b963 100644 --- a/internal/service/ssh/upload_pack_test.go +++ b/internal/service/ssh/upload_pack_test.go @@ -59,13 +59,11 @@ func (cmd cloneCommand) execute(t *testing.T) error { return nil } -func (cmd cloneCommand) test(t *testing.T, localRepoPath string) (string, string, string, string, error) { +func (cmd cloneCommand) test(t *testing.T, localRepoPath string) (string, string, string, string) { defer os.RemoveAll(localRepoPath) err := cmd.execute(t) - if err != nil { - return "", "", "", "", err - } + require.NoError(t, err) storagePath := testhelper.GitlabTestStoragePath() testRepoPath := path.Join(storagePath, testRepo.GetRelativePath()) @@ -76,7 +74,7 @@ func (cmd cloneCommand) test(t *testing.T, localRepoPath string) (string, string remoteTags := text.ChompBytes(testhelper.MustRunCommand(t, nil, "git", "-C", testRepoPath, "tag")) localTags := text.ChompBytes(testhelper.MustRunCommand(t, nil, "git", "-C", localRepoPath, "tag")) - return localHead, remoteHead, localTags, remoteTags, nil + return localHead, remoteHead, localTags, remoteTags } func TestFailedUploadPackRequestDueToTimeout(t *testing.T) { @@ -211,9 +209,7 @@ func TestUploadPackCloneSuccess(t *testing.T) { command: tc.cmd, server: serverSocketPath, } - - lHead, rHead, _, _, err := cmd.test(t, localRepoPath) - require.NoError(t, err, "clone failed") + lHead, rHead, _, _ := cmd.test(t, localRepoPath) require.Equal(t, lHead, rHead, "local and remote head not equal") }) } @@ -251,8 +247,7 @@ func TestUploadPackCloneSuccessWithGitProtocol(t *testing.T) { gitProtocol: git.ProtocolV2, } - lHead, rHead, _, _, err := cmd.test(t, localRepoPath) - require.NoError(t, err, "clone failed") + lHead, rHead, _, _ := cmd.test(t, localRepoPath) require.Equal(t, lHead, rHead, "local and remote head not equal") envData, err := testhelper.GetGitEnvData() @@ -275,11 +270,8 @@ func TestUploadPackCloneHideTags(t *testing.T) { server: serverSocketPath, gitConfig: "transfer.hideRefs=refs/tags", } - _, _, lTags, rTags, err := cmd.test(t, localRepoPath) + _, _, lTags, rTags := cmd.test(t, localRepoPath) - if err != nil { - t.Fatalf("clone failed: %v", err) - } if lTags == rTags { t.Fatalf("local and remote tags are equal. clone failed: %q != %q", lTags, rTags) } @@ -302,10 +294,8 @@ func TestUploadPackCloneFailure(t *testing.T) { command: exec.Command("git", "clone", "git@localhost:test/test.git", localRepoPath), server: serverSocketPath, } - _, _, _, _, err := cmd.test(t, localRepoPath) - if err == nil { - t.Fatalf("clone didn't fail") - } + err := cmd.execute(t) + require.Error(t, err, "clone didn't fail") } func testPostUploadPackFailedResponse(t *testing.T, stream gitalypb.SSHService_SSHUploadPackClient) error { |