Skip to content

Commit

Permalink
Automatically copy changelog message to the backport or put no-change…
Browse files Browse the repository at this point in the history
…log label
  • Loading branch information
AntonAM committed Dec 6, 2023
1 parent 7ade89f commit 07a5800
Show file tree
Hide file tree
Showing 6 changed files with 187 additions and 55 deletions.
29 changes: 24 additions & 5 deletions bot/internal/bot/backport.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,9 +30,10 @@ import (
"strings"
"text/template"

"github.com/gravitational/shared-workflows/bot/internal/github"

"github.com/gravitational/trace"
"golang.org/x/exp/slices"

"github.com/gravitational/shared-workflows/bot/internal/github"
)

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

var rows []row

g := git
if b.c.Git != nil {
g = b.c.Git
}

// Loop over all requested backport branches and create backport branch and
// GitHub Pull Request.
for _, base := range branches {
Expand All @@ -92,7 +98,7 @@ func (b *Bot) Backport(ctx context.Context) error {
base,
pull,
head,
git,
g,
)
if err != nil {
log.Printf("Failed to create backport branch:\n%v\n", trace.DebugReport(err))
Expand All @@ -104,6 +110,16 @@ func (b *Bot) Backport(ctx context.Context) error {
continue
}

labels := []string{NoChangelogLabel}
bodyText := fmt.Sprintf("Backport #%v to %v", b.c.Environment.Number, base)
if entries := b.getChangelogEntries(pull.UnsafeBody); len(entries) > 0 && !slices.Contains(pull.UnsafeLabels, NoChangelogLabel) {
bodyText += "\n\n"
labels = labels[:0]
for _, entry := range entries {
bodyText += fmt.Sprintf("%s%s\n", ChangelogPrefix, entry)
}
}

rows = append(rows, row{
Branch: base,
Failed: false,
Expand All @@ -117,7 +133,8 @@ func (b *Bot) Backport(ctx context.Context) error {
RawQuery: url.Values{
"expand": []string{"1"},
"title": []string{fmt.Sprintf("[%v] %v", strings.Trim(base, "branch/"), pull.UnsafeTitle)},
"body": []string{fmt.Sprintf("Backport #%v to %v", b.c.Environment.Number, base)},
"body": []string{bodyText},
"labels": labels,
}.Encode(),
},
})
Expand Down Expand Up @@ -162,8 +179,10 @@ func (b *Bot) BackportLocal(ctx context.Context, branch string) error {
return nil
}

const botBackportBranchPrefix = "bot/backport"

func (b *Bot) backportBranchName(base string) string {
return fmt.Sprintf("bot/backport-%v-%v", b.c.Environment.Number, base)
return fmt.Sprintf("%s-%v-%v", botBackportBranchPrefix, b.c.Environment.Number, base)
}

// findBranches looks through the labels attached to a Pull Request for all the
Expand Down
113 changes: 113 additions & 0 deletions bot/internal/bot/backport_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,14 @@ limitations under the License.
package bot

import (
"context"
"testing"

"github.com/stretchr/testify/require"

"github.com/gravitational/shared-workflows/bot/internal/env"
"github.com/gravitational/shared-workflows/bot/internal/github"
"github.com/gravitational/shared-workflows/bot/internal/review"
)

func TestFindBranches(t *testing.T) {
Expand All @@ -35,3 +40,111 @@ func TestFindBranches(t *testing.T) {
"master",
})
}

func TestBackport(t *testing.T) {
buildTestBot := func(github Client) (*Bot, context.Context) {
r, _ := review.New(&review.Config{
CodeReviewers: map[string]review.Reviewer{"dev": review.Reviewer{
Team: "core",
}},
CodeReviewersOmit: map[string]bool{},
DocsReviewers: map[string]review.Reviewer{},
DocsReviewersOmit: map[string]bool{},
Admins: []string{},
})

return &Bot{
c: &Config{
Environment: &env.Environment{
Organization: "foo",
Author: "dev",
Repository: "bar",
Number: 42,
UnsafeBase: "branch/v8",
UnsafeHead: "fix",
},
GitHub: github,
Review: r,
Git: gitDryRun,
},
}, context.Background()
}

tests := []struct {
desc string
github Client
assertFunc require.ValueAssertionFunc
}{
{
desc: "pr without backport labels",
github: &fakeGithub{},
assertFunc: require.Empty,
},
{
desc: "pr with backport label, no changelog",
github: &fakeGithub{
pull: github.PullRequest{
Author: "dev",
Repository: "Teleport",
Number: 42,
UnsafeTitle: "Best PR",
UnsafeBody: "This is PR body",
UnsafeLabels: []string{"backport/branch/v7"},
},
jobs: []github.Job{{Name: "Job1", ID: 1}},
},
assertFunc: func(t require.TestingT, i interface{}, i2 ...interface{}) {
comments, ok := i.([]github.Comment)
require.True(t, ok)
require.Len(t, comments, 1)
require.Equal(t,
`
@dev See the table below for backport results.
| Branch | Result |
|--------|--------|
| 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) |
`, comments[0].Body)
},
},
{
desc: "pr with backport label and with changelog",
github: &fakeGithub{
pull: github.PullRequest{
Author: "dev",
Repository: "Teleport",
Number: 42,
UnsafeTitle: "Best PR",
UnsafeBody: "This is PR body\n\nchangelog: important change",
UnsafeLabels: []string{"backport/branch/v7"},
},
jobs: []github.Job{{Name: "Job1", ID: 1}},
},
assertFunc: func(t require.TestingT, i interface{}, i2 ...interface{}) {
comments, ok := i.([]github.Comment)
require.True(t, ok)
require.Len(t, comments, 1)
require.Equal(t,
`
@dev See the table below for backport results.
| Branch | Result |
|--------|--------|
| 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) |
`, comments[0].Body)
},
},
}

for _, test := range tests {
t.Run(test.desc, func(t *testing.T) {
b, ctx := buildTestBot(test.github)

err := b.Backport(ctx)
require.NoError(t, err)

comments, _ := b.c.GitHub.ListComments(nil, "", "", 0)
test.assertFunc(t, comments)
})
}
}
9 changes: 6 additions & 3 deletions bot/internal/bot/bot.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,11 +20,11 @@ import (
"context"
"strings"

"github.com/gravitational/trace"

"github.com/gravitational/shared-workflows/bot/internal/env"
"github.com/gravitational/shared-workflows/bot/internal/github"
"github.com/gravitational/shared-workflows/bot/internal/review"

"github.com/gravitational/trace"
)

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

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

// CreatePullRequest will create a Pull Request.
Expand Down Expand Up @@ -97,6 +97,9 @@ type Config struct {

// Review is used to get code and docs reviewers.
Review *review.Assignments

// Git is used to run git commands, uses dry run in tests.
Git func(...string) error
}

// CheckAndSetDefaults checks and sets defaults.
Expand Down
11 changes: 8 additions & 3 deletions bot/internal/bot/bot_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,11 +20,11 @@ import (
"context"
"testing"

"github.com/stretchr/testify/require"

"github.com/gravitational/shared-workflows/bot/internal/env"
"github.com/gravitational/shared-workflows/bot/internal/github"
"github.com/gravitational/shared-workflows/bot/internal/review"

"github.com/stretchr/testify/require"
)

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

func (f *fakeGithub) ListWorkflowJobs(ctx context.Context, organization string, repository string, runID int64) ([]github.Job, error) {
return nil, nil
return f.jobs, nil
}

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

func (f *fakeGithub) CreateComment(ctx context.Context, organization string, repository string, number int, comment string) error {
f.comments = append(f.comments, github.Comment{
Body: comment,
})

return nil
}

Expand Down
20 changes: 10 additions & 10 deletions bot/internal/bot/changelog.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,9 +51,11 @@ func (b *Bot) CheckChangelog(ctx context.Context) error {
return nil
}

changelogEntries, err := b.getChangelogEntries(ctx, pull.UnsafeBody)
if err != nil {
return trace.Wrap(err, "failed to get changelog entry")
changelogEntries := b.getChangelogEntries(pull.UnsafeBody)
if len(changelogEntries) == 0 {
return trace.Wrap(
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),
"failed to get changelog entry")
}

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

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

changelogMatches := changelogRegex.FindAllString(prBody, -1)
if len(changelogMatches) == 0 {
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)
}

for i, changelogMatch := range changelogMatches {
changelogMatches[i] = changelogMatch[len(ChangelogPrefix):] // Case insensitive prefix removal
}

log.Printf("Found changelog entries %v", changelogMatches)
return changelogMatches, nil
if len(changelogMatches) > 0 {
log.Printf("Found changelog entries %v", changelogMatches)
}
return changelogMatches
}

// Checks for common issues with the changelog entry.
Expand Down
Loading

0 comments on commit 07a5800

Please sign in to comment.