Skip to content

Commit

Permalink
feat: improve assign reviewer command (#433)
Browse files Browse the repository at this point in the history
  • Loading branch information
zolamk authored Nov 24, 2022
1 parent 41be8ad commit cb6d90d
Showing 9 changed files with 270 additions and 96 deletions.
64 changes: 47 additions & 17 deletions engine/commands/assignReviewer.go
Original file line number Diff line number Diff line change
@@ -8,39 +8,69 @@ import (
"errors"
"fmt"
"regexp"
"strconv"
"strings"

"github.com/spf13/cobra"
)

var assignReviewerRegex = regexp.MustCompile(`^\/reviewpad assign-reviewers\s+((?:[a-zA-Z0-9\-]+[a-zA-Z0-9])(?:,\s*[a-zA-Z0-9\-]+[a-zA-Z0-9])*)(?:\s+(\d+))?(?:\s+(random|round-robin|reviewpad))?$`)
func AssignReviewerCmd() *cobra.Command {
assignReviewerCmd := &cobra.Command{
Use: "assign-reviewers",
Short: "Assign reviewers to a pull request",
Long: "Assigns a defined amount of reviewers to the pull request from the provided list of reviewers.",
SilenceUsage: true,
SilenceErrors: true,
Args: func(cmd *cobra.Command, args []string) error {
if len(args) != 1 {
return fmt.Errorf("accepts 1 arg(s), received %d", len(args))
}

func AssignReviewer(matches []string) ([]string, error) {
var err error
if !regexp.MustCompile(`^([a-zA-Z0-9\-]+[a-zA-Z0-9])(,[a-zA-Z0-9\-]+[a-zA-Z0-9])*$`).MatchString(args[0]) {
return errors.New("reviewers must be a list of comma separated valid github usernames")
}

if len(matches) < 2 {
return nil, errors.New("invalid assign reviewer command")
return nil
},
RunE: AssignReviewer,
}

reviewersList := strings.Split(strings.ReplaceAll(matches[1], " ", ""), ",")
flags := assignReviewerCmd.Flags()

flags.StringP("review-policy", "p", "reviewpad", "The policy followed for reviewer assignment. The valid values can only be: random, round-robin, reviewpad. By default, the policy is reviewpad.")

flags.Uint8P("total-reviewers", "t", 0, "The total number of reviewers to assign to. By default, it assigns to all reviewers.")

return assignReviewerCmd
}

func AssignReviewer(cmd *cobra.Command, args []string) error {
flags := cmd.Flags()

reviewersList := strings.Split(strings.ReplaceAll(args[0], " ", ""), ",")

availableReviewers := `"` + strings.Join(reviewersList, `","`) + `"`

totalRequiredReviewers := len(reviewersList)
totalRequiredReviewers, err := flags.GetUint8("total-reviewers")
if err != nil {
return err
}

if len(matches) > 2 && matches[2] != "" {
totalRequiredReviewers, err = strconv.Atoi(matches[2])
if err != nil {
return nil, err
}
if totalRequiredReviewers == 0 {
totalRequiredReviewers = uint8(len(reviewersList))
}

policy := "reviewpad"
policy, err := flags.GetString("review-policy")
if err != nil {
return err
}

if len(matches) > 3 && matches[3] != "" {
policy = matches[3]
if policy != "reviewpad" && policy != "round-robin" && policy != "random" {
return fmt.Errorf("invalid review policy specified: %s", policy)
}

action := fmt.Sprintf(`$assignReviewer([%s], %d, %q)`, availableReviewers, totalRequiredReviewers, policy)

return []string{action}, nil
cmd.Print(action)

return nil
}
118 changes: 72 additions & 46 deletions engine/commands/assignReviewer_test.go
Original file line number Diff line number Diff line change
@@ -5,8 +5,8 @@
package commands_test

import (
"bytes"
"errors"
"strconv"
"testing"

"github.com/reviewpad/reviewpad/v3/engine/commands"
@@ -15,82 +15,108 @@ import (

func TestAssignReviewer(t *testing.T) {
testCases := map[string]struct {
matches []string
wantActions []string
wantErr error
args []string
wantAction string
wantErr error
}{
"when arguments are empty": {
matches: []string{},
wantErr: errors.New("invalid assign reviewer command"),
},
"when invalid number of arguments": {
matches: []string{
"/reviewpad assign-reviewers",
args: []string{
"john",
"jane",
},
wantErr: errors.New("accepts 1 arg(s), received 2"),
},
"when list of reviewers has invalid github username": {
args: []string{
"john2,jane-",
},
wantErr: errors.New("invalid assign reviewer command"),
wantErr: errors.New("reviewers must be a list of comma separated valid github usernames"),
},
"when review policy is invalid": {
args: []string{
"jane,john",
"--total-reviewers=1",
"--review-policy=unknown",
},
wantErr: errors.New("invalid review policy specified: unknown"),
},
"when number of reviewers is not a number": {
matches: []string{
"/reviewpad assign-reviewers john, jane, john2, jane27 z random",
"john, jane, john2, jane27",
"z",
"random",
args: []string{
"john,jane,john2,jane27",
"--total-reviewers=z",
},
wantErr: &strconv.NumError{Func: "Atoi", Num: "z", Err: errors.New("invalid syntax")},
wantErr: errors.New("invalid argument \"z\" for \"-t, --total-reviewers\" flag: strconv.ParseUint: parsing \"z\": invalid syntax"),
},
"when missing number of reviewers and policy": {
matches: []string{
"/reviewpad assign-reviewers john",
args: []string{
"john",
"",
"",
},
wantActions: []string{`$assignReviewer(["john"], 1, "reviewpad")`},
wantAction: `$assignReviewer(["john"], 1, "reviewpad")`,
},
"when missing policy": {
matches: []string{
"/reviewpad assign-reviewers john-123, jane 1",
"john-123, jane",
"1",
"",
args: []string{
"john-123,jane",
"--total-reviewers=5",
},
wantActions: []string{`$assignReviewer(["john-123","jane"], 1, "reviewpad")`},
wantAction: `$assignReviewer(["john-123","jane"], 5, "reviewpad")`,
},
"when only one reviewer is provided": {
matches: []string{
"/reviewpad assign-reviewers john-123-jane 1 reviewpad",
args: []string{
"john-123-jane",
"1",
"reviewpad",
"--total-reviewers=1",
"--review-policy=reviewpad",
},
wantActions: []string{`$assignReviewer(["john-123-jane"], 1, "reviewpad")`},
wantAction: `$assignReviewer(["john-123-jane"], 1, "reviewpad")`,
},
"when only two reviewers is provided": {
matches: []string{
"/reviewpad assign-reviewers jane, john 2 random",
"jane, john",
"when only two reviewers are provided": {
args: []string{
"jane,john",
"--total-reviewers",
"2",
"--review-policy",
"random",
},
wantActions: []string{`$assignReviewer(["jane","john"], 2, "random")`},
wantAction: `$assignReviewer(["jane","john"], 2, "random")`,
},
"when number of provided reviewers is greater than requested reviewers": {
matches: []string{
"/reviewpad assign-reviewers jane, john 1 reviewpad",
"jane, john",
"1",
"reviewpad",
args: []string{
"jane,john",
"--total-reviewers=1",
"--review-policy",
"round-robin",
},
wantActions: []string{`$assignReviewer(["jane","john"], 1, "reviewpad")`},
wantAction: `$assignReviewer(["jane","john"], 1, "round-robin")`,
},
"when number of provided reviewers is less than requested reviewers": {
args: []string{
"jane,john,jane123",
"--total-reviewers",
"5",
"--review-policy=round-robin",
},
wantAction: `$assignReviewer(["jane","john","jane123"], 5, "round-robin")`,
},
"when missing number of reviewers": {
args: []string{
"jane,john,jane123",
"--review-policy=reviewpad",
},
wantAction: `$assignReviewer(["jane","john","jane123"], 3, "reviewpad")`,
},
}

for name, test := range testCases {
t.Run(name, func(t *testing.T) {
actions, err := commands.AssignReviewer(test.matches)
out := new(bytes.Buffer)
cmd := commands.AssignReviewerCmd()

cmd.SetOut(out)
cmd.SetArgs(test.args)

err := cmd.Execute()

assert.Equal(t, test.wantErr, err)
assert.Equal(t, test.wantActions, actions)
assert.Equal(t, test.wantAction, out.String())
})
}
}
32 changes: 29 additions & 3 deletions engine/commands/commands.go
Original file line number Diff line number Diff line change
@@ -5,9 +5,35 @@
package commands

import (
"regexp"
"errors"
"io"

"github.com/spf13/cobra"
)

var Commands = map[*regexp.Regexp]func(matches []string) ([]string, error){
assignReviewerRegex: AssignReviewer,
func NewCommands(out io.Writer, args []string) *cobra.Command {
root := &cobra.Command{
Use: "/reviewpad",
Hidden: true,
CompletionOptions: cobra.CompletionOptions{
DisableDefaultCmd: true,
},
SilenceUsage: true,
SilenceErrors: true,
RunE: func(cmd *cobra.Command, args []string) error {
cmd.Flags().Lookup("help").Hidden = true
return errors.New(cmd.UsageString())
},
DisableFlagParsing: true,
}

root.SetHelpCommand(&cobra.Command{Hidden: true})

root.SetOut(out)

root.SetArgs(args)

root.AddCommand(AssignReviewerCmd())

return root
}
51 changes: 38 additions & 13 deletions engine/exec.go
Original file line number Diff line number Diff line change
@@ -5,11 +5,15 @@
package engine

import (
"bytes"
"fmt"
"log"
"strings"

"github.com/google/go-github/v48/github"
"github.com/mattn/go-shellwords"
"github.com/reviewpad/reviewpad/v3/engine/commands"
"github.com/reviewpad/reviewpad/v3/utils"
"github.com/reviewpad/reviewpad/v3/utils/fmtio"
)

@@ -133,21 +137,15 @@ func Eval(file *ReviewpadFile, env *Env) (*Program, error) {
program := BuildProgram(make([]*Statement, 0))

// process commands
if env.EventData != nil && env.EventData.EventName == "issue_comment" {
for r, command := range commands.Commands {
matches := r.FindAllStringSubmatch(*env.EventData.Comment.Body, 1)

if len(matches) == 1 {
actions, err := command(matches[0])
if err != nil {
return nil, err
}
if utils.IsReviewpadCommand(env.EventData) {
action, err := processCommand(env, *env.EventData.Comment.Body)
if err != nil {
return nil, err
}

program.append(actions)
program.append([]string{action})

return program, nil
}
}
return program, nil
}

// process rules
@@ -257,3 +255,30 @@ func Eval(file *ReviewpadFile, env *Env) (*Program, error) {

return program, nil
}

func processCommand(env *Env, comment string) (string, error) {
out := new(bytes.Buffer)
comment = strings.TrimPrefix(comment, "/reviewpad")

args, err := shellwords.Parse(comment)
if err != nil {
return "", err
}

root := commands.NewCommands(out, args)

err = root.Execute()
if err != nil {
comment := fmt.Sprintf("%s\n```\n🚫 error\n\n%s\n```", *env.EventData.Comment.Body, err.Error())

if _, _, err := env.GithubClient.EditComment(env.Ctx, env.TargetEntity.Owner, env.TargetEntity.Repo, *env.EventData.Comment.ID, &github.IssueComment{
Body: github.String(comment),
}); err != nil {
return "", err
}

return "", err
}

return out.String(), nil
}
Loading

0 comments on commit cb6d90d

Please sign in to comment.