Repository: jenkins-x/lighthouse
Pull request #1045 (2020-09-11 12:54:20)

refactor: assign plugin
This PR refactors assign plugin.

jenkins-x-bot: [APPROVALNOTIFIER] This PR is **NOT APPROVED**

This pull-request has been approved by:
To complete the [pull request process](https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process), please assign **eddycharly**
You can assign the PR to them by writing `/assign @eddycharly` in a comment when ready.

The full list of commands accepted by this bot can be found [here](https://go.k8s.io/bot-commands?repo=jenkins-x%2Flighthouse).

<details open>
Needs approval from an approver in each of these files:

- **[OWNERS](https://github.com/jenkins-x/lighthouse/blob/master/OWNERS)**

Approvers can indicate their approval by writing `/approve` in a comment
Approvers can cancel approval by writing `/approve cancel` in a comment
</details>
<!-- META={"approvers":["eddycharly"]} -->
eddycharly: /hold

must come after https://github.com/jenkins-x/lighthouse/pull/1044
jenkins-x-bot: @eddycharly: The following test **failed**, say `/retest` to rerun them all:

Test name | Commit | Details | Rerun command
--- | --- | --- | ---
bbs | e5a733b75299cf553a6e2c15a9ef4bfbd360a6e5 | [link](https://dashboard-jx.jenkins-x.live/teams/jx/projects/jenkins-x/lighthouse/PR-1045/9) | `/test bbs`

[View all Builds for this Pull Request](https://dashboard-jx.jenkins-x.live/teams/jx/projects/jenkins-x/lighthouse/PR-1045) 

<details>

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md).  If you have questions or suggestions related to my behavior, please file an issue against the [jenkins-x/lighthouse](https://github.com/jenkins-x/lighthouse/issues/new?title=Command%20issue:) repository. I understand the commands that are listed [here](https://go.k8s.io/bot-commands).
</details>
<!-- test report -->
jenkins-x-bot: @eddycharly: The following tests **failed**, say `/retest` to rerun them all:

Test name | Commit | Details | Rerun command
--- | --- | --- | ---
bbs | e5a733b75299cf553a6e2c15a9ef4bfbd360a6e5 | [link](https://dashboard-jx.jenkins-x.live/teams/jx/projects/jenkins-x/lighthouse/PR-1045/9) | `/test bbs`
tekton | e5a733b75299cf553a6e2c15a9ef4bfbd360a6e5 | [link](https://dashboard-jx.jenkins-x.live/teams/jx/projects/jenkins-x/lighthouse/PR-1045/12) | `/test tekton`
gitlab | e5a733b75299cf553a6e2c15a9ef4bfbd360a6e5 | [link](https://dashboard-jx.jenkins-x.live/teams/jx/projects/jenkins-x/lighthouse/PR-1045/10) | `/test gitlab`

[View all Builds for this Pull Request](https://dashboard-jx.jenkins-x.live/teams/jx/projects/jenkins-x/lighthouse/PR-1045) 

<details>

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md).  If you have questions or suggestions related to my behavior, please file an issue against the [jenkins-x/lighthouse](https://github.com/jenkins-x/lighthouse/issues/new?title=Command%20issue:) repository. I understand the commands that are listed [here](https://go.k8s.io/bot-commands).
</details>
<!-- test report -->
jenkins-x-bot: @eddycharly: The following tests **failed**, say `/retest` to rerun them all:

Test name | Commit | Details | Rerun command
--- | --- | --- | ---
bbs | e5a733b75299cf553a6e2c15a9ef4bfbd360a6e5 | [link](https://dashboard-jx.jenkins-x.live/teams/jx/projects/jenkins-x/lighthouse/PR-1045/9) | `/test bbs`
tekton | e5a733b75299cf553a6e2c15a9ef4bfbd360a6e5 | [link](https://dashboard-jx.jenkins-x.live/teams/jx/projects/jenkins-x/lighthouse/PR-1045/12) | `/test tekton`
gitlab | e5a733b75299cf553a6e2c15a9ef4bfbd360a6e5 | [link](https://dashboard-jx.jenkins-x.live/teams/jx/projects/jenkins-x/lighthouse/PR-1045/10) | `/test gitlab`
github | e5a733b75299cf553a6e2c15a9ef4bfbd360a6e5 | [link](https://dashboard-jx.jenkins-x.live/teams/jx/projects/jenkins-x/lighthouse/PR-1045/11) | `/test github`
ghe | e5a733b75299cf553a6e2c15a9ef4bfbd360a6e5 | [link](https://dashboard-jx.jenkins-x.live/teams/jx/projects/jenkins-x/lighthouse/PR-1045/7) | `/test ghe`

[View all Builds for this Pull Request](https://dashboard-jx.jenkins-x.live/teams/jx/projects/jenkins-x/lighthouse/PR-1045) 

<details>

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md).  If you have questions or suggestions related to my behavior, please file an issue against the [jenkins-x/lighthouse](https://github.com/jenkins-x/lighthouse/issues/new?title=Command%20issue:) repository. I understand the commands that are listed [here](https://go.k8s.io/bot-commands).
</details>
<!-- test report -->
eddycharly: /hold cancel
jenkins-x-bot: @eddycharly: The following test **failed**, say `/retest` to rerun them all:

Test name | Commit | Details | Rerun command
--- | --- | --- | ---
pr-build | 02b7e65b37a8b33ad50bf6e7e378d5f7e5c2a524 | [link](https://dashboard-jx.jenkins-x.live/teams/jx/projects/jenkins-x/lighthouse/PR-1045/19) | `/test pr-build`

[View all Builds for this Pull Request](https://dashboard-jx.jenkins-x.live/teams/jx/projects/jenkins-x/lighthouse/PR-1045) 

<details>

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md).  If you have questions or suggestions related to my behavior, please file an issue against the [jenkins-x/lighthouse](https://github.com/jenkins-x/lighthouse/issues/new?title=Command%20issue:) repository. I understand the commands that are listed [here](https://go.k8s.io/bot-commands).
</details>
<!-- test report -->
abayer: /lgtm
/retest
jenkins-x-bot: [APPROVALNOTIFIER] This PR is **APPROVED**

This pull-request has been approved by: *[abayer](https://github.com/jenkins-x/lighthouse/pull/1045#issuecomment-691140658 "LGTM")*

The full list of commands accepted by this bot can be found [here](https://go.k8s.io/bot-commands?repo=jenkins-x%2Flighthouse).

The pull request process is described [here](https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process)

<details >
Needs approval from an approver in each of these files:

- ~~[OWNERS](https://github.com#issuecomment-691140658/jenkins-x/lighthouse/blob/master/OWNERS)~~ [abayer]

Approvers can indicate their approval by writing `/approve` in a comment
Approvers can cancel approval by writing `/approve cancel` in a comment
</details>
<!-- META={"approvers":[]} -->

Patch:
diff --git a/pkg/plugins/assign/assign.go b/pkg/plugins/assign/assign.go
index 56d6ed94..f59e6e63 100644
--- a/pkg/plugins/assign/assign.go
+++ b/pkg/plugins/assign/assign.go
@@ -32,17 +32,18 @@ import (
 const pluginName = "assign"
 
 var (
-	assignRe = regexp.MustCompile(`(?mi)^/(?:lh-)?(un)?assign(( @?(?:")?[-\w]+?)*(?:")?)\s*$`)
-	// CCRegexp parses and validates /cc commands, also used by blunderbuss
-	CCRegexp = regexp.MustCompile(`(?mi)^/(?:lh-)?(un)?cc(( +@?(?:")?[-/\w]+?)*(?:")?)\s*$`)
+	match = regexp.MustCompile(`(?mi)^/(?:lh-)?(un)?(cc|assign)(( @?(?:")?[-/\w]+?)*(?:")?)\s*$`)
 )
 
 var (
 	plugin = plugins.Plugin{
 		Description: "The assign plugin assigns or requests reviews from users. Specific users can be assigned with the command '/assign @user1' or have reviews requested of them with the command '/cc @user1'. If no user is specified the commands default to targeting the user who created the command. Assignments and requested reviews can be removed in the same way that they are added by prefixing the commands with 'un'.",
 		Commands: []plugins.Command{{
-			GenericCommentHandler: handleGenericComment,
-			Filter:                func(e scmprovider.GenericCommentEvent) bool { return e.Action == scm.ActionCreate },
+			Filter: func(e scmprovider.GenericCommentEvent) bool { return e.Action == scm.ActionCreate },
+			Regex:  match,
+			GenericCommentHandler: func(match []string, pc plugins.Agent, e scmprovider.GenericCommentEvent) error {
+				return handleGenericComment(match[1] != "un", match[2], match[3], pc, e)
+			},
 			Help: []pluginhelp.Command{{
 				Usage:       "/[un]assign [[@]<username>...]",
 				Description: "Assigns an assignee to the PR",
@@ -75,10 +76,10 @@ type scmProviderClient interface {
 	QuoteAuthorForComment(string) string
 }
 
-func handleGenericComment(_ []string, pc plugins.Agent, e scmprovider.GenericCommentEvent) error {
-	err := handle(newAssignHandler(e, pc.SCMProviderClient, pc.Logger))
+func handleGenericComment(add bool, command string, arg string, pc plugins.Agent, e scmprovider.GenericCommentEvent) error {
+	err := handle(add, command, arg, newAssignHandler(e, pc.SCMProviderClient, pc.Logger))
 	if e.IsPR {
-		err = combineErrors(err, handle(newReviewHandler(e, pc.SCMProviderClient, pc.Logger)))
+		err = combineErrors(err, handle(add, command, arg, newReviewHandler(e, pc.SCMProviderClient, pc.Logger)))
 	}
 	return err
 }
@@ -109,25 +110,22 @@ func combineErrors(err1, err2 error) error {
 // functions to identify the users to add and/or remove and then passes the appropriate users to the
 // handler's add and remove functions. If add fails to add some of the users, a response comment is
 // created where the body of the response is generated by the handler's addFailureResponse function.
-func handle(h *handler) error {
+func handle(add bool, command string, arg string, h *handler) error {
 	e := h.event
 	org := e.Repo.Namespace
 	repo := e.Repo.Name
-	matches := h.regexp.FindAllStringSubmatch(e.Body, -1)
-	if matches == nil {
+	if command != h.command {
 		return nil
 	}
 	users := make(map[string]bool)
-	for _, re := range matches {
-		add := re[1] != "un" // un<cmd> == !add
-		if re[2] == "" {
-			users[e.Author.Login] = add
-		} else {
-			for _, login := range parseLogins(re[2]) {
-				users[login] = add
-			}
+	if arg == "" {
+		users[e.Author.Login] = add
+	} else {
+		for _, login := range parseLogins(arg) {
+			users[login] = add
 		}
 	}
+
 	var toAdd, toRemove []string
 	for login, add := range users {
 		if add {
@@ -174,9 +172,8 @@ type handler struct {
 
 	// event is a pointer to the gitprovider.GenericCommentEvent struct that triggered the handler.
 	event *scmprovider.GenericCommentEvent
-	// regexp is the regular expression describing the command. It must have an optional 'un' prefix
-	// as the first subgroup and the arguments to the command as the second subgroup.
-	regexp *regexp.Regexp
+	// command is the name of the command to be executed (assign or cc).
+	command string
 	// spc is the scmProviderClient to use for creating response comments in the event of a failure.
 	spc scmProviderClient
 
@@ -197,7 +194,7 @@ func newAssignHandler(e scmprovider.GenericCommentEvent, spc scmProviderClient,
 		remove:             spc.UnassignIssue,
 		add:                spc.AssignIssue,
 		event:              &e,
-		regexp:             assignRe,
+		command:            "assign",
 		spc:                spc,
 		log:                log,
 		userType:           "assignee(s)",
@@ -215,7 +212,7 @@ func newReviewHandler(e scmprovider.GenericCommentEvent, spc scmProviderClient,
 		remove:             spc.UnrequestReview,
 		add:                spc.RequestReview,
 		event:              &e,
-		regexp:             CCRegexp,
+		command:            "cc",
 		spc:                spc,
 		log:                log,
 		userType:           "reviewer(s)",


Test patch:
diff --git a/pkg/plugins/assign/assign_test.go b/pkg/plugins/assign/assign_test.go
index ff6ef178..0acd6a82 100644
--- a/pkg/plugins/assign/assign_test.go
+++ b/pkg/plugins/assign/assign_test.go
@@ -413,16 +413,24 @@ func TestAssignAndReview(t *testing.T) {
 		t.Run(tc.name, func(t *testing.T) {
 			fc := newFakeClient([]string{"hello-world", "allow_underscore", "cjwagner", "merlin", "kubernetes/sig-testing-misc"})
 			e := scmprovider.GenericCommentEvent{
+				Action: scm.ActionCreate,
 				Body:   tc.body,
 				Author: scm.User{Login: tc.commenter},
 				Repo:   scm.Repository{Name: "repo", Namespace: "org"},
 				Number: 5,
 			}
-			if err := handle(newAssignHandler(e, fc, logrus.WithField("plugin", pluginName))); err != nil {
-				t.Fatalf("For case %s, didn't expect error from handle: %v", tc.name, err)
+			cmd := plugin.Commands[0]
+			matches, err := cmd.GetMatches(&e)
+			if err != nil {
+				t.Fatalf("(%s): Unexpected error from handle: %v.", tc.name, err)
 			}
-			if err := handle(newReviewHandler(e, fc, logrus.WithField("plugin", pluginName))); err != nil {
-				t.Fatalf("For case %s, didn't expect error from handle: %v", tc.name, err)
+			for _, m := range matches {
+				if err := handle(m[1] != "un", m[2], m[3], newAssignHandler(e, fc, logrus.WithField("plugin", pluginName))); err != nil {
+					t.Fatalf("For case %s, didn't expect error from handle: %v", tc.name, err)
+				}
+				if err := handle(m[1] != "un", m[2], m[3], newReviewHandler(e, fc, logrus.WithField("plugin", pluginName))); err != nil {
+					t.Fatalf("For case %s, didn't expect error from handle: %v", tc.name, err)
+				}
 			}
 
 			if tc.commented != fc.commented {