Skip to content

Commit

Permalink
bot: Allow setB to be a single approver (#209)
Browse files Browse the repository at this point in the history
  • Loading branch information
jimbishopp authored Jan 4, 2024
1 parent 714fd43 commit ae09746
Show file tree
Hide file tree
Showing 3 changed files with 49 additions and 15 deletions.
1 change: 1 addition & 0 deletions bot/internal/bot/check.go
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@ func (b *Bot) Check(ctx context.Context) error {
}

changes := classifyChanges(b.c, files)
log.Printf("Check: required approvals: %d", changes.ApproverCount)

if changes.Large {
comment := fmt.Sprintf("@%v - this PR will require admin approval to merge due to its size. "+
Expand Down
10 changes: 7 additions & 3 deletions bot/internal/review/review.go
Original file line number Diff line number Diff line change
Expand Up @@ -518,10 +518,14 @@ func (r *Assignments) checkInternalCodeReviews(e *env.Environment, changes env.C
// PRs can be approved if you either have multiple code owners that approve
// or code owner and code reviewer. An exception is for PRs that
// only modify paths that require a single approver.
if checkN(setA, reviews) >= changes.ApproverCount {
a := checkN(setA, reviews)
b := checkN(setB, reviews)
switch {
case changes.ApproverCount == 1 && (a >= 1 || b >= 1):
return nil
}
if check(setA, reviews) && check(setB, reviews) {
case a >= changes.ApproverCount:
return nil
case a > 0 && b > 0 && a+b >= changes.ApproverCount:
return nil
}

Expand Down
53 changes: 41 additions & 12 deletions bot/internal/review/review_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -669,16 +669,17 @@ func TestCheckInternal(t *testing.T) {
},
}
tests := []struct {
desc string
author string
repository string
reviews []github.Review
docs bool
code bool
large bool
release bool
result bool
files []github.PullRequestFile
desc string
author string
repository string
reviews []github.Review
docs bool
code bool
large bool
release bool
singleApproval bool
result bool
files []github.PullRequestFile
}{
{
desc: "no-reviews-fail",
Expand Down Expand Up @@ -1050,20 +1051,48 @@ func TestCheckInternal(t *testing.T) {
large: false,
result: false,
},
{
desc: "cloud-single-approval-setA-success",
repository: "cloud",
author: Dependabot,
reviews: []github.Review{
{Author: "12", State: Approved},
},
docs: false,
code: true,
singleApproval: true,
result: true,
},
{
desc: "cloud-single-approval-setB-success",
repository: "cloud",
author: Dependabot,
reviews: []github.Review{
{Author: "13", State: Approved},
},
docs: false,
code: true,
singleApproval: true,
result: true,
},
}
for _, test := range tests {
t.Run(test.desc, func(t *testing.T) {
e := &env.Environment{
Repository: test.repository,
Author: test.author,
}
err := r.CheckInternal(e, test.reviews, env.Changes{
changes := env.Changes{
Docs: test.docs,
Code: test.code,
Large: test.large,
Release: test.release,
ApproverCount: env.DefaultApproverCount,
}, test.files)
}
if test.singleApproval {
changes.ApproverCount = 1
}
err := r.CheckInternal(e, test.reviews, changes, test.files)
if test.result {
require.NoError(t, err)
} else {
Expand Down

0 comments on commit ae09746

Please sign in to comment.