feat(create_prs): check whether PR description file has been updated (#…

…145) * prompt for unchanged or empty description * check for todos only * test title and body separately * no need to check dir name * extract into vars --------- Co-authored-by: Danny Ranson <[email protected]>
Skyscanner · Aug 26, 2024 · 55ca75f · 55ca75f
1 parent 40ea0b8
commit 55ca75f
Show file tree

Hide file tree

Showing 4 changed files with 166 additions and 2 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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)
+}