diff --git a/cmd/create_prs/create_prs.go b/cmd/create_prs/create_prs.go index 4f58e3a..2ad3983 100644 --- a/cmd/create_prs/create_prs.go +++ b/cmd/create_prs/create_prs.go @@ -16,8 +16,11 @@ package create_prs import ( + "fmt" + "github.com/skyscanner/turbolift/internal/prompt" "os" "path" + "strings" "time" "github.com/spf13/cobra" @@ -32,6 +35,7 @@ import ( var ( gh github.GitHub = github.NewRealGitHub() g git.Git = git.NewRealGit() + p prompt.Prompt = prompt.NewRealPrompt() ) var ( @@ -68,6 +72,11 @@ func run(c *cobra.Command, _ []string) { readCampaignActivity.EndWithFailure(err) return } + if prDescriptionUnchanged(dir) { + if !p.AskConfirm(fmt.Sprintf("It looks like the PR title and/or description may not have been updated in %s. Are you sure you want to proceed?", prDescriptionFile)) { + return + } + } readCampaignActivity.EndWithSuccess() doneCount := 0 @@ -131,3 +140,9 @@ func run(c *cobra.Command, _ []string) { logger.Warnf("turbolift create-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 prDescriptionUnchanged(dir *campaign.Campaign) bool { + originalPrTitleTodo := "TODO: Title of Pull Request" + originalPrBodyTodo := "TODO: This file will serve as both a README and the description of the PR." + return strings.Contains(dir.PrTitle, originalPrTitleTodo) || strings.Contains(dir.PrBody, originalPrBodyTodo) || dir.PrTitle == "" +} diff --git a/cmd/create_prs/create_prs_test.go b/cmd/create_prs/create_prs_test.go index 398b817..672eb23 100644 --- a/cmd/create_prs/create_prs_test.go +++ b/cmd/create_prs/create_prs_test.go @@ -19,11 +19,98 @@ import ( "bytes" "github.com/skyscanner/turbolift/internal/git" "github.com/skyscanner/turbolift/internal/github" + "github.com/skyscanner/turbolift/internal/prompt" "github.com/skyscanner/turbolift/internal/testsupport" "github.com/stretchr/testify/assert" "testing" ) +func TestItWarnsIfDescriptionFileTemplateIsUnchanged(t *testing.T) { + fakeGitHub := github.NewAlwaysFailsFakeGitHub() + gh = fakeGitHub + fakeGit := git.NewAlwaysSucceedsFakeGit() + g = fakeGit + fakePrompt := prompt.NewFakePromptNo() + p = fakePrompt + + testsupport.PrepareTempCampaign(true, "org/repo1", "org/repo2") + testsupport.UseDefaultPrDescription() + + out, err := runCommand() + assert.NoError(t, err) + assert.NotContains(t, out, "Creating PR in org/repo1") + assert.NotContains(t, out, "Creating PR in org/repo2") + assert.NotContains(t, out, "turbolift create-prs completed") + assert.NotContains(t, out, "2 OK, 0 skipped") + + fakePrompt.AssertCalledWith(t, "It looks like the PR title and/or description may not have been updated in README.md. Are you sure you want to proceed?") +} + +func TestItWarnsIfOnlyPrTitleIsUnchanged(t *testing.T) { + fakeGitHub := github.NewAlwaysFailsFakeGitHub() + gh = fakeGitHub + fakeGit := git.NewAlwaysSucceedsFakeGit() + g = fakeGit + fakePrompt := prompt.NewFakePromptNo() + p = fakePrompt + + testsupport.PrepareTempCampaign(true, "org/repo1", "org/repo2") + testsupport.UsePrTitleTodoOnly() + + out, err := runCommand() + assert.NoError(t, err) + assert.NotContains(t, out, "Creating PR in org/repo1") + assert.NotContains(t, out, "Creating PR in org/repo2") + assert.NotContains(t, out, "turbolift create-prs completed") + assert.NotContains(t, out, "2 OK, 0 skipped") + + fakePrompt.AssertCalledWith(t, "It looks like the PR title and/or description may not have been updated in README.md. Are you sure you want to proceed?") +} + +func TestItWarnsIfOnlyPrBodyIsUnchanged(t *testing.T) { + fakeGitHub := github.NewAlwaysFailsFakeGitHub() + gh = fakeGitHub + fakeGit := git.NewAlwaysSucceedsFakeGit() + g = fakeGit + fakePrompt := prompt.NewFakePromptNo() + p = fakePrompt + + testsupport.PrepareTempCampaign(true, "org/repo1", "org/repo2") + testsupport.UsePrBodyTodoOnly() + + out, err := runCommand() + assert.NoError(t, err) + assert.NotContains(t, out, "Creating PR in org/repo1") + assert.NotContains(t, out, "Creating PR in org/repo2") + assert.NotContains(t, out, "turbolift create-prs completed") + assert.NotContains(t, out, "2 OK, 0 skipped") + + fakePrompt.AssertCalledWith(t, "It looks like the PR title and/or description may not have been updated in README.md. Are you sure you want to proceed?") +} + +func TestItWarnsIfDescriptionFileIsEmpty(t *testing.T) { + fakeGitHub := github.NewAlwaysFailsFakeGitHub() + gh = fakeGitHub + fakeGit := git.NewAlwaysSucceedsFakeGit() + g = fakeGit + fakePrompt := prompt.NewFakePromptNo() + p = fakePrompt + + customDescriptionFileName := "custom.md" + + testsupport.PrepareTempCampaign(true, "org/repo1", "org/repo2") + testsupport.CreateOrUpdatePrDescriptionFile(customDescriptionFileName, "", "") + + out, err := runCommandWithAlternativeDescriptionFile(customDescriptionFileName) + assert.NoError(t, err) + assert.NotContains(t, out, "Creating PR in org/repo1") + assert.NotContains(t, out, "Creating PR in org/repo2") + assert.NotContains(t, out, "turbolift create-prs completed") + assert.NotContains(t, out, "2 OK, 0 skipped") + + fakePrompt.AssertCalledWith(t, "It looks like the PR title and/or description may not have been updated in custom.md. Are you sure you want to proceed?") +} + func TestItLogsCreatePrErrorsButContinuesToTryAll(t *testing.T) { fakeGitHub := github.NewAlwaysFailsFakeGitHub() gh = fakeGitHub @@ -106,6 +193,31 @@ func TestItLogsCreateDraftPr(t *testing.T) { }) } +func TestItCreatesPrsFromAlternativeDescriptionFile(t *testing.T) { + fakeGitHub := github.NewAlwaysSucceedsFakeGitHub() + gh = fakeGitHub + fakeGit := git.NewAlwaysSucceedsFakeGit() + g = fakeGit + + customDescriptionFileName := "custom.md" + + testsupport.PrepareTempCampaign(true, "org/repo1", "org/repo2") + testsupport.CreateOrUpdatePrDescriptionFile(customDescriptionFileName, "custom PR title", "custom PR body") + + out, err := runCommandWithAlternativeDescriptionFile(customDescriptionFileName) + assert.NoError(t, err) + assert.Contains(t, out, "Reading campaign data (repos.txt, custom.md)") + assert.Contains(t, out, "Creating PR in org/repo1") + assert.Contains(t, out, "Creating PR in org/repo2") + assert.Contains(t, out, "turbolift create-prs completed") + assert.Contains(t, out, "2 OK, 0 skipped") + + fakeGitHub.AssertCalledWith(t, [][]string{ + {"work/org/repo1", "custom PR title"}, + {"work/org/repo2", "custom PR title"}, + }) +} + func runCommand() (string, error) { cmd := NewCreatePRsCmd() outBuffer := bytes.NewBufferString("") @@ -118,6 +230,19 @@ func runCommand() (string, error) { return outBuffer.String(), nil } +func runCommandWithAlternativeDescriptionFile(fileName string) (string, error) { + cmd := NewCreatePRsCmd() + prDescriptionFile = fileName + outBuffer := bytes.NewBufferString("") + cmd.SetOut(outBuffer) + err := cmd.Execute() + + if err != nil { + return outBuffer.String(), err + } + return outBuffer.String(), nil +} + func runCommandDraft() (string, error) { cmd := NewCreatePRsCmd() isDraft = true diff --git a/internal/prompt/prompt.go b/internal/prompt/prompt.go index d9f1876..95da677 100644 --- a/internal/prompt/prompt.go +++ b/internal/prompt/prompt.go @@ -2,8 +2,10 @@ package prompt import ( "strings" + "testing" "github.com/manifoldco/promptui" + "github.com/stretchr/testify/assert" ) type Prompt interface { @@ -49,12 +51,19 @@ func (f FakePromptYes) AskConfirm(_ string) bool { } // Mock Prompt that always returns false -type FakePromptNo struct{} +type FakePromptNo struct { + call string +} func NewFakePromptNo() *FakePromptNo { return &FakePromptNo{} } -func (f FakePromptNo) AskConfirm(_ string) bool { +func (f *FakePromptNo) AskConfirm(question string) bool { + f.call = question return false } + +func (f *FakePromptNo) AssertCalledWith(t *testing.T, expected string) { + assert.Equal(t, expected, f.call) +} diff --git a/internal/testsupport/testsupport.go b/internal/testsupport/testsupport.go index 062ddf6..e2d9a85 100644 --- a/internal/testsupport/testsupport.go +++ b/internal/testsupport/testsupport.go @@ -24,6 +24,9 @@ import ( "strings" ) +var originalPrTitleTodo = "TODO: Title of Pull Request" +var originalPrBodyTodo = "TODO: This file will serve as both a README and the description of the PR." + func Pwd() string { dir, _ := os.Getwd() return filepath.Base(dir) @@ -81,3 +84,15 @@ func CreateOrUpdatePrDescriptionFile(filename string, prTitle string, prBody str panic(err) } } + +func UseDefaultPrDescription() { + CreateOrUpdatePrDescriptionFile("README.md", originalPrTitleTodo, originalPrBodyTodo) +} + +func UsePrTitleTodoOnly() { + CreateOrUpdatePrDescriptionFile("README.md", originalPrTitleTodo, "updated PR body") +} + +func UsePrBodyTodoOnly() { + CreateOrUpdatePrDescriptionFile("README.md", "updated PR title", originalPrBodyTodo) +}