Skip to content

Commit

Permalink
feat: support multiple calls per comment (#520)
Browse files Browse the repository at this point in the history 
Signed-off-by: hi-rustin <[email protected]>

close #510

```release-note
Cherrypicker support multiple calls per comment.
```
  • Loading branch information
@Rustin170506
Rustin170506 authored May 5, 2021
1 parent 434f280 commit 91061d0
Show file tree
Hide file tree
Showing 2 changed files with 95 additions and 44 deletions.
45 changes: 31 additions & 14 deletions internal/pkg/externalplugins/cherrypicker/cherrypicker.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -254,7 +254,12 @@ func (s *Server) handleIssueComment(l *logrus.Entry, ic github.IssueCommentEvent
if len(cherryPickMatches) == 0 || len(cherryPickMatches[0]) != 2 {
return nil
}
targetBranch := strings.TrimSpace(cherryPickMatches[0][1])

targetBranchesSet := sets.NewString()
for _, match := range cherryPickMatches {
targetBranch := strings.TrimSpace(match[1])
targetBranchesSet.Insert(targetBranch)
}

if ic.Issue.State != "closed" {
if !opts.AllowAll {
Expand All @@ -271,7 +276,8 @@ func (s *Server) handleIssueComment(l *logrus.Entry, ic github.IssueCommentEvent
}
}
resp := fmt.Sprintf("once the present PR merges, "+
"I will cherry-pick it on top of %s in a new PR and assign it to you.", targetBranch)
"I will cherry-pick it on top of %s in the new PR and assign it to you.",
strings.Join(targetBranchesSet.List(), "/"))
l.Info(resp)
return s.GitHubClient.CreateComment(org, repo, num, tiexternalplugins.FormatICResponse(ic.Comment, resp))
}
Expand All @@ -289,12 +295,6 @@ func (s *Server) handleIssueComment(l *logrus.Entry, ic github.IssueCommentEvent
return s.GitHubClient.CreateComment(org, repo, num, tiexternalplugins.FormatICResponse(ic.Comment, resp))
}

if baseBranch == targetBranch {
resp := fmt.Sprintf("base branch (%s) needs to differ from target branch (%s).", baseBranch, targetBranch)
l.Info(resp)
return s.GitHubClient.CreateComment(org, repo, num, tiexternalplugins.FormatICResponse(ic.Comment, resp))
}

if !opts.AllowAll {
// Only org members should be able to do cherry-picks.
ok, err := s.GitHubClient.IsMember(org, commentAuthor)
Expand All @@ -309,12 +309,29 @@ func (s *Server) handleIssueComment(l *logrus.Entry, ic github.IssueCommentEvent
}
}

*l = *l.WithFields(logrus.Fields{
"requestor": ic.Comment.User.Login,
"target_branch": targetBranch,
})
l.Debug("Cherrypick request.")
return s.handle(l, ic.Comment.User.Login, &ic.Comment, org, repo, targetBranch, pr)
for _, targetBranch := range targetBranchesSet.List() {
if baseBranch == targetBranch {
resp := fmt.Sprintf("base branch (%s) needs to differ from target branch (%s).", baseBranch, targetBranch)
l.Info(resp)
if err := s.GitHubClient.CreateComment(org, repo, num,
tiexternalplugins.FormatICResponse(ic.Comment, resp)); err != nil {
l.WithError(err).WithField("response", resp).Error("Failed to create comment.")
}
continue
}

*l = *l.WithFields(logrus.Fields{
"requestor": ic.Comment.User.Login,
"target_branch": targetBranch,
})
l.Debug("Cherrypick request.")
err := s.handle(l, ic.Comment.User.Login, &ic.Comment, org, repo, targetBranch, pr)
if err != nil {
l.WithError(err).Error("Cherrypick failed.")
}
}

return nil
}

func (s *Server) handlePullRequest(l *logrus.Entry, pre github.PullRequestEvent) error {
Expand Down
94 changes: 64 additions & 30 deletions internal/pkg/externalplugins/cherrypicker/cherrypicker_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,7 @@ func (f *fghc) EnsureFork(forkingUser, org, repo string) (string, error) {
return repo, nil
}

var expectedFmt = `title=%q body=%q head=%s base=%s labels=%v reviewers=%v assignees=%v`
var prFmt = `title=%q body=%q head=%s base=%s labels=%v reviewers=%v assignees=%v`

func prToString(pr github.PullRequest) string {
var labels []string
Expand All @@ -163,7 +163,7 @@ func prToString(pr github.PullRequest) string {
for _, assignee := range pr.Assignees {
assignees = append(assignees, assignee.Login)
}
return fmt.Sprintf(expectedFmt, pr.Title, pr.Body, pr.Head.Ref, pr.Base.Ref, labels, reviewers, assignees)
return fmt.Sprintf(prFmt, pr.Title, pr.Body, pr.Head.Ref, pr.Base.Ref, labels, reviewers, assignees)
}

func (f *fghc) CreateIssue(org, repo, title, body string, milestone int, labels, assignees []string) (int, error) {
Expand Down Expand Up @@ -298,8 +298,12 @@ func testCherryPickIC(clients localgit.Clients, t *testing.T) {
if err := lg.AddCommit("foo", "bar", initialFiles); err != nil {
t.Fatalf("Adding initial commit: %v", err)
}
if err := lg.CheckoutNewBranch("foo", "bar", "stage"); err != nil {
t.Fatalf("Checking out pull branch: %v", err)

expectedBranches := []string{"stage", "release-1.5"}
for _, branch := range expectedBranches {
if err := lg.CheckoutNewBranch("foo", "bar", branch); err != nil {
t.Fatalf("Checking out pull branch: %v", err)
}
}

ghc := &fghc{
Expand Down Expand Up @@ -344,20 +348,29 @@ func testCherryPickIC(clients localgit.Clients, t *testing.T) {
User: github.User{
Login: "wiseguy",
},
Body: "/cherrypick stage",
Body: "/cherrypick stage\r\n/cherrypick release-1.5\r\n/cherrypick master",
},
}

botUser := &github.UserData{Login: "ci-robot", Email: "[email protected]"}
expectedTitle := "This is a fix for X (#2)"
expectedBody := "This is an automated cherry-pick of #2\n\nThis PR updates the magic number.\n\n"
expectedBase := "stage"
expectedHead := fmt.Sprintf(botUser.Login+":"+cherryPickBranchFmt, 2, expectedBase)
expectedLabels := []string{"type/cherrypick-for-stage"}
expectedReviewers := []string{"user1"}
expectedAssignees := []string{"wiseguy"}
expected := fmt.Sprintf(expectedFmt, expectedTitle, expectedBody, expectedHead,
expectedBase, expectedLabels, expectedReviewers, expectedAssignees)
var expectedFn = func(branch string) string {
expectedTitle := "This is a fix for X (#2)"
expectedBody := "This is an automated cherry-pick of #2\n\nThis PR updates the magic number.\n\n"
expectedHead := fmt.Sprintf(botUser.Login+":"+cherryPickBranchFmt, 2, branch)
expectedAssignees := []string{"wiseguy"}

var expectedLabels []string
for _, label := range ghc.pr.Labels {
expectedLabels = append(expectedLabels, label.Name)
}
expectedLabels = append(expectedLabels, "type/cherrypick-for-"+branch)
var expectedReviewers []string
for _, reviewer := range ghc.pr.RequestedReviewers {
expectedReviewers = append(expectedReviewers, reviewer.Login)
}
return fmt.Sprintf(prFmt, expectedTitle, expectedBody, expectedHead,
branch, expectedLabels, expectedReviewers, expectedAssignees)
}

getSecret := func() []byte {
return []byte("sha=abcdefg")
Expand Down Expand Up @@ -388,9 +401,29 @@ func testCherryPickIC(clients localgit.Clients, t *testing.T) {
if err := s.handleIssueComment(logrus.NewEntry(logrus.StandardLogger()), ic); err != nil {
t.Fatalf("unexpected error: %v", err)
}
got := prToString(ghc.prs[0])
if got != expected {
t.Errorf("Expected (%d):\n%s\nGot (%d):\n%+v\n", len(expected), expected, len(got), got)

if len(ghc.prs) != len(expectedBranches) {
t.Fatalf("Expected %d PRs, got %d", len(expectedBranches), len(ghc.prs))
}

expectedPrs := make(map[string]string)
for _, branch := range expectedBranches {
expectedPrs[expectedFn(branch)] = branch
}

seenBranches := make(map[string]bool)
for _, p := range ghc.prs {
pr := prToString(p)
branch, present := expectedPrs[pr]
if !present {
t.Errorf("Unexpected PR:\n%s\nExpected to target one of the following branches: %v\n",
pr, expectedBranches)
} else {
seenBranches[branch] = present
}
}
if len(seenBranches) != len(expectedBranches) {
t.Fatalf("Expected to see PRs for %d branches, got %d (%v)", len(expectedBranches), len(seenBranches), seenBranches)
}
}

Expand Down Expand Up @@ -584,13 +617,13 @@ func testCherryPickPR(clients localgit.Clients, t *testing.T) {
for _, label := range pr.PullRequest.Labels {
expectedLabels = append(expectedLabels, label.Name)
}
var reviewers []string
var expectedReviewers []string
for _, reviewer := range pr.PullRequest.RequestedReviewers {
reviewers = append(reviewers, reviewer.Login)
expectedReviewers = append(expectedReviewers, reviewer.Login)
}
expectedAssignees := []string{"approver"}
return fmt.Sprintf(expectedFmt, expectedTitle, expectedBody, expectedHead,
branch, expectedLabels, reviewers, expectedAssignees)
return fmt.Sprintf(prFmt, expectedTitle, expectedBody, expectedHead,
branch, expectedLabels, expectedReviewers, expectedAssignees)
}

if len(ghc.prs) != len(expectedBranches) {
Expand All @@ -601,15 +634,16 @@ func testCherryPickPR(clients localgit.Clients, t *testing.T) {
for _, branch := range expectedBranches {
expectedPrs[expectedFn(branch)] = branch
}
seenBranches := make(map[string]struct{})
seenBranches := make(map[string]bool)
for _, p := range ghc.prs {
pr := prToString(p)
branch, present := expectedPrs[pr]
if !present {
t.Errorf("Unexpected PR:\n%s\nExpected to target one of the following branches: %v\n",
pr, expectedBranches)
} else {
seenBranches[branch] = present
}
seenBranches[branch] = struct{}{}
}
if len(seenBranches) != len(expectedBranches) {
t.Fatalf("Expected to see PRs for %d branches, got %d (%v)", len(expectedBranches), len(seenBranches), seenBranches)
Expand Down Expand Up @@ -803,13 +837,13 @@ func testCherryPickPRWithLabels(clients localgit.Clients, t *testing.T) {
expectedLabels = append(expectedLabels, label.Name)
}
expectedLabels = append(expectedLabels, "type/cherrypick-for-"+branch)
var reviewers []string
var expectedReviewers []string
for _, reviewer := range pr(evt).PullRequest.RequestedReviewers {
reviewers = append(reviewers, reviewer.Login)
expectedReviewers = append(expectedReviewers, reviewer.Login)
}
expectedAssignees := []string{"developer"}
return fmt.Sprintf(expectedFmt, expectedTitle, expectedBody, expectedHead,
branch, expectedLabels, reviewers, expectedAssignees)
return fmt.Sprintf(prFmt, expectedTitle, expectedBody, expectedHead,
branch, expectedLabels, expectedReviewers, expectedAssignees)
}

expectedPRs := 2
Expand All @@ -825,17 +859,17 @@ func testCherryPickPRWithLabels(clients localgit.Clients, t *testing.T) {
}

expectedBranches := []string{"release-1.5", "release-1.6"}
seenBranches := make(map[string]struct{})
seenBranches := make(map[string]bool)
for _, p := range ghc.prs {
pr := prToString(p)
if pr != expectedFn("release-1.5") && pr != expectedFn("release-1.6") {
t.Errorf("Unexpected PR:\n%s\nExpected to target one of the following branches: %v", pr, expectedBranches)
}
if pr == expectedFn("release-1.5") {
seenBranches["release-1.5"] = struct{}{}
seenBranches["release-1.5"] = true
}
if pr == expectedFn("release-1.6") {
seenBranches["release-1.6"] = struct{}{}
seenBranches["release-1.6"] = true
}
}
if len(seenBranches) != expectedPRs {
Expand Down

0 comments on commit 91061d0

Please sign in to comment.