Skip to content

Commit 55ca75f

Browse files
authored
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]>
1 parent 40ea0b8 commit 55ca75f

File tree

4 files changed

+166
-2
lines changed

4 files changed

+166
-2
lines changed

cmd/create_prs/create_prs.go

+15
Original file line numberDiff line numberDiff line change
@@ -16,8 +16,11 @@
1616
package create_prs
1717

1818
import (
19+
"fmt"
20+
"github.com/skyscanner/turbolift/internal/prompt"
1921
"os"
2022
"path"
23+
"strings"
2124
"time"
2225

2326
"github.com/spf13/cobra"
@@ -32,6 +35,7 @@ import (
3235
var (
3336
gh github.GitHub = github.NewRealGitHub()
3437
g git.Git = git.NewRealGit()
38+
p prompt.Prompt = prompt.NewRealPrompt()
3539
)
3640

3741
var (
@@ -68,6 +72,11 @@ func run(c *cobra.Command, _ []string) {
6872
readCampaignActivity.EndWithFailure(err)
6973
return
7074
}
75+
if prDescriptionUnchanged(dir) {
76+
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)) {
77+
return
78+
}
79+
}
7180
readCampaignActivity.EndWithSuccess()
7281

7382
doneCount := 0
@@ -131,3 +140,9 @@ func run(c *cobra.Command, _ []string) {
131140
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"))
132141
}
133142
}
143+
144+
func prDescriptionUnchanged(dir *campaign.Campaign) bool {
145+
originalPrTitleTodo := "TODO: Title of Pull Request"
146+
originalPrBodyTodo := "TODO: This file will serve as both a README and the description of the PR."
147+
return strings.Contains(dir.PrTitle, originalPrTitleTodo) || strings.Contains(dir.PrBody, originalPrBodyTodo) || dir.PrTitle == ""
148+
}

cmd/create_prs/create_prs_test.go

+125
Original file line numberDiff line numberDiff line change
@@ -19,11 +19,98 @@ import (
1919
"bytes"
2020
"github.com/skyscanner/turbolift/internal/git"
2121
"github.com/skyscanner/turbolift/internal/github"
22+
"github.com/skyscanner/turbolift/internal/prompt"
2223
"github.com/skyscanner/turbolift/internal/testsupport"
2324
"github.com/stretchr/testify/assert"
2425
"testing"
2526
)
2627

28+
func TestItWarnsIfDescriptionFileTemplateIsUnchanged(t *testing.T) {
29+
fakeGitHub := github.NewAlwaysFailsFakeGitHub()
30+
gh = fakeGitHub
31+
fakeGit := git.NewAlwaysSucceedsFakeGit()
32+
g = fakeGit
33+
fakePrompt := prompt.NewFakePromptNo()
34+
p = fakePrompt
35+
36+
testsupport.PrepareTempCampaign(true, "org/repo1", "org/repo2")
37+
testsupport.UseDefaultPrDescription()
38+
39+
out, err := runCommand()
40+
assert.NoError(t, err)
41+
assert.NotContains(t, out, "Creating PR in org/repo1")
42+
assert.NotContains(t, out, "Creating PR in org/repo2")
43+
assert.NotContains(t, out, "turbolift create-prs completed")
44+
assert.NotContains(t, out, "2 OK, 0 skipped")
45+
46+
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?")
47+
}
48+
49+
func TestItWarnsIfOnlyPrTitleIsUnchanged(t *testing.T) {
50+
fakeGitHub := github.NewAlwaysFailsFakeGitHub()
51+
gh = fakeGitHub
52+
fakeGit := git.NewAlwaysSucceedsFakeGit()
53+
g = fakeGit
54+
fakePrompt := prompt.NewFakePromptNo()
55+
p = fakePrompt
56+
57+
testsupport.PrepareTempCampaign(true, "org/repo1", "org/repo2")
58+
testsupport.UsePrTitleTodoOnly()
59+
60+
out, err := runCommand()
61+
assert.NoError(t, err)
62+
assert.NotContains(t, out, "Creating PR in org/repo1")
63+
assert.NotContains(t, out, "Creating PR in org/repo2")
64+
assert.NotContains(t, out, "turbolift create-prs completed")
65+
assert.NotContains(t, out, "2 OK, 0 skipped")
66+
67+
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?")
68+
}
69+
70+
func TestItWarnsIfOnlyPrBodyIsUnchanged(t *testing.T) {
71+
fakeGitHub := github.NewAlwaysFailsFakeGitHub()
72+
gh = fakeGitHub
73+
fakeGit := git.NewAlwaysSucceedsFakeGit()
74+
g = fakeGit
75+
fakePrompt := prompt.NewFakePromptNo()
76+
p = fakePrompt
77+
78+
testsupport.PrepareTempCampaign(true, "org/repo1", "org/repo2")
79+
testsupport.UsePrBodyTodoOnly()
80+
81+
out, err := runCommand()
82+
assert.NoError(t, err)
83+
assert.NotContains(t, out, "Creating PR in org/repo1")
84+
assert.NotContains(t, out, "Creating PR in org/repo2")
85+
assert.NotContains(t, out, "turbolift create-prs completed")
86+
assert.NotContains(t, out, "2 OK, 0 skipped")
87+
88+
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?")
89+
}
90+
91+
func TestItWarnsIfDescriptionFileIsEmpty(t *testing.T) {
92+
fakeGitHub := github.NewAlwaysFailsFakeGitHub()
93+
gh = fakeGitHub
94+
fakeGit := git.NewAlwaysSucceedsFakeGit()
95+
g = fakeGit
96+
fakePrompt := prompt.NewFakePromptNo()
97+
p = fakePrompt
98+
99+
customDescriptionFileName := "custom.md"
100+
101+
testsupport.PrepareTempCampaign(true, "org/repo1", "org/repo2")
102+
testsupport.CreateOrUpdatePrDescriptionFile(customDescriptionFileName, "", "")
103+
104+
out, err := runCommandWithAlternativeDescriptionFile(customDescriptionFileName)
105+
assert.NoError(t, err)
106+
assert.NotContains(t, out, "Creating PR in org/repo1")
107+
assert.NotContains(t, out, "Creating PR in org/repo2")
108+
assert.NotContains(t, out, "turbolift create-prs completed")
109+
assert.NotContains(t, out, "2 OK, 0 skipped")
110+
111+
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?")
112+
}
113+
27114
func TestItLogsCreatePrErrorsButContinuesToTryAll(t *testing.T) {
28115
fakeGitHub := github.NewAlwaysFailsFakeGitHub()
29116
gh = fakeGitHub
@@ -106,6 +193,31 @@ func TestItLogsCreateDraftPr(t *testing.T) {
106193
})
107194
}
108195

196+
func TestItCreatesPrsFromAlternativeDescriptionFile(t *testing.T) {
197+
fakeGitHub := github.NewAlwaysSucceedsFakeGitHub()
198+
gh = fakeGitHub
199+
fakeGit := git.NewAlwaysSucceedsFakeGit()
200+
g = fakeGit
201+
202+
customDescriptionFileName := "custom.md"
203+
204+
testsupport.PrepareTempCampaign(true, "org/repo1", "org/repo2")
205+
testsupport.CreateOrUpdatePrDescriptionFile(customDescriptionFileName, "custom PR title", "custom PR body")
206+
207+
out, err := runCommandWithAlternativeDescriptionFile(customDescriptionFileName)
208+
assert.NoError(t, err)
209+
assert.Contains(t, out, "Reading campaign data (repos.txt, custom.md)")
210+
assert.Contains(t, out, "Creating PR in org/repo1")
211+
assert.Contains(t, out, "Creating PR in org/repo2")
212+
assert.Contains(t, out, "turbolift create-prs completed")
213+
assert.Contains(t, out, "2 OK, 0 skipped")
214+
215+
fakeGitHub.AssertCalledWith(t, [][]string{
216+
{"work/org/repo1", "custom PR title"},
217+
{"work/org/repo2", "custom PR title"},
218+
})
219+
}
220+
109221
func runCommand() (string, error) {
110222
cmd := NewCreatePRsCmd()
111223
outBuffer := bytes.NewBufferString("")
@@ -118,6 +230,19 @@ func runCommand() (string, error) {
118230
return outBuffer.String(), nil
119231
}
120232

233+
func runCommandWithAlternativeDescriptionFile(fileName string) (string, error) {
234+
cmd := NewCreatePRsCmd()
235+
prDescriptionFile = fileName
236+
outBuffer := bytes.NewBufferString("")
237+
cmd.SetOut(outBuffer)
238+
err := cmd.Execute()
239+
240+
if err != nil {
241+
return outBuffer.String(), err
242+
}
243+
return outBuffer.String(), nil
244+
}
245+
121246
func runCommandDraft() (string, error) {
122247
cmd := NewCreatePRsCmd()
123248
isDraft = true

internal/prompt/prompt.go

+11-2
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,10 @@ package prompt
22

33
import (
44
"strings"
5+
"testing"
56

67
"github.com/manifoldco/promptui"
8+
"github.com/stretchr/testify/assert"
79
)
810

911
type Prompt interface {
@@ -49,12 +51,19 @@ func (f FakePromptYes) AskConfirm(_ string) bool {
4951
}
5052

5153
// Mock Prompt that always returns false
52-
type FakePromptNo struct{}
54+
type FakePromptNo struct {
55+
call string
56+
}
5357

5458
func NewFakePromptNo() *FakePromptNo {
5559
return &FakePromptNo{}
5660
}
5761

58-
func (f FakePromptNo) AskConfirm(_ string) bool {
62+
func (f *FakePromptNo) AskConfirm(question string) bool {
63+
f.call = question
5964
return false
6065
}
66+
67+
func (f *FakePromptNo) AssertCalledWith(t *testing.T, expected string) {
68+
assert.Equal(t, expected, f.call)
69+
}

internal/testsupport/testsupport.go

+15
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,9 @@ import (
2424
"strings"
2525
)
2626

27+
var originalPrTitleTodo = "TODO: Title of Pull Request"
28+
var originalPrBodyTodo = "TODO: This file will serve as both a README and the description of the PR."
29+
2730
func Pwd() string {
2831
dir, _ := os.Getwd()
2932
return filepath.Base(dir)
@@ -81,3 +84,15 @@ func CreateOrUpdatePrDescriptionFile(filename string, prTitle string, prBody str
8184
panic(err)
8285
}
8386
}
87+
88+
func UseDefaultPrDescription() {
89+
CreateOrUpdatePrDescriptionFile("README.md", originalPrTitleTodo, originalPrBodyTodo)
90+
}
91+
92+
func UsePrTitleTodoOnly() {
93+
CreateOrUpdatePrDescriptionFile("README.md", originalPrTitleTodo, "updated PR body")
94+
}
95+
96+
func UsePrBodyTodoOnly() {
97+
CreateOrUpdatePrDescriptionFile("README.md", "updated PR title", originalPrBodyTodo)
98+
}

0 commit comments

Comments
 (0)