From 77115a8c04c56058319454c6ac42561871f55d75 Mon Sep 17 00:00:00 2001 From: Danny Ranson Date: Mon, 7 Oct 2024 15:55:17 +0100 Subject: [PATCH 1/5] implement symlinks and tests --- README.md | 11 ++ cmd/foreach/foreach.go | 67 ++++++++++- cmd/foreach/foreach_test.go | 180 ++++++++++++++++++++++++++++ internal/testsupport/testsupport.go | 13 ++ 4 files changed, 265 insertions(+), 6 deletions(-) diff --git a/README.md b/README.md index afcc3b5..0b7ab7c 100644 --- a/README.md +++ b/README.md @@ -156,6 +156,17 @@ At any time, if you need to update your working copy branches from the upstream, It is highly recommended that you run tests against affected repos, if it will help validate the changes you have made. +#### Logging and re-running with foreach + +Every time a command is run with `turbolift foreach`, logging output for each repository is collected in a temporary directory +divided into `successful` and `failed` subdirectories. Each of these also contains a separate file listing all the repositories that succeeded or failed. + +You can use `--successful` or `--failed` to run a foreach command only against the repositories that succeeded or failed in the preceding command. + +``` +turbolift foreach --failed -- make test +``` + ### Committing changes When ready to commit changes across all repos, run: diff --git a/cmd/foreach/foreach.go b/cmd/foreach/foreach.go index 79dffc2..ca7be25 100644 --- a/cmd/foreach/foreach.go +++ b/cmd/foreach/foreach.go @@ -35,15 +35,19 @@ import ( var exec executor.Executor = executor.NewRealExecutor() var ( - repoFile = "repos.txt" + repoFile string + successful bool + failed bool overallResultsDirectory string successfulResultsDirectory string successfulReposFileName string + successfulReposSymlink = ".latest_successful" failedResultsDirectory string failedReposFileName string + failedReposSymlink = ".latest_failed" ) func formatArguments(arguments []string) string { @@ -54,6 +58,17 @@ func formatArguments(arguments []string) string { return strings.Join(quotedArgs, " ") } +func moreThanOne(args ...bool) bool { + b := map[bool]int{ + false: 0, + true: 0, + } + for _, v := range args { + b[v] += 1 + } + return b[true] > 1 +} + func NewForeachCmd() *cobra.Command { cmd := &cobra.Command{ Use: "foreach [flags] -- COMMAND [ARGUMENT...]", @@ -65,7 +80,9 @@ marks that no further options should be interpreted by turbolift.`, Args: cobra.MinimumNArgs(1), } - cmd.Flags().StringVar(&repoFile, "repos", "repos.txt", "A file containing a list of repositories to clone.") + cmd.Flags().StringVar(&repoFile, "repos", "", "A file containing a list of repositories to clone.") + cmd.Flags().BoolVar(&successful, "successful", false, "Indication of whether to run against previously successful repos only.") + cmd.Flags().BoolVar(&failed, "failed", false, "Indication of whether to run against previously failed repos only.") return cmd } @@ -77,6 +94,23 @@ func runE(c *cobra.Command, args []string) error { return errors.New("Use -- to separate command") } + customRepoFile := repoFile != "" + if moreThanOne(successful, failed, customRepoFile) { + return errors.New("only one repositories flag or option may be specified: either --successful; --failed; or --repos ") + } + if successful { + var err error + if repoFile, err = os.Readlink(successfulReposSymlink); err != nil { + return errors.New("no previous successful foreach logs found") + } + } else if failed { + var err error + if repoFile, err = os.Readlink(failedReposSymlink); err != nil { + return errors.New("no previous failed foreach logs found") + } + } else if !customRepoFile { + repoFile = "repos.txt" + } readCampaignActivity := logger.StartActivity("Reading campaign data (%s)", repoFile) options := campaign.NewCampaignOptions() options.RepoFilename = repoFile @@ -91,7 +125,7 @@ func runE(c *cobra.Command, args []string) error { // the user something they could copy and paste. prettyArgs := formatArguments(args) - setupOutputFiles(dir.Name, prettyArgs) + setupOutputFiles(dir.Name, prettyArgs, logger) logger.Printf("Logs for all executions will be stored under %s", overallResultsDirectory) @@ -128,14 +162,14 @@ func runE(c *cobra.Command, args []string) error { } 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) + logger.Printf("Names of successful repos have been written to %s. Use --successful to run the next foreach command against these repos", successfulReposFileName) + logger.Printf("Names of failed repos have been written to %s. Use --failed to run the next foreach command against these repos", failedReposFileName) return nil } // sets up a temporary directory to store success/failure logs etc -func setupOutputFiles(campaignName string, command string) { +func setupOutputFiles(campaignName string, command string, logger *logging.Logger) { overallResultsDirectory, _ = os.MkdirTemp("", fmt.Sprintf("turbolift-foreach-%s-", campaignName)) successfulResultsDirectory = path.Join(overallResultsDirectory, "successful") failedResultsDirectory = path.Join(overallResultsDirectory, "failed") @@ -151,6 +185,27 @@ func setupOutputFiles(campaignName string, command string) { defer successfulReposFile.Close() defer failedReposFile.Close() + if _, err := os.Lstat(successfulReposSymlink); err == nil { + err := os.Remove(successfulReposSymlink) + if err != nil { + logger.Warnf("Failed to remove previous symlink for successful repos: %v", err) + } + } + err := os.Symlink(successfulReposFileName, successfulReposSymlink) + if err != nil { + logger.Warnf("Failed to create symlink for successful repos: %v", err) + } + if _, err := os.Lstat(failedReposSymlink); err == nil { + err := os.Remove(failedReposSymlink) + if err != nil { + logger.Warnf("Failed to remove previous symlink for failed repos: %v", err) + } + } + err = os.Symlink(failedReposFileName, failedReposSymlink) + if err != nil { + logger.Warnf("Failed to create symlink for failed repos: %v", err) + } + _, _ = 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)) } diff --git a/cmd/foreach/foreach_test.go b/cmd/foreach/foreach_test.go index 9b8afbc..7b6ebcc 100644 --- a/cmd/foreach/foreach_test.go +++ b/cmd/foreach/foreach_test.go @@ -204,6 +204,133 @@ func TestItCreatesLogFiles(t *testing.T) { assert.NoError(t, err, "Expected the failure log file for org/repo2 to exist") } +func TestItRunsAgainstSuccessfulReposOnly(t *testing.T) { + fakeExecutor := executor.NewAlternatingSuccessFakeExecutor() + exec = fakeExecutor + + testsupport.PrepareTempCampaign(true, "org/repo1", "org/repo2", "org/repo3") + testsupport.CreateAnotherRepoFile("successful.txt", "org/repo1", "org/repo3") + testsupport.CreateNewSymlink("successful.txt", ".latest_successful") + + out, err := runCommandReposSuccessful("--", "some", "command") + assert.NoError(t, err) + assert.Contains(t, out, "turbolift foreach completed") + assert.Contains(t, out, "1 OK, 0 skipped, 1 errored") + assert.Contains(t, out, "org/repo1") + assert.Contains(t, out, "org/repo3") + assert.NotContains(t, out, "org/repo2") + + fakeExecutor.AssertCalledWith(t, [][]string{ + {"work/org/repo1", "some", "command"}, + {"work/org/repo3", "some", "command"}, + }) + + // check that the symlink has been updated + successfulRepoFile, err := os.Readlink(".latest_successful") + if err != nil { + panic(err) + } + assert.NotEqual(t, successfulRepoFile, "successful.txt") +} + +func TestItRunsAgainstFailedReposOnly(t *testing.T) { + fakeExecutor := executor.NewAlternatingSuccessFakeExecutor() + exec = fakeExecutor + + testsupport.PrepareTempCampaign(true, "org/repo1", "org/repo2", "org/repo3") + testsupport.CreateAnotherRepoFile("failed.txt", "org/repo1", "org/repo3") + testsupport.CreateNewSymlink("failed.txt", ".latest_failed") + + out, err := runCommandReposFailed("--", "some", "command") + assert.NoError(t, err) + assert.Contains(t, out, "turbolift foreach completed") + assert.Contains(t, out, "1 OK, 0 skipped, 1 errored") + assert.Contains(t, out, "org/repo1") + assert.Contains(t, out, "org/repo3") + assert.NotContains(t, out, "org/repo2") + + fakeExecutor.AssertCalledWith(t, [][]string{ + {"work/org/repo1", "some", "command"}, + {"work/org/repo3", "some", "command"}, + }) + + // check that the symlink has been updated + failedRepoFile, err := os.Readlink(".latest_failed") + if err != nil { + panic(err) + } + assert.NotEqual(t, failedRepoFile, "failed.txt") +} + +func TestItCreatesSymlinksSuccessfully(t *testing.T) { + fakeExecutor := executor.NewAlternatingSuccessFakeExecutor() + exec = fakeExecutor + + testsupport.PrepareTempCampaign(true, "org/repo1", "org/repo2", "org/repo3") + + out, err := runCommand("--", "some", "command") + assert.NoError(t, err) + assert.Contains(t, out, "turbolift foreach completed") + assert.Contains(t, out, "2 OK, 0 skipped, 1 errored") + + successfulRepoFile, err := os.Readlink(".latest_successful") + if err != nil { + panic(err) + } + successfulRepos, err := os.ReadFile(successfulRepoFile) + if err != nil { + panic(err) + } + assert.Contains(t, string(successfulRepos), "org/repo1") + assert.Contains(t, string(successfulRepos), "org/repo3") + assert.NotContains(t, string(successfulRepos), "org/repo2") + + failedRepoFile, err := os.Readlink(".latest_failed") + if err != nil { + panic(err) + } + failedRepos, err := os.ReadFile(failedRepoFile) + if err != nil { + panic(err) + } + assert.Contains(t, string(failedRepos), "org/repo2") + assert.NotContains(t, string(failedRepos), "org/repo1") + assert.NotContains(t, string(failedRepos), "org/repo3") +} + +func TestItRunsAgainstCustomReposFile(t *testing.T) { + fakeExecutor := executor.NewAlternatingSuccessFakeExecutor() + exec = fakeExecutor + + testsupport.PrepareTempCampaign(true, "org/repo1", "org/repo2", "org/repo3") + testsupport.CreateAnotherRepoFile("custom_repofile.txt", "org/repo1", "org/repo3") + + out, err := runCommandReposCustom("--", "some", "command") + assert.NoError(t, err) + assert.Contains(t, out, "turbolift foreach completed") + assert.Contains(t, out, "1 OK, 0 skipped, 1 errored") + assert.Contains(t, out, "org/repo1") + assert.Contains(t, out, "org/repo3") + assert.NotContains(t, out, "org/repo2") + + fakeExecutor.AssertCalledWith(t, [][]string{ + {"work/org/repo1", "some", "command"}, + {"work/org/repo3", "some", "command"}, + }) +} + +func TestItDoesNotAllowMultipleReposArguments(t *testing.T) { + fakeExecutor := executor.NewAlwaysSucceedsFakeExecutor() + exec = fakeExecutor + + testsupport.PrepareTempCampaign(true, "org/repo1", "org/repo2", "org/repo3") + + _, err := runCommandReposMultiple("--", "some", "command") + assert.Error(t, err, "only one repositories flag or option may be specified: either --successful; --failed; or --repos ") + + fakeExecutor.AssertCalledWith(t, [][]string{}) +} + func runCommand(args ...string) (string, error) { cmd := NewForeachCmd() outBuffer := bytes.NewBufferString("") @@ -215,3 +342,56 @@ func runCommand(args ...string) (string, error) { } return outBuffer.String(), nil } + +func runCommandReposSuccessful(args ...string) (string, error) { + cmd := NewForeachCmd() + successful = true + outBuffer := bytes.NewBufferString("") + cmd.SetOut(outBuffer) + cmd.SetArgs(args) + err := cmd.Execute() + if err != nil { + return outBuffer.String(), err + } + return outBuffer.String(), nil +} + +func runCommandReposFailed(args ...string) (string, error) { + cmd := NewForeachCmd() + failed = true + outBuffer := bytes.NewBufferString("") + cmd.SetOut(outBuffer) + cmd.SetArgs(args) + err := cmd.Execute() + if err != nil { + return outBuffer.String(), err + } + return outBuffer.String(), nil +} + +func runCommandReposCustom(args ...string) (string, error) { + cmd := NewForeachCmd() + repoFile = "custom_repofile.txt" + outBuffer := bytes.NewBufferString("") + cmd.SetOut(outBuffer) + cmd.SetArgs(args) + err := cmd.Execute() + if err != nil { + return outBuffer.String(), err + } + return outBuffer.String(), nil +} + +func runCommandReposMultiple(args ...string) (string, error) { + cmd := NewForeachCmd() + successful = true + repoFile = "custom_repofile.txt" + outBuffer := bytes.NewBufferString("") + cmd.SetOut(outBuffer) + cmd.SetArgs(args) + err := cmd.Execute() + if err != nil { + return outBuffer.String(), err + } + return outBuffer.String(), nil +} diff --git a/internal/testsupport/testsupport.go b/internal/testsupport/testsupport.go index e2d9a85..862cd88 100644 --- a/internal/testsupport/testsupport.go +++ b/internal/testsupport/testsupport.go @@ -96,3 +96,16 @@ func UsePrTitleTodoOnly() { func UsePrBodyTodoOnly() { CreateOrUpdatePrDescriptionFile("README.md", "updated PR title", originalPrBodyTodo) } + +func CreateNewSymlink(target string, linkName string) { + if _, err := os.Lstat(linkName); err == nil { + err = os.Remove(linkName) + if err != nil { + panic(err) + } + } + err := os.Symlink(target, linkName) + if err != nil { + panic(err) + } +} From e04a2ee0bb7a7b81021477eb4cad99705701d2b6 Mon Sep 17 00:00:00 2001 From: Danny Ranson Date: Fri, 1 Nov 2024 17:14:45 +0000 Subject: [PATCH 2/5] use a single symlink --- cmd/foreach/foreach.go | 38 ++++++++--------- cmd/foreach/foreach_test.go | 63 +++++++++++++++++------------ internal/testsupport/testsupport.go | 22 ++-------- 3 files changed, 57 insertions(+), 66 deletions(-) diff --git a/cmd/foreach/foreach.go b/cmd/foreach/foreach.go index ca7be25..5ca584c 100644 --- a/cmd/foreach/foreach.go +++ b/cmd/foreach/foreach.go @@ -43,13 +43,13 @@ var ( successfulResultsDirectory string successfulReposFileName string - successfulReposSymlink = ".latest_successful" failedResultsDirectory string failedReposFileName string - failedReposSymlink = ".latest_failed" ) +const previousResultsSymlink = ".previous_results" + func formatArguments(arguments []string) string { quotedArgs := make([]string, len(arguments)) for i, arg := range arguments { @@ -99,18 +99,21 @@ func runE(c *cobra.Command, args []string) error { return errors.New("only one repositories flag or option may be specified: either --successful; --failed; or --repos ") } if successful { - var err error - if repoFile, err = os.Readlink(successfulReposSymlink); err != nil { - return errors.New("no previous successful foreach logs found") + previousResults, err := os.Readlink(previousResultsSymlink) + if err != nil { + return errors.New("no previous foreach logs found") } + repoFile = path.Join(previousResults, "successful", "repos.txt") } else if failed { - var err error - if repoFile, err = os.Readlink(failedReposSymlink); err != nil { - return errors.New("no previous failed foreach logs found") + previousResults, err := os.Readlink(previousResultsSymlink) + if err != nil { + return errors.New("no previous foreach logs found") } + repoFile = path.Join(previousResults, "failed", "repos.txt") } else if !customRepoFile { repoFile = "repos.txt" } + readCampaignActivity := logger.StartActivity("Reading campaign data (%s)", repoFile) options := campaign.NewCampaignOptions() options.RepoFilename = repoFile @@ -185,25 +188,16 @@ func setupOutputFiles(campaignName string, command string, logger *logging.Logge defer successfulReposFile.Close() defer failedReposFile.Close() - if _, err := os.Lstat(successfulReposSymlink); err == nil { - err := os.Remove(successfulReposSymlink) + // create symlink to the results + if _, err := os.Lstat(previousResultsSymlink); err == nil { + err := os.Remove(previousResultsSymlink) if err != nil { logger.Warnf("Failed to remove previous symlink for successful repos: %v", err) } } - err := os.Symlink(successfulReposFileName, successfulReposSymlink) - if err != nil { - logger.Warnf("Failed to create symlink for successful repos: %v", err) - } - if _, err := os.Lstat(failedReposSymlink); err == nil { - err := os.Remove(failedReposSymlink) - if err != nil { - logger.Warnf("Failed to remove previous symlink for failed repos: %v", err) - } - } - err = os.Symlink(failedReposFileName, failedReposSymlink) + err := os.Symlink(overallResultsDirectory, previousResultsSymlink) if err != nil { - logger.Warnf("Failed to create symlink for failed repos: %v", err) + logger.Warnf("Failed to create symlink to foreach results: %v", err) } _, _ = 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)) diff --git a/cmd/foreach/foreach_test.go b/cmd/foreach/foreach_test.go index 7b6ebcc..ae1d92e 100644 --- a/cmd/foreach/foreach_test.go +++ b/cmd/foreach/foreach_test.go @@ -18,7 +18,9 @@ package foreach import ( "bytes" "os" + "path" "regexp" + "strings" "testing" "github.com/stretchr/testify/assert" @@ -209,8 +211,8 @@ func TestItRunsAgainstSuccessfulReposOnly(t *testing.T) { exec = fakeExecutor testsupport.PrepareTempCampaign(true, "org/repo1", "org/repo2", "org/repo3") - testsupport.CreateAnotherRepoFile("successful.txt", "org/repo1", "org/repo3") - testsupport.CreateNewSymlink("successful.txt", ".latest_successful") + setUpSymlink() + defer os.RemoveAll("mock_output") out, err := runCommandReposSuccessful("--", "some", "command") assert.NoError(t, err) @@ -224,13 +226,6 @@ func TestItRunsAgainstSuccessfulReposOnly(t *testing.T) { {"work/org/repo1", "some", "command"}, {"work/org/repo3", "some", "command"}, }) - - // check that the symlink has been updated - successfulRepoFile, err := os.Readlink(".latest_successful") - if err != nil { - panic(err) - } - assert.NotEqual(t, successfulRepoFile, "successful.txt") } func TestItRunsAgainstFailedReposOnly(t *testing.T) { @@ -238,8 +233,8 @@ func TestItRunsAgainstFailedReposOnly(t *testing.T) { exec = fakeExecutor testsupport.PrepareTempCampaign(true, "org/repo1", "org/repo2", "org/repo3") - testsupport.CreateAnotherRepoFile("failed.txt", "org/repo1", "org/repo3") - testsupport.CreateNewSymlink("failed.txt", ".latest_failed") + setUpSymlink() + defer os.RemoveAll("mock_output") out, err := runCommandReposFailed("--", "some", "command") assert.NoError(t, err) @@ -253,13 +248,6 @@ func TestItRunsAgainstFailedReposOnly(t *testing.T) { {"work/org/repo1", "some", "command"}, {"work/org/repo3", "some", "command"}, }) - - // check that the symlink has been updated - failedRepoFile, err := os.Readlink(".latest_failed") - if err != nil { - panic(err) - } - assert.NotEqual(t, failedRepoFile, "failed.txt") } func TestItCreatesSymlinksSuccessfully(t *testing.T) { @@ -273,10 +261,12 @@ func TestItCreatesSymlinksSuccessfully(t *testing.T) { assert.Contains(t, out, "turbolift foreach completed") assert.Contains(t, out, "2 OK, 0 skipped, 1 errored") - successfulRepoFile, err := os.Readlink(".latest_successful") + resultsDir, err := os.Readlink(".previous_results") if err != nil { panic(err) } + + successfulRepoFile := path.Join(resultsDir, "successful", "repos.txt") successfulRepos, err := os.ReadFile(successfulRepoFile) if err != nil { panic(err) @@ -285,14 +275,8 @@ func TestItCreatesSymlinksSuccessfully(t *testing.T) { assert.Contains(t, string(successfulRepos), "org/repo3") assert.NotContains(t, string(successfulRepos), "org/repo2") - failedRepoFile, err := os.Readlink(".latest_failed") - if err != nil { - panic(err) - } + failedRepoFile := path.Join(resultsDir, "failed", "repos.txt") failedRepos, err := os.ReadFile(failedRepoFile) - if err != nil { - panic(err) - } assert.Contains(t, string(failedRepos), "org/repo2") assert.NotContains(t, string(failedRepos), "org/repo1") assert.NotContains(t, string(failedRepos), "org/repo3") @@ -331,6 +315,33 @@ func TestItDoesNotAllowMultipleReposArguments(t *testing.T) { fakeExecutor.AssertCalledWith(t, [][]string{}) } +func setUpSymlink() { + err := os.MkdirAll("mock_output/successful", 0755) + if err != nil { + panic(err) + } + err = os.MkdirAll("mock_output/failed", 0755) + if err != nil { + panic(err) + } + err = os.Symlink("mock_output", ".previous_results") + if err != nil { + panic(err) + } + _, err = os.Create("mock_output/successful/repos.txt") + if err != nil { + panic(err) + } + _, err = os.Create("mock_output/failed/repos.txt") + if err != nil { + panic(err) + } + repos := []string{"org/repo1", "org/repo3"} + delimitedList := strings.Join(repos, "\n") + _ = os.WriteFile("mock_output/successful/repos.txt", []byte(delimitedList), os.ModePerm|0o644) + _ = os.WriteFile("mock_output/failed/repos.txt", []byte(delimitedList), os.ModePerm|0o644) +} + func runCommand(args ...string) (string, error) { cmd := NewForeachCmd() outBuffer := bytes.NewBufferString("") diff --git a/internal/testsupport/testsupport.go b/internal/testsupport/testsupport.go index 862cd88..55c94ee 100644 --- a/internal/testsupport/testsupport.go +++ b/internal/testsupport/testsupport.go @@ -17,7 +17,6 @@ package testsupport import ( "fmt" - "io/ioutil" "os" "path" "path/filepath" @@ -33,7 +32,7 @@ func Pwd() string { } func CreateAndEnterTempDirectory() string { - tempDir, _ := ioutil.TempDir("", "turbolift-test-*") + tempDir, _ := os.MkdirTemp("", "turbolift-test-*") err := os.Chdir(tempDir) if err != nil { panic(err) @@ -45,7 +44,7 @@ func PrepareTempCampaign(createDirs bool, repos ...string) string { tempDir := CreateAndEnterTempDirectory() delimitedList := strings.Join(repos, "\n") - err := ioutil.WriteFile("repos.txt", []byte(delimitedList), os.ModePerm|0o644) + err := os.WriteFile("repos.txt", []byte(delimitedList), os.ModePerm|0o644) if err != nil { panic(err) } @@ -61,7 +60,7 @@ func PrepareTempCampaign(createDirs bool, repos ...string) string { } dummyPrDescription := "# PR title\nPR body" - err = ioutil.WriteFile("README.md", []byte(dummyPrDescription), os.ModePerm|0o644) + err = os.WriteFile("README.md", []byte(dummyPrDescription), os.ModePerm|0o644) if err != nil { panic(err) } @@ -71,7 +70,7 @@ func PrepareTempCampaign(createDirs bool, repos ...string) string { func CreateAnotherRepoFile(filename string, repos ...string) { delimitedList := strings.Join(repos, "\n") - err := ioutil.WriteFile(filename, []byte(delimitedList), os.ModePerm|0o644) + err := os.WriteFile(filename, []byte(delimitedList), os.ModePerm|0o644) if err != nil { panic(err) } @@ -96,16 +95,3 @@ func UsePrTitleTodoOnly() { func UsePrBodyTodoOnly() { CreateOrUpdatePrDescriptionFile("README.md", "updated PR title", originalPrBodyTodo) } - -func CreateNewSymlink(target string, linkName string) { - if _, err := os.Lstat(linkName); err == nil { - err = os.Remove(linkName) - if err != nil { - panic(err) - } - } - err := os.Symlink(target, linkName) - if err != nil { - panic(err) - } -} From f00fec08db307c0b1c74d0e6300c38866b78ef97 Mon Sep 17 00:00:00 2001 From: Danny Ranson Date: Mon, 4 Nov 2024 09:48:54 +0000 Subject: [PATCH 3/5] update error handling --- README.md | 17 ++++++++++++++++- cmd/foreach/foreach_test.go | 31 +++++++++++++++++++++---------- 2 files changed, 37 insertions(+), 11 deletions(-) diff --git a/README.md b/README.md index 0b7ab7c..4bd1345 100644 --- a/README.md +++ b/README.md @@ -159,7 +159,22 @@ It is highly recommended that you run tests against affected repos, if it will h #### Logging and re-running with foreach Every time a command is run with `turbolift foreach`, logging output for each repository is collected in a temporary directory -divided into `successful` and `failed` subdirectories. Each of these also contains a separate file listing all the repositories that succeeded or failed. +with the following structure: + +``` +temp-dir + \ successful + \ repos.txt # a list of repos where the command succeeded + \ org + \ repo + \ logs.txt # logs from the specific foreach execution on this repo + .... + \ failed + \ repos.txt # a list of repos where the command succeeded + \ org + \ repo + \ logs.txt # logs from the specific foreach execution on this repo +``` You can use `--successful` or `--failed` to run a foreach command only against the repositories that succeeded or failed in the preceding command. diff --git a/cmd/foreach/foreach_test.go b/cmd/foreach/foreach_test.go index ae1d92e..205d88d 100644 --- a/cmd/foreach/foreach_test.go +++ b/cmd/foreach/foreach_test.go @@ -211,7 +211,10 @@ func TestItRunsAgainstSuccessfulReposOnly(t *testing.T) { exec = fakeExecutor testsupport.PrepareTempCampaign(true, "org/repo1", "org/repo2", "org/repo3") - setUpSymlink() + err := setUpSymlink() + if err != nil { + t.Errorf("Error setting up symlink: %s", err) + } defer os.RemoveAll("mock_output") out, err := runCommandReposSuccessful("--", "some", "command") @@ -233,7 +236,10 @@ func TestItRunsAgainstFailedReposOnly(t *testing.T) { exec = fakeExecutor testsupport.PrepareTempCampaign(true, "org/repo1", "org/repo2", "org/repo3") - setUpSymlink() + err := setUpSymlink() + if err != nil { + t.Errorf("Error setting up symlink: %s", err) + } defer os.RemoveAll("mock_output") out, err := runCommandReposFailed("--", "some", "command") @@ -263,13 +269,14 @@ func TestItCreatesSymlinksSuccessfully(t *testing.T) { resultsDir, err := os.Readlink(".previous_results") if err != nil { - panic(err) + + t.Errorf("Error reading symlink: %s", err) } successfulRepoFile := path.Join(resultsDir, "successful", "repos.txt") successfulRepos, err := os.ReadFile(successfulRepoFile) if err != nil { - panic(err) + t.Errorf("Error reading successful repos: %s", err) } assert.Contains(t, string(successfulRepos), "org/repo1") assert.Contains(t, string(successfulRepos), "org/repo3") @@ -277,6 +284,9 @@ func TestItCreatesSymlinksSuccessfully(t *testing.T) { failedRepoFile := path.Join(resultsDir, "failed", "repos.txt") failedRepos, err := os.ReadFile(failedRepoFile) + if err != nil { + t.Errorf("Error reading failed repos: %s", err) + } assert.Contains(t, string(failedRepos), "org/repo2") assert.NotContains(t, string(failedRepos), "org/repo1") assert.NotContains(t, string(failedRepos), "org/repo3") @@ -315,31 +325,32 @@ func TestItDoesNotAllowMultipleReposArguments(t *testing.T) { fakeExecutor.AssertCalledWith(t, [][]string{}) } -func setUpSymlink() { +func setUpSymlink() error { err := os.MkdirAll("mock_output/successful", 0755) if err != nil { - panic(err) + return err } err = os.MkdirAll("mock_output/failed", 0755) if err != nil { - panic(err) + return err } err = os.Symlink("mock_output", ".previous_results") if err != nil { - panic(err) + return err } _, err = os.Create("mock_output/successful/repos.txt") if err != nil { - panic(err) + return err } _, err = os.Create("mock_output/failed/repos.txt") if err != nil { - panic(err) + return err } repos := []string{"org/repo1", "org/repo3"} delimitedList := strings.Join(repos, "\n") _ = os.WriteFile("mock_output/successful/repos.txt", []byte(delimitedList), os.ModePerm|0o644) _ = os.WriteFile("mock_output/failed/repos.txt", []byte(delimitedList), os.ModePerm|0o644) + return nil } func runCommand(args ...string) (string, error) { From 74301a3fb31f88b707b7dadee26b95e799237863 Mon Sep 17 00:00:00 2001 From: Danny Ranson Date: Mon, 4 Nov 2024 09:53:21 +0000 Subject: [PATCH 4/5] readme update --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 4bd1345..82bb867 100644 --- a/README.md +++ b/README.md @@ -176,7 +176,7 @@ temp-dir \ logs.txt # logs from the specific foreach execution on this repo ``` -You can use `--successful` or `--failed` to run a foreach command only against the repositories that succeeded or failed in the preceding command. +You can use `--successful` or `--failed` to run a foreach command only against the repositories that succeeded or failed in the preceding foreach execution. ``` turbolift foreach --failed -- make test From 3388a62b0c03a1840fbd9fa963cc351b10d20a76 Mon Sep 17 00:00:00 2001 From: Danny Ranson Date: Tue, 5 Nov 2024 16:22:08 +0000 Subject: [PATCH 5/5] minor changes --- cmd/foreach/foreach.go | 16 ++++++++-------- cmd/foreach/foreach_test.go | 4 ++-- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/cmd/foreach/foreach.go b/cmd/foreach/foreach.go index 5ca584c..cd9cf6e 100644 --- a/cmd/foreach/foreach.go +++ b/cmd/foreach/foreach.go @@ -35,7 +35,7 @@ import ( var exec executor.Executor = executor.NewRealExecutor() var ( - repoFile string + repoFile = "repos.txt" successful bool failed bool @@ -48,7 +48,7 @@ var ( failedReposFileName string ) -const previousResultsSymlink = ".previous_results" +const previousResultsSymlink = "..turbolift_previous_results" func formatArguments(arguments []string) string { quotedArgs := make([]string, len(arguments)) @@ -80,7 +80,7 @@ marks that no further options should be interpreted by turbolift.`, Args: cobra.MinimumNArgs(1), } - cmd.Flags().StringVar(&repoFile, "repos", "", "A file containing a list of repositories to clone.") + cmd.Flags().StringVar(&repoFile, "repos", "repos.txt", "A file containing a list of repositories to clone.") cmd.Flags().BoolVar(&successful, "successful", false, "Indication of whether to run against previously successful repos only.") cmd.Flags().BoolVar(&failed, "failed", false, "Indication of whether to run against previously failed repos only.") @@ -94,9 +94,9 @@ func runE(c *cobra.Command, args []string) error { return errors.New("Use -- to separate command") } - customRepoFile := repoFile != "" - if moreThanOne(successful, failed, customRepoFile) { - return errors.New("only one repositories flag or option may be specified: either --successful; --failed; or --repos ") + isCustomRepoFile := repoFile != "repos.txt" + if moreThanOne(successful, failed, isCustomRepoFile) { + return errors.New("a maximum of one repositories flag / option may be specified: either --successful; --failed; or --repos ") } if successful { previousResults, err := os.Readlink(previousResultsSymlink) @@ -104,14 +104,14 @@ func runE(c *cobra.Command, args []string) error { return errors.New("no previous foreach logs found") } repoFile = path.Join(previousResults, "successful", "repos.txt") + logger.Printf("Running against previously successful repos only") } else if failed { previousResults, err := os.Readlink(previousResultsSymlink) if err != nil { return errors.New("no previous foreach logs found") } repoFile = path.Join(previousResults, "failed", "repos.txt") - } else if !customRepoFile { - repoFile = "repos.txt" + logger.Printf("Running against previously failed repos only") } readCampaignActivity := logger.StartActivity("Reading campaign data (%s)", repoFile) diff --git a/cmd/foreach/foreach_test.go b/cmd/foreach/foreach_test.go index 205d88d..78a737b 100644 --- a/cmd/foreach/foreach_test.go +++ b/cmd/foreach/foreach_test.go @@ -267,7 +267,7 @@ func TestItCreatesSymlinksSuccessfully(t *testing.T) { assert.Contains(t, out, "turbolift foreach completed") assert.Contains(t, out, "2 OK, 0 skipped, 1 errored") - resultsDir, err := os.Readlink(".previous_results") + resultsDir, err := os.Readlink("..turbolift_previous_results") if err != nil { t.Errorf("Error reading symlink: %s", err) @@ -334,7 +334,7 @@ func setUpSymlink() error { if err != nil { return err } - err = os.Symlink("mock_output", ".previous_results") + err = os.Symlink("mock_output", "..turbolift_previous_results") if err != nil { return err }