diff --git a/contribs/github-bot/README.md b/contribs/github-bot/README.md index 5f1bcd19098..1a15c461787 100644 --- a/contribs/github-bot/README.md +++ b/contribs/github-bot/README.md @@ -11,21 +11,18 @@ The GitHub Bot is designed to automate and streamline the process of managing pu The bot operates by defining a set of rules that are evaluated against each pull request passed as parameter. These rules are categorized into automatic and manual checks: - **Automatic Checks**: These are rules that the bot evaluates automatically. If a pull request meets the conditions specified in the rule, then the corresponding requirements are exectued. For example, ensuring that changes to specific directories are reviewed by specific team members. -- **Manual Checks**: These require human intervention. If a pull request meets the conditions specified in the rule, then a checkbox that can be checked only by specified teams is displayed on the bot comment. For example, determining if infrastructure needs to be updated based on changes in specific files. +- **Manual Checks**: These require human intervention. If a pull request meets the conditions specified in the rule, then a checkbox that can be checked only by specified teams is displayed on the bot comment. For example, determining if infrastructure needs to be updated based on changes to specific files. The bot configuration is defined in Go and is located in the file [config.go](./config.go). -### Conditions - -// TODO - -### Requirements - -// TODO - ### GitHub Token -// TODO +For the bot to make requests to the GitHub API, it needs a Personal Access Token. The fine-grained permissions to assign to the token for the bot to function are: + +- `pull_requests` scope to read is the bare minimum to run the bot in dry-run mode +- `pull_requests` scope to write to be able to update bot comment, assign user, apply label and request review +- `contents` scope to read to be able to check if the head branch is up to date with another one +- `commit_statuses` scope to write to be able to update pull request bot status check ## Usage diff --git a/contribs/github-bot/requirement/reviewer.go b/contribs/github-bot/requirement/reviewer.go index 3fcb7c1afa9..b45432bc042 100644 --- a/contribs/github-bot/requirement/reviewer.go +++ b/contribs/github-bot/requirement/reviewer.go @@ -111,9 +111,8 @@ func (r *reviewByTeamMembers) IsSatisfied(pr *github.PullRequest, details treepr // Check how many members of this team already approved this PR approved := uint(0) - members := r.gh.ListTeamMembers(r.team) for _, review := range r.gh.ListPrReviews(pr.GetNumber()) { - for _, member := range members { + for _, member := range r.gh.ListTeamMembers(r.team) { if review.GetUser().GetLogin() == member.GetLogin() { if review.GetState() == "APPROVED" { approved += 1 diff --git a/contribs/github-bot/requirement/reviewer_test.go b/contribs/github-bot/requirement/reviewer_test.go index 8327dec17d4..f45fbb9c3cc 100644 --- a/contribs/github-bot/requirement/reviewer_test.go +++ b/contribs/github-bot/requirement/reviewer_test.go @@ -2,6 +2,7 @@ package requirement import ( "context" + "fmt" "net/http" "testing" @@ -103,5 +104,123 @@ func TestReviewByUser(t *testing.T) { func TestReviewByTeamMembers(t *testing.T) { t.Parallel() - // TODO + reviewers := github.Reviewers{ + Teams: []*github.Team{ + {Slug: github.String("team1")}, + {Slug: github.String("team2")}, + {Slug: github.String("team3")}, + }, + } + + members := map[string][]*github.User{ + "team1": { + {Login: github.String("user1")}, + {Login: github.String("user2")}, + {Login: github.String("user3")}, + }, + "team2": { + {Login: github.String("user3")}, + {Login: github.String("user4")}, + {Login: github.String("user5")}, + }, + "team3": { + {Login: github.String("user4")}, + {Login: github.String("user5")}, + }, + } + + reviews := []*github.PullRequestReview{ + { + User: &github.User{Login: github.String("user1")}, + State: github.String("APPROVED"), + }, { + User: &github.User{Login: github.String("user2")}, + State: github.String("APPROVED"), + }, { + User: &github.User{Login: github.String("user3")}, + State: github.String("APPROVED"), + }, { + User: &github.User{Login: github.String("user4")}, + State: github.String("REQUEST_CHANGES"), + }, { + User: &github.User{Login: github.String("user5")}, + State: github.String("REQUEST_CHANGES"), + }, + } + + for _, testCase := range []struct { + name string + team string + count uint + isSatisfied bool + testRequest bool + }{ + {"3/3 team members approved;", "team1", 3, true, false}, + {"1/1 team member approved", "team2", 1, true, false}, + {"1/2 team member approved", "team2", 2, false, false}, + {"0/1 team member approved", "team3", 1, false, false}, + {"0/1 team member approved", "team3", 1, false, true}, + {"team doesn't exist", "team4", 1, false, true}, + } { + t.Run(testCase.name, func(t *testing.T) { + t.Parallel() + + firstRequest := true + requested := false + mockedHTTPClient := mock.NewMockedHTTPClient( + mock.WithRequestMatchHandler( + mock.EndpointPattern{ + Pattern: "/repos/pulls/0/requested_reviewers", + Method: "GET", + }, + http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + if firstRequest { + if testCase.testRequest { + w.Write(mock.MustMarshal(github.Reviewers{})) + } else { + w.Write(mock.MustMarshal(reviewers)) + } + firstRequest = false + } else { + requested = true + } + }), + ), + mock.WithRequestMatchPages( + mock.EndpointPattern{ + Pattern: fmt.Sprintf("/orgs/teams/%s/members", testCase.team), + Method: "GET", + }, + members[testCase.team], + ), + mock.WithRequestMatchPages( + mock.EndpointPattern{ + Pattern: "/repos/pulls/0/reviews", + Method: "GET", + }, + reviews, + ), + ) + + gh := &client.GitHub{ + Client: github.NewClient(mockedHTTPClient), + Ctx: context.Background(), + Logger: logger.NewNoopLogger(), + } + + pr := &github.PullRequest{} + details := treeprint.New() + requirement := ReviewByTeamMembers(gh, testCase.team, testCase.count) + + if requirement.IsSatisfied(pr, details) != testCase.isSatisfied { + t.Errorf("requirement should have a satisfied status: %t", testCase.isSatisfied) + } + if !utils.TestLastNodeStatus(t, testCase.isSatisfied, details) { + t.Errorf("requirement details should have a status: %t", testCase.isSatisfied) + } + if testCase.testRequest != requested { + t.Errorf("requirement should have requested to create item: %t", testCase.testRequest) + } + }) + } }