Skip to content

Commit

Permalink
feat(github-bot): support review team flows (#3530)
Browse files Browse the repository at this point in the history
Fixes #3072, Fixes #3093.

Should add support for the required workflows on the GitHub bot, for the
review team.

cc @Kouteki @jefft0
  • Loading branch information
thehowl authored Jan 21, 2025
1 parent bcfc002 commit c74fb57
Show file tree
Hide file tree
Showing 14 changed files with 516 additions and 36 deletions.
16 changes: 16 additions & 0 deletions contribs/github-bot/Makefile
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
GNOROOT_DIR ?= $(abspath $(lastword $(MAKEFILE_LIST))/../../../)

rundep := go run -modfile ../../misc/devdeps/go.mod
golangci_lint := $(rundep) github.com/golangci/golangci-lint/cmd/golangci-lint

.PHONY: install
install:
go install .

.PHONY: lint
lint:
$(golangci_lint) --config ../../.github/golangci.yml run ./...

.PHONY: test
test:
go test $(GOTEST_FLAGS) -v ./...
21 changes: 21 additions & 0 deletions contribs/github-bot/internal/conditions/author.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,3 +58,24 @@ func (a *authorInTeam) IsMet(pr *github.PullRequest, details treeprint.Tree) boo
func AuthorInTeam(gh *client.GitHub, team string) Condition {
return &authorInTeam{gh: gh, team: team}
}

type authorAssociationIs struct {
assoc string
}

var _ Condition = &authorAssociationIs{}

func (a *authorAssociationIs) IsMet(pr *github.PullRequest, details treeprint.Tree) bool {
detail := fmt.Sprintf("Pull request author has author_association: %q", a.assoc)

return utils.AddStatusNode(pr.GetAuthorAssociation() == a.assoc, detail, details)
}

// AuthorAssociationIs asserts that the author of the PR has the given value for
// the GitHub "author association" field, on the PR.
//
// See https://docs.github.com/en/graphql/reference/enums#commentauthorassociation
// for a list of possible values and descriptions.
func AuthorAssociationIs(association string) Condition {
return &authorAssociationIs{assoc: association}
}
27 changes: 27 additions & 0 deletions contribs/github-bot/internal/conditions/author_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -91,3 +91,30 @@ func TestAuthorInTeam(t *testing.T) {
})
}
}

func TestAuthorAssociationIs(t *testing.T) {
t.Parallel()

for _, testCase := range []struct {
name string
association string
associationWant string
isMet bool
}{
{"has", "MEMBER", "MEMBER", true},
{"hasNot", "COLLABORATOR", "MEMBER", false},
} {
t.Run(testCase.name, func(t *testing.T) {
t.Parallel()

pr := &github.PullRequest{
AuthorAssociation: github.String(testCase.association),
}
details := treeprint.New()
condition := AuthorAssociationIs(testCase.associationWant)

assert.Equal(t, condition.IsMet(pr, details), testCase.isMet, fmt.Sprintf("condition should have a met status: %t", testCase.isMet))
assert.True(t, utils.TestLastNodeStatus(t, testCase.isMet, details), fmt.Sprintf("condition details should have a status: %t", testCase.isMet))
})
}
}
2 changes: 1 addition & 1 deletion contribs/github-bot/internal/conditions/draft.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import (
// Draft Condition.
type draft struct{}

var _ Condition = &baseBranch{}
var _ Condition = &draft{}

func (*draft) IsMet(pr *github.PullRequest, details treeprint.Tree) bool {
return utils.AddStatusNode(pr.GetDraft(), "This pull request is a draft", details)
Expand Down
13 changes: 13 additions & 0 deletions contribs/github-bot/internal/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,19 @@ func Config(gh *client.GitHub) ([]AutomaticCheck, []ManualCheck) {
If: c.Label("don't merge"),
Then: r.Never(),
},
{
Description: "Pending initial approval by a review team member (and label matches review triage state)",
If: c.Not(c.AuthorInTeam(gh, "tech-staff")),
Then: r.
If(r.Or(r.ReviewByOrgMembers(gh, 1), r.Draft())).
// Either there was a first approval from a member, and we
// assert that the label for triage-pending is removed...
Then(r.Not(r.Label(gh, "review/triage-pending", r.LabelRemove))).
// Or there was not, and we apply the triage pending label.
// The requirement should always fail, to mark the PR is not
// ready to be merged.
Else(r.And(r.Label(gh, "review/triage-pending", r.LabelApply), r.Never())),
},
}

manual := []ManualCheck{
Expand Down
66 changes: 66 additions & 0 deletions contribs/github-bot/internal/requirements/boolean.go
Original file line number Diff line number Diff line change
Expand Up @@ -96,3 +96,69 @@ func (n *not) IsSatisfied(pr *github.PullRequest, details treeprint.Tree) bool {
func Not(req Requirement) Requirement {
return &not{req}
}

// IfCondition executes the condition, and based on the result then runs Then
// or Else.
type IfCondition struct {
cond Requirement
then Requirement
els Requirement
}

var _ Requirement = &IfCondition{}

func (i *IfCondition) IsSatisfied(pr *github.PullRequest, details treeprint.Tree) bool {
if i.then == nil {
i.then = Always()
}
ifBranch := details.AddBranch("")
condBranch := ifBranch.AddBranch("")

var (
target Requirement
targetName string
)

if i.cond.IsSatisfied(pr, condBranch) {
condBranch.SetValue(fmt.Sprintf("%s Condition", utils.Success))
target, targetName = i.then, "Then"
} else {
condBranch.SetValue(fmt.Sprintf("%s Condition", utils.Fail))
target, targetName = i.els, "Else"
}

targBranch := ifBranch.AddBranch("")
if target == nil || target.IsSatisfied(pr, targBranch) {
ifBranch.SetValue(fmt.Sprintf("%s If", utils.Success))
targBranch.SetValue(fmt.Sprintf("%s %s", utils.Success, targetName))
return true
} else {
ifBranch.SetValue(fmt.Sprintf("%s If", utils.Fail))
targBranch.SetValue(fmt.Sprintf("%s %s", utils.Fail, targetName))
return false
}
}

// If returns a conditional requirement, which runs Then if cond evaluates
// successfully, or Else otherwise.
//
// Then / Else are optional, and always evaluate to true by default.
func If(cond Requirement) *IfCondition {
return &IfCondition{cond: cond}
}

func (i *IfCondition) Then(then Requirement) *IfCondition {
if i.then != nil {
panic("'Then' is already set")
}
i.then = then
return i
}

func (i *IfCondition) Else(els Requirement) *IfCondition {
if i.els != nil {
panic("'Else' is already set")
}
i.els = els
return i
}
77 changes: 77 additions & 0 deletions contribs/github-bot/internal/requirements/boolean_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -94,3 +94,80 @@ func TestNot(t *testing.T) {
})
}
}

func TestIfCond(t *testing.T) {
t.Parallel()

for _, testCase := range []struct {
name string
req Requirement
isSatisfied bool
}{
{"if always", If(Always()), true},
{"if never", If(Never()), true},
{"if always then always", If(Always()).Then(Always()), true},
{"if never else always", If(Never()).Else(Always()), true},
{"if always then never", If(Always()).Then(Never()), false},
{"if never else never", If(Never()).Else(Never()), false},
} {
t.Run(testCase.name, func(t *testing.T) {
t.Parallel()

pr := &github.PullRequest{}
details := treeprint.New()

actual := testCase.req.IsSatisfied(pr, details)
assert.Equal(t, testCase.isSatisfied, actual,
"requirement should have a satisfied status: %t", testCase.isSatisfied)
assert.True(t,
utils.TestNodeStatus(t, testCase.isSatisfied, details.(*treeprint.Node).Nodes[0]),
"requirement details should have a status: %t", testCase.isSatisfied)
})
}
}

type reqFunc func(*github.PullRequest, treeprint.Tree) bool

func (r reqFunc) IsSatisfied(gh *github.PullRequest, details treeprint.Tree) bool {
return r(gh, details)
}

func TestIfCond_ConditionalExecution(t *testing.T) {
t.Run("executeThen", func(t *testing.T) {
thenExec, elseExec := 0, 0
If(Always()).
Then(reqFunc(func(*github.PullRequest, treeprint.Tree) bool {
thenExec++
return true
})).
Else(reqFunc(func(*github.PullRequest, treeprint.Tree) bool {
elseExec++
return true
})).IsSatisfied(nil, treeprint.New())
assert.Equal(t, 1, thenExec, "Then should be executed 1 time")
assert.Equal(t, 0, elseExec, "Else should be executed 0 time")
})
t.Run("executeElse", func(t *testing.T) {
thenExec, elseExec := 0, 0
If(Never()).
Then(reqFunc(func(*github.PullRequest, treeprint.Tree) bool {
thenExec++
return true
})).
Else(reqFunc(func(*github.PullRequest, treeprint.Tree) bool {
elseExec++
return true
})).IsSatisfied(nil, treeprint.New())
assert.Equal(t, 0, thenExec, "Then should be executed 0 time")
assert.Equal(t, 1, elseExec, "Else should be executed 1 time")
})
}

func TestIfCond_NoRepeats(t *testing.T) {
assert.Panics(t, func() {
If(Always()).Then(Always()).Then(Always())
}, "two Then should panic")
assert.Panics(t, func() {
If(Always()).Else(Always()).Else(Always())
}, "two Else should panic")
}
21 changes: 21 additions & 0 deletions contribs/github-bot/internal/requirements/draft.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
package requirements

import (
"github.com/gnolang/gno/contribs/github-bot/internal/utils"

"github.com/google/go-github/v64/github"
"github.com/xlab/treeprint"
)

// Draft Condition.
type draft struct{}

var _ Requirement = &draft{}

func (*draft) IsSatisfied(pr *github.PullRequest, details treeprint.Tree) bool {
return utils.AddStatusNode(pr.GetDraft(), "This pull request is a draft", details)
}

func Draft() Requirement {
return &draft{}
}
34 changes: 34 additions & 0 deletions contribs/github-bot/internal/requirements/draft_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
package requirements

import (
"fmt"
"testing"

"github.com/gnolang/gno/contribs/github-bot/internal/utils"
"github.com/google/go-github/v64/github"
"github.com/stretchr/testify/assert"
"github.com/xlab/treeprint"
)

func TestDraft(t *testing.T) {
t.Parallel()

for _, testCase := range []struct {
name string
isMet bool
}{
{"draft is true", true},
{"draft is false", false},
} {
t.Run(testCase.name, func(t *testing.T) {
t.Parallel()

pr := &github.PullRequest{Draft: &testCase.isMet}
details := treeprint.New()
req := Draft()

assert.Equal(t, req.IsSatisfied(pr, details), testCase.isMet, fmt.Sprintf("condition should have a met status: %t", testCase.isMet))
assert.True(t, utils.TestLastNodeStatus(t, testCase.isMet, details), fmt.Sprintf("condition details should have a status: %t", testCase.isMet))
})
}
}
Loading

0 comments on commit c74fb57

Please sign in to comment.