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

ci: fixes bot workflow and comment update #3229

Merged
merged 10 commits into from
Nov 28, 2024
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
12 changes: 7 additions & 5 deletions .github/workflows/bot.yml
Original file line number Diff line number Diff line change
Expand Up @@ -44,16 +44,18 @@ jobs:
steps:
- name: Checkout code
uses: actions/checkout@v4

- name: Install Go
uses: actions/setup-go@v5
with:
go-version-file: go.mod
thehowl marked this conversation as resolved.
Show resolved Hide resolved
go-version-file: contribs/github-bot/go.mod

- name: Generate matrix from event
id: pr-numbers
working-directory: contribs/github-bot
env:
GH_TOKEN: ${{ secrets.GITHUB_TOKEN }}
run: go run . matrix >> "$GITHUB_OUTPUT"
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
run: echo "pr-numbers=$(go run . matrix)" >> "$GITHUB_OUTPUT"

# This job processes each pull request in the matrix individually while ensuring
# that a same PR cannot be processed concurrently by mutliple runners
Expand All @@ -76,10 +78,10 @@ jobs:
- name: Install Go
uses: actions/setup-go@v5
with:
go-version-file: go.mod
go-version-file: contribs/github-bot/go.mod

- name: Run GitHub Bot
working-directory: contribs/github-bot
env:
GITHUB_TOKEN: ${{ secrets.GH_BOT_PAT }}
run: go run . -pr-numbers '${{ matrix.pr-number }}' -verbose
run: go run . check -pr-numbers '${{ matrix.pr-number }}' -verbose
9 changes: 3 additions & 6 deletions contribs/github-bot/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -27,14 +27,11 @@ For the bot to make requests to the GitHub API, it needs a Personal Access Token
## Usage

```bash
> go install github.com/gnolang/gno/contribs/github-bot@latest
// (go: downloading ...)

> github-bot --help
> github-bot check --help
USAGE
github-bot [flags]
github-bot check [flags]

This tool checks if the requirements for a PR to be merged are satisfied (defined in config.go) and displays PR status checks accordingly.
This tool checks if the requirements for a pull request to be merged are satisfied (defined in config.go) and displays PR status checks accordingly.
A valid GitHub Token must be provided by setting the GITHUB_TOKEN environment variable.

FLAGS
Expand Down
4 changes: 2 additions & 2 deletions contribs/github-bot/comment.go
Original file line number Diff line number Diff line change
Expand Up @@ -175,9 +175,9 @@

// If teams specified in rule, check if actor is a member of one of them.
if len(teams) > 0 {
if gh.IsUserInTeams(actionCtx.Actor, teams) {
if !gh.IsUserInTeams(actionCtx.Actor, teams) { // If user not allowed

Check warning on line 178 in contribs/github-bot/comment.go

View check run for this annotation

Codecov / codecov/patch

contribs/github-bot/comment.go#L178

Added line #L178 was not covered by tests
if !gh.DryRun {
gh.SetBotComment(previous, int(prNum))
gh.SetBotComment(previous, int(prNum)) // Restore previous state

Check warning on line 180 in contribs/github-bot/comment.go

View check run for this annotation

Codecov / codecov/patch

contribs/github-bot/comment.go#L180

Added line #L180 was not covered by tests
}
return errors.New("checkbox edited by a user not allowed to")
}
Expand Down
71 changes: 29 additions & 42 deletions contribs/github-bot/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@
r "github.com/gnolang/gno/contribs/github-bot/internal/requirements"
)

type Teams []string

// Automatic check that will be performed by the bot.
type automaticCheck struct {
description string
Expand All @@ -17,73 +19,58 @@
type manualCheck struct {
description string
ifC c.Condition // If the condition is met, a checkbox will be displayed on bot comment.
teams []string // Members of these teams can check the checkbox to make the check pass.
teams Teams // Members of these teams can check the checkbox to make the check pass.
}

// This function returns the configuration of the bot consisting of automatic and manual checks
// in which the GitHub client is injected.
func config(gh *client.GitHub) ([]automaticCheck, []manualCheck) {
auto := []automaticCheck{
{
description: "Changes to 'tm2' folder should be reviewed/authored by at least one member of both EU and US teams",
ifC: c.And(
c.FileChanged(gh, "tm2"),
c.BaseBranch("master"),
),
thenR: r.And(
r.Or(
r.ReviewByTeamMembers(gh, "eu", 1),
r.AuthorInTeam(gh, "eu"),
),
r.Or(
r.ReviewByTeamMembers(gh, "us", 1),
r.AuthorInTeam(gh, "us"),
),
),
},
{
description: "A maintainer must be able to edit this pull request",
description: "Maintainers must be able to edit this pull request",

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

View check run for this annotation

Codecov / codecov/patch

contribs/github-bot/config.go#L30

Added line #L30 was not covered by tests
ifC: c.Always(),
thenR: r.MaintainerCanModify(),
},
{
description: "The pull request head branch must be up-to-date with its base",
ifC: c.Always(), // Or only if c.BaseBranch("main") ?
ifC: c.Always(),

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

View check run for this annotation

Codecov / codecov/patch

contribs/github-bot/config.go#L36

Added line #L36 was not covered by tests
thenR: r.UpToDateWith(gh, r.PR_BASE),
},
{
description: "Changes to 'docs' folder must be reviewed/authored by at least one devrel and one tech-staff",
ifC: c.FileChanged(gh, "^docs/"),
thenR: r.Or(
r.And(
r.AuthorInTeam(gh, "devrels"),
r.ReviewByTeamMembers(gh, "tech-staff", 1),
),
r.And(
r.AuthorInTeam(gh, "tech-staff"),
r.ReviewByTeamMembers(gh, "devrels", 1),
),
),
},

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

View check run for this annotation

Codecov / codecov/patch

contribs/github-bot/config.go#L39-L52

Added lines #L39 - L52 were not covered by tests
}

manual := []manualCheck{
{
description: "Determine if infra needs to be updated",
ifC: c.And(
c.BaseBranch("master"),
c.Or(
c.FileChanged(gh, "misc/deployments"),
c.FileChanged(gh, `misc/docker-\.*`),
c.FileChanged(gh, "tm2/pkg/p2p"),
),
),
teams: []string{"tech-staff"},
description: "The pull request description provides enough details",
ifC: c.Not(c.AuthorInTeam(gh, "core-contributors")),
teams: Teams{"core-contributors"},

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

View check run for this annotation

Codecov / codecov/patch

contribs/github-bot/config.go#L57-L59

Added lines #L57 - L59 were not covered by tests
},
{
description: "Ensure the code style is satisfactory",
description: "Determine if infra needs to be updated before merging",

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

View check run for this annotation

Codecov / codecov/patch

contribs/github-bot/config.go#L62

Added line #L62 was not covered by tests
ifC: c.And(
c.BaseBranch("master"),
c.Or(
c.FileChanged(gh, `.*\.go`),
c.FileChanged(gh, `.*\.js`),
c.FileChanged(gh, `Dockerfile`),
c.FileChanged(gh, `^misc/deployments`),
c.FileChanged(gh, `^misc/docker-`),
c.FileChanged(gh, `^.github/workflows/releaser.*\.yml$`),
c.FileChanged(gh, `^.github/workflows/portal-loop\.yml$`),

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

View check run for this annotation

Codecov / codecov/patch

contribs/github-bot/config.go#L66-L70

Added lines #L66 - L70 were not covered by tests
),
),
teams: []string{"tech-staff"},
},
{
description: "Ensure the documentation is accurate and relevant",
ifC: c.FileChanged(gh, `.*\.md`),
teams: []string{
"tech-staff",
"devrels",
},
teams: Teams{"devops"},

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

View check run for this annotation

Codecov / codecov/patch

contribs/github-bot/config.go#L73

Added line #L73 was not covered by tests
},
}

Expand Down
2 changes: 1 addition & 1 deletion contribs/github-bot/internal/client/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@
opts.Page = response.NextPage
}

return nil, errors.New("bot comment not found")
return nil, ErrBotCommentNotFound

Check warning on line 83 in contribs/github-bot/internal/client/client.go

View check run for this annotation

Codecov / codecov/patch

contribs/github-bot/internal/client/client.go#L83

Added line #L83 was not covered by tests
}

// SetBotComment creates a bot's comment on the provided PR number
Expand Down
4 changes: 2 additions & 2 deletions contribs/github-bot/internal/conditions/branch_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,9 @@ func TestHeadBaseBranch(t *testing.T) {
}{
{"perfectly match", "base", "base", true},
{"prefix match", "^dev/", "dev/test-bot", true},
{"prefix doesn't match", "dev/$", "dev/test-bot", false},
{"prefix doesn't match", "^/test-bot", "dev/test-bot", false},
{"suffix match", "/test-bot$", "dev/test-bot", true},
{"suffix doesn't match", "^/test-bot", "dev/test-bot", false},
{"suffix doesn't match", "dev/$", "dev/test-bot", false},
{"doesn't match", "base", "notatall", false},
} {
t.Run(testCase.name, func(t *testing.T) {
Expand Down
21 changes: 21 additions & 0 deletions contribs/github-bot/internal/conditions/draft.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
package conditions

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 _ Condition = &baseBranch{}

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

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

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()
condition := Draft()

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))
})
}
}
4 changes: 2 additions & 2 deletions contribs/github-bot/internal/conditions/file_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,9 +33,9 @@ func TestFileChanged(t *testing.T) {
{"empty file list", "foo", []*github.CommitFile{}, false},
{"file list contains exact match", "foo", filenames, true},
{"file list contains prefix match", "^fo", filenames, true},
{"file list contains prefix doesn't match", "fo$", filenames, false},
{"file list contains prefix doesn't match", "^oo", filenames, false},
{"file list contains suffix match", "oo$", filenames, true},
{"file list contains suffix doesn't match", "^oo", filenames, false},
{"file list contains suffix doesn't match", "fo$", filenames, false},
{"file list doesn't contains match", "foobar", filenames, false},
} {
t.Run(testCase.name, func(t *testing.T) {
Expand Down
4 changes: 2 additions & 2 deletions contribs/github-bot/internal/conditions/label_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,9 +29,9 @@ func TestLabel(t *testing.T) {
{"empty label list", "label", []*github.Label{}, false},
{"label list contains exact match", "label", labels, true},
{"label list contains prefix match", "^lab", labels, true},
{"label list contains prefix doesn't match", "lab$", labels, false},
{"label list contains prefix doesn't match", "^bel", labels, false},
{"label list contains suffix match", "bel$", labels, true},
{"label list contains suffix doesn't match", "^bel", labels, false},
{"label list contains suffix doesn't match", "lab$", labels, false},
{"label list doesn't contains match", "baleb", labels, false},
} {
t.Run(testCase.name, func(t *testing.T) {
Expand Down
2 changes: 1 addition & 1 deletion contribs/github-bot/internal/params/prlist.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@
prNumsStr[i] = strconv.Itoa(prNum)
}

return []byte(strings.Join(prNumsStr, ",")), nil
return []byte(strings.Join(prNumsStr, ", ")), nil

Check warning on line 26 in contribs/github-bot/internal/params/prlist.go

View check run for this annotation

Codecov / codecov/patch

contribs/github-bot/internal/params/prlist.go#L26

Added line #L26 was not covered by tests
}

// UnmarshalText implements encoding.TextUnmarshaler.
Expand Down
6 changes: 3 additions & 3 deletions contribs/github-bot/internal/requirements/assignee_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -64,9 +64,9 @@ func TestAssignee(t *testing.T) {
details := treeprint.New()
requirement := Assignee(gh, testCase.user)

assert.False(t, !requirement.IsSatisfied(pr, details) && !testCase.dryRun, "requirement should have a satisfied status: true")
assert.False(t, !utils.TestLastNodeStatus(t, true, details) && !testCase.dryRun, "requirement details should have a status: true")
assert.False(t, !testCase.exists && !requested && !testCase.dryRun, "requirement should have requested to create item")
assert.True(t, requirement.IsSatisfied(pr, details) || testCase.dryRun, "requirement should have a satisfied status: true")
assert.True(t, utils.TestLastNodeStatus(t, true, details) || testCase.dryRun, "requirement details should have a status: true")
assert.True(t, testCase.exists || requested || testCase.dryRun, "requirement should have requested to create item")
})
}
}
5 changes: 5 additions & 0 deletions contribs/github-bot/internal/requirements/branch.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,12 @@
if u.base == PR_BASE {
base = pr.GetBase().GetRef()
}

head := pr.GetHead().GetRef()
// If pull request is open from a fork, prepend head ref with fork owner login
if pr.GetHead().GetRepo().GetFullName() != pr.GetBase().GetRepo().GetFullName() {
head = fmt.Sprintf("%s:%s", pr.GetHead().GetRepo().GetOwner().GetLogin(), pr.GetHead().GetRef())
}

Check warning on line 35 in contribs/github-bot/internal/requirements/branch.go

View check run for this annotation

Codecov / codecov/patch

contribs/github-bot/internal/requirements/branch.go#L34-L35

Added lines #L34 - L35 were not covered by tests

cmp, _, err := u.gh.Client.Repositories.CompareCommits(u.gh.Ctx, u.gh.Owner, u.gh.Repo, base, head, nil)
if err != nil {
Expand Down
21 changes: 7 additions & 14 deletions contribs/github-bot/internal/requirements/label_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,17 +32,10 @@ func TestLabel(t *testing.T) {
exists bool
}{
{"empty label list", "label", []*github.Label{}, false, false},
{"empty label list with dry-run", "label", []*github.Label{}, true, false},
{"label list contains exact match", "label", labels, false, true},
{"label list contains prefix match", "^lab", labels, false, true},
{"label list contains prefix doesn't match", "lab$", labels, false, false},
{"label list contains prefix doesn't match with dry-run", "lab$", labels, true, false},
{"label list contains suffix match", "bel$", labels, false, true},
{"label list contains suffix match with dry-run", "bel$", labels, true, true},
{"label list contains suffix doesn't match", "^bel", labels, false, false},
{"label list contains suffix doesn't match with dry-run", "^bel", labels, true, false},
{"label list doesn't contains match", "baleb", labels, false, false},
{"label list doesn't contains match with dry-run", "baleb", labels, true, false},
{"empty label list with dry-run", "user", []*github.Label{}, true, false},
{"label list contains label", "label", labels, false, true},
{"label list doesn't contain label", "label2", labels, false, false},
{"label list doesn't contain label with dry-run", "label", labels, true, false},
} {
t.Run(testCase.name, func(t *testing.T) {
t.Parallel()
Expand Down Expand Up @@ -71,9 +64,9 @@ func TestLabel(t *testing.T) {
details := treeprint.New()
requirement := Label(gh, testCase.pattern)

assert.False(t, !requirement.IsSatisfied(pr, details) && !testCase.dryRun, "requirement should have a satisfied status: true")
assert.False(t, !utils.TestLastNodeStatus(t, true, details) && !testCase.dryRun, "requirement details should have a status: true")
assert.False(t, !testCase.exists && !requested && !testCase.dryRun, "requirement should have requested to create item")
assert.True(t, requirement.IsSatisfied(pr, details) || testCase.dryRun, "requirement should have a satisfied status: true")
assert.True(t, utils.TestLastNodeStatus(t, true, details) || testCase.dryRun, "requirement details should have a status: true")
assert.True(t, testCase.exists || requested || testCase.dryRun, "requirement should have requested to create item")
})
}
}
8 changes: 7 additions & 1 deletion contribs/github-bot/matrix.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,13 @@
return err
}

fmt.Println(prList)
// Print PR list for GitHub Actions matrix definition
bytes, err := prList.MarshalText()
if err != nil {
return fmt.Errorf("unable to marshal PR list: %w", err)
}
fmt.Printf("[%s]", string(bytes))

Check warning on line 57 in contribs/github-bot/matrix.go

View check run for this annotation

Codecov / codecov/patch

contribs/github-bot/matrix.go#L52-L57

Added lines #L52 - L57 were not covered by tests
return nil
}

Expand Down
Loading