Skip to content

Commit d06c6cc

Browse files
AntonAMAntonAM
authored
Merge pull request #195 from gravitational/anton/autobackport-changelog
Automatically copy changelog message to the backport or put no-changelog label
2 parents 7ade89f + 07a5800 commit d06c6cc

File tree

6 files changed

+187
-55
lines changed

6 files changed

+187
-55
lines changed

‎bot/internal/bot/backport.go‎

Lines changed: 24 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -30,9 +30,10 @@ import (
3030
"strings"
3131
"text/template"
3232

33-
"github.com/gravitational/shared-workflows/bot/internal/github"
34-
3533
"github.com/gravitational/trace"
34+
"golang.org/x/exp/slices"
35+
36+
"github.com/gravitational/shared-workflows/bot/internal/github"
3637
)
3738

3839
// Backport will create backport Pull Requests (if requested) when a Pull
@@ -79,6 +80,11 @@ func (b *Bot) Backport(ctx context.Context) error {
7980

8081
var rows []row
8182

83+
g := git
84+
if b.c.Git != nil {
85+
g = b.c.Git
86+
}
87+
8288
// Loop over all requested backport branches and create backport branch and
8389
// GitHub Pull Request.
8490
for _, base := range branches {
@@ -92,7 +98,7 @@ func (b *Bot) Backport(ctx context.Context) error {
9298
base,
9399
pull,
94100
head,
95-
git,
101+
g,
96102
)
97103
if err != nil {
98104
log.Printf("Failed to create backport branch:\n%v\n", trace.DebugReport(err))
@@ -104,6 +110,16 @@ func (b *Bot) Backport(ctx context.Context) error {
104110
continue
105111
}
106112

113+
labels := []string{NoChangelogLabel}
114+
bodyText := fmt.Sprintf("Backport #%v to %v", b.c.Environment.Number, base)
115+
if entries := b.getChangelogEntries(pull.UnsafeBody); len(entries) > 0 && !slices.Contains(pull.UnsafeLabels, NoChangelogLabel) {
116+
bodyText += "\n\n"
117+
labels = labels[:0]
118+
for _, entry := range entries {
119+
bodyText += fmt.Sprintf("%s%s\n", ChangelogPrefix, entry)
120+
}
121+
}
122+
107123
rows = append(rows, row{
108124
Branch: base,
109125
Failed: false,
@@ -117,7 +133,8 @@ func (b *Bot) Backport(ctx context.Context) error {
117133
RawQuery: url.Values{
118134
"expand": []string{"1"},
119135
"title": []string{fmt.Sprintf("[%v] %v", strings.Trim(base, "branch/"), pull.UnsafeTitle)},
120-
"body": []string{fmt.Sprintf("Backport #%v to %v", b.c.Environment.Number, base)},
136+
"body": []string{bodyText},
137+
"labels": labels,
121138
}.Encode(),
122139
},
123140
})
@@ -162,8 +179,10 @@ func (b *Bot) BackportLocal(ctx context.Context, branch string) error {
162179
return nil
163180
}
164181

182+
const botBackportBranchPrefix = "bot/backport"
183+
165184
func (b *Bot) backportBranchName(base string) string {
166-
return fmt.Sprintf("bot/backport-%v-%v", b.c.Environment.Number, base)
185+
return fmt.Sprintf("%s-%v-%v", botBackportBranchPrefix, b.c.Environment.Number, base)
167186
}
168187

169188
// findBranches looks through the labels attached to a Pull Request for all the

‎bot/internal/bot/backport_test.go‎

Lines changed: 113 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,9 +17,14 @@ limitations under the License.
1717
package bot
1818

1919
import (
20+
"context"
2021
"testing"
2122

2223
"github.com/stretchr/testify/require"
24+
25+
"github.com/gravitational/shared-workflows/bot/internal/env"
26+
"github.com/gravitational/shared-workflows/bot/internal/github"
27+
"github.com/gravitational/shared-workflows/bot/internal/review"
2328
)
2429

2530
func TestFindBranches(t *testing.T) {
@@ -35,3 +40,111 @@ func TestFindBranches(t *testing.T) {
3540
"master",
3641
})
3742
}
43+
44+
func TestBackport(t *testing.T) {
45+
buildTestBot := func(github Client) (*Bot, context.Context) {
46+
r, _ := review.New(&review.Config{
47+
CodeReviewers: map[string]review.Reviewer{"dev": review.Reviewer{
48+
Team: "core",
49+
}},
50+
CodeReviewersOmit: map[string]bool{},
51+
DocsReviewers: map[string]review.Reviewer{},
52+
DocsReviewersOmit: map[string]bool{},
53+
Admins: []string{},
54+
})
55+
56+
return &Bot{
57+
c: &Config{
58+
Environment: &env.Environment{
59+
Organization: "foo",
60+
Author: "dev",
61+
Repository: "bar",
62+
Number: 42,
63+
UnsafeBase: "branch/v8",
64+
UnsafeHead: "fix",
65+
},
66+
GitHub: github,
67+
Review: r,
68+
Git: gitDryRun,
69+
},
70+
}, context.Background()
71+
}
72+
73+
tests := []struct {
74+
desc string
75+
github Client
76+
assertFunc require.ValueAssertionFunc
77+
}{
78+
{
79+
desc: "pr without backport labels",
80+
github: &fakeGithub{},
81+
assertFunc: require.Empty,
82+
},
83+
{
84+
desc: "pr with backport label, no changelog",
85+
github: &fakeGithub{
86+
pull: github.PullRequest{
87+
Author: "dev",
88+
Repository: "Teleport",
89+
Number: 42,
90+
UnsafeTitle: "Best PR",
91+
UnsafeBody: "This is PR body",
92+
UnsafeLabels: []string{"backport/branch/v7"},
93+
},
94+
jobs: []github.Job{{Name: "Job1", ID: 1}},
95+
},
96+
assertFunc: func(t require.TestingT, i interface{}, i2 ...interface{}) {
97+
comments, ok := i.([]github.Comment)
98+
require.True(t, ok)
99+
require.Len(t, comments, 1)
100+
require.Equal(t,
101+
`
102+
@dev See the table below for backport results.
103+
104+
| Branch | Result |
105+
|--------|--------|
106+
| branch/v7 | [Create PR](https://github.com/foo/bar/compare/branch/v7...bot/backport-42-branch/v7?body=Backport+%2342+to+branch%2Fv7&expand=1&labels=no-changelog&title=%5Bv7%5D+Best+PR) |
107+
`, comments[0].Body)
108+
},
109+
},
110+
{
111+
desc: "pr with backport label and with changelog",
112+
github: &fakeGithub{
113+
pull: github.PullRequest{
114+
Author: "dev",
115+
Repository: "Teleport",
116+
Number: 42,
117+
UnsafeTitle: "Best PR",
118+
UnsafeBody: "This is PR body\n\nchangelog: important change",
119+
UnsafeLabels: []string{"backport/branch/v7"},
120+
},
121+
jobs: []github.Job{{Name: "Job1", ID: 1}},
122+
},
123+
assertFunc: func(t require.TestingT, i interface{}, i2 ...interface{}) {
124+
comments, ok := i.([]github.Comment)
125+
require.True(t, ok)
126+
require.Len(t, comments, 1)
127+
require.Equal(t,
128+
`
129+
@dev See the table below for backport results.
130+
131+
| Branch | Result |
132+
|--------|--------|
133+
| branch/v7 | [Create PR](https://github.com/foo/bar/compare/branch/v7...bot/backport-42-branch/v7?body=Backport+%2342+to+branch%2Fv7%0A%0Achangelog%3A+important+change%0A&expand=1&title=%5Bv7%5D+Best+PR) |
134+
`, comments[0].Body)
135+
},
136+
},
137+
}
138+
139+
for _, test := range tests {
140+
t.Run(test.desc, func(t *testing.T) {
141+
b, ctx := buildTestBot(test.github)
142+
143+
err := b.Backport(ctx)
144+
require.NoError(t, err)
145+
146+
comments, _ := b.c.GitHub.ListComments(nil, "", "", 0)
147+
test.assertFunc(t, comments)
148+
})
149+
}
150+
}

‎bot/internal/bot/bot.go‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -20,11 +20,11 @@ import (
2020
"context"
2121
"strings"
2222

23+
"github.com/gravitational/trace"
24+
2325
"github.com/gravitational/shared-workflows/bot/internal/env"
2426
"github.com/gravitational/shared-workflows/bot/internal/github"
2527
"github.com/gravitational/shared-workflows/bot/internal/review"
26-
27-
"github.com/gravitational/trace"
2828
)
2929

3030
// Client implements the GitHub API.
@@ -44,7 +44,7 @@ type Client interface {
4444
// GetPullRequest returns a specific Pull Request.
4545
GetPullRequest(ctx context.Context, organization string, repository string, number int) (github.PullRequest, error)
4646

47-
// GetPullRequestWithCommitsd returns a specific Pull Request with commits.
47+
// GetPullRequestWithCommits returns a specific Pull Request with commits.
4848
GetPullRequestWithCommits(ctx context.Context, organization string, repository string, number int) (github.PullRequest, error)
4949

5050
// CreatePullRequest will create a Pull Request.
@@ -97,6 +97,9 @@ type Config struct {
9797

9898
// Review is used to get code and docs reviewers.
9999
Review *review.Assignments
100+
101+
// Git is used to run git commands, uses dry run in tests.
102+
Git func(...string) error
100103
}
101104

102105
// CheckAndSetDefaults checks and sets defaults.

‎bot/internal/bot/bot_test.go‎

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -20,11 +20,11 @@ import (
2020
"context"
2121
"testing"
2222

23+
"github.com/stretchr/testify/require"
24+
2325
"github.com/gravitational/shared-workflows/bot/internal/env"
2426
"github.com/gravitational/shared-workflows/bot/internal/github"
2527
"github.com/gravitational/shared-workflows/bot/internal/review"
26-
27-
"github.com/stretchr/testify/require"
2828
)
2929

3030
// TestClassifyChanges checks that PR contents are correctly parsed for docs and
@@ -311,6 +311,7 @@ func TestDoNotMerge(t *testing.T) {
311311
type fakeGithub struct {
312312
files []github.PullRequestFile
313313
pull github.PullRequest
314+
jobs []github.Job
314315
reviewers []string
315316
reviews []github.Review
316317
orgMembers map[string]struct{}
@@ -364,7 +365,7 @@ func (f *fakeGithub) ListWorkflowRuns(ctx context.Context, organization string,
364365
}
365366

366367
func (f *fakeGithub) ListWorkflowJobs(ctx context.Context, organization string, repository string, runID int64) ([]github.Job, error) {
367-
return nil, nil
368+
return f.jobs, nil
368369
}
369370

370371
func (f *fakeGithub) DeleteWorkflowRun(ctx context.Context, organization string, repository string, runID int64) error {
@@ -377,6 +378,10 @@ func (f *fakeGithub) IsOrgMember(ctx context.Context, user string, org string) (
377378
}
378379

379380
func (f *fakeGithub) CreateComment(ctx context.Context, organization string, repository string, number int, comment string) error {
381+
f.comments = append(f.comments, github.Comment{
382+
Body: comment,
383+
})
384+
380385
return nil
381386
}
382387

‎bot/internal/bot/changelog.go‎

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -51,9 +51,11 @@ func (b *Bot) CheckChangelog(ctx context.Context) error {
5151
return nil
5252
}
5353

54-
changelogEntries, err := b.getChangelogEntries(ctx, pull.UnsafeBody)
55-
if err != nil {
56-
return trace.Wrap(err, "failed to get changelog entry")
54+
changelogEntries := b.getChangelogEntries(pull.UnsafeBody)
55+
if len(changelogEntries) == 0 {
56+
return trace.Wrap(
57+
b.logFailedCheck(ctx, "Changelog entry not found in the PR body. Please add a %q label to the PR, or changelog lines starting with `%s` followed by the changelog entries for the PR.", NoChangelogLabel, ChangelogPrefix),
58+
"failed to get changelog entry")
5759
}
5860

5961
for _, changelogEntry := range changelogEntries {
@@ -66,20 +68,18 @@ func (b *Bot) CheckChangelog(ctx context.Context) error {
6668
return nil
6769
}
6870

69-
func (b *Bot) getChangelogEntries(ctx context.Context, prBody string) ([]string, error) {
71+
func (b *Bot) getChangelogEntries(prBody string) []string {
7072
changelogRegex := regexp.MustCompile(ChangelogRegex)
7173

7274
changelogMatches := changelogRegex.FindAllString(prBody, -1)
73-
if len(changelogMatches) == 0 {
74-
return nil, b.logFailedCheck(ctx, "Changelog entry not found in the PR body. Please add a %q label to the PR, or changelog lines starting with `%s` followed by the changelog entries for the PR.", NoChangelogLabel, ChangelogPrefix)
75-
}
76-
7775
for i, changelogMatch := range changelogMatches {
7876
changelogMatches[i] = changelogMatch[len(ChangelogPrefix):] // Case insensitive prefix removal
7977
}
8078

81-
log.Printf("Found changelog entries %v", changelogMatches)
82-
return changelogMatches, nil
79+
if len(changelogMatches) > 0 {
80+
log.Printf("Found changelog entries %v", changelogMatches)
81+
}
82+
return changelogMatches
8383
}
8484

8585
// Checks for common issues with the changelog entry.

0 commit comments

Comments
 (0)