From be8dfba7a2c382666e3fed68be36e885e80cc742 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jes=C3=BAs=20Carpintero?= Date: Wed, 1 May 2024 15:05:54 +0200 Subject: [PATCH 1/9] fix(foreach): supports quoted commands (#132) * Add support to execute commands with spaces * update string builder * use strconv.Quote * Add tests --- cmd/foreach/foreach.go | 9 ++++++++- cmd/foreach/foreach_test.go | 17 +++++++++++++++++ 2 files changed, 25 insertions(+), 1 deletion(-) diff --git a/cmd/foreach/foreach.go b/cmd/foreach/foreach.go index 691836b..fe55cda 100644 --- a/cmd/foreach/foreach.go +++ b/cmd/foreach/foreach.go @@ -18,6 +18,7 @@ package foreach import ( "os" "path" + "strconv" "strings" "github.com/spf13/cobra" @@ -98,10 +99,16 @@ func run(c *cobra.Command, args []string) { } readCampaignActivity.EndWithSuccess() + for i := range args { + if strings.Contains(args[i], " ") { + args[i] = strconv.Quote(args[i]) + } + } + command := strings.Join(args, " ") + var doneCount, skippedCount, errorCount int for _, repo := range dir.Repos { repoDirPath := path.Join("work", repo.OrgName, repo.RepoName) // i.e. work/org/repo - command := strings.Join(args, " ") execActivity := logger.StartActivity("Executing %s in %s", command, repoDirPath) diff --git a/cmd/foreach/foreach_test.go b/cmd/foreach/foreach_test.go index a674faa..de3de2b 100644 --- a/cmd/foreach/foreach_test.go +++ b/cmd/foreach/foreach_test.go @@ -151,6 +151,23 @@ func TestItRunsCommandInShellAgainstWorkingCopies(t *testing.T) { }) } +func TestItRunsCommandQuotedInShellAgainstWorkingCopied(t *testing.T) { + fakeExecutor := executor.NewAlwaysSucceedsFakeExecutor() + exec = fakeExecutor + + testsupport.PrepareTempCampaign(true, "org/repo1", "org/repo2") + + out, err := runCommand("some", "command", "with spaces") + assert.NoError(t, err) + assert.Contains(t, out, "turbolift foreach completed") + assert.Contains(t, out, "2 OK, 0 skipped") + + fakeExecutor.AssertCalledWith(t, [][]string{ + {"work/org/repo1", userShell(), "-c", "some command \"with spaces\""}, + {"work/org/repo2", userShell(), "-c", "some command \"with spaces\""}, + }) +} + func TestItSkipsMissingWorkingCopies(t *testing.T) { fakeExecutor := executor.NewAlwaysSucceedsFakeExecutor() exec = fakeExecutor From db482a6a73389ed0b6edbd00d5a4c3e33c513a35 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Mon, 6 May 2024 15:10:16 +0100 Subject: [PATCH 2/9] Bump golangci/golangci-lint-action from 4.0.0 to 5.1.0 (#134) Bumps [golangci/golangci-lint-action](https://github.com/golangci/golangci-lint-action) from 4.0.0 to 5.1.0. - [Release notes](https://github.com/golangci/golangci-lint-action/releases) - [Commits](https://github.com/golangci/golangci-lint-action/compare/3cfe3a4abbb849e10058ce4af15d205b6da42804...9d1e0624a798bb64f6c3cea93db47765312263dc) --- updated-dependencies: - dependency-name: golangci/golangci-lint-action dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Daniel Ranson <92924979+Dan7-7-7@users.noreply.github.com> --- .github/workflows/golangci-lint.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/golangci-lint.yml b/.github/workflows/golangci-lint.yml index a27d8a8..c490a8f 100644 --- a/.github/workflows/golangci-lint.yml +++ b/.github/workflows/golangci-lint.yml @@ -13,7 +13,7 @@ jobs: steps: - uses: actions/checkout@v4 - name: golangci-lint - uses: golangci/golangci-lint-action@3cfe3a4abbb849e10058ce4af15d205b6da42804 # v4.0.0 + uses: golangci/golangci-lint-action@9d1e0624a798bb64f6c3cea93db47765312263dc # v5.1.0 with: # Optional: version of golangci-lint to use in form of v1.2 or v1.2.3 or `latest` to use the latest version version: v1.42.1 From c7bb350a3f8a01355370dabbef3243f9d738fd45 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Mon, 3 Jun 2024 15:44:56 +0100 Subject: [PATCH 3/9] Bump goreleaser/goreleaser-action from 5.0.0 to 5.1.0 (#138) Bumps [goreleaser/goreleaser-action](https://github.com/goreleaser/goreleaser-action) from 5.0.0 to 5.1.0. - [Release notes](https://github.com/goreleaser/goreleaser-action/releases) - [Commits](https://github.com/goreleaser/goreleaser-action/compare/7ec5c2b0c6cdda6e8bbb49444bc797dd33d74dd8...5742e2a039330cbb23ebf35f046f814d4c6ff811) --- updated-dependencies: - dependency-name: goreleaser/goreleaser-action dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- .github/workflows/release.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 45eddfc..27b8a26 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -20,7 +20,7 @@ jobs: go-version: ^1.16 - name: Run GoReleaser - uses: goreleaser/goreleaser-action@7ec5c2b0c6cdda6e8bbb49444bc797dd33d74dd8 # v5.0.0 + uses: goreleaser/goreleaser-action@5742e2a039330cbb23ebf35f046f814d4c6ff811 # v5.1.0 with: args: release --clean env: From 77c3a547525fa44e36e7f10ad32b0bc5e8b6d1aa Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Mon, 3 Jun 2024 15:47:50 +0100 Subject: [PATCH 4/9] Bump golangci/golangci-lint-action from 5.1.0 to 6.0.1 (#137) Bumps [golangci/golangci-lint-action](https://github.com/golangci/golangci-lint-action) from 5.1.0 to 6.0.1. - [Release notes](https://github.com/golangci/golangci-lint-action/releases) - [Commits](https://github.com/golangci/golangci-lint-action/compare/9d1e0624a798bb64f6c3cea93db47765312263dc...a4f60bb28d35aeee14e6880718e0c85ff1882e64) --- updated-dependencies: - dependency-name: golangci/golangci-lint-action dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Richard North --- .github/workflows/golangci-lint.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/golangci-lint.yml b/.github/workflows/golangci-lint.yml index c490a8f..eea3c7d 100644 --- a/.github/workflows/golangci-lint.yml +++ b/.github/workflows/golangci-lint.yml @@ -13,7 +13,7 @@ jobs: steps: - uses: actions/checkout@v4 - name: golangci-lint - uses: golangci/golangci-lint-action@9d1e0624a798bb64f6c3cea93db47765312263dc # v5.1.0 + uses: golangci/golangci-lint-action@a4f60bb28d35aeee14e6880718e0c85ff1882e64 # v6.0.1 with: # Optional: version of golangci-lint to use in form of v1.2 or v1.2.3 or `latest` to use the latest version version: v1.42.1 From f0b8dc4a85726832fcdbedde5164629f08de5d85 Mon Sep 17 00:00:00 2001 From: Jorge Dominguez Date: Tue, 11 Jun 2024 15:30:22 +0200 Subject: [PATCH 5/9] feat(pr-status): add build status to pr-status list table (#129) * Add build status to pr-status list table * Use correct field for status * Update README * Rename field * add pending status check --------- Co-authored-by: Jorge Dominguez Co-authored-by: Richard North Co-authored-by: Danny Ranson Co-authored-by: Daniel Ranson <92924979+Dan7-7-7@users.noreply.github.com> --- README.md | 20 +++---- cmd/prstatus/prstatus.go | 21 ++++++- cmd/prstatus/prstatus_test.go | 101 +++++++++++++++++++++++++++++++--- internal/github/github.go | 25 +++++---- 4 files changed, 137 insertions(+), 30 deletions(-) diff --git a/README.md b/README.md index f0593dc..52960ff 100644 --- a/README.md +++ b/README.md @@ -204,16 +204,16 @@ Viewing a detailed list of status per repo: ``` $ turbolift pr-status --list ... -Repository State Reviews URL -redacted/redacted OPEN REVIEW_REQUIRED https://github.redacted/redacted/redacted/pull/262 -redacted/redacted OPEN REVIEW_REQUIRED https://github.redacted/redacted/redacted/pull/515 -redacted/redacted OPEN REVIEW_REQUIRED https://github.redacted/redacted/redacted/pull/342 -redacted/redacted MERGED APPROVED https://github.redacted/redacted/redacted/pull/407 -redacted/redacted MERGED REVIEW_REQUIRED https://github.redacted/redacted/redacted/pull/220 -redacted/redacted OPEN REVIEW_REQUIRED https://github.redacted/redacted/redacted/pull/105 -redacted/redacted MERGED APPROVED https://github.redacted/redacted/redacted/pull/532 -redacted/redacted MERGED APPROVED https://github.redacted/redacted/redacted/pull/268 -redacted/redacted OPEN REVIEW_REQUIRED https://github.redacted/redacted/redacted/pull/438 +Repository State Reviews Build status URL +redacted/redacted OPEN REVIEW_REQUIRED SUCCESS https://github.redacted/redacted/redacted/pull/262 +redacted/redacted OPEN REVIEW_REQUIRED SUCCESS https://github.redacted/redacted/redacted/pull/515 +redacted/redacted OPEN REVIEW_REQUIRED SUCCESS https://github.redacted/redacted/redacted/pull/342 +redacted/redacted MERGED APPROVED SUCCESS https://github.redacted/redacted/redacted/pull/407 +redacted/redacted MERGED REVIEW_REQUIRED SUCCESS https://github.redacted/redacted/redacted/pull/220 +redacted/redacted OPEN REVIEW_REQUIRED FAILURE https://github.redacted/redacted/redacted/pull/105 +redacted/redacted MERGED APPROVED SUCCESS https://github.redacted/redacted/redacted/pull/532 +redacted/redacted MERGED APPROVED SUCCESS https://github.redacted/redacted/redacted/pull/268 +redacted/redacted OPEN REVIEW_REQUIRED FAILURE https://github.redacted/redacted/redacted/pull/438 ... ``` diff --git a/cmd/prstatus/prstatus.go b/cmd/prstatus/prstatus.go index 5a642e1..10f06a6 100644 --- a/cmd/prstatus/prstatus.go +++ b/cmd/prstatus/prstatus.go @@ -88,7 +88,7 @@ func run(c *cobra.Command, _ []string) { statuses := make(map[string]int) reactions := make(map[string]int) - detailsTable := table.New("Repository", "State", "Reviews", "URL") + detailsTable := table.New("Repository", "State", "Reviews", "Checks status", "URL") detailsTable.WithHeaderFormatter(color.New(color.Underline).SprintfFunc()) detailsTable.WithFirstColumnFormatter(color.New(color.FgCyan).SprintfFunc()) detailsTable.WithWriter(logger.Writer()) @@ -118,7 +118,24 @@ func run(c *cobra.Command, _ []string) { reactions[reaction.Content] += reaction.Users.TotalCount } - detailsTable.AddRow(repo.FullRepoName, prStatus.State, prStatus.ReviewDecision, prStatus.Url) + failedCheck := false + pendingCheck := false + for _, check := range prStatus.StatusCheckRollup { + if strings.Contains(check.State, "FAILURE") { + failedCheck = true + } else if strings.Contains(check.State, "PENDING") { + pendingCheck = true + } + } + + checksStatus := "SUCCESS" + if failedCheck { + checksStatus = "FAILURE" + } else if pendingCheck { + checksStatus = "PENDING" + } + + detailsTable.AddRow(repo.FullRepoName, prStatus.State, prStatus.ReviewDecision, checksStatus, prStatus.Url) checkStatusActivity.EndWithSuccess() } diff --git a/cmd/prstatus/prstatus_test.go b/cmd/prstatus/prstatus_test.go index 5c9c3db..6fcffd5 100644 --- a/cmd/prstatus/prstatus_test.go +++ b/cmd/prstatus/prstatus_test.go @@ -56,17 +56,20 @@ func TestItLogsSummaryInformation(t *testing.T) { func TestItLogsDetailedInformation(t *testing.T) { prepareFakeResponses() - testsupport.PrepareTempCampaign(true, "org/repo1", "org/repo2", "org/repo3") + testsupport.PrepareTempCampaign(true, "org/repo1", "org/repo2", "org/repo3", "org/repo4", "org/repo5", "org/repo6") out, err := runCommand(true) assert.NoError(t, err) // Should still show summary info - assert.Regexp(t, "Open\\s+1", out) - assert.Regexp(t, "👍\\s+4", out) - - assert.Regexp(t, "org/repo1\\s+OPEN\\s+REVIEW_REQUIRED", out) - assert.Regexp(t, "org/repo2\\s+MERGED\\s+APPROVED", out) - assert.Regexp(t, "org/repo3\\s+CLOSED", out) + assert.Regexp(t, "Open\\s+4", out) + assert.Regexp(t, "👍\\s+13", out) + + assert.Regexp(t, "org/repo1\\s+OPEN\\s+REVIEW_REQUIRED\\s+FAILURE", out) + assert.Regexp(t, "org/repo2\\s+MERGED\\s+APPROVED\\s+SUCCESS", out) + assert.Regexp(t, "org/repo3\\s+CLOSED\\s+FAILURE", out) + assert.Regexp(t, "org/repo4\\s+OPEN\\s+REVIEW_REQUIRED\\s+PENDING", out) + assert.Regexp(t, "org/repo5\\s+OPEN\\s+REVIEW_REQUIRED\\s+FAILURE", out) + assert.Regexp(t, "org/repo6\\s+OPEN\\s+REVIEW_REQUIRED\\s+PENDING", out) } func TestItSkipsUnclonedRepos(t *testing.T) { @@ -119,6 +122,14 @@ func prepareFakeResponses() { dummyData := map[string]*github.PrStatus{ "work/org/repo1": { State: "OPEN", + StatusCheckRollup: []github.StatusCheckRollup{ + { + State: "FAILURE", + }, + { + State: "SUCCESS", + }, + }, ReactionGroups: []github.ReactionGroup{ { Content: "THUMBS_UP", @@ -137,6 +148,14 @@ func prepareFakeResponses() { }, "work/org/repo2": { State: "MERGED", + StatusCheckRollup: []github.StatusCheckRollup{ + { + State: "SUCCESS", + }, + { + State: "SUCCESS", + }, + }, ReactionGroups: []github.ReactionGroup{ { Content: "THUMBS_UP", @@ -148,7 +167,13 @@ func prepareFakeResponses() { ReviewDecision: "APPROVED", }, "work/org/repo3": { - State: "CLOSED", + State: "CLOSED", + Mergeable: "UNKNOWN", + StatusCheckRollup: []github.StatusCheckRollup{ + { + State: "FAILURE", + }, + }, ReactionGroups: []github.ReactionGroup{ { Content: "THUMBS_DOWN", @@ -158,6 +183,66 @@ func prepareFakeResponses() { }, }, }, + "work/org/repo4": { + State: "OPEN", + StatusCheckRollup: []github.StatusCheckRollup{ + { + State: "SUCCESS", + }, + { + State: "PENDING", + }, + }, + ReactionGroups: []github.ReactionGroup{ + { + Content: "THUMBS_UP", + Users: github.ReactionGroupUsers{ + TotalCount: 3, + }, + }, + }, + ReviewDecision: "REVIEW_REQUIRED", + }, + "work/org/repo5": { + State: "OPEN", + StatusCheckRollup: []github.StatusCheckRollup{ + { + State: "FAILURE", + }, + { + State: "PENDING", + }, + }, + ReactionGroups: []github.ReactionGroup{ + { + Content: "THUMBS_UP", + Users: github.ReactionGroupUsers{ + TotalCount: 3, + }, + }, + }, + ReviewDecision: "REVIEW_REQUIRED", + }, + "work/org/repo6": { + State: "OPEN", + StatusCheckRollup: []github.StatusCheckRollup{ + { + State: "PENDING", + }, + { + State: "PENDING", + }, + }, + ReactionGroups: []github.ReactionGroup{ + { + Content: "THUMBS_UP", + Users: github.ReactionGroupUsers{ + TotalCount: 3, + }, + }, + }, + ReviewDecision: "REVIEW_REQUIRED", + }, } fakeGitHub := github.NewFakeGitHub(nil, func(workingDir string) (interface{}, error) { if workingDir == "work/org/repoWithError" { diff --git a/internal/github/github.go b/internal/github/github.go index a8f9519..c729125 100644 --- a/internal/github/github.go +++ b/internal/github/github.go @@ -108,15 +108,16 @@ type PrStatusResponse struct { // https://github.com/cli/cli/blob/4b415f80d79e57 } type PrStatus struct { - Closed bool `json:"closed"` - HeadRefName string `json:"headRefName"` - Mergeable string `json:"mergeable"` - Number int `json:"number"` - ReactionGroups []ReactionGroup `json:"reactionGroups"` - ReviewDecision string `json:"reviewDecision"` - State string `json:"state"` - Title string `json:"title"` - Url string `json:"url"` + Closed bool `json:"closed"` + HeadRefName string `json:"headRefName"` + Mergeable string `json:"mergeable"` + Number int `json:"number"` + ReactionGroups []ReactionGroup `json:"reactionGroups"` + ReviewDecision string `json:"reviewDecision"` + State string `json:"state"` + StatusCheckRollup []StatusCheckRollup `json:"statusCheckRollup"` + Title string `json:"title"` + Url string `json:"url"` } type ReactionGroupUsers struct { @@ -128,6 +129,10 @@ type ReactionGroup struct { Users ReactionGroupUsers } +type StatusCheckRollup struct { + State string +} + // GetPR is a helper function to retrieve the PR associated with the branch Name type NoPRFoundError struct { Path string @@ -139,7 +144,7 @@ func (e *NoPRFoundError) Error() string { } func (r *RealGitHub) GetPR(output io.Writer, workingDir string, branchName string) (*PrStatus, error) { - s, err := execInstance.ExecuteAndCapture(output, workingDir, "gh", "pr", "status", "--json", "closed,headRefName,mergeable,number,reactionGroups,reviewDecision,state,title,url") + s, err := execInstance.ExecuteAndCapture(output, workingDir, "gh", "pr", "status", "--json", "closed,headRefName,mergeable,number,reactionGroups,reviewDecision,state,statusCheckRollup,title,url") if err != nil { return nil, err } From 6997ad4e0dd8983e223e8ae34e505be447d86e46 Mon Sep 17 00:00:00 2001 From: Annette Wilson <33626088+annettejanewilson@users.noreply.github.com> Date: Mon, 17 Jun 2024 15:05:26 +0100 Subject: [PATCH 6/9] fix(foreach): Remove shell invocation from foreach (#136) * Remove shell invocation from foreach As discussed in [issue 135], turbolift applies shell escaping inconsistently on the arguments in foreach. Upon consideration, I think there is no good reason to apply shell escaping at all. It makes turbolift more fragile, more confusing and ultimately less flexible. Instead, do not use a shell at all and all these problems disappear. None of the examples in the README use this functionality. If users do need to invoke a shell, for example to do redirection, it is much more explicit and understandable for them if they do it themselves. This PR: * Removes the use of a shell, instead directly executing a subprocess based on the arguments provided. * Removes custom handling of arguments. Users are directed to the standard GNU-style `--` to indicate that everything else on the command-line is an argument, not an option. This makes the code substantially simpler. [issue 135]: #135 * Format foreach shell arguments nicely Shell escape the arguments so you can tell when arguments have spaces in them. * Require -- before foreach command Since we are deliberately removing custom handling of arguments, users will find that if they don't include a double hypen (`--`) separator before their foreach command, any options they pass to their command will be interpreted by turbolift and either result in errors, or worse, being quietly omitted from their command (e.g. `-v`), which could be surprising and frustrating! To head this off, refuse immediately if `--` either doesn't appear on the command-line, or appears after one or more arguments. * Reword foreach usage for accuracy Make it clear that the double-hyphen separator `--` is mandatory and not just a good idea. * Wrap long usage message Turns out Cobra doesn't wrap usage messages, so they look pretty ugly if we don't pick a pretty conservative width and wrap by hand. * Fix [flags] appearing at end of usage Turns out Cobra searches your usage line for the specific text "[flags]" and if it doesn't find it, shoves it on the end. Gross. For that reason, let's remove explicit callout of any flags and move it back to *before* the double-hyphen. * Improve logged command format and add tests Wrap braces around the logged command to delimit it clearly. Add unit tests to cover argument formatting. * Add foreach absolute path example --------- Co-authored-by: Daniel Ranson <92924979+Dan7-7-7@users.noreply.github.com> --- README.md | 27 ++++-- cmd/foreach/foreach.go | 81 ++++++------------ cmd/foreach/foreach_test.go | 163 +++++++++++------------------------- go.mod | 1 + go.sum | 2 + 5 files changed, 97 insertions(+), 177 deletions(-) diff --git a/README.md b/README.md index 52960ff..afcc3b5 100644 --- a/README.md +++ b/README.md @@ -111,8 +111,8 @@ Occasionally you may need to work on different repo files. For instance the repo The default repo file is called `repos.txt` but you can override this on any command with the `--repos` flag. ```console -turbolift foreach --repos repoFile1.txt sed 's/pattern1/replacement1/g' -turbolift foreach --repos repoFile2.txt sed 's/pattern2/replacement2/g' +turbolift foreach --repos repoFile1.txt -- sed 's/pattern1/replacement1/g' +turbolift foreach --repos repoFile2.txt -- sed 's/pattern2/replacement2/g' ``` @@ -132,16 +132,27 @@ You can do this manually using an editor, using `sed` and similar commands, or u **You are free to use any tools that help get the job done.** -If you wish to, you can run the same command against every repo using `turbolift foreach ...` (where `...` is the shell command you want to run). +If you wish to, you can run the same command against every repo using `turbolift foreach -- ...` (where `...` is the command you want to run). For example, you might choose to: -* `turbolift foreach rm somefile` - to delete a particular file -* `turbolift foreach sed -i '' 's/foo/bar/g' somefile` - to find/replace in a common file -* `turbolift foreach make test` - for example, to run tests (using any appropriate command to invoke the tests) -* `turbolift foreach git add somefile` - to stage a file that you have created +* `turbolift foreach -- rm somefile` - to delete a particular file +* `turbolift foreach -- sed -i '' 's/foo/bar/g' somefile` - to find/replace in a common file +* `turbolift foreach -- make test` - for example, to run tests (using any appropriate command to invoke the tests) +* `turbolift foreach -- git add somefile` - to stage a file that you have created +* `turbolift foreach -- sh -c 'grep needle haystack.txt > output.txt'` - use a shell to run a command using redirection -At any time, if you need to update your working copy branches from the upstream, you can run `turbolift foreach git pull upstream master`. +Remember that when the command runs the working directory will be the +repository root. If you want to refer to files from elsewhere you need +to provide an absolute path. You may find the `pwd` command helpful here. +For example, to run a shell script from the current directory against +each repository: + +``` +turbolift foreach -- sh "$(pwd)/script.sh" +``` + +At any time, if you need to update your working copy branches from the upstream, you can run `turbolift foreach -- git pull upstream master`. It is highly recommended that you run tests against affected repos, if it will help validate the changes you have made. diff --git a/cmd/foreach/foreach.go b/cmd/foreach/foreach.go index fe55cda..14bf4ca 100644 --- a/cmd/foreach/foreach.go +++ b/cmd/foreach/foreach.go @@ -16,9 +16,9 @@ package foreach import ( + "errors" "os" "path" - "strconv" "strings" "github.com/spf13/cobra" @@ -27,66 +27,46 @@ import ( "github.com/skyscanner/turbolift/internal/colors" "github.com/skyscanner/turbolift/internal/executor" "github.com/skyscanner/turbolift/internal/logging" + + "github.com/alessio/shellescape" ) var exec executor.Executor = executor.NewRealExecutor() var ( repoFile string = "repos.txt" - helpFlag bool = false ) -func parseForeachArgs(args []string) []string { - strippedArgs := make([]string, 0) -MAIN: - for i := 0; i < len(args); i++ { - switch args[i] { - case "--repos": - repoFile = args[i+1] - i = i + 1 - case "--help": - helpFlag = true - default: - // we've parsed everything that could be parsed; this is now the command - strippedArgs = append(strippedArgs, args[i:]...) - break MAIN - } +func formatArguments(arguments []string) string { + quotedArgs := make([]string, len(arguments)) + for i, arg := range arguments { + quotedArgs[i] = shellescape.Quote(arg) } - - return strippedArgs + return strings.Join(quotedArgs, " ") } func NewForeachCmd() *cobra.Command { cmd := &cobra.Command{ - Use: "foreach [flags] SHELL_COMMAND", - Short: "Run a shell command against each working copy", - Run: run, - Args: cobra.MinimumNArgs(1), - DisableFlagsInUseLine: true, - DisableFlagParsing: true, + Use: "foreach [flags] -- COMMAND [ARGUMENT...]", + Short: "Run COMMAND against each working copy", + Long: +`Run COMMAND against each working copy. Make sure to include a +double hyphen -- with space on both sides before COMMAND, as this +marks that no further options should be interpreted by turbolift.`, + RunE: runE, + Args: cobra.MinimumNArgs(1), } - // this flag will not be parsed (DisabledFlagParsing is on) but is here for the help context and auto complete cmd.Flags().StringVar(&repoFile, "repos", "repos.txt", "A file containing a list of repositories to clone.") return cmd } -func run(c *cobra.Command, args []string) { +func runE(c *cobra.Command, args []string) error { logger := logging.NewLogger(c) - /* - Parsing is disabled for this command to make sure it doesn't capture flags from the subsequent command. - E.g.: turbolift foreach ls -l <- here, the -l would be captured by foreach, not by ls - Because of this, we need a manual parsing of the arguments. - Assumption is the foreach arguments will be parsed before the command and its arguments. - */ - args = parseForeachArgs(args) - - // check if the help flag was toggled - if helpFlag { - _ = c.Usage() - return + if c.ArgsLenAtDash() != 0 { + return errors.New("Use -- to separate command") } readCampaignActivity := logger.StartActivity("Reading campaign data (%s)", repoFile) @@ -95,22 +75,19 @@ func run(c *cobra.Command, args []string) { dir, err := campaign.OpenCampaign(options) if err != nil { readCampaignActivity.EndWithFailure(err) - return + return nil } readCampaignActivity.EndWithSuccess() - for i := range args { - if strings.Contains(args[i], " ") { - args[i] = strconv.Quote(args[i]) - } - } - command := strings.Join(args, " ") + // We shell escape these to avoid ambiguity in our logs, and give + // the user something they could copy and paste. + prettyArgs := formatArguments(args) var doneCount, skippedCount, errorCount int for _, repo := range dir.Repos { repoDirPath := path.Join("work", repo.OrgName, repo.RepoName) // i.e. work/org/repo - execActivity := logger.StartActivity("Executing %s in %s", command, repoDirPath) + execActivity := logger.StartActivity("Executing { %s } in %s", prettyArgs, repoDirPath) // skip if the working copy does not exist if _, err = os.Stat(repoDirPath); os.IsNotExist(err) { @@ -119,13 +96,7 @@ func run(c *cobra.Command, args []string) { continue } - // Execute within a shell so that piping, redirection, etc are possible - shellCommand := os.Getenv("SHELL") - if shellCommand == "" { - shellCommand = "sh" - } - shellArgs := []string{"-c", command} - err := exec.Execute(execActivity.Writer(), repoDirPath, shellCommand, shellArgs...) + err := exec.Execute(execActivity.Writer(), repoDirPath, args[0], args[1:]...) if err != nil { execActivity.EndWithFailure(err) @@ -141,4 +112,6 @@ func run(c *cobra.Command, args []string) { } else { logger.Warnf("turbolift foreach completed with %s %s(%s, %s, %s)\n", colors.Red("errors"), colors.Normal(), colors.Green(doneCount, " OK"), colors.Yellow(skippedCount, " skipped"), colors.Red(errorCount, " errored")) } + + return nil } diff --git a/cmd/foreach/foreach_test.go b/cmd/foreach/foreach_test.go index de3de2b..da97ece 100644 --- a/cmd/foreach/foreach_test.go +++ b/cmd/foreach/foreach_test.go @@ -26,101 +26,6 @@ import ( "github.com/skyscanner/turbolift/internal/testsupport" ) -func TestParseForEachArgs(t *testing.T) { - testCases := []struct { - Name string - Args []string - ExpectedCommand []string - ExpectedRepoFileName string - ExpectedHelpFlag bool - }{ - { - Name: "simple command", - Args: []string{"ls", "-l"}, - ExpectedCommand: []string{"ls", "-l"}, - ExpectedRepoFileName: "repos.txt", - ExpectedHelpFlag: false, - }, - { - Name: "advanced command", - Args: []string{"sed", "-e", "'s/foo/bar/'", "-e", "'s/bar/baz/'"}, - ExpectedCommand: []string{"sed", "-e", "'s/foo/bar/'", "-e", "'s/bar/baz/'"}, - ExpectedRepoFileName: "repos.txt", - ExpectedHelpFlag: false, - }, - { - Name: "simple command with repo flag", - Args: []string{"--repos", "test.txt", "ls", "-l"}, - ExpectedCommand: []string{"ls", "-l"}, - ExpectedRepoFileName: "test.txt", - ExpectedHelpFlag: false, - }, - { - Name: "advanced command with repos flag", - Args: []string{"--repos", "test2.txt", "sed", "-e", "'s/foo/bar/'", "-e", "'s/bar/baz/'"}, - ExpectedCommand: []string{"sed", "-e", "'s/foo/bar/'", "-e", "'s/bar/baz/'"}, - ExpectedRepoFileName: "test2.txt", - ExpectedHelpFlag: false, - }, - { - Name: "repos flag should only be caught when at the beginning", - Args: []string{"ls", "-l", "--repos", "random.txt"}, - ExpectedCommand: []string{"ls", "-l", "--repos", "random.txt"}, - ExpectedRepoFileName: "repos.txt", - ExpectedHelpFlag: false, - }, - { - Name: "random flag is not caught", - Args: []string{"--random", "arg", "ls", "-l"}, - ExpectedCommand: []string{"--random", "arg", "ls", "-l"}, - ExpectedRepoFileName: "repos.txt", - ExpectedHelpFlag: false, - }, - { - Name: "Help flag is triggered", - Args: []string{"--help"}, - ExpectedCommand: []string{}, - ExpectedRepoFileName: "repos.txt", - ExpectedHelpFlag: true, - }, - { - Name: "Help flag is triggered after the repo one", - Args: []string{"--repos", "example.txt", "--help", "thecommand"}, - ExpectedCommand: []string{"thecommand"}, - ExpectedRepoFileName: "example.txt", - ExpectedHelpFlag: true, - }, - { - Name: "Help flag is triggered before the repo one", - Args: []string{"--help", "--repos", "example.txt", "newcommand", "anotherarg"}, - ExpectedCommand: []string{"newcommand", "anotherarg"}, - ExpectedRepoFileName: "example.txt", - ExpectedHelpFlag: true, - }, - { - Name: "Help flag is not triggered from a subsequent command", - Args: []string{"command", "--help"}, - ExpectedCommand: []string{"command", "--help"}, - ExpectedRepoFileName: "repos.txt", - ExpectedHelpFlag: false, - }, - } - - for _, tc := range testCases { - t.Run(tc.Name, func(t *testing.T) { - actual := parseForeachArgs(tc.Args) - t.Log(actual) - assert.EqualValues(t, tc.ExpectedCommand, actual) - assert.Equal(t, repoFile, tc.ExpectedRepoFileName) - assert.Equal(t, helpFlag, tc.ExpectedHelpFlag) - - // Cleanup to default repo file name - repoFile = "repos.txt" - helpFlag = false - }) - } -} - func TestItRejectsEmptyArgs(t *testing.T) { fakeExecutor := executor.NewAlwaysSucceedsFakeExecutor() exec = fakeExecutor @@ -128,43 +33,59 @@ func TestItRejectsEmptyArgs(t *testing.T) { testsupport.PrepareTempCampaign(true, "org/repo1", "org/repo2") out, err := runCommand([]string{}...) - assert.Errorf(t, err, "requires at least 1 arg(s), only received 0") + assert.Error(t, err, "Expected an error to be returned") assert.Contains(t, out, "Usage") fakeExecutor.AssertCalledWith(t, [][]string{}) } -func TestItRunsCommandInShellAgainstWorkingCopies(t *testing.T) { +func TestItRejectsCommandWithoutDashes(t *testing.T) { fakeExecutor := executor.NewAlwaysSucceedsFakeExecutor() exec = fakeExecutor testsupport.PrepareTempCampaign(true, "org/repo1", "org/repo2") out, err := runCommand("some", "command") + assert.Error(t, err, "Expected an error to be returned") + assert.Contains(t, out, "Usage") + + fakeExecutor.AssertCalledWith(t, [][]string{}) +} + +func TestItRunsCommandWithoutShellAgainstWorkingCopies(t *testing.T) { + fakeExecutor := executor.NewAlwaysSucceedsFakeExecutor() + exec = fakeExecutor + + testsupport.PrepareTempCampaign(true, "org/repo1", "org/repo2") + + out, err := runCommand("--", "some", "command") assert.NoError(t, err) assert.Contains(t, out, "turbolift foreach completed") assert.Contains(t, out, "2 OK, 0 skipped") fakeExecutor.AssertCalledWith(t, [][]string{ - {"work/org/repo1", userShell(), "-c", "some command"}, - {"work/org/repo2", userShell(), "-c", "some command"}, + {"work/org/repo1", "some", "command"}, + {"work/org/repo2", "some", "command"}, }) } -func TestItRunsCommandQuotedInShellAgainstWorkingCopied(t *testing.T) { +func TestItRunsCommandWithSpacesAgainstWorkingCopied(t *testing.T) { fakeExecutor := executor.NewAlwaysSucceedsFakeExecutor() exec = fakeExecutor testsupport.PrepareTempCampaign(true, "org/repo1", "org/repo2") - out, err := runCommand("some", "command", "with spaces") + out, err := runCommand("--", "some", "command", "with spaces") assert.NoError(t, err) assert.Contains(t, out, "turbolift foreach completed") assert.Contains(t, out, "2 OK, 0 skipped") + assert.Contains(t, out, + "Executing { some command 'with spaces' } in work/org/repo1", + "It should format the executed command accurately") fakeExecutor.AssertCalledWith(t, [][]string{ - {"work/org/repo1", userShell(), "-c", "some command \"with spaces\""}, - {"work/org/repo2", userShell(), "-c", "some command \"with spaces\""}, + {"work/org/repo1", "some", "command", "with spaces"}, + {"work/org/repo2", "some", "command", "with spaces"}, }) } @@ -175,13 +96,13 @@ func TestItSkipsMissingWorkingCopies(t *testing.T) { testsupport.PrepareTempCampaign(true, "org/repo1", "org/repo2") _ = os.Remove("work/org/repo2") - out, err := runCommand("some", "command") + out, err := runCommand("--", "some", "command") assert.NoError(t, err) assert.Contains(t, out, "turbolift foreach completed") assert.Contains(t, out, "1 OK, 1 skipped") fakeExecutor.AssertCalledWith(t, [][]string{ - {"work/org/repo1", userShell(), "-c", "some command"}, + {"work/org/repo1", "some", "command"}, }) } @@ -191,14 +112,14 @@ func TestItContinuesOnAndRecordsFailures(t *testing.T) { testsupport.PrepareTempCampaign(true, "org/repo1", "org/repo2") - out, err := runCommand("some", "command") + out, err := runCommand("--", "some", "command") assert.NoError(t, err) assert.Contains(t, out, "turbolift foreach completed with errors") assert.Contains(t, out, "0 OK, 0 skipped, 2 errored") fakeExecutor.AssertCalledWith(t, [][]string{ - {"work/org/repo1", userShell(), "-c", "some command"}, - {"work/org/repo2", userShell(), "-c", "some command"}, + {"work/org/repo1", "some", "command"}, + {"work/org/repo2", "some", "command"}, }) } @@ -208,12 +129,12 @@ func TestHelpFlagReturnsUsage(t *testing.T) { testsupport.PrepareTempCampaign(true, "org/repo1", "org/repo2") - out, err := runCommand("--help", "command1") + out, err := runCommand("--help", "--", "command1") t.Log(out) assert.NoError(t, err) // should return usage assert.Contains(t, out, "Usage:") - assert.Contains(t, out, "foreach [flags] SHELL_COMMAND") + assert.Contains(t, out, "foreach [flags] -- COMMAND [ARGUMENT...]") assert.Contains(t, out, "Flags:") assert.Contains(t, out, "help for foreach") @@ -221,12 +142,24 @@ func TestHelpFlagReturnsUsage(t *testing.T) { fakeExecutor.AssertCalledWith(t, [][]string{}) } -func userShell() string { - shell := os.Getenv("SHELL") - if shell == "" { - return "sh" +func TestFormatArguments(t *testing.T) { + // Don't go too heavy here. We are not seeking to exhaustively test + // shellescape. We just want to make sure formatArguments works. + var tests = []struct { + input []string + expected string + title string + }{ + {[]string{""}, `''`, "Empty arg should be quoted"}, + {[]string{"one two"}, `'one two'`, "Arg with space should be quoted"}, + {[]string{"one"}, `one`, "Plain arg should not need quotes"}, + {[]string{}, ``, "Empty arg list should give empty string"}, + {[]string{"x", "", "y y"}, `x '' 'y y'`, "Args should be separated with spaces"}, + } + for _, test := range tests { + actual := formatArguments(test.input) + assert.Equal(t, actual, test.expected, test.title) } - return shell } func runCommand(args ...string) (string, error) { diff --git a/go.mod b/go.mod index 0c462d6..66fd088 100644 --- a/go.mod +++ b/go.mod @@ -3,6 +3,7 @@ module github.com/skyscanner/turbolift go 1.16 require ( + github.com/alessio/shellescape v1.4.2 // indirect github.com/briandowns/spinner v1.15.0 github.com/fatih/color v1.12.0 github.com/manifoldco/promptui v0.9.0 diff --git a/go.sum b/go.sum index 167c699..0009688 100644 --- a/go.sum +++ b/go.sum @@ -16,6 +16,8 @@ github.com/BurntSushi/xgb v0.0.0-20160522181843-27f122750802/go.mod h1:IVnqGOEym github.com/OneOfOne/xxhash v1.2.2/go.mod h1:HSdplMjZKSmBqAxg5vPj2TmRDmfkzw+cTzAElWljhcU= github.com/alecthomas/template v0.0.0-20160405071501-a0175ee3bccc/go.mod h1:LOuyumcjzFXgccqObfd/Ljyb9UuFJ6TxHnclSeseNhc= github.com/alecthomas/units v0.0.0-20151022065526-2efee857e7cf/go.mod h1:ybxpYRFXyAe+OPACYpWeL0wqObRcbAqCMya13uyzqw0= +github.com/alessio/shellescape v1.4.2 h1:MHPfaU+ddJ0/bYWpgIeUnQUqKrlJ1S7BfEYPM4uEoM0= +github.com/alessio/shellescape v1.4.2/go.mod h1:PZAiSCk0LJaZkiCSkPv8qIobYglO3FPpyFjDCtHLS30= github.com/armon/circbuf v0.0.0-20150827004946-bbbad097214e/go.mod h1:3U/XgcO3hCbHZ8TKRvWD2dDTCfh9M9ya+I9JpbB7O8o= github.com/armon/go-metrics v0.0.0-20180917152333-f0300d1749da/go.mod h1:Q73ZrmVTwzkszR9V5SSuryQ31EELlFMUz1kKyl939pY= github.com/armon/go-radix v0.0.0-20180808171621-7fddfc383310/go.mod h1:ufUuZ+zHj4x4TnLV4JWEpy2hxWSpsRywHrMgIH9cCH8= From fb6a1c47f0e5aa69f49b3a49166a3b1847334ab7 Mon Sep 17 00:00:00 2001 From: Daniel Ranson <92924979+Dan7-7-7@users.noreply.github.com> Date: Mon, 15 Jul 2024 15:05:01 +0100 Subject: [PATCH 7/9] fix(clone): provide helpful message when fork conflicts cause an error (#139) * suggest fix for fork conflict * newlines --------- Co-authored-by: Danny Ranson --- cmd/clone/clone.go | 1 + cmd/clone/clone_test.go | 2 ++ 2 files changed, 3 insertions(+) diff --git a/cmd/clone/clone.go b/cmd/clone/clone.go index 8e964dd..c88a38c 100644 --- a/cmd/clone/clone.go +++ b/cmd/clone/clone.go @@ -126,6 +126,7 @@ func run(c *cobra.Command, _ []string) { err = g.Pull(pullFromUpstreamActivity.Writer(), repoDirPath, "upstream", defaultBranch) if err != nil { pullFromUpstreamActivity.EndWithFailure(err) + logger.Printf("\nWe weren't able to pull the latest upstream changes into your fork of %s. This is probably because you have a pre-existing fork with commits ahead of upstream. Please change this or delete your fork, and try again.\n", repo.FullRepoName) errorCount++ continue } diff --git a/cmd/clone/clone_test.go b/cmd/clone/clone_test.go index 04b077d..5359e86 100644 --- a/cmd/clone/clone_test.go +++ b/cmd/clone/clone_test.go @@ -204,6 +204,8 @@ func TestItLogsPullErrorsButContinuesToTryAll(t *testing.T) { assert.NoError(t, err) assert.Contains(t, out, "Pulling latest changes from org1/repo1") assert.Contains(t, out, "Pulling latest changes from org2/repo2") + assert.Contains(t, out, "We weren't able to pull the latest upstream changes into your fork of org1/repo1") + assert.Contains(t, out, "We weren't able to pull the latest upstream changes into your fork of org2/repo2") assert.Contains(t, out, "turbolift clone completed with errors") assert.Contains(t, out, "2 repos errored") From 6197cf2dd3d26c641d5480f6d76f073d300d0a80 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Mon, 5 Aug 2024 09:03:41 +0100 Subject: [PATCH 8/9] Bump golangci/golangci-lint-action from 6.0.1 to 6.1.0 (#142) Bumps [golangci/golangci-lint-action](https://github.com/golangci/golangci-lint-action) from 6.0.1 to 6.1.0. - [Release notes](https://github.com/golangci/golangci-lint-action/releases) - [Commits](https://github.com/golangci/golangci-lint-action/compare/a4f60bb28d35aeee14e6880718e0c85ff1882e64...aaa42aa0628b4ae2578232a66b541047968fac86) --- updated-dependencies: - dependency-name: golangci/golangci-lint-action dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- .github/workflows/golangci-lint.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/golangci-lint.yml b/.github/workflows/golangci-lint.yml index eea3c7d..f739ea7 100644 --- a/.github/workflows/golangci-lint.yml +++ b/.github/workflows/golangci-lint.yml @@ -13,7 +13,7 @@ jobs: steps: - uses: actions/checkout@v4 - name: golangci-lint - uses: golangci/golangci-lint-action@a4f60bb28d35aeee14e6880718e0c85ff1882e64 # v6.0.1 + uses: golangci/golangci-lint-action@aaa42aa0628b4ae2578232a66b541047968fac86 # v6.1.0 with: # Optional: version of golangci-lint to use in form of v1.2 or v1.2.3 or `latest` to use the latest version version: v1.42.1 From b5cd6d215b30e661c8fc3caeb9a684654058a6c9 Mon Sep 17 00:00:00 2001 From: Richard North Date: Mon, 5 Aug 2024 09:23:17 +0100 Subject: [PATCH 9/9] Add file output to foreach command (#141) * Add file output to foreach command * Reorder * Simplify code --- cmd/foreach/foreach.go | 70 ++++++++++++++++++++++++++++-- cmd/foreach/foreach_test.go | 42 ++++++++++++++++++ internal/executor/fake_executor.go | 22 ++++++++++ internal/logging/activity.go | 4 ++ 4 files changed, 135 insertions(+), 3 deletions(-) diff --git a/cmd/foreach/foreach.go b/cmd/foreach/foreach.go index 14bf4ca..79dffc2 100644 --- a/cmd/foreach/foreach.go +++ b/cmd/foreach/foreach.go @@ -17,6 +17,7 @@ package foreach import ( "errors" + "fmt" "os" "path" "strings" @@ -34,7 +35,15 @@ import ( var exec executor.Executor = executor.NewRealExecutor() var ( - repoFile string = "repos.txt" + repoFile = "repos.txt" + + overallResultsDirectory string + + successfulResultsDirectory string + successfulReposFileName string + + failedResultsDirectory string + failedReposFileName string ) func formatArguments(arguments []string) string { @@ -49,8 +58,7 @@ func NewForeachCmd() *cobra.Command { cmd := &cobra.Command{ Use: "foreach [flags] -- COMMAND [ARGUMENT...]", Short: "Run COMMAND against each working copy", - Long: -`Run COMMAND against each working copy. Make sure to include a + Long: `Run COMMAND against each working copy. Make sure to include a double hyphen -- with space on both sides before COMMAND, as this marks that no further options should be interpreted by turbolift.`, RunE: runE, @@ -83,6 +91,10 @@ func runE(c *cobra.Command, args []string) error { // the user something they could copy and paste. prettyArgs := formatArguments(args) + setupOutputFiles(dir.Name, prettyArgs) + + logger.Printf("Logs for all executions will be stored under %s", overallResultsDirectory) + var doneCount, skippedCount, errorCount int for _, repo := range dir.Repos { repoDirPath := path.Join("work", repo.OrgName, repo.RepoName) // i.e. work/org/repo @@ -99,9 +111,11 @@ func runE(c *cobra.Command, args []string) error { err := exec.Execute(execActivity.Writer(), repoDirPath, args[0], args[1:]...) if err != nil { + emitOutcomeToFiles(repo, failedReposFileName, failedResultsDirectory, execActivity.Logs(), logger) execActivity.EndWithFailure(err) errorCount++ } else { + emitOutcomeToFiles(repo, successfulReposFileName, successfulResultsDirectory, execActivity.Logs(), logger) execActivity.EndWithSuccessAndEmitLogs() doneCount++ } @@ -113,5 +127,55 @@ func runE(c *cobra.Command, args []string) error { logger.Warnf("turbolift foreach completed with %s %s(%s, %s, %s)\n", colors.Red("errors"), colors.Normal(), colors.Green(doneCount, " OK"), colors.Yellow(skippedCount, " skipped"), colors.Red(errorCount, " errored")) } + logger.Printf("Logs for all executions have been stored under %s", overallResultsDirectory) + logger.Printf("Names of successful repos have been written to %s", successfulReposFileName) + logger.Printf("Names of failed repos have been written to %s", failedReposFileName) + return nil } + +// sets up a temporary directory to store success/failure logs etc +func setupOutputFiles(campaignName string, command string) { + overallResultsDirectory, _ = os.MkdirTemp("", fmt.Sprintf("turbolift-foreach-%s-", campaignName)) + successfulResultsDirectory = path.Join(overallResultsDirectory, "successful") + failedResultsDirectory = path.Join(overallResultsDirectory, "failed") + _ = os.MkdirAll(successfulResultsDirectory, 0755) + _ = os.MkdirAll(failedResultsDirectory, 0755) + + successfulReposFileName = path.Join(successfulResultsDirectory, "repos.txt") + failedReposFileName = path.Join(failedResultsDirectory, "repos.txt") + + // create the files + successfulReposFile, _ := os.Create(successfulReposFileName) + failedReposFile, _ := os.Create(failedReposFileName) + defer successfulReposFile.Close() + defer failedReposFile.Close() + + _, _ = successfulReposFile.WriteString(fmt.Sprintf("# This file contains the list of repositories that were successfully processed by turbolift foreach\n# for the command: %s\n", command)) + _, _ = failedReposFile.WriteString(fmt.Sprintf("# This file contains the list of repositories that failed to be processed by turbolift foreach\n# for the command: %s\n", command)) +} + +func emitOutcomeToFiles(repo campaign.Repo, reposFileName string, logsDirectoryParent string, executionLogs string, logger *logging.Logger) { + // write the repo name to the repos file + reposFile, _ := os.OpenFile(reposFileName, os.O_RDWR|os.O_APPEND, 0644) + defer reposFile.Close() + _, err := reposFile.WriteString(repo.FullRepoName + "\n") + if err != nil { + logger.Errorf("Failed to write repo name to %s: %s", reposFile.Name(), err) + } + + // write logs to a file under the logsParent directory, in a directory structure that mirrors that of the work directory + logsDir := path.Join(logsDirectoryParent, repo.FullRepoName) + logsFile := path.Join(logsDir, "logs.txt") + err = os.MkdirAll(logsDir, 0755) + if err != nil { + logger.Errorf("Failed to create directory %s: %s", logsDir, err) + } + + logs, _ := os.Create(logsFile) + defer logs.Close() + _, err = logs.WriteString(executionLogs) + if err != nil { + logger.Errorf("Failed to write logs to %s: %s", logsFile, err) + } +} diff --git a/cmd/foreach/foreach_test.go b/cmd/foreach/foreach_test.go index da97ece..9b8afbc 100644 --- a/cmd/foreach/foreach_test.go +++ b/cmd/foreach/foreach_test.go @@ -18,6 +18,7 @@ package foreach import ( "bytes" "os" + "regexp" "testing" "github.com/stretchr/testify/assert" @@ -162,6 +163,47 @@ func TestFormatArguments(t *testing.T) { } } +func TestItCreatesLogFiles(t *testing.T) { + fakeExecutor := executor.NewAlternatingSuccessFakeExecutor() + exec = fakeExecutor + + testsupport.PrepareTempCampaign(true, "org/repo1", "org/repo2") + + out, err := runCommand("--", "some", "command") + assert.NoError(t, err) + assert.Contains(t, out, "turbolift foreach completed") + assert.Contains(t, out, "1 OK, 0 skipped, 1 errored") + + // Logs should describe where output was written + r := regexp.MustCompile(`Logs for all executions have been stored under (.+)`) + matches := r.FindStringSubmatch(out) + assert.Len(t, matches, 2, "Expected to find the log directory path") + path := matches[1] + + // check that expected static directories and files exist + _, err = os.Stat(path) + assert.NoError(t, err, "Expected the log directory to exist") + + _, err = os.Stat(path + "/successful") + assert.NoError(t, err, "Expected the successful log directory to exist") + + _, err = os.Stat(path + "/failed") + assert.NoError(t, err, "Expected the failure log directory to exist") + + _, err = os.Stat(path + "/successful/repos.txt") + assert.NoError(t, err, "Expected the successful repos.txt file to exist") + + _, err = os.Stat(path + "/failed/repos.txt") + assert.NoError(t, err, "Expected the failure repos.txt file to exist") + + // check that the expected logs files exist + _, err = os.Stat(path + "/successful/org/repo1/logs.txt") + assert.NoError(t, err, "Expected the successful log file for org/repo1 to exist") + + _, err = os.Stat(path + "/failed/org/repo2/logs.txt") + assert.NoError(t, err, "Expected the failure log file for org/repo2 to exist") +} + func runCommand(args ...string) (string, error) { cmd := NewForeachCmd() outBuffer := bytes.NewBufferString("") diff --git a/internal/executor/fake_executor.go b/internal/executor/fake_executor.go index 68282eb..f31f336 100644 --- a/internal/executor/fake_executor.go +++ b/internal/executor/fake_executor.go @@ -70,3 +70,25 @@ func NewAlwaysFailsFakeExecutor() *FakeExecutor { return "", errors.New("synthetic error") }) } + +func NewAlternatingSuccessFakeExecutor() *FakeExecutor { + i := 0 + return NewFakeExecutor( + func(s string, s2 string, s3 ...string) error { + i++ + if i%2 == 1 { + return nil + } else { + return errors.New("synthetic error") + } + }, + func(s string, s2 string, s3 ...string) (string, error) { + i++ + if i%2 == 1 { + return "", nil + } else { + return "", errors.New("synthetic error") + } + }, + ) +} diff --git a/internal/logging/activity.go b/internal/logging/activity.go index 06e93e7..976d706 100644 --- a/internal/logging/activity.go +++ b/internal/logging/activity.go @@ -110,3 +110,7 @@ func (a *Activity) Writer() io.Writer { activity: a, } } + +func (a *Activity) Logs() string { + return strings.Join(a.logs, "\n") +}