Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(github-bot): support review team flows #3530

Merged
merged 6 commits into from
Jan 21, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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 @@
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())),
},

Check warning on line 69 in contribs/github-bot/internal/config/config.go

View check run for this annotation

Codecov / codecov/patch

contribs/github-bot/internal/config/config.go#L57-L69

Added lines #L57 - L69 were not covered by tests
thehowl marked this conversation as resolved.
Show resolved Hide resolved
}

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
Loading