From be27e9cd44a938988adca00546aceeff13669652 Mon Sep 17 00:00:00 2001 From: Danny Ranson Date: Thu, 12 Dec 2024 16:41:05 +0000 Subject: [PATCH 1/4] push command --- cmd/updateprs/updateprs.go | 62 +++++++++++++++++++++++++++++-- cmd/updateprs/updateprs_test.go | 66 +++++++++++++++++++++++++++++++++ 2 files changed, 125 insertions(+), 3 deletions(-) diff --git a/cmd/updateprs/updateprs.go b/cmd/updateprs/updateprs.go index 612813d..6972940 100644 --- a/cmd/updateprs/updateprs.go +++ b/cmd/updateprs/updateprs.go @@ -18,6 +18,7 @@ package updateprs import ( "errors" "fmt" + "github.com/skyscanner/turbolift/internal/git" "os" "github.com/spf13/cobra" @@ -31,12 +32,14 @@ import ( var ( gh github.GitHub = github.NewRealGitHub() + g git.Git = git.NewRealGit() p prompt.Prompt = prompt.NewRealPrompt() ) var ( closeFlag bool updateDescriptionFlag bool + pushFlag bool yesFlag bool repoFile string prDescriptionFile string @@ -51,6 +54,7 @@ func NewUpdatePRsCmd() *cobra.Command { cmd.Flags().BoolVar(&closeFlag, "close", false, "Close all generated PRs") cmd.Flags().BoolVar(&updateDescriptionFlag, "amend-description", false, "Update PR titles and descriptions") + cmd.Flags().BoolVar(&pushFlag, "push", false, "Push new commits") cmd.Flags().BoolVar(&yesFlag, "yes", false, "Skips the confirmation prompt") cmd.Flags().StringVar(&repoFile, "repos", "repos.txt", "A file containing a list of repositories to clone.") cmd.Flags().StringVar(&prDescriptionFile, "description", "README.md", "A file containing the title and description for the PRs.") @@ -71,8 +75,8 @@ func onlyOne(args ...bool) bool { return b[true] == 1 } -func validateFlags(closeFlag bool, updateDescriptionFlag bool) error { - if !onlyOne(closeFlag, updateDescriptionFlag) { +func validateFlags(closeFlag bool, updateDescriptionFlag bool, pushFlag bool) error { + if !onlyOne(closeFlag, updateDescriptionFlag, pushFlag) { return errors.New("update-prs needs one and only one action flag") } return nil @@ -81,7 +85,7 @@ func validateFlags(closeFlag bool, updateDescriptionFlag bool) error { // we keep the args as one of the subfunctions might need it one day. func run(c *cobra.Command, args []string) { logger := logging.NewLogger(c) - if err := validateFlags(closeFlag, updateDescriptionFlag); err != nil { + if err := validateFlags(closeFlag, updateDescriptionFlag, pushFlag); err != nil { logger.Errorf("Error while parsing the flags: %v", err) return } @@ -90,6 +94,8 @@ func run(c *cobra.Command, args []string) { runClose(c, args) } else if updateDescriptionFlag { runUpdatePrDescription(c, args) + } else if pushFlag { + runPush(c, args) } } @@ -206,3 +212,53 @@ func runUpdatePrDescription(c *cobra.Command, _ []string) { logger.Warnf("turbolift update-prs 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")) } } + +func runPush(c *cobra.Command, _ []string) { + logger := logging.NewLogger(c) + + readCampaignActivity := logger.StartActivity("Reading campaign data (%s)", repoFile) + options := campaign.NewCampaignOptions() + options.RepoFilename = repoFile + dir, err := campaign.OpenCampaign(options) + if err != nil { + readCampaignActivity.EndWithFailure(err) + return + } + readCampaignActivity.EndWithSuccess() + + // Prompting for confirmation + if !yesFlag { + if !p.AskConfirm(fmt.Sprintf("Push new commits to %s campaign PRs for all repos in %s?", dir.Name, repoFile)) { + return + } + } + + doneCount := 0 + skippedCount := 0 + errorCount := 0 + + for _, repo := range dir.Repos { + pushActivity := logger.StartActivity("Pushing changes in %s to origin", repo.FullRepoName) + // skip if the working copy does not exist + if _, err = os.Stat(repo.FullRepoPath()); os.IsNotExist(err) { + pushActivity.EndWithWarningf("Directory %s does not exist - has it been cloned?", repo.FullRepoPath()) + skippedCount++ + continue + } + + err := g.Push(pushActivity.Writer(), repo.FullRepoPath(), "origin", dir.Name) + if err != nil { + pushActivity.EndWithFailure(err) + errorCount++ + continue + } + pushActivity.EndWithSuccess() + doneCount++ + } + + if errorCount == 0 { + logger.Successf("turbolift update-prs completed %s(%s, %s)\n", colors.Normal(), colors.Green(doneCount, " OK"), colors.Yellow(skippedCount, " skipped")) + } else { + logger.Warnf("turbolift update-prs 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")) + } +} diff --git a/cmd/updateprs/updateprs_test.go b/cmd/updateprs/updateprs_test.go index d710a0e..005f46c 100644 --- a/cmd/updateprs/updateprs_test.go +++ b/cmd/updateprs/updateprs_test.go @@ -2,6 +2,7 @@ package updateprs import ( "bytes" + "github.com/skyscanner/turbolift/internal/git" "path/filepath" "testing" @@ -165,6 +166,45 @@ func TestItDoesNotUpdateDescriptionsIfNotConfirmed(t *testing.T) { fakeGitHub.AssertCalledWith(t, [][]string{}) } +func TestItPushesNewCommits(t *testing.T) { + fakeGitHub := github.NewAlwaysSucceedsFakeGitHub() + gh = fakeGitHub + fakeGit := git.NewAlwaysSucceedsFakeGit() + g = fakeGit + + tempDir := testsupport.PrepareTempCampaign(true, "org/repo1", "org/repo2") + + out, err := runPushCommandAuto() + assert.NoError(t, err) + assert.Contains(t, out, "Pushing changes in org/repo1 to origin") + assert.Contains(t, out, "Pushing changes in org/repo2 to origin") + assert.Contains(t, out, "turbolift update-prs completed") + assert.Contains(t, out, "2 OK, 0 skipped") + + fakeGit.AssertCalledWith(t, [][]string{ + {"push", "work/org/repo1", filepath.Base(tempDir)}, + {"push", "work/org/repo2", filepath.Base(tempDir)}, + }) +} + +func TestItDoesNotPushIfNotConfirmed(t *testing.T) { + fakeGitHub := github.NewAlwaysSucceedsFakeGitHub() + gh = fakeGitHub + fakePrompt := prompt.NewFakePromptNo() + p = fakePrompt + + _ = testsupport.PrepareTempCampaign(true, "org/repo1", "org/repo2") + + out, err := runPushCommandConfirm() + assert.NoError(t, err) + assert.NotContains(t, out, "Pushing changes in org/repo1 to origin") + assert.NotContains(t, out, "Pushing changes in org/repo2 to origin") + assert.NotContains(t, out, "turbolift update-prs completed") + assert.NotContains(t, out, "2 OK") + + fakeGitHub.AssertCalledWith(t, [][]string{}) +} + func runCloseCommandAuto() (string, error) { cmd := NewUpdatePRsCmd() closeFlag = true @@ -217,3 +257,29 @@ func runUpdateDescriptionCommandConfirm() (string, error) { } return outBuffer.String(), nil } + +func runPushCommandAuto() (string, error) { + cmd := NewUpdatePRsCmd() + pushFlag = true + yesFlag = true + outBuffer := bytes.NewBufferString("") + cmd.SetOut(outBuffer) + err := cmd.Execute() + if err != nil { + return outBuffer.String(), err + } + return outBuffer.String(), nil +} + +func runPushCommandConfirm() (string, error) { + cmd := NewUpdatePRsCmd() + pushFlag = true + yesFlag = false + outBuffer := bytes.NewBufferString("") + cmd.SetOut(outBuffer) + err := cmd.Execute() + if err != nil { + return outBuffer.String(), err + } + return outBuffer.String(), nil +} From 432b05dc81db981eb3fa8ed073205124b5a27817 Mon Sep 17 00:00:00 2001 From: Danny Ranson Date: Mon, 16 Dec 2024 14:55:25 +0000 Subject: [PATCH 2/4] adjust readme section on update-prs --- README.md | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/README.md b/README.md index 516a39e..8636968 100644 --- a/README.md +++ b/README.md @@ -260,23 +260,23 @@ redacted/redacted OPEN REVIEW_REQUIRE Use the `update-prs` command to update PRs after creating them. Current options for updating PRs are: -##### Update PR titles and descriptions with `--amend-description` +- `--push` to push new commits +- `--amend-description` to update PR titles and descriptions +- `--close` to close PRs -```turbolift update-prs --amend-description [--yes]``` - -By default, turbolift will read a revised PR Title and Description from `README.md`. The title is taken from the first heading line, and the description is the remainder of the file contents. - -As with creating PRs, if you need Turbolift to read these values from an alternative file, use the flag `--description PATH_TO_FILE`. - -```turblift update-prs --amend-description --description prDescriptionFile1.md``` +If the flag `--yes` is not passed with an `update-prs` command, a confirmation prompt will be presented. +As always, use the `--repos` flag to specify an alternative repo file to the default `repos.txt`. -##### Close PRs with the `--close` flag +##### Examples ```turbolift update-prs --close [--yes]``` +```turbolift update-prs --push [--yes]``` +```turbolift update-prs --amend-description [--yes]``` +```turblift update-prs --amend-description --description prDescriptionFile1.md``` -If the flag `--yes` is not passed with an `update-prs` command, a confirmation prompt will be presented to the user. - -As always, use the `--repos` flag to specify an alternative repo file to repos.txt. +Note that when updating PR descriptions, as when creating PRs, the `--description` flag can be used to specify an +alternative description file to the default `README.md`. +The updated title is taken from the first line of the file, and the updated description is the remainder of the file contents. ## Status: Preview From 326e4a343ece82eca1ad9fd45cb65ac921657135 Mon Sep 17 00:00:00 2001 From: Danny Ranson Date: Mon, 16 Dec 2024 15:14:55 +0000 Subject: [PATCH 3/4] remove double question mark --- cmd/updateprs/updateprs.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/cmd/updateprs/updateprs.go b/cmd/updateprs/updateprs.go index 6972940..0c419dd 100644 --- a/cmd/updateprs/updateprs.go +++ b/cmd/updateprs/updateprs.go @@ -115,7 +115,7 @@ func runClose(c *cobra.Command, _ []string) { // Prompting for confirmation if !yesFlag { // TODO: add the number of PRs that it will actually close - if !p.AskConfirm(fmt.Sprintf("Close %s campaign PRs for all repos in %s?", dir.Name, repoFile)) { + if !p.AskConfirm(fmt.Sprintf("Close %s campaign PRs for all repos in %s", dir.Name, repoFile)) { return } } @@ -172,7 +172,7 @@ func runUpdatePrDescription(c *cobra.Command, _ []string) { // Prompting for confirmation if !yesFlag { - if !p.AskConfirm(fmt.Sprintf("Update %s campaign PR titles and descriptions for all repos listed in %s?", dir.Name, repoFile)) { + if !p.AskConfirm(fmt.Sprintf("Update %s campaign PR titles and descriptions for all repos listed in %s", dir.Name, repoFile)) { return } } @@ -228,7 +228,7 @@ func runPush(c *cobra.Command, _ []string) { // Prompting for confirmation if !yesFlag { - if !p.AskConfirm(fmt.Sprintf("Push new commits to %s campaign PRs for all repos in %s?", dir.Name, repoFile)) { + if !p.AskConfirm(fmt.Sprintf("Push new commits to %s campaign PRs for all repos in %s", dir.Name, repoFile)) { return } } From f0d5eaf39beae5451e787e6ba34626a01f9f981b Mon Sep 17 00:00:00 2001 From: Danny Ranson Date: Mon, 16 Dec 2024 15:18:42 +0000 Subject: [PATCH 4/4] add test for failed push --- README.md | 3 +-- cmd/updateprs/updateprs_test.go | 21 +++++++++++++++++++++ 2 files changed, 22 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index 8636968..d1c4d03 100644 --- a/README.md +++ b/README.md @@ -271,8 +271,7 @@ As always, use the `--repos` flag to specify an alternative repo file to the def ```turbolift update-prs --close [--yes]``` ```turbolift update-prs --push [--yes]``` -```turbolift update-prs --amend-description [--yes]``` -```turblift update-prs --amend-description --description prDescriptionFile1.md``` +```turbolift update-prs --amend-description [--description prDescriptionFile1.md] [--yes]``` Note that when updating PR descriptions, as when creating PRs, the `--description` flag can be used to specify an alternative description file to the default `README.md`. diff --git a/cmd/updateprs/updateprs_test.go b/cmd/updateprs/updateprs_test.go index 005f46c..01485c3 100644 --- a/cmd/updateprs/updateprs_test.go +++ b/cmd/updateprs/updateprs_test.go @@ -187,6 +187,27 @@ func TestItPushesNewCommits(t *testing.T) { }) } +func TestItLogsPushErrorsButContinuesToTryAll(t *testing.T) { + fakeGitHub := github.NewAlwaysSucceedsFakeGitHub() + gh = fakeGitHub + fakeGit := git.NewAlwaysFailsFakeGit() + g = fakeGit + + tempDir := testsupport.PrepareTempCampaign(true, "org/repo1", "org/repo2") + + out, err := runPushCommandAuto() + assert.NoError(t, err) + assert.Contains(t, out, "Pushing changes in org/repo1 to origin") + assert.Contains(t, out, "Pushing changes in org/repo2 to origin") + assert.Contains(t, out, "turbolift update-prs completed with errors") + assert.Contains(t, out, "2 errored") + + fakeGit.AssertCalledWith(t, [][]string{ + {"push", "work/org/repo1", filepath.Base(tempDir)}, + {"push", "work/org/repo2", filepath.Base(tempDir)}, + }) +} + func TestItDoesNotPushIfNotConfirmed(t *testing.T) { fakeGitHub := github.NewAlwaysSucceedsFakeGitHub() gh = fakeGitHub