diff options
author | Paul Okstad <pokstad@gitlab.com> | 2019-03-20 23:19:00 +0300 |
---|---|---|
committer | Paul Okstad <pokstad@gitlab.com> | 2019-03-20 23:19:00 +0300 |
commit | 92cb0b175267dee4993dacff0e11e1628bf9bc67 (patch) | |
tree | f4892136fb060e041f8baf013a0360c6b9d3edd8 | |
parent | f309a06e14740430380f96f5d2e212b525ca63cf (diff) | |
parent | 996ac5da5c8cf5de7cb364e07aae49d81eb5eef2 (diff) |
Merge branch 'chomp-bytes' into 'master'
Introduce text.ChompBytes helper
See merge request gitlab-org/gitaly!1144
-rw-r--r-- | changelogs/unreleased/chomp-bytes.yml | 5 | ||||
-rw-r--r-- | internal/git/log/last_commit.go | 4 | ||||
-rw-r--r-- | internal/git/objectpool/link.go | 4 | ||||
-rw-r--r-- | internal/helper/text/chomp.go | 8 | ||||
-rw-r--r-- | internal/service/commit/languages.go | 4 | ||||
-rw-r--r-- | internal/service/operations/commit_files_test.go | 4 | ||||
-rw-r--r-- | internal/service/operations/merge_test.go | 15 | ||||
-rw-r--r-- | internal/service/operations/tags_test.go | 4 | ||||
-rw-r--r-- | internal/service/repository/gc_test.go | 6 | ||||
-rw-r--r-- | internal/service/repository/merge_base.go | 4 | ||||
-rw-r--r-- | internal/service/repository/write_config_test.go | 4 | ||||
-rw-r--r-- | internal/service/smarthttp/receive_pack_test.go | 5 | ||||
-rw-r--r-- | internal/service/wiki/get_page_versions_test.go | 8 | ||||
-rw-r--r-- | internal/supervisor/monitor.go | 4 | ||||
-rw-r--r-- | internal/testhelper/commit.go | 4 | ||||
-rw-r--r-- | internal/testhelper/tag.go | 5 | ||||
-rw-r--r-- | internal/testhelper/testhelper.go | 3 |
17 files changed, 54 insertions, 37 deletions
diff --git a/changelogs/unreleased/chomp-bytes.yml b/changelogs/unreleased/chomp-bytes.yml new file mode 100644 index 000000000..3cc448493 --- /dev/null +++ b/changelogs/unreleased/chomp-bytes.yml @@ -0,0 +1,5 @@ +--- +title: Introduce text.ChompBytes helper +merge_request: 1144 +author: +type: other diff --git a/internal/git/log/last_commit.go b/internal/git/log/last_commit.go index 4f08e0acb..0f356d570 100644 --- a/internal/git/log/last_commit.go +++ b/internal/git/log/last_commit.go @@ -3,13 +3,13 @@ package log import ( "context" "io/ioutil" - "strings" grpc_logrus "github.com/grpc-ecosystem/go-grpc-middleware/logging/logrus" log "github.com/sirupsen/logrus" "gitlab.com/gitlab-org/gitaly-proto/go/gitalypb" "gitlab.com/gitlab-org/gitaly/internal/command" "gitlab.com/gitlab-org/gitaly/internal/git" + "gitlab.com/gitlab-org/gitaly/internal/helper/text" ) // LastCommitForPath returns the last commit which modified path. @@ -24,7 +24,7 @@ func LastCommitForPath(ctx context.Context, repo *gitalypb.Repository, revision return nil, err } - return GetCommit(ctx, repo, strings.TrimSpace(string(commitID))) + return GetCommit(ctx, repo, text.ChompBytes(commitID)) } // GitLogCommand returns a Command that executes git log with the given the arguments diff --git a/internal/git/objectpool/link.go b/internal/git/objectpool/link.go index 1b5614c1b..389b24534 100644 --- a/internal/git/objectpool/link.go +++ b/internal/git/objectpool/link.go @@ -8,9 +8,9 @@ import ( "io/ioutil" "os" "path/filepath" - "strings" "gitlab.com/gitlab-org/gitaly/internal/git/remote" + "gitlab.com/gitlab-org/gitaly/internal/helper/text" "gitlab.com/gitlab-org/gitaly-proto/go/gitalypb" "gitlab.com/gitlab-org/gitaly/internal/git" @@ -46,7 +46,7 @@ func (o *ObjectPool) Link(ctx context.Context, repo *gitalypb.Repository) error actualContent, err := ioutil.ReadFile(altPath) if err == nil { - if strings.TrimSuffix(string(actualContent), "\n") == expectedContent { + if text.ChompBytes(actualContent) == expectedContent { return nil } diff --git a/internal/helper/text/chomp.go b/internal/helper/text/chomp.go new file mode 100644 index 000000000..6ce64cafa --- /dev/null +++ b/internal/helper/text/chomp.go @@ -0,0 +1,8 @@ +package text + +import "strings" + +// ChompBytes converts b to a string with its trailing newline, if present, removed. +func ChompBytes(b []byte) string { + return strings.TrimSuffix(string(b), "\n") +} diff --git a/internal/service/commit/languages.go b/internal/service/commit/languages.go index 166ff5b8c..73ef24d6e 100644 --- a/internal/service/commit/languages.go +++ b/internal/service/commit/languages.go @@ -4,11 +4,11 @@ import ( "context" "io/ioutil" "sort" - "strings" "gitlab.com/gitlab-org/gitaly-proto/go/gitalypb" "gitlab.com/gitlab-org/gitaly/internal/git" "gitlab.com/gitlab-org/gitaly/internal/helper" + "gitlab.com/gitlab-org/gitaly/internal/helper/text" "gitlab.com/gitlab-org/gitaly/internal/linguist" "gitlab.com/gitlab-org/gitaly/internal/service/ref" "google.golang.org/grpc/codes" @@ -86,5 +86,5 @@ func lookupRevision(ctx context.Context, repo *gitalypb.Repository, revision str return "", err } - return strings.TrimSpace(string(revParseBytes)), nil + return text.ChompBytes(revParseBytes), nil } diff --git a/internal/service/operations/commit_files_test.go b/internal/service/operations/commit_files_test.go index e26a05946..19b8eeedd 100644 --- a/internal/service/operations/commit_files_test.go +++ b/internal/service/operations/commit_files_test.go @@ -3,12 +3,12 @@ package operations_test import ( "fmt" "strconv" - "strings" "testing" "github.com/stretchr/testify/require" "gitlab.com/gitlab-org/gitaly-proto/go/gitalypb" "gitlab.com/gitlab-org/gitaly/internal/git/log" + "gitlab.com/gitlab-org/gitaly/internal/helper/text" "gitlab.com/gitlab-org/gitaly/internal/service/operations" "gitlab.com/gitlab-org/gitaly/internal/testhelper" "google.golang.org/grpc/codes" @@ -223,7 +223,7 @@ func TestSuccessfulUserCommitFilesRequestForceCommit(t *testing.T) { require.NoError(t, err) mergeBaseOut := testhelper.MustRunCommand(t, nil, "git", "-C", testRepoPath, "merge-base", targetBranchCommit.Id, startBranchCommit.Id) - mergeBaseID := strings.TrimSuffix(string(mergeBaseOut), "\n") + 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) diff --git a/internal/service/operations/merge_test.go b/internal/service/operations/merge_test.go index aab11c5d9..a682e4ff3 100644 --- a/internal/service/operations/merge_test.go +++ b/internal/service/operations/merge_test.go @@ -13,6 +13,7 @@ import ( "github.com/stretchr/testify/require" "gitlab.com/gitlab-org/gitaly-proto/go/gitalypb" gitlog "gitlab.com/gitlab-org/gitaly/internal/git/log" + "gitlab.com/gitlab-org/gitaly/internal/helper/text" "gitlab.com/gitlab-org/gitaly/internal/testhelper" "google.golang.org/grpc/codes" ) @@ -273,7 +274,7 @@ func TestFailedMergeDueToHooks(t *testing.T) { require.NoError(t, err, "consume EOF") currentBranchHead := testhelper.MustRunCommand(t, nil, "git", "-C", testRepoPath, "rev-parse", mergeBranchName) - require.Equal(t, mergeBranchHeadBefore, strings.TrimSpace(string(currentBranchHead)), "branch head updated") + require.Equal(t, mergeBranchHeadBefore, text.ChompBytes(currentBranchHead), "branch head updated") }) } @@ -315,7 +316,7 @@ func TestSuccessfulUserFFBranchRequest(t *testing.T) { require.NoError(t, err) require.Equal(t, expectedResponse, resp) newBranchHead := testhelper.MustRunCommand(t, nil, "git", "-C", testRepoPath, "rev-parse", branchName) - require.Equal(t, commitID, strings.TrimSpace(string(newBranchHead)), "branch head not updated") + require.Equal(t, commitID, text.ChompBytes(newBranchHead), "branch head not updated") } func TestFailedUserFFBranchRequest(t *testing.T) { @@ -481,7 +482,7 @@ func TestSuccessfulUserMergeToRefRequest(t *testing.T) { targetRef []byte emptyRef bool sourceSha string - message []byte + message string }{ { desc: "empty target ref merge", @@ -490,7 +491,7 @@ func TestSuccessfulUserMergeToRefRequest(t *testing.T) { targetRef: emptyTargetRef, emptyRef: true, sourceSha: commitToMerge, - message: []byte(mergeCommitMessage), + message: mergeCommitMessage, }, { desc: "existing target ref", @@ -499,7 +500,7 @@ func TestSuccessfulUserMergeToRefRequest(t *testing.T) { targetRef: existingTargetRef, emptyRef: false, sourceSha: commitToMerge, - message: []byte(mergeCommitMessage), + message: mergeCommitMessage, }, } @@ -514,7 +515,7 @@ func TestSuccessfulUserMergeToRefRequest(t *testing.T) { Branch: testCase.branch, TargetRef: testCase.targetRef, SourceSha: testCase.sourceSha, - Message: testCase.message, + Message: []byte(testCase.message), } commitBeforeRefMerge, fetchRefBeforeMergeErr := gitlog.GetCommit(ctx, testRepo, string(testCase.targetRef)) @@ -533,7 +534,7 @@ func TestSuccessfulUserMergeToRefRequest(t *testing.T) { // Asserts commit parent SHAs require.Equal(t, []string{mergeBranchHeadBefore, testCase.sourceSha}, commit.ParentIds, "merge commit parents must be the sha before HEAD and source sha") - require.True(t, strings.HasPrefix(string(commit.Body), string(testCase.message)), "expected %q to start with %q", commit.Body, string(testCase.message)) + require.True(t, strings.HasPrefix(string(commit.Body), testCase.message), "expected %q to start with %q", commit.Body, testCase.message) // Asserts author author := commit.Author diff --git a/internal/service/operations/tags_test.go b/internal/service/operations/tags_test.go index b458fb000..e8d10d1d4 100644 --- a/internal/service/operations/tags_test.go +++ b/internal/service/operations/tags_test.go @@ -3,12 +3,12 @@ package operations import ( "os" "os/exec" - "strings" "testing" "github.com/stretchr/testify/require" "gitlab.com/gitlab-org/gitaly-proto/go/gitalypb" "gitlab.com/gitlab-org/gitaly/internal/git/log" + "gitlab.com/gitlab-org/gitaly/internal/helper/text" "gitlab.com/gitlab-org/gitaly/internal/testhelper" "google.golang.org/grpc/codes" ) @@ -168,7 +168,7 @@ func TestSuccessfulUserCreateTagRequest(t *testing.T) { defer exec.Command("git", "-C", testRepoPath, "tag", "-d", inputTagName).Run() id := testhelper.MustRunCommand(t, nil, "git", "-C", testRepoPath, "rev-parse", inputTagName) - testCase.expectedTag.Id = strings.TrimSpace(string(id)) + testCase.expectedTag.Id = text.ChompBytes(id) require.NoError(t, err) require.Equal(t, testCase.expectedTag, response.Tag) diff --git a/internal/service/repository/gc_test.go b/internal/service/repository/gc_test.go index 651a5dfea..b8f258ddd 100644 --- a/internal/service/repository/gc_test.go +++ b/internal/service/repository/gc_test.go @@ -7,13 +7,13 @@ import ( "os" "path" "path/filepath" - "strings" "testing" "time" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "gitlab.com/gitlab-org/gitaly-proto/go/gitalypb" + "gitlab.com/gitlab-org/gitaly/internal/helper/text" "gitlab.com/gitlab-org/gitaly/internal/testhelper" "google.golang.org/grpc/codes" ) @@ -235,7 +235,7 @@ func TestCleanupInvalidKeepAroundRefs(t *testing.T) { // The existing keeparound still exists commitSha := testhelper.MustRunCommand(t, nil, "git", "-C", testRepoPath, "rev-parse", existingRefName) - require.Equal(t, existingSha, strings.TrimSpace(string(commitSha))) + require.Equal(t, existingSha, text.ChompBytes(commitSha)) //The invalid one was removed _, err = os.Stat(bogusPath) @@ -244,7 +244,7 @@ func TestCleanupInvalidKeepAroundRefs(t *testing.T) { if testcase.shouldExist { keepAroundName := fmt.Sprintf("refs/keep-around/%s", testcase.refName) commitSha := testhelper.MustRunCommand(t, nil, "git", "-C", testRepoPath, "rev-parse", keepAroundName) - require.Equal(t, testcase.refName, strings.TrimSpace(string(commitSha))) + require.Equal(t, testcase.refName, text.ChompBytes(commitSha)) } else { _, err := os.Stat(refPath) require.True(t, os.IsNotExist(err), "expected 'does not exist' error, got %v", err) diff --git a/internal/service/repository/merge_base.go b/internal/service/repository/merge_base.go index 68850ce70..890779ebd 100644 --- a/internal/service/repository/merge_base.go +++ b/internal/service/repository/merge_base.go @@ -3,10 +3,10 @@ package repository import ( "context" "io/ioutil" - "strings" "gitlab.com/gitlab-org/gitaly-proto/go/gitalypb" "gitlab.com/gitlab-org/gitaly/internal/git" + "gitlab.com/gitlab-org/gitaly/internal/helper/text" "google.golang.org/grpc/codes" "google.golang.org/grpc/status" ) @@ -35,7 +35,7 @@ func (s *server) FindMergeBase(ctx context.Context, req *gitalypb.FindMergeBaseR return nil, err } - mergeBaseStr := strings.TrimSpace(string(mergeBase)) + mergeBaseStr := text.ChompBytes(mergeBase) if err := cmd.Wait(); err != nil { // On error just return an empty merge base diff --git a/internal/service/repository/write_config_test.go b/internal/service/repository/write_config_test.go index 135025b50..fd8770cb6 100644 --- a/internal/service/repository/write_config_test.go +++ b/internal/service/repository/write_config_test.go @@ -1,11 +1,11 @@ package repository import ( - "strings" "testing" "github.com/stretchr/testify/require" "gitlab.com/gitlab-org/gitaly-proto/go/gitalypb" + "gitlab.com/gitlab-org/gitaly/internal/helper/text" "gitlab.com/gitlab-org/gitaly/internal/testhelper" "google.golang.org/grpc/codes" ) @@ -50,7 +50,7 @@ func TestWriteConfigSuccessful(t *testing.T) { require.Empty(t, string(c.GetError())) actualConfig := testhelper.MustRunCommand(t, nil, "git", "-C", testRepoPath, "config", "gitlab.fullpath") - require.Equal(t, tc.setPath, strings.TrimRight(string(actualConfig), "\n")) + require.Equal(t, tc.setPath, text.ChompBytes(actualConfig)) }) } } diff --git a/internal/service/smarthttp/receive_pack_test.go b/internal/service/smarthttp/receive_pack_test.go index 6525aa72e..8223b0ae8 100644 --- a/internal/service/smarthttp/receive_pack_test.go +++ b/internal/service/smarthttp/receive_pack_test.go @@ -17,6 +17,7 @@ import ( "gitlab.com/gitlab-org/gitaly/internal/config" "gitlab.com/gitlab-org/gitaly/internal/git" "gitlab.com/gitlab-org/gitaly/internal/git/hooks" + "gitlab.com/gitlab-org/gitaly/internal/helper/text" "gitlab.com/gitlab-org/gitaly/internal/testhelper" "gitlab.com/gitlab-org/gitaly/streamio" "google.golang.org/grpc/codes" @@ -224,7 +225,7 @@ func createCommit(t *testing.T, repoPath string, fileContents []byte) (oldHead s committerEmail := "scrooge@mcduck.com" // The latest commit ID on the remote repo - oldHead = strings.TrimSpace(string(testhelper.MustRunCommand(t, nil, "git", "-C", repoPath, "rev-parse", "master"))) + oldHead = text.ChompBytes(testhelper.MustRunCommand(t, nil, "git", "-C", repoPath, "rev-parse", "master")) changedFile := "README.md" require.NoError(t, ioutil.WriteFile(path.Join(repoPath, changedFile), fileContents, 0644)) @@ -236,7 +237,7 @@ func createCommit(t *testing.T, repoPath string, fileContents []byte) (oldHead s "commit", "-m", commitMsg) // The commit ID we want to push to the remote repo - newHead = strings.TrimSpace(string(testhelper.MustRunCommand(t, nil, "git", "-C", repoPath, "rev-parse", "master"))) + newHead = text.ChompBytes(testhelper.MustRunCommand(t, nil, "git", "-C", repoPath, "rev-parse", "master")) return oldHead, newHead } diff --git a/internal/service/wiki/get_page_versions_test.go b/internal/service/wiki/get_page_versions_test.go index aafdf6e69..6da221901 100644 --- a/internal/service/wiki/get_page_versions_test.go +++ b/internal/service/wiki/get_page_versions_test.go @@ -3,12 +3,12 @@ package wiki import ( "bytes" "io" - "strings" "testing" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "gitlab.com/gitlab-org/gitaly-proto/go/gitalypb" + "gitlab.com/gitlab-org/gitaly/internal/helper/text" "gitlab.com/gitlab-org/gitaly/internal/testhelper" ) @@ -59,19 +59,19 @@ func TestWikiGetPageVersionsRequest(t *testing.T) { versions: []*gitalypb.WikiPageVersion{ { Commit: &gitalypb.GitCommit{ - Id: strings.TrimRight(string(v2cid), "\n"), + Id: text.ChompBytes(v2cid), Body: []byte("Update WikiGétPageVersions"), Subject: []byte("Update WikiGétPageVersions"), Author: gitAuthor, Committer: gitAuthor, - ParentIds: []string{strings.TrimRight(string(v1cid), "\n")}, + ParentIds: []string{text.ChompBytes(v1cid)}, BodySize: 26, }, Format: "markdown", }, { Commit: &gitalypb.GitCommit{ - Id: strings.TrimRight(string(v1cid), "\n"), + Id: text.ChompBytes(v1cid), Body: []byte("Add WikiGétPageVersions"), Subject: []byte("Add WikiGétPageVersions"), Author: gitAuthor, diff --git a/internal/supervisor/monitor.go b/internal/supervisor/monitor.go index 1be009638..4544e259a 100644 --- a/internal/supervisor/monitor.go +++ b/internal/supervisor/monitor.go @@ -3,11 +3,11 @@ package supervisor import ( "os/exec" "strconv" - "strings" "time" "github.com/prometheus/client_golang/prometheus" log "github.com/sirupsen/logrus" + "gitlab.com/gitlab-org/gitaly/internal/helper/text" ) var ( @@ -82,7 +82,7 @@ func getRss(pid int) int { return 0 } - rss, err := strconv.Atoi(strings.TrimSpace(string(psRss))) + rss, err := strconv.Atoi(text.ChompBytes(psRss)) if err != nil { return 0 } diff --git a/internal/testhelper/commit.go b/internal/testhelper/commit.go index fe3b48d1d..81596352e 100644 --- a/internal/testhelper/commit.go +++ b/internal/testhelper/commit.go @@ -6,10 +6,10 @@ import ( "os" "os/exec" "path" - "strings" "testing" "github.com/stretchr/testify/require" + "gitlab.com/gitlab-org/gitaly/internal/helper/text" ) // CreateCommitOpts holds extra options for CreateCommit. @@ -50,7 +50,7 @@ func CreateCommit(t *testing.T, repoPath, branchName string, opts *CreateCommitO "commit-tree", "-F", "-", "-p", parentID, parentID + "^{tree}", } newCommit := MustRunCommand(t, stdin, "git", commitArgs...) - newCommitID := strings.TrimSpace(string(newCommit)) + newCommitID := text.ChompBytes(newCommit) MustRunCommand(t, nil, "git", "-C", repoPath, "update-ref", "refs/heads/"+branchName, newCommitID) return newCommitID diff --git a/internal/testhelper/tag.go b/internal/testhelper/tag.go index 21e2e9a21..8ab0e64df 100644 --- a/internal/testhelper/tag.go +++ b/internal/testhelper/tag.go @@ -3,8 +3,9 @@ package testhelper import ( "bytes" "fmt" - "strings" "testing" + + "gitlab.com/gitlab-org/gitaly/internal/helper/text" ) // CreateTagOpts holds extra options for CreateTag. @@ -41,5 +42,5 @@ func CreateTag(t *testing.T, repoPath, tagName, targetID string, opts *CreateTag MustRunCommand(t, stdin, "git", args...) tagID := MustRunCommand(t, nil, "git", "-C", repoPath, "show-ref", "-s", tagName) - return strings.TrimSpace(string(tagID)) + return text.ChompBytes(tagID) } diff --git a/internal/testhelper/testhelper.go b/internal/testhelper/testhelper.go index ea679e795..8ceb75413 100644 --- a/internal/testhelper/testhelper.go +++ b/internal/testhelper/testhelper.go @@ -29,6 +29,7 @@ import ( "gitlab.com/gitlab-org/gitaly/internal/command" "gitlab.com/gitlab-org/gitaly/internal/config" "gitlab.com/gitlab-org/gitaly/internal/helper/fieldextractors" + "gitlab.com/gitlab-org/gitaly/internal/helper/text" gitalylog "gitlab.com/gitlab-org/gitaly/internal/log" "gitlab.com/gitlab-org/gitaly/internal/storage" "google.golang.org/grpc" @@ -332,7 +333,7 @@ func mustFindNoRunningChildProcess() { out, err := pgrep.Output() if err == nil { - pidsComma := strings.Replace(strings.TrimSpace(string(out)), ",", "\n", -1) + pidsComma := strings.Replace(text.ChompBytes(out), "\n", ",", -1) psOut, _ := exec.Command("ps", "-o", "pid,args", "-p", pidsComma).Output() panic(fmt.Errorf("found running child processes %s:\n%s", pidsComma, psOut)) } |