Skip to content

Commit

Permalink
Added unit tests
Browse files Browse the repository at this point in the history
  • Loading branch information
melinath committed Dec 21, 2023
1 parent 1d3882d commit 8951923
Show file tree
Hide file tree
Showing 5 changed files with 175 additions and 9 deletions.
4 changes: 2 additions & 2 deletions .ci/magician/cmd/mock_github_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ type mockGithub struct {
userType github.UserType
requestedReviewers []github.User
previousReviewers []github.User
teamMembers []github.User
teamMembers map[string][]github.User
calledMethods map[string][][]any
}

Expand All @@ -48,7 +48,7 @@ func (m *mockGithub) GetPullRequestPreviousReviewers(prNumber string) ([]github.

func (m *mockGithub) GetTeamMembers(organization, team string) ([]github.User, error) {
m.calledMethods["GetTeamMembers"] = append(m.calledMethods["GetTeamMembers"], []any{organization, team})
return m.teamMembers, nil
return m.teamMembers[team], nil
}

func (m *mockGithub) RequestPullRequestReviewer(prNumber string, reviewer string) error {
Expand Down
25 changes: 18 additions & 7 deletions .ci/magician/cmd/request_service_reviewers.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ var requestServiceReviewersCmd = &cobra.Command{
If a PR has more than 3 service labels, the command will not do anything.
`,
Args: cobra.ExactArgs(6),
Args: cobra.ExactArgs(1),
Run: func(cmd *cobra.Command, args []string) {
prNumber := args[0]
fmt.Println("PR Number: ", prNumber)
Expand Down Expand Up @@ -91,13 +91,15 @@ func execRequestServiceReviewers(prNumber string, gh GithubClient, enrolledTeams

// For each service team, check if one of the team members is already a reviewer. Rerequest
// review if there is and choose a random reviewer from the list if there isn't.
reviewersMap := make(map[string]struct{})
toRequest := []string{}
requestedReviewersMap := make(map[string]struct{})
for _, reviewer := range requestedReviewers {
reviewersMap[reviewer.Login] = struct{}{}
requestedReviewersMap[reviewer.Login] = struct{}{}
}

previousReviewersMap := make(map[string]struct{})
for _, reviewer := range previousReviewers {
reviewersMap[reviewer.Login] = struct{}{}
previousReviewersMap[reviewer.Login] = struct{}{}
}

exitCode := 0
Expand All @@ -109,15 +111,24 @@ func execRequestServiceReviewers(prNumber string, gh GithubClient, enrolledTeams
continue
}
hasReviewer := false
reviewerPool := []string{}
for _, member := range members {
if _, ok := reviewersMap[member.Login]; ok {
// Exclude PR author
if member.Login != pullRequest.User.Login {
reviewerPool = append(reviewerPool, member.Login)
}
// Don't re-request review if there's an active review request
if _, ok := requestedReviewersMap[member.Login]; ok {
hasReviewer = true
}
if _, ok := previousReviewersMap[member.Login]; ok {
hasReviewer = true
toRequest = append(toRequest, member.Login)
}
}

if !hasReviewer {
toRequest = append(toRequest, members[rand.Intn(len(members))].Login)
if !hasReviewer && len(reviewerPool) > 0 {
toRequest = append(toRequest, reviewerPool[rand.Intn(len(reviewerPool))])
}
}

Expand Down
146 changes: 146 additions & 0 deletions .ci/magician/cmd/request_service_reviewers_test.go
Original file line number Diff line number Diff line change
@@ -1 +1,147 @@
package cmd

import (
"github.com/stretchr/testify/assert"
"magician/github"
"testing"
)

var enrolledTeamsYaml = []byte(`
service/google-x:
team: google-x
resources:
- google_x_resource
service/google-y:
team: google-y
resources:
- google_y_resource
service/google-z:
resources:
- google_z_resource
`)

func TestExecRequestServiceReviewersMembershipChecker_CoreContributorFlow(t *testing.T) {
cases := map[string]struct {
pullRequest github.PullRequest
requestedReviewers []string
previousReviewers []string
teamMembers map[string][]string
expectSpecificReviewers []string
}{
"no service labels means no service team reviewers": {
pullRequest: github.PullRequest{
User: github.User{Login: "googler_author"},
},
expectSpecificReviewers: []string{},
},
"unregistered service labels will not trigger review": {
pullRequest: github.PullRequest{
User: github.User{Login: "googler_author"},
Labels: []github.Label{{Name: "service/google-a"}},
},
expectSpecificReviewers: []string{},
},
"no configured team means no reviewers": {
pullRequest: github.PullRequest{
User: github.User{Login: "googler_author"},
Labels: []github.Label{{Name: "service/google-z"}},
},
expectSpecificReviewers: []string{},
},
"no previous reviewers means all reviews will be requested": {
pullRequest: github.PullRequest{
User: github.User{Login: "googler_author"},
Labels: []github.Label{{Name: "service/google-x"}},
},
teamMembers: map[string][]string{"google-x": []string{"googler_team_member"}},
expectSpecificReviewers: []string{"googler_team_member"},
},
"previous reviewers will be re-requested": {
pullRequest: github.PullRequest{
User: github.User{Login: "googler_author"},
Labels: []github.Label{{Name: "service/google-x"}},
},
previousReviewers: []string{"googler_team_member"},
teamMembers: map[string][]string{"google-x": []string{"googler_team_member", "googler_team_member_2", "googler_team_member_3", "googler_team_member_4", "googler_team_member_5"}},
expectSpecificReviewers: []string{"googler_team_member"},
},
"active reviewers will not be re-requested": {
pullRequest: github.PullRequest{
User: github.User{Login: "googler_author"},
Labels: []github.Label{{Name: "service/google-x"}},
},
requestedReviewers: []string{"googler_team_member"},
teamMembers: map[string][]string{"google-x": []string{"googler_team_member"}},
expectSpecificReviewers: []string{},
},
"authors will not be requested on their own PRs": {
pullRequest: github.PullRequest{
User: github.User{Login: "googler_team_member"},
Labels: []github.Label{{Name: "service/google-x"}},
},
teamMembers: map[string][]string{"google-x": []string{"googler_team_member"}},
expectSpecificReviewers: []string{},
},
"other team members be requested even if the author is excluded": {
pullRequest: github.PullRequest{
User: github.User{Login: "googler_team_member"},
Labels: []github.Label{{Name: "service/google-x"}},
},
teamMembers: map[string][]string{"google-x": []string{"googler_team_member", "googler_team_member_2"}},
expectSpecificReviewers: []string{"googler_team_member_2"},
},
"multiple teams can be requested at once": {
pullRequest: github.PullRequest{
User: github.User{Login: "googler_author"},
Labels: []github.Label{{Name: "service/google-x"}, {Name: "service/google-y"}, {Name: "service/google-z"}},
},
teamMembers: map[string][]string{"google-x": []string{"googler_team_member"}, "google-y": []string{"googler_y_team_member"}},
expectSpecificReviewers: []string{"googler_team_member", "googler_y_team_member"},
},
">3 service teams will not be requested": {
pullRequest: github.PullRequest{
User: github.User{Login: "googler_author"},
Labels: []github.Label{{Name: "service/google-x"}, {Name: "service/google-y"}, {Name: "service/google-z"}, {Name: "service/google-a"}},
},
teamMembers: map[string][]string{"google-x": []string{"googler_team_member"}, "google-y": []string{"googler_y_team_member"}},
expectSpecificReviewers: []string{},
},
}
for tn, tc := range cases {
t.Run(tn, func(t *testing.T) {
requestedReviewers := []github.User{}
for _, login := range tc.requestedReviewers {
requestedReviewers = append(requestedReviewers, github.User{Login: login})
}
previousReviewers := []github.User{}
for _, login := range tc.previousReviewers {
previousReviewers = append(previousReviewers, github.User{Login: login})
}
teamMembers := map[string][]github.User{}
for team, logins := range tc.teamMembers {
teamMembers[team] = []github.User{}
for _, login := range logins {
teamMembers[team] = append(teamMembers[team], github.User{Login: login})
}
}
gh := &mockGithub{
pullRequest: tc.pullRequest,
requestedReviewers: requestedReviewers,
previousReviewers: previousReviewers,
teamMembers: teamMembers,
calledMethods: make(map[string][][]any),
}

execRequestServiceReviewers("1", gh, enrolledTeamsYaml)

actualReviewers := []string{}
for _, args := range gh.calledMethods["RequestPullRequestReviewer"] {
actualReviewers = append(actualReviewers, args[1].(string))
}

if tc.expectSpecificReviewers != nil {
assert.Equal(t, tc.expectSpecificReviewers, actualReviewers)
}
})
}
}
4 changes: 4 additions & 0 deletions .ci/magician/go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -18,12 +18,15 @@ require github.com/otiai10/copy v1.12.0
require (
cloud.google.com/go/compute v1.18.0 // indirect
cloud.google.com/go/compute/metadata v0.2.3 // indirect
github.com/davecgh/go-spew v1.1.1 // indirect
github.com/golang/glog v1.1.1 // indirect
github.com/golang/groupcache v0.0.0-20210331224755-41bb18bfe9da // indirect
github.com/golang/protobuf v1.5.2 // indirect
github.com/google/uuid v1.3.0 // indirect
github.com/googleapis/enterprise-certificate-proxy v0.2.3 // indirect
github.com/googleapis/gax-go/v2 v2.7.0 // indirect
github.com/pmezard/go-difflib v1.0.0 // indirect
github.com/stretchr/testify v1.8.4 // indirect
go.opencensus.io v0.24.0 // indirect
golang.org/x/net v0.15.0 // indirect
golang.org/x/oauth2 v0.6.0 // indirect
Expand All @@ -34,4 +37,5 @@ require (
google.golang.org/grpc v1.53.0 // indirect
google.golang.org/protobuf v1.28.1 // indirect
gopkg.in/yaml.v2 v2.4.0 // indirect
gopkg.in/yaml.v3 v3.0.1 // indirect
)
5 changes: 5 additions & 0 deletions .ci/magician/go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ github.com/client9/misspell v0.3.4/go.mod h1:qj6jICC3Q7zFZvVWo7KLAzC3yx5G7kyvSDk
github.com/cncf/udpa/go v0.0.0-20191209042840-269d4d468f6f/go.mod h1:M8M6+tZqaGXZJjfX53e64911xZQV5JYwmTeXPW+k8Sc=
github.com/cpuguy83/go-md2man/v2 v2.0.2/go.mod h1:tgQtvFlXSQOSOSIRvRPT7W67SCa46tRHOmNcaadrF8o=
github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c=
github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
github.com/envoyproxy/go-control-plane v0.9.0/go.mod h1:YTl/9mNaCwkRvm6d1a2C3ymFceY/DCBVvsKhRF0iEA4=
github.com/envoyproxy/go-control-plane v0.9.1-0.20191026205805-5f8ba28d4473/go.mod h1:YTl/9mNaCwkRvm6d1a2C3ymFceY/DCBVvsKhRF0iEA4=
Expand Down Expand Up @@ -58,6 +59,7 @@ github.com/inconshreveable/mousetrap v1.1.0/go.mod h1:vpF70FUmC8bwa3OWnCshd2FqLf
github.com/otiai10/copy v1.12.0 h1:cLMgSQnXBs1eehF0Wy/FAGsgDTDmAqFR7rQylBb1nDY=
github.com/otiai10/copy v1.12.0/go.mod h1:rSaLseMUsZFFbsFGc7wCJnnkTAvdc5L6VWxPE4308Ww=
github.com/otiai10/mint v1.5.1 h1:XaPLeE+9vGbuyEHem1JNk3bYc7KKqyI/na0/mLd/Kks=
github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM=
github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4=
github.com/prometheus/client_model v0.0.0-20190812154241-14fe0d1b01d4/go.mod h1:xMI15A0UPsDsEKsMN9yxemIoYk6Tm2C1GtYGdfGttqA=
github.com/russross/blackfriday/v2 v2.1.0/go.mod h1:+Rmxgy9KzJVeS9/2gXHxylqXiyQDYRxCVz55jmeOWTM=
Expand All @@ -71,6 +73,8 @@ github.com/stretchr/objx v0.5.0/go.mod h1:Yh+to48EsGEfYuaHDzXPcE3xhTkx73EhmCGUpE
github.com/stretchr/testify v1.7.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg=
github.com/stretchr/testify v1.8.0/go.mod h1:yNjHg4UonilssWZ8iaSj1OCr/vHnekPRkoO+kdMU+MU=
github.com/stretchr/testify v1.8.1/go.mod h1:w2LPCIKwWwSfY2zedu0+kehJoqGctiVI29o6fzry7u4=
github.com/stretchr/testify v1.8.4 h1:CcVxjf3Q8PM0mHUKJCdn+eZZtm5yQwehR5yeSVQQcUk=
github.com/stretchr/testify v1.8.4/go.mod h1:sz/lmYIOXD/1dqDmKjjqLyZ2RngseejIcXlSw2iwfAo=
go.opencensus.io v0.24.0 h1:y73uSU6J157QMP2kn2r30vwW1A2W2WFwSCGnAVxeaD0=
go.opencensus.io v0.24.0/go.mod h1:vNK8G9p7aAivkbmorf4v+7Hgx+Zs0yY+0fOtgBfjQKo=
golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w=
Expand Down Expand Up @@ -150,6 +154,7 @@ gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8
gopkg.in/yaml.v2 v2.4.0 h1:D8xgwECY7CYvx+Y2n4sBz93Jn9JRvxdiyyo8CTfuKaY=
gopkg.in/yaml.v2 v2.4.0/go.mod h1:RDklbk79AGWmwhnvt/jBztapEOGDOx6ZbXqjP6csGnQ=
gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM=
gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA=
gopkg.in/yaml.v3 v3.0.1/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM=
honnef.co/go/tools v0.0.0-20190102054323-c2f93a96b099/go.mod h1:rf3lG4BRIbNafJWhAfAdb/ePZxsR/4RtNHQocxwk9r4=
honnef.co/go/tools v0.0.0-20190523083050-ea95bdfd59fc/go.mod h1:rf3lG4BRIbNafJWhAfAdb/ePZxsR/4RtNHQocxwk9r4=

0 comments on commit 8951923

Please sign in to comment.