Skip to content

Commit d9c5bd8

Browse files
ardakuyumcuardakuyumcu
committedFeb 28, 2020
Add check for case where base ref did not change
1 parent fa4b815 commit d9c5bd8

File tree

2 files changed

+87
-79
lines changed

2 files changed

+87
-79
lines changed
 

‎prow/plugins/cherrypickunapproved/cherrypick-unapproved.go

+7-2
Original file line numberDiff line numberDiff line change
@@ -116,10 +116,15 @@ func handlePR(gc githubClient, log *logrus.Entry, pr *github.PullRequestEvent, c
116116
} `json:"base"`
117117
}
118118
if err := json.Unmarshal(pr.Changes, &changes); err != nil {
119-
// we're detecting this best-effort so we can forget about
120-
// the event
119+
// we're detecting this best-effort so we can forget about the event
121120
return nil
122121
}
122+
123+
if changes.Base.Ref.From == "" {
124+
// PR base ref did not change, ignore the event
125+
return nil
126+
}
127+
123128
if branchRe.MatchString(branch) && !branchRe.MatchString(changes.Base.Ref.From) {
124129
// base ref changed from a branch not allowed for cherry-picks to a branch that is allowed for cherry-picks
125130
return ensureLabels(gc, org, repo, pr, log, cp, commentBody)

‎prow/plugins/cherrypickunapproved/cherrypick-unapproved_test.go

+80-77
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,6 @@ package cherrypickunapproved
1818

1919
import (
2020
"encoding/json"
21-
"fmt"
2221
"reflect"
2322
"regexp"
2423
"testing"
@@ -90,7 +89,7 @@ type fakePruner struct{}
9089

9190
func (fp *fakePruner) PruneComments(shouldPrune func(github.IssueComment) bool) {}
9291

93-
func makeFakePullRequestEvent(action github.PullRequestEventAction, branch, oldBranch string) github.PullRequestEvent {
92+
func makeFakePullRequestEvent(action github.PullRequestEventAction, branch string, changes json.RawMessage) github.PullRequestEvent {
9493
event := github.PullRequestEvent{
9594
Action: action,
9695
PullRequest: github.PullRequest{
@@ -100,103 +99,107 @@ func makeFakePullRequestEvent(action github.PullRequestEventAction, branch, oldB
10099
},
101100
}
102101

103-
if oldBranch != "" {
104-
event.Changes = json.RawMessage(fmt.Sprintf(`{"base": {"ref": {"from": "%s"}, "sha": {"from": "sha"}}}`, oldBranch))
102+
if changes != nil {
103+
event.Changes = changes
105104
}
106105

107106
return event
108107
}
109108

110109
func TestCherryPickUnapprovedLabel(t *testing.T) {
111110
var testcases = []struct {
112-
name string
113-
branch string
114-
previousBranch string
115-
action github.PullRequestEventAction
116-
labels []string
117-
added []string
118-
removed []string
119-
expectComment bool
111+
name string
112+
branch string
113+
changes json.RawMessage
114+
action github.PullRequestEventAction
115+
labels []string
116+
added []string
117+
removed []string
118+
expectComment bool
120119
}{
121120
{
122-
name: "unsupported PR action -> no-op",
123-
branch: "release-1.10",
124-
previousBranch: "",
125-
action: github.PullRequestActionClosed,
126-
labels: []string{},
127-
added: []string{},
128-
removed: []string{},
129-
expectComment: false,
121+
name: "unsupported PR action -> no-op",
122+
branch: "release-1.10",
123+
action: github.PullRequestActionClosed,
124+
labels: []string{},
125+
added: []string{},
126+
removed: []string{},
127+
expectComment: false,
130128
},
131129
{
132-
name: "branch that does match regexp -> no-op",
133-
branch: "master",
134-
previousBranch: "",
135-
action: github.PullRequestActionOpened,
136-
labels: []string{},
137-
added: []string{},
138-
removed: []string{},
139-
expectComment: false,
130+
name: "branch that does match regexp -> no-op",
131+
branch: "master",
132+
action: github.PullRequestActionOpened,
133+
labels: []string{},
134+
added: []string{},
135+
removed: []string{},
136+
expectComment: false,
140137
},
141138
{
142-
name: "has cpUnapproved -> no-op",
143-
branch: "release-1.10",
144-
previousBranch: "",
145-
action: github.PullRequestActionOpened,
146-
labels: []string{labels.CpUnapproved},
147-
added: []string{},
148-
removed: []string{},
149-
expectComment: false,
139+
name: "has cpUnapproved -> no-op",
140+
branch: "release-1.10",
141+
action: github.PullRequestActionOpened,
142+
labels: []string{labels.CpUnapproved},
143+
added: []string{},
144+
removed: []string{},
145+
expectComment: false,
150146
},
151147
{
152-
name: "has both cpApproved and cpUnapproved -> remove cpUnapproved",
153-
branch: "release-1.10",
154-
previousBranch: "",
155-
action: github.PullRequestActionOpened,
156-
labels: []string{labels.CpApproved, labels.CpUnapproved},
157-
added: []string{},
158-
removed: []string{labels.CpUnapproved},
159-
expectComment: false,
148+
name: "has both cpApproved and cpUnapproved -> remove cpUnapproved",
149+
branch: "release-1.10",
150+
action: github.PullRequestActionOpened,
151+
labels: []string{labels.CpApproved, labels.CpUnapproved},
152+
added: []string{},
153+
removed: []string{labels.CpUnapproved},
154+
expectComment: false,
160155
},
161156
{
162-
name: "does not have any labels, PR opened against a release branch -> add cpUnapproved and comment",
163-
branch: "release-1.10",
164-
previousBranch: "",
165-
action: github.PullRequestActionOpened,
166-
labels: []string{},
167-
added: []string{labels.CpUnapproved},
168-
removed: []string{},
169-
expectComment: true,
157+
name: "does not have any labels, PR opened against a release branch -> add cpUnapproved and comment",
158+
branch: "release-1.10",
159+
action: github.PullRequestActionOpened,
160+
labels: []string{},
161+
added: []string{labels.CpUnapproved},
162+
removed: []string{},
163+
expectComment: true,
170164
},
171165
{
172-
name: "does not have any labels, PR reopened against a release branch -> add cpUnapproved and comment",
173-
branch: "release-1.10",
174-
previousBranch: "",
175-
action: github.PullRequestActionReopened,
176-
labels: []string{},
177-
added: []string{labels.CpUnapproved},
178-
removed: []string{},
179-
expectComment: true,
166+
name: "does not have any labels, PR reopened against a release branch -> add cpUnapproved and comment",
167+
branch: "release-1.10",
168+
action: github.PullRequestActionReopened,
169+
labels: []string{},
170+
added: []string{labels.CpUnapproved},
171+
removed: []string{},
172+
expectComment: true,
180173
},
181174
{
182-
name: "PR base branch master edited to release -> add cpUnapproved and comment",
183-
branch: "release-1.10",
184-
previousBranch: "master",
185-
action: github.PullRequestActionEdited,
186-
labels: []string{},
187-
added: []string{labels.CpUnapproved},
188-
removed: []string{},
189-
expectComment: true,
175+
name: "PR base branch master edited to release -> add cpUnapproved and comment",
176+
branch: "release-1.10",
177+
action: github.PullRequestActionEdited,
178+
changes: json.RawMessage(`{"base": {"ref": {"from": "master"}, "sha": {"from": "sha"}}}`),
179+
labels: []string{},
180+
added: []string{labels.CpUnapproved},
181+
removed: []string{},
182+
expectComment: true,
190183
},
191184
{
192-
name: "PR base branch edited from release to master -> remove cpApproved and cpUnapproved",
193-
branch: "master",
194-
previousBranch: "release-1.10",
195-
action: github.PullRequestActionEdited,
196-
labels: []string{labels.CpApproved, labels.CpUnapproved},
197-
added: []string{},
198-
removed: []string{labels.CpApproved, labels.CpUnapproved},
199-
expectComment: false,
185+
name: "PR base branch edited from release to master -> remove cpApproved and cpUnapproved",
186+
branch: "master",
187+
action: github.PullRequestActionEdited,
188+
changes: json.RawMessage(`{"base": {"ref": {"from": "release-1.10"}, "sha": {"from": "sha"}}}`),
189+
labels: []string{labels.CpApproved, labels.CpUnapproved},
190+
added: []string{},
191+
removed: []string{labels.CpApproved, labels.CpUnapproved},
192+
expectComment: false,
193+
},
194+
{
195+
name: "PR title changed -> no-op",
196+
branch: "release-1.10",
197+
action: github.PullRequestActionEdited,
198+
changes: json.RawMessage(`{"title": {"from": "Update README.md"}}`),
199+
labels: []string{labels.CpApproved, labels.CpUnapproved},
200+
added: []string{},
201+
removed: []string{},
202+
expectComment: false,
200203
},
201204
}
202205

@@ -208,7 +211,7 @@ func TestCherryPickUnapprovedLabel(t *testing.T) {
208211
commentsAdded: make(map[int][]string, 0),
209212
}
210213

211-
event := makeFakePullRequestEvent(tc.action, tc.branch, tc.previousBranch)
214+
event := makeFakePullRequestEvent(tc.action, tc.branch, tc.changes)
212215
branchRe := regexp.MustCompile(`^release-.*$`)
213216
comment := "dummy cumment"
214217
err := handlePR(fc, logrus.WithField("plugin", "fake-cherrypick-unapproved"), &event, &fakePruner{}, branchRe, comment)

0 commit comments

Comments
 (0)
Please sign in to comment.