From a9c8a939d682269abf62474d2005511d5b434e35 Mon Sep 17 00:00:00 2001 From: Richard North Date: Tue, 23 Jul 2024 10:26:53 +0100 Subject: [PATCH 1/4] Add file output to foreach command --- cmd/foreach/foreach.go | 75 +++++++++++++++++++++++++++++- cmd/foreach/foreach_test.go | 42 +++++++++++++++++ internal/executor/fake_executor.go | 22 +++++++++ internal/logging/activity.go | 4 ++ 4 files changed, 141 insertions(+), 2 deletions(-) diff --git a/cmd/foreach/foreach.go b/cmd/foreach/foreach.go index 14bf4ca..747ef87 100644 --- a/cmd/foreach/foreach.go +++ b/cmd/foreach/foreach.go @@ -17,6 +17,7 @@ package foreach import ( "errors" + "fmt" "os" "path" "strings" @@ -49,8 +50,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 +83,10 @@ func runE(c *cobra.Command, args []string) error { // the user something they could copy and paste. prettyArgs := formatArguments(args) + o := setupOutputFiles(dir.Name, prettyArgs) + + logger.Printf("Logs for all executions will be stored under %s", o.overallResultsDirectory) + var doneCount, skippedCount, errorCount int for _, repo := range dir.Repos { repoDirPath := path.Join("work", repo.OrgName, repo.RepoName) // i.e. work/org/repo @@ -101,7 +105,9 @@ func runE(c *cobra.Command, args []string) error { if err != nil { execActivity.EndWithFailure(err) errorCount++ + emitOutcomeToFiles(repo, o.failedReposFile, o.failedResultsDirectory, execActivity.Logs(), logger) } else { + emitOutcomeToFiles(repo, o.successfulReposFile, o.successfulResultsDirectory, execActivity.Logs(), logger) execActivity.EndWithSuccessAndEmitLogs() doneCount++ } @@ -113,5 +119,70 @@ 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", o.overallResultsDirectory) + logger.Printf("Names of successful repos have been written to %s", o.successfulReposFile.Name()) + logger.Printf("Names of failed repos have been written to %s", o.failedReposFile.Name()) + return nil } + +type outputLogFileDestinations struct { + overallResultsDirectory string + + successfulResultsDirectory string + successfulReposFile *os.File + + failedResultsDirectory string + failedReposFile *os.File +} + +// sets up a temporary directory to store success/failure logs etc +func setupOutputFiles(campaignName string, command string) outputLogFileDestinations { + resultsDirectory, _ := os.MkdirTemp("", fmt.Sprintf("turbolift-foreach-%s-", campaignName)) + successfulResultsDirectory := path.Join(resultsDirectory, "successful") + failedResultsDirectory := path.Join(resultsDirectory, "failed") + _ = os.MkdirAll(successfulResultsDirectory, 0755) + _ = os.MkdirAll(failedResultsDirectory, 0755) + + successfulReposTxt := path.Join(successfulResultsDirectory, "repos.txt") + failedReposTxt := path.Join(failedResultsDirectory, "repos.txt") + + // create the files + successfulReposFile, _ := os.Create(successfulReposTxt) + failedReposFile, _ := os.Create(failedReposTxt) + + _, _ = 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)) + + return outputLogFileDestinations{ + overallResultsDirectory: resultsDirectory, + + successfulResultsDirectory: successfulResultsDirectory, + successfulReposFile: successfulReposFile, + + failedResultsDirectory: failedResultsDirectory, + failedReposFile: failedReposFile, + } +} + +func emitOutcomeToFiles(repo campaign.Repo, reposFile *os.File, logsDirectoryParent string, executionLogs string, logger *logging.Logger) { + // write the repo name to the repos file + _, 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) + _, 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") +} From 9c1f65ce648bb64661d72bd2902f42241230468a Mon Sep 17 00:00:00 2001 From: Richard North Date: Tue, 23 Jul 2024 10:35:30 +0100 Subject: [PATCH 2/4] Reorder --- cmd/foreach/foreach.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/foreach/foreach.go b/cmd/foreach/foreach.go index 747ef87..c26f159 100644 --- a/cmd/foreach/foreach.go +++ b/cmd/foreach/foreach.go @@ -103,9 +103,9 @@ func runE(c *cobra.Command, args []string) error { err := exec.Execute(execActivity.Writer(), repoDirPath, args[0], args[1:]...) if err != nil { + emitOutcomeToFiles(repo, o.failedReposFile, o.failedResultsDirectory, execActivity.Logs(), logger) execActivity.EndWithFailure(err) errorCount++ - emitOutcomeToFiles(repo, o.failedReposFile, o.failedResultsDirectory, execActivity.Logs(), logger) } else { emitOutcomeToFiles(repo, o.successfulReposFile, o.successfulResultsDirectory, execActivity.Logs(), logger) execActivity.EndWithSuccessAndEmitLogs() From bee3fddf45b94864e917d7093d8a73f75fe5513e Mon Sep 17 00:00:00 2001 From: Richard North Date: Tue, 23 Jul 2024 11:51:15 +0100 Subject: [PATCH 3/4] Simplify code --- cmd/foreach/foreach.go | 67 +++++++++++++++++++----------------------- 1 file changed, 30 insertions(+), 37 deletions(-) diff --git a/cmd/foreach/foreach.go b/cmd/foreach/foreach.go index c26f159..4c04a87 100644 --- a/cmd/foreach/foreach.go +++ b/cmd/foreach/foreach.go @@ -35,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 { @@ -83,9 +91,9 @@ func runE(c *cobra.Command, args []string) error { // the user something they could copy and paste. prettyArgs := formatArguments(args) - o := setupOutputFiles(dir.Name, prettyArgs) + setupOutputFiles(dir.Name, prettyArgs) - logger.Printf("Logs for all executions will be stored under %s", o.overallResultsDirectory) + logger.Printf("Logs for all executions will be stored under %s", overallResultsDirectory) var doneCount, skippedCount, errorCount int for _, repo := range dir.Repos { @@ -103,11 +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, o.failedReposFile, o.failedResultsDirectory, execActivity.Logs(), logger) + emitOutcomeToFiles(repo, failedReposFileName, failedResultsDirectory, execActivity.Logs(), logger) execActivity.EndWithFailure(err) errorCount++ } else { - emitOutcomeToFiles(repo, o.successfulReposFile, o.successfulResultsDirectory, execActivity.Logs(), logger) + emitOutcomeToFiles(repo, successfulReposFileName, successfulResultsDirectory, execActivity.Logs(), logger) execActivity.EndWithSuccessAndEmitLogs() doneCount++ } @@ -119,58 +127,42 @@ 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", o.overallResultsDirectory) - logger.Printf("Names of successful repos have been written to %s", o.successfulReposFile.Name()) - logger.Printf("Names of failed repos have been written to %s", o.failedReposFile.Name()) + 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 } -type outputLogFileDestinations struct { - overallResultsDirectory string - - successfulResultsDirectory string - successfulReposFile *os.File - - failedResultsDirectory string - failedReposFile *os.File -} - // sets up a temporary directory to store success/failure logs etc -func setupOutputFiles(campaignName string, command string) outputLogFileDestinations { - resultsDirectory, _ := os.MkdirTemp("", fmt.Sprintf("turbolift-foreach-%s-", campaignName)) - successfulResultsDirectory := path.Join(resultsDirectory, "successful") - failedResultsDirectory := path.Join(resultsDirectory, "failed") +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) - successfulReposTxt := path.Join(successfulResultsDirectory, "repos.txt") - failedReposTxt := path.Join(failedResultsDirectory, "repos.txt") + successfulReposFileName = path.Join(successfulResultsDirectory, "repos.txt") + failedReposFileName = path.Join(failedResultsDirectory, "repos.txt") // create the files - successfulReposFile, _ := os.Create(successfulReposTxt) - failedReposFile, _ := os.Create(failedReposTxt) + successfulReposFile, _ := os.Create(successfulReposFileName) + failedReposFile, _ := os.Create(failedReposFileName) _, _ = 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)) - - return outputLogFileDestinations{ - overallResultsDirectory: resultsDirectory, - - successfulResultsDirectory: successfulResultsDirectory, - successfulReposFile: successfulReposFile, - - failedResultsDirectory: failedResultsDirectory, - failedReposFile: failedReposFile, - } + successfulReposFile.Close() + failedReposFile.Close() } -func emitOutcomeToFiles(repo campaign.Repo, reposFile *os.File, logsDirectoryParent string, executionLogs string, logger *logging.Logger) { +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) _, err := reposFile.WriteString(repo.FullRepoName + "\n") if err != nil { logger.Errorf("Failed to write repo name to %s: %s", reposFile.Name(), err) } + reposFile.Close() // 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) @@ -185,4 +177,5 @@ func emitOutcomeToFiles(repo campaign.Repo, reposFile *os.File, logsDirectoryPar if err != nil { logger.Errorf("Failed to write logs to %s: %s", logsFile, err) } + logs.Close() } From 3e11664daf16056ac8fe2caf3626509f2e38b791 Mon Sep 17 00:00:00 2001 From: Richard North Date: Mon, 5 Aug 2024 08:51:15 +0100 Subject: [PATCH 4/4] Defer closing of files --- cmd/foreach/foreach.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/cmd/foreach/foreach.go b/cmd/foreach/foreach.go index 4c04a87..79dffc2 100644 --- a/cmd/foreach/foreach.go +++ b/cmd/foreach/foreach.go @@ -148,21 +148,21 @@ func setupOutputFiles(campaignName string, command string) { // 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)) - successfulReposFile.Close() - failedReposFile.Close() } 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) } - reposFile.Close() // 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) @@ -173,9 +173,9 @@ func emitOutcomeToFiles(repo campaign.Repo, reposFileName string, logsDirectoryP } 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) } - logs.Close() }