You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@pulsar.apache.org by GitBox <gi...@apache.org> on 2022/08/29 11:33:30 UTC
[GitHub] [pulsar-test-infra] nodece opened a new pull request, #69: Refactor docbot and add tests
nodece opened a new pull request, #69:
URL: https://github.com/apache/pulsar-test-infra/pull/69
### Motivation
1. Fix #59
2. Add tests to cover this feature
### Verify
I verified this on my repo, please help verify the following case on your repo if you have time, thanks!
1. Checked single checkbox
2. Checked multiple checkboxes
3. Unchecked any checkboxes, waited for CI to have done, then checked one checkbox
Workflow:
```
name: Documentation Bot
on:
pull_request_target :
types:
- opened
- edited
- labeled
- unlabeled
concurrency:
group: ${{ github.workflow }}-${{ github.ref }}-${{ github.event.number }}
cancel-in-progress: true
jobs:
label:
permissions:
pull-requests: write
runs-on: ubuntu-latest
steps:
- name: Labeling
uses: nodece/pulsar-test-infra/docbot@master
env:
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
LABEL_WATCH_LIST: 'doc,doc-required,doc-not-needed,doc-complete'
LABEL_MISSING: 'doc-label-missing'
```
You also need to setting your `Workflow permissions`, see https://github.com/$username/$repo/settings/actions.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar-test-infra] nodece commented on pull request #69: Refactor docbot and add tests
Posted by GitBox <gi...@apache.org>.
nodece commented on PR #69:
URL: https://github.com/apache/pulsar-test-infra/pull/69#issuecomment-1230166815
Covert to draft for testing.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar-test-infra] tisonkun commented on pull request #69: Refactor docbot and add tests
Posted by GitBox <gi...@apache.org>.
tisonkun commented on PR #69:
URL: https://github.com/apache/pulsar-test-infra/pull/69#issuecomment-1236972369
@maxsxu Aha, I see your point. @nodece doesn't describe the changes he made despite of refactoring.
I read the new code file by file and confirm that the functionality changes are included as this https://github.com/apache/pulsar-test-infra/issues/59#issuecomment-1216452240 described. That's handle all actions in the same way. The remaining is semantically identical refactoring and some tests.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar-test-infra] nodece commented on a diff in pull request #69: Refactor docbot and add tests
Posted by GitBox <gi...@apache.org>.
nodece commented on code in PR #69:
URL: https://github.com/apache/pulsar-test-infra/pull/69#discussion_r962760646
##########
docbot/action_test.go:
##########
@@ -0,0 +1,326 @@
+package main
+
+import (
+ "context"
+ "fmt"
+ "os"
+ "strings"
+ "testing"
+
+ "github.com/google/go-github/v45/github"
+ "github.com/migueleliasweb/go-github-mock/src/mock"
+)
+
+func repoLabels() []*github.Label {
+ labels := []string{"doc-required", "doc-not-needed", "doc", "doc-complete", "doc-label-missing"}
+
+ result := make([]*github.Label, 0)
+ for _, label := range labels {
+ name := label
+ result = append(result, &github.Label{Name: &name})
+ }
+
+ return result
+}
+
+func mustNewActionConfig() *ActionConfig {
+ _ = os.Setenv("GITHUB_REPOSITORY", "apache/pulsar")
+ _ = os.Setenv("LABEL_WATCH_LIST", "doc,doc-required,doc-not-needed,doc-complete")
+ _ = os.Setenv("LABEL_MISSING", "doc-label-missing")
+
+ config, err := NewActionConfig()
+ if err != nil {
+ panic(err)
+ }
+
+ return config
+}
+
+func assertMessageLabel(t *testing.T, err error, message string) {
+ t.Helper()
+
+ if err == nil {
+ t.Fatal("Expect err not nil")
+ }
+
+ if err.Error() != message {
+ t.Fatal("Expect err equals " + message)
+ }
+}
+
+func TestSingleChecked(t *testing.T) {
+ id := int64(1)
+ body := fmt.Sprintf(`
+Check the box below or label this PR directly.
+
+Need to update docs?
+
+- [ ] %s
+(Your PR needs to update docs and you will update later)
+
+- [x] %s
+(Please explain why)
+
+- [ ] %s
+(Your PR contains doc changes)
+
+- [ ] %s
+(Docs have been already added)
+`, "`doc-required`", "`doc-not-needed`", "`doc`", "`doc-complete`")
+
+ mockedHTTPClient := mock.NewMockedHTTPClient(
+ mock.WithRequestMatch(
+ mock.GetReposPullsByOwnerByRepoByPullNumber,
+ github.PullRequest{
+ ID: &id,
+ Body: &body,
+ Labels: nil,
+ },
+ ), mock.WithRequestMatch(
+ mock.GetReposLabelsByOwnerByRepo,
+ repoLabels(),
+ ),
+ mock.WithRequestMatch(mock.PostReposIssuesLabelsByOwnerByRepoByIssueNumber, nil),
+ )
+
+ config := mustNewActionConfig()
+ action := NewActionWithClient(context.Background(), config, github.NewClient(mockedHTTPClient))
+
+ err := action.Run(1, "opened")
+ if err != nil {
+ t.Fatal(err)
+ }
+}
+
+func TestMultipleChecked(t *testing.T) {
+ id := int64(1)
+ body := fmt.Sprintf(`
+Check the box below or label this PR directly.
+
+Need to update docs?
+
+- [ ] %s
+(Your PR needs to update docs and you will update later)
+
+- [x] %s
+(Please explain why)
+
+- [x] %s
+(Your PR contains doc changes)
+
+- [ ] %s
+(Docs have been already added)
+`, "`doc-required`", "`doc-not-needed`", "`doc`", "`doc-complete`")
+
+ mockedHTTPClient := mock.NewMockedHTTPClient(
+ mock.WithRequestMatch(
+ mock.GetReposPullsByOwnerByRepoByPullNumber,
+ github.PullRequest{
+ ID: &id,
+ Body: &body,
+ Labels: nil,
+ },
+ ), mock.WithRequestMatch(
+ mock.GetReposLabelsByOwnerByRepo,
+ repoLabels(),
+ ),
+ mock.WithRequestMatch(mock.PostReposIssuesLabelsByOwnerByRepoByIssueNumber, nil),
+ )
+
+ const key = "ENABLE_LABEL_MULTIPLE"
+ value := os.Getenv(key)
+ defer func() {
+ // reset
+ _ = os.Setenv(key, value)
+ }()
+ _ = os.Setenv("ENABLE_LABEL_MULTIPLE", "true")
+
+ config := mustNewActionConfig()
+ action := NewActionWithClient(context.Background(), config, github.NewClient(mockedHTTPClient))
+
+ err := action.Run(1, "opened")
+ if err != nil {
+ t.Fatal(err)
+ }
+}
+
+func TestUnchecked(t *testing.T) {
+ id := int64(1)
+ body := fmt.Sprintf(`
+Check the box below or label this PR directly.
+
+Need to update docs?
+
+- [ ] %s
+(Your PR needs to update docs and you will update later)
+
+- [ ] %s
+(Please explain why)
+
+- [ ] %s
+(Your PR contains doc changes)
+
+- [ ] %s
+(Docs have been already added)
+`, "`doc-required`", "`doc-not-needed`", "`doc`", "`doc-complete`")
+
+ mockedHTTPClient := mock.NewMockedHTTPClient(
+ mock.WithRequestMatch(
+ mock.GetReposPullsByOwnerByRepoByPullNumber,
+ github.PullRequest{
+ ID: &id,
+ Body: &body,
+ Labels: nil,
+ },
+ ), mock.WithRequestMatch(
+ mock.GetReposLabelsByOwnerByRepo,
+ repoLabels(),
+ ),
+ mock.WithRequestMatch(mock.PostReposIssuesLabelsByOwnerByRepoByIssueNumber, nil),
+ mock.WithRequestMatch(mock.GetReposIssuesCommentsByOwnerByRepoByIssueNumber, nil),
+ mock.WithRequestMatch(mock.PostReposIssuesCommentsByOwnerByRepoByIssueNumber, nil),
+ )
+
+ config := mustNewActionConfig()
+ action := NewActionWithClient(context.Background(), config, github.NewClient(mockedHTTPClient))
+
+ err := action.Run(1, "opened")
+ assertMessageLabel(t, err, MessageLabelMissing)
+}
+
+func TestMultipleChecked_WhenMultipleLabelsNotEnabled(t *testing.T) {
+ id := int64(1)
+ body := fmt.Sprintf(`
+Check the box below or label this PR directly.
+
+Need to update docs?
+
+- [ ] %s
+(Your PR needs to update docs and you will update later)
+
+- [x] %s
+(Please explain why)
+
+- [x] %s
+(Your PR contains doc changes)
+
+- [ ] %s
+(Docs have been already added)
+`, "`doc-required`", "`doc-not-needed`", "`doc`", "`doc-complete`")
+
+ mockedHTTPClient := mock.NewMockedHTTPClient(
+ mock.WithRequestMatch(
+ mock.GetReposPullsByOwnerByRepoByPullNumber,
+ github.PullRequest{
+ ID: &id,
+ Body: &body,
+ Labels: nil,
+ },
+ ), mock.WithRequestMatch(
+ mock.GetReposLabelsByOwnerByRepo,
+ repoLabels(),
+ ),
+ mock.WithRequestMatch(mock.PostReposIssuesLabelsByOwnerByRepoByIssueNumber, nil),
+ mock.WithRequestMatch(mock.GetReposIssuesCommentsByOwnerByRepoByIssueNumber, nil),
+ mock.WithRequestMatch(mock.PostReposIssuesCommentsByOwnerByRepoByIssueNumber, nil),
+ )
+
+ config := mustNewActionConfig()
+ action := NewActionWithClient(context.Background(), config, github.NewClient(mockedHTTPClient))
+
+ err := action.Run(1, "opened")
+ assertMessageLabel(t, err, MessageLabelMultiple)
+}
+
+func TestSingleChecked_WhenLabelMissingExist(t *testing.T) {
+ id := int64(1)
+ body := fmt.Sprintf(`
+Check the box below or label this PR directly.
+
+Need to update docs?
+
+- [ ] %s
+(Your PR needs to update docs and you will update later)
+
+- [x] %s
+(Please explain why)
+
+- [ ] %s
+(Your PR contains doc changes)
+
+- [ ] %s
+(Docs have been already added)
+`, "`doc-required`", "`doc-not-needed`", "`doc`", "`doc-complete`")
+
+ labelMissing := "doc-label-missing"
+ mockedHTTPClient := mock.NewMockedHTTPClient(
+ mock.WithRequestMatch(
+ mock.GetReposPullsByOwnerByRepoByPullNumber,
+ github.PullRequest{
+ ID: &id,
+ Body: &body,
+ Labels: []*github.Label{{Name: &labelMissing}},
+ },
+ ), mock.WithRequestMatch(
+ mock.GetReposLabelsByOwnerByRepo,
+ repoLabels(),
+ ),
+ mock.WithRequestMatch(mock.PostReposIssuesLabelsByOwnerByRepoByIssueNumber, nil),
+ mock.WithRequestMatch(mock.DeleteReposIssuesLabelsByOwnerByRepoByIssueNumberByName, nil),
+ )
+
+ config := mustNewActionConfig()
+ action := NewActionWithClient(context.Background(), config, github.NewClient(mockedHTTPClient))
+
+ err := action.Run(1, "opened")
+ if err != nil {
+ t.Fatal(err)
+ }
+}
+
+func TestUnchecked_WhenLabelMissingExist(t *testing.T) {
+ id := int64(1)
+ body := fmt.Sprintf(`
+Check the box below or label this PR directly.
+
+Need to update docs?
+
+- [ ] %s
+(Your PR needs to update docs and you will update later)
+
+- [ ] %s
+(Please explain why)
+
+- [ ] %s
+(Your PR contains doc changes)
+
+- [ ] %s
+(Docs have been already added)
+`, "`doc-required`", "`doc-not-needed`", "`doc`", "`doc-complete`")
+
+ labelMissing := "doc-label-missing"
+ mockedHTTPClient := mock.NewMockedHTTPClient(
+ mock.WithRequestMatch(
+ mock.GetReposPullsByOwnerByRepoByPullNumber,
+ github.PullRequest{
+ ID: &id,
+ Body: &body,
+ Labels: []*github.Label{{Name: &labelMissing}},
+ },
+ ), mock.WithRequestMatch(
+ mock.GetReposLabelsByOwnerByRepo,
+ repoLabels(),
+ ),
+ )
+
+ config := mustNewActionConfig()
+ action := NewActionWithClient(context.Background(), config, github.NewClient(mockedHTTPClient))
+
+ err := action.Run(1, "opened")
+ assertMessageLabel(t, err, MessageLabelMissing)
+}
+
+func TestName(t *testing.T) {
+ split := strings.Split("docs, docs-required,", ",")
+ t.Log(split)
+}
Review Comment:
Removed
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar-test-infra] tisonkun commented on a diff in pull request #69: Refactor docbot and add tests
Posted by GitBox <gi...@apache.org>.
tisonkun commented on code in PR #69:
URL: https://github.com/apache/pulsar-test-infra/pull/69#discussion_r962756759
##########
docbot/action_test.go:
##########
@@ -0,0 +1,320 @@
+package main
+
+import (
Review Comment:
Make sense. IIRC Golang will download test dependencies even in production env :(
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar-test-infra] tisonkun commented on a diff in pull request #69: Refactor docbot and add tests
Posted by GitBox <gi...@apache.org>.
tisonkun commented on code in PR #69:
URL: https://github.com/apache/pulsar-test-infra/pull/69#discussion_r962618120
##########
docbot/action_config.go:
##########
@@ -0,0 +1,119 @@
+package main
+
+import (
+ "fmt"
+ "os"
+ "strings"
+)
+
+type ActionConfig struct {
+ token *string
+ repo *string
+ owner *string
+
+ labelPattern *string
+ labelWatchSet map[string]struct{}
+ labelMissing *string
+ enableLabelMissing *bool
+ enableLabelMultiple *bool
+}
+
+func NewActionConfig() (*ActionConfig, error) {
+ ownerRepoSlug := os.Getenv("GITHUB_REPOSITORY")
+ ownerRepo := strings.Split(ownerRepoSlug, "/")
+ if len(ownerRepo) != 2 {
+ return nil, fmt.Errorf("GITHUB_REPOSITORY is not found")
+ }
+ owner, repo := ownerRepo[0], ownerRepo[1]
+
+ token := os.Getenv("GITHUB_TOKEN")
+
+ labelPattern := os.Getenv("LABEL_PATTERN")
+ if len(labelPattern) == 0 {
+ labelPattern = "- \\[(.*?)\\] ?`(.+?)`"
+ }
+
+ labelWatchListSlug := os.Getenv("LABEL_WATCH_LIST")
+ labelWatchList := strings.Split(strings.TrimSpace(labelWatchListSlug), ",")
+ labelWatchSet := make(map[string]struct{})
+ for _, l := range labelWatchList {
+ labelWatchSet[l] = struct{}{}
Review Comment:
```suggestion
if r := strings.TrimSpace(l); len(r) > 0 {
labelWatchSet[r] = struct{}{}
}
```
For `LABEL_WATCH_LIST == 'docs, docs-required,'`.
##########
docbot/action.go:
##########
@@ -0,0 +1,346 @@
+package main
+
+import (
+ "context"
+ "errors"
+ "fmt"
+ "regexp"
+ "strings"
+
+ "github.com/apache/pulsar-test-infra/docbot/pkg/logger"
+ "github.com/google/go-github/v45/github"
+ "golang.org/x/oauth2"
+)
+
+const (
+ MessageLabelMissing = `Please provide a correct documentation label for your PR.
+Instructions see [Pulsar Documentation Label Guide](https://docs.google.com/document/d/1Qw7LHQdXWBW9t2-r-A7QdFDBwmZh6ytB4guwMoXHqc0).`
+ MessageLabelMultiple = `Please select only one documentation label for your PR.
+Instructions see [Pulsar Documentation Label Guide](https://docs.google.com/document/d/1Qw7LHQdXWBW9t2-r-A7QdFDBwmZh6ytB4guwMoXHqc0).`
+)
+
+type Action struct {
+ config *ActionConfig
+
+ globalContext context.Context
+ client *github.Client
+
+ prNumber int
+}
+
+func NewAction(ac *ActionConfig) *Action {
+ ctx := context.Background()
+ ts := oauth2.StaticTokenSource(
+ &oauth2.Token{AccessToken: ac.GetToken()},
+ )
+
+ tc := oauth2.NewClient(ctx, ts)
+
+ return NewActionWithClient(ctx, ac, github.NewClient(tc))
+}
+
+func NewActionWithClient(ctx context.Context, ac *ActionConfig, client *github.Client) *Action {
+ return &Action{
+ config: ac,
+ globalContext: ctx,
+ client: client,
+ }
+}
+
+func (a *Action) Run(prNumber int, actionType string) error {
+ a.prNumber = prNumber
+
+ switch actionType {
+ case "opened", "edited", "labeled", "unlabeled":
+ return a.labeling()
+ }
+ return nil
+}
+
+func (a *Action) labeling() error {
+ pr, _, err := a.client.PullRequests.Get(a.globalContext, a.config.GetOwner(), a.config.GetRepo(), a.prNumber)
+ if err != nil {
+ return fmt.Errorf("get PR: %v", err)
+ }
+
+ var bodyLabels map[string]bool
+ if pr.Body != nil {
+ bodyLabels = a.extractLabels(*pr.Body)
+ }
+
+ logger.Infoln("@List repo labels")
+ repoLabels, err := a.getRepoLabels()
+ if err != nil {
+ return fmt.Errorf("list repo labels: %v", err)
+ }
+ logger.Infof("Repo labels: %v\n", repoLabels)
+
+ prLabels := a.labelsToMap(pr.Labels)
+ logger.Infof("PR labels: %v\n", prLabels)
+
+ // Get expected labels
+ // Only handle labels already exist in repo
+ expectedLabelsMap := make(map[string]bool)
+ checkedCount := 0
+ for label, checked := range bodyLabels {
+ if _, exist := repoLabels[label]; !exist {
+ logger.Infof("Found label %v not exist int repo\n", label)
+ continue
+ }
+ expectedLabelsMap[label] = checked
+ if checked {
+ checkedCount++
+ }
+ }
+ logger.Infof("Expected labels: %v\n", expectedLabelsMap)
+
+ labelsToRemove := make(map[string]struct{}, 0)
+ labelsToAdd := make(map[string]struct{}, 0)
+
+ if checkedCount == 0 {
+ logger.Infoln("Label missing")
+ for label := range a.config.labelWatchSet {
+ _, found := prLabels[label]
+ if found {
+ labelsToRemove[label] = struct{}{}
+ }
+ }
+ _, found := prLabels[a.config.GetLabelMissing()]
+ if !found {
+ labelsToAdd[a.config.GetLabelMissing()] = struct{}{}
+ } else {
+ logger.Infoln("Already added missing label.")
+ return errors.New(MessageLabelMissing)
+ }
+ } else {
+ if !a.config.GetEnableLabelMultiple() && checkedCount > 1 {
+ logger.Infoln("Multiple labels not enabled")
+ err = a.AddAndCleanupHelpComment(pr.User.GetLogin(), MessageLabelMultiple)
+ if err != nil {
+ return err
+ }
+ return errors.New(MessageLabelMultiple)
+ }
+
+ _, found := prLabels[a.config.GetLabelMissing()]
+ if found {
+ labelsToRemove[a.config.GetLabelMissing()] = struct{}{}
+ }
+
+ for label, checked := range expectedLabelsMap {
+ _, found := prLabels[label]
+ if found {
+ continue
+ }
+ if checked {
+ labelsToAdd[label] = struct{}{}
+ }
+ }
+ }
+
+ if len(labelsToAdd) == 0 {
+ logger.Infoln("No labels to add.")
+ } else {
+ labels := a.labelsSetToString(labelsToAdd)
+ logger.Infof("Labels to add: %v\n", labels)
+ err = a.AddLabels(labels)
+ if err != nil {
+ logger.Errorf("Failed add labels %v: %v\n", labelsToAdd, err)
+ return err
+ }
+ }
+
+ if len(labelsToRemove) == 0 {
+ logger.Infoln("No labels to remove.")
+ } else {
+ labels := a.labelsSetToString(labelsToRemove)
+ logger.Infof("Labels to remove: %v\n", labels)
+ for _, label := range labels {
+ err = a.RemoveLabel(label)
+ if err != nil {
+ logger.Errorf("Failed remove labels %v: %v\n", labelsToRemove, err)
+ return err
+ }
+ }
+ }
+
+ if checkedCount == 0 {
+ err := a.AddAndCleanupHelpComment(pr.User.GetLogin(), MessageLabelMissing)
+ if err != nil {
+ return err
+ }
+ return errors.New(MessageLabelMissing)
+ }
+
+ return nil
+}
+
+func (a *Action) extractLabels(prBody string) map[string]bool {
+ r := regexp.MustCompile(a.config.GetLabelPattern())
+ targets := r.FindAllStringSubmatch(prBody, -1)
+ labels := make(map[string]bool)
+
+ //// Init labels from watch list
+ //for label := range a.config.labelWatchSet {
+ // labels[label] = false
+ //}
Review Comment:
```suggestion
```
##########
docbot/action.go:
##########
@@ -0,0 +1,346 @@
+package main
+
+import (
+ "context"
+ "errors"
+ "fmt"
+ "regexp"
+ "strings"
+
+ "github.com/apache/pulsar-test-infra/docbot/pkg/logger"
+ "github.com/google/go-github/v45/github"
+ "golang.org/x/oauth2"
+)
+
+const (
+ MessageLabelMissing = `Please provide a correct documentation label for your PR.
+Instructions see [Pulsar Documentation Label Guide](https://docs.google.com/document/d/1Qw7LHQdXWBW9t2-r-A7QdFDBwmZh6ytB4guwMoXHqc0).`
+ MessageLabelMultiple = `Please select only one documentation label for your PR.
+Instructions see [Pulsar Documentation Label Guide](https://docs.google.com/document/d/1Qw7LHQdXWBW9t2-r-A7QdFDBwmZh6ytB4guwMoXHqc0).`
+)
+
+type Action struct {
+ config *ActionConfig
+
+ globalContext context.Context
+ client *github.Client
+
+ prNumber int
+}
+
+func NewAction(ac *ActionConfig) *Action {
+ ctx := context.Background()
+ ts := oauth2.StaticTokenSource(
+ &oauth2.Token{AccessToken: ac.GetToken()},
+ )
+
+ tc := oauth2.NewClient(ctx, ts)
+
+ return NewActionWithClient(ctx, ac, github.NewClient(tc))
+}
+
+func NewActionWithClient(ctx context.Context, ac *ActionConfig, client *github.Client) *Action {
+ return &Action{
+ config: ac,
+ globalContext: ctx,
+ client: client,
+ }
+}
+
+func (a *Action) Run(prNumber int, actionType string) error {
+ a.prNumber = prNumber
+
+ switch actionType {
+ case "opened", "edited", "labeled", "unlabeled":
+ return a.labeling()
+ }
+ return nil
+}
+
+func (a *Action) labeling() error {
+ pr, _, err := a.client.PullRequests.Get(a.globalContext, a.config.GetOwner(), a.config.GetRepo(), a.prNumber)
+ if err != nil {
+ return fmt.Errorf("get PR: %v", err)
+ }
+
+ var bodyLabels map[string]bool
+ if pr.Body != nil {
+ bodyLabels = a.extractLabels(*pr.Body)
+ }
+
+ logger.Infoln("@List repo labels")
+ repoLabels, err := a.getRepoLabels()
+ if err != nil {
+ return fmt.Errorf("list repo labels: %v", err)
+ }
+ logger.Infof("Repo labels: %v\n", repoLabels)
+
+ prLabels := a.labelsToMap(pr.Labels)
+ logger.Infof("PR labels: %v\n", prLabels)
+
+ // Get expected labels
+ // Only handle labels already exist in repo
+ expectedLabelsMap := make(map[string]bool)
+ checkedCount := 0
+ for label, checked := range bodyLabels {
+ if _, exist := repoLabels[label]; !exist {
+ logger.Infof("Found label %v not exist int repo\n", label)
+ continue
+ }
+ expectedLabelsMap[label] = checked
+ if checked {
+ checkedCount++
+ }
+ }
+ logger.Infof("Expected labels: %v\n", expectedLabelsMap)
+
+ labelsToRemove := make(map[string]struct{}, 0)
+ labelsToAdd := make(map[string]struct{}, 0)
+
+ if checkedCount == 0 {
+ logger.Infoln("Label missing")
+ for label := range a.config.labelWatchSet {
+ _, found := prLabels[label]
+ if found {
+ labelsToRemove[label] = struct{}{}
+ }
+ }
+ _, found := prLabels[a.config.GetLabelMissing()]
+ if !found {
+ labelsToAdd[a.config.GetLabelMissing()] = struct{}{}
+ } else {
+ logger.Infoln("Already added missing label.")
+ return errors.New(MessageLabelMissing)
+ }
+ } else {
+ if !a.config.GetEnableLabelMultiple() && checkedCount > 1 {
+ logger.Infoln("Multiple labels not enabled")
+ err = a.AddAndCleanupHelpComment(pr.User.GetLogin(), MessageLabelMultiple)
+ if err != nil {
+ return err
+ }
+ return errors.New(MessageLabelMultiple)
+ }
+
+ _, found := prLabels[a.config.GetLabelMissing()]
+ if found {
+ labelsToRemove[a.config.GetLabelMissing()] = struct{}{}
+ }
+
+ for label, checked := range expectedLabelsMap {
+ _, found := prLabels[label]
+ if found {
+ continue
+ }
+ if checked {
+ labelsToAdd[label] = struct{}{}
+ }
+ }
+ }
+
+ if len(labelsToAdd) == 0 {
+ logger.Infoln("No labels to add.")
+ } else {
+ labels := a.labelsSetToString(labelsToAdd)
+ logger.Infof("Labels to add: %v\n", labels)
+ err = a.AddLabels(labels)
+ if err != nil {
+ logger.Errorf("Failed add labels %v: %v\n", labelsToAdd, err)
+ return err
+ }
+ }
+
+ if len(labelsToRemove) == 0 {
+ logger.Infoln("No labels to remove.")
+ } else {
+ labels := a.labelsSetToString(labelsToRemove)
+ logger.Infof("Labels to remove: %v\n", labels)
+ for _, label := range labels {
+ err = a.RemoveLabel(label)
+ if err != nil {
+ logger.Errorf("Failed remove labels %v: %v\n", labelsToRemove, err)
+ return err
+ }
+ }
+ }
+
+ if checkedCount == 0 {
+ err := a.AddAndCleanupHelpComment(pr.User.GetLogin(), MessageLabelMissing)
+ if err != nil {
+ return err
+ }
+ return errors.New(MessageLabelMissing)
+ }
+
+ return nil
+}
+
+func (a *Action) extractLabels(prBody string) map[string]bool {
+ r := regexp.MustCompile(a.config.GetLabelPattern())
+ targets := r.FindAllStringSubmatch(prBody, -1)
+ labels := make(map[string]bool)
+
+ //// Init labels from watch list
+ //for label := range a.config.labelWatchSet {
+ // labels[label] = false
+ //}
+
+ for _, v := range targets {
+ checked := strings.ToLower(strings.TrimSpace(v[1])) == "x"
+ name := strings.TrimSpace(v[2])
+
+ // Filter uninterested labels
+ if _, exist := a.config.labelWatchSet[name]; !exist {
+ continue
+ }
+
+ labels[name] = checked
+ }
+
+ return labels
+}
+
+func (a *Action) getRepoLabels() (map[string]struct{}, error) {
+ ctx := context.Background()
+ listOptions := &github.ListOptions{PerPage: 100}
+ repoLabels := make(map[string]struct{}, 0)
+ for {
+ rLabels, resp, err := a.client.Issues.ListLabels(ctx, a.config.GetOwner(), a.config.GetRepo(), listOptions)
+ if err != nil {
+ return nil, err
+ }
+
+ for _, label := range rLabels {
+ repoLabels[label.GetName()] = struct{}{}
+ }
+ if resp.NextPage == 0 {
+ break
+ }
+ listOptions.Page = resp.NextPage
+ }
+ return repoLabels, nil
+}
+
+func (a *Action) getPRLabels() ([]*github.Label, error) {
+ ctx := context.Background()
+ listOptions := &github.ListOptions{PerPage: 100}
+ issueLabels := make([]*github.Label, 0)
+ for {
+ iLabels, resp, err := a.client.Issues.ListLabelsByIssue(ctx, a.config.GetOwner(), a.config.GetRepo(),
+ a.prNumber, listOptions)
+ if err != nil {
+ return nil, err
+ }
+ issueLabels = append(issueLabels, iLabels...)
+ if resp.NextPage == 0 {
+ break
+ }
+ listOptions.Page = resp.NextPage
+ }
+ return issueLabels, nil
+}
+
+func (a *Action) labelsToMap(labels []*github.Label) map[string]struct{} {
+ result := make(map[string]struct{}, 0)
+ for _, label := range labels {
+ result[label.GetName()] = struct{}{}
+ }
+ return result
+}
+
+func (a *Action) labelsSetToString(labels map[string]struct{}) []string {
+ result := []string{}
+ for label := range labels {
+ result = append(result, label)
+ }
+ return result
+}
+
+func (a *Action) GetLabelInvalidCommentIDs(body string) ([]int64, error) {
+ ctx := context.Background()
+ listOptions := &github.IssueListCommentsOptions{}
+ listOptions.PerPage = 100
+ commentIDs := make([]int64, 0)
+ for {
+ comments, resp, err := a.client.Issues.ListComments(ctx, a.config.GetOwner(), a.config.GetRepo(),
+ a.prNumber, listOptions)
+ if err != nil {
+ return nil, err
+ }
+ for _, item := range comments {
+ if strings.Contains(*item.Body, body) {
+ commentIDs = append(commentIDs, *item.ID)
+ }
+ }
+
+ if resp.NextPage == 0 {
+ break
+ }
+ listOptions.Page = resp.NextPage
+ }
+
+ return commentIDs, nil
+}
+
+func (a *Action) CreateComment(body string) error {
+ _, _, err := a.client.Issues.CreateComment(a.globalContext, a.config.GetOwner(), a.config.GetRepo(),
+ a.prNumber, &github.IssueComment{Body: func(v string) *string { return &v }(body)})
+ return err
+}
+
+func (a *Action) EditComment(commentID int64, body string) error {
+ _, _, err := a.client.Issues.EditComment(a.globalContext, a.config.GetOwner(), a.config.GetRepo(),
+ commentID, &github.IssueComment{Body: func(v string) *string { return &v }(body)})
+ return err
+}
Review Comment:
```suggestion
```
Unused.
##########
docbot/action.go:
##########
@@ -0,0 +1,346 @@
+package main
+
+import (
+ "context"
+ "errors"
+ "fmt"
+ "regexp"
+ "strings"
+
+ "github.com/apache/pulsar-test-infra/docbot/pkg/logger"
+ "github.com/google/go-github/v45/github"
+ "golang.org/x/oauth2"
+)
+
+const (
+ MessageLabelMissing = `Please provide a correct documentation label for your PR.
+Instructions see [Pulsar Documentation Label Guide](https://docs.google.com/document/d/1Qw7LHQdXWBW9t2-r-A7QdFDBwmZh6ytB4guwMoXHqc0).`
+ MessageLabelMultiple = `Please select only one documentation label for your PR.
+Instructions see [Pulsar Documentation Label Guide](https://docs.google.com/document/d/1Qw7LHQdXWBW9t2-r-A7QdFDBwmZh6ytB4guwMoXHqc0).`
+)
+
+type Action struct {
+ config *ActionConfig
+
+ globalContext context.Context
+ client *github.Client
+
+ prNumber int
+}
+
+func NewAction(ac *ActionConfig) *Action {
+ ctx := context.Background()
+ ts := oauth2.StaticTokenSource(
+ &oauth2.Token{AccessToken: ac.GetToken()},
+ )
+
+ tc := oauth2.NewClient(ctx, ts)
+
+ return NewActionWithClient(ctx, ac, github.NewClient(tc))
+}
+
+func NewActionWithClient(ctx context.Context, ac *ActionConfig, client *github.Client) *Action {
+ return &Action{
+ config: ac,
+ globalContext: ctx,
+ client: client,
+ }
+}
+
+func (a *Action) Run(prNumber int, actionType string) error {
+ a.prNumber = prNumber
+
+ switch actionType {
+ case "opened", "edited", "labeled", "unlabeled":
+ return a.labeling()
+ }
+ return nil
+}
+
+func (a *Action) labeling() error {
Review Comment:
```suggestion
func (a *Action) checkLabels() error {
```
##########
docbot/action_test.go:
##########
@@ -0,0 +1,320 @@
+package main
+
+import (
Review Comment:
I suggest use [testify](https://github.com/stretchr/testify) to simplify assertions.
##########
docbot/action.go:
##########
@@ -0,0 +1,346 @@
+package main
+
+import (
+ "context"
+ "errors"
+ "fmt"
+ "regexp"
+ "strings"
+
+ "github.com/apache/pulsar-test-infra/docbot/pkg/logger"
+ "github.com/google/go-github/v45/github"
+ "golang.org/x/oauth2"
+)
+
+const (
+ MessageLabelMissing = `Please provide a correct documentation label for your PR.
+Instructions see [Pulsar Documentation Label Guide](https://docs.google.com/document/d/1Qw7LHQdXWBW9t2-r-A7QdFDBwmZh6ytB4guwMoXHqc0).`
+ MessageLabelMultiple = `Please select only one documentation label for your PR.
+Instructions see [Pulsar Documentation Label Guide](https://docs.google.com/document/d/1Qw7LHQdXWBW9t2-r-A7QdFDBwmZh6ytB4guwMoXHqc0).`
+)
+
+type Action struct {
+ config *ActionConfig
+
+ globalContext context.Context
+ client *github.Client
+
+ prNumber int
+}
+
+func NewAction(ac *ActionConfig) *Action {
+ ctx := context.Background()
+ ts := oauth2.StaticTokenSource(
+ &oauth2.Token{AccessToken: ac.GetToken()},
+ )
+
+ tc := oauth2.NewClient(ctx, ts)
+
+ return NewActionWithClient(ctx, ac, github.NewClient(tc))
+}
+
+func NewActionWithClient(ctx context.Context, ac *ActionConfig, client *github.Client) *Action {
+ return &Action{
+ config: ac,
+ globalContext: ctx,
+ client: client,
+ }
+}
+
+func (a *Action) Run(prNumber int, actionType string) error {
+ a.prNumber = prNumber
+
+ switch actionType {
+ case "opened", "edited", "labeled", "unlabeled":
+ return a.labeling()
+ }
+ return nil
+}
+
+func (a *Action) labeling() error {
+ pr, _, err := a.client.PullRequests.Get(a.globalContext, a.config.GetOwner(), a.config.GetRepo(), a.prNumber)
+ if err != nil {
+ return fmt.Errorf("get PR: %v", err)
+ }
+
+ var bodyLabels map[string]bool
+ if pr.Body != nil {
+ bodyLabels = a.extractLabels(*pr.Body)
+ }
+
+ logger.Infoln("@List repo labels")
+ repoLabels, err := a.getRepoLabels()
+ if err != nil {
+ return fmt.Errorf("list repo labels: %v", err)
+ }
+ logger.Infof("Repo labels: %v\n", repoLabels)
+
+ prLabels := a.labelsToMap(pr.Labels)
+ logger.Infof("PR labels: %v\n", prLabels)
+
+ // Get expected labels
+ // Only handle labels already exist in repo
+ expectedLabelsMap := make(map[string]bool)
+ checkedCount := 0
+ for label, checked := range bodyLabels {
+ if _, exist := repoLabels[label]; !exist {
+ logger.Infof("Found label %v not exist int repo\n", label)
+ continue
+ }
+ expectedLabelsMap[label] = checked
+ if checked {
+ checkedCount++
+ }
+ }
+ logger.Infof("Expected labels: %v\n", expectedLabelsMap)
+
+ labelsToRemove := make(map[string]struct{}, 0)
+ labelsToAdd := make(map[string]struct{}, 0)
+
+ if checkedCount == 0 {
+ logger.Infoln("Label missing")
+ for label := range a.config.labelWatchSet {
+ _, found := prLabels[label]
+ if found {
+ labelsToRemove[label] = struct{}{}
+ }
+ }
+ _, found := prLabels[a.config.GetLabelMissing()]
+ if !found {
+ labelsToAdd[a.config.GetLabelMissing()] = struct{}{}
+ } else {
+ logger.Infoln("Already added missing label.")
+ return errors.New(MessageLabelMissing)
+ }
+ } else {
+ if !a.config.GetEnableLabelMultiple() && checkedCount > 1 {
+ logger.Infoln("Multiple labels not enabled")
+ err = a.AddAndCleanupHelpComment(pr.User.GetLogin(), MessageLabelMultiple)
+ if err != nil {
+ return err
+ }
+ return errors.New(MessageLabelMultiple)
+ }
+
+ _, found := prLabels[a.config.GetLabelMissing()]
+ if found {
+ labelsToRemove[a.config.GetLabelMissing()] = struct{}{}
+ }
+
+ for label, checked := range expectedLabelsMap {
+ _, found := prLabels[label]
+ if found {
+ continue
+ }
+ if checked {
+ labelsToAdd[label] = struct{}{}
+ }
+ }
+ }
+
+ if len(labelsToAdd) == 0 {
+ logger.Infoln("No labels to add.")
+ } else {
+ labels := a.labelsSetToString(labelsToAdd)
+ logger.Infof("Labels to add: %v\n", labels)
+ err = a.AddLabels(labels)
+ if err != nil {
+ logger.Errorf("Failed add labels %v: %v\n", labelsToAdd, err)
+ return err
+ }
+ }
+
+ if len(labelsToRemove) == 0 {
+ logger.Infoln("No labels to remove.")
+ } else {
+ labels := a.labelsSetToString(labelsToRemove)
+ logger.Infof("Labels to remove: %v\n", labels)
+ for _, label := range labels {
+ err = a.RemoveLabel(label)
+ if err != nil {
+ logger.Errorf("Failed remove labels %v: %v\n", labelsToRemove, err)
+ return err
+ }
+ }
+ }
+
+ if checkedCount == 0 {
+ err := a.AddAndCleanupHelpComment(pr.User.GetLogin(), MessageLabelMissing)
+ if err != nil {
+ return err
+ }
+ return errors.New(MessageLabelMissing)
+ }
+
+ return nil
+}
+
+func (a *Action) extractLabels(prBody string) map[string]bool {
+ r := regexp.MustCompile(a.config.GetLabelPattern())
+ targets := r.FindAllStringSubmatch(prBody, -1)
+ labels := make(map[string]bool)
+
+ //// Init labels from watch list
+ //for label := range a.config.labelWatchSet {
+ // labels[label] = false
+ //}
+
+ for _, v := range targets {
+ checked := strings.ToLower(strings.TrimSpace(v[1])) == "x"
+ name := strings.TrimSpace(v[2])
+
+ // Filter uninterested labels
+ if _, exist := a.config.labelWatchSet[name]; !exist {
+ continue
+ }
+
+ labels[name] = checked
+ }
+
+ return labels
+}
+
+func (a *Action) getRepoLabels() (map[string]struct{}, error) {
+ ctx := context.Background()
+ listOptions := &github.ListOptions{PerPage: 100}
+ repoLabels := make(map[string]struct{}, 0)
+ for {
+ rLabels, resp, err := a.client.Issues.ListLabels(ctx, a.config.GetOwner(), a.config.GetRepo(), listOptions)
+ if err != nil {
+ return nil, err
+ }
+
+ for _, label := range rLabels {
+ repoLabels[label.GetName()] = struct{}{}
+ }
+ if resp.NextPage == 0 {
+ break
+ }
+ listOptions.Page = resp.NextPage
+ }
+ return repoLabels, nil
+}
+
+func (a *Action) getPRLabels() ([]*github.Label, error) {
+ ctx := context.Background()
+ listOptions := &github.ListOptions{PerPage: 100}
+ issueLabels := make([]*github.Label, 0)
+ for {
+ iLabels, resp, err := a.client.Issues.ListLabelsByIssue(ctx, a.config.GetOwner(), a.config.GetRepo(),
+ a.prNumber, listOptions)
+ if err != nil {
+ return nil, err
+ }
+ issueLabels = append(issueLabels, iLabels...)
+ if resp.NextPage == 0 {
+ break
+ }
+ listOptions.Page = resp.NextPage
+ }
+ return issueLabels, nil
+}
+
Review Comment:
```suggestion
```
Unused.
##########
docbot/action.go:
##########
@@ -0,0 +1,346 @@
+package main
+
+import (
+ "context"
+ "errors"
+ "fmt"
+ "regexp"
+ "strings"
+
+ "github.com/apache/pulsar-test-infra/docbot/pkg/logger"
+ "github.com/google/go-github/v45/github"
+ "golang.org/x/oauth2"
+)
+
+const (
+ MessageLabelMissing = `Please provide a correct documentation label for your PR.
+Instructions see [Pulsar Documentation Label Guide](https://docs.google.com/document/d/1Qw7LHQdXWBW9t2-r-A7QdFDBwmZh6ytB4guwMoXHqc0).`
+ MessageLabelMultiple = `Please select only one documentation label for your PR.
+Instructions see [Pulsar Documentation Label Guide](https://docs.google.com/document/d/1Qw7LHQdXWBW9t2-r-A7QdFDBwmZh6ytB4guwMoXHqc0).`
+)
+
+type Action struct {
+ config *ActionConfig
+
+ globalContext context.Context
+ client *github.Client
+
+ prNumber int
+}
+
+func NewAction(ac *ActionConfig) *Action {
+ ctx := context.Background()
+ ts := oauth2.StaticTokenSource(
+ &oauth2.Token{AccessToken: ac.GetToken()},
+ )
+
+ tc := oauth2.NewClient(ctx, ts)
+
+ return NewActionWithClient(ctx, ac, github.NewClient(tc))
+}
+
+func NewActionWithClient(ctx context.Context, ac *ActionConfig, client *github.Client) *Action {
+ return &Action{
+ config: ac,
+ globalContext: ctx,
+ client: client,
+ }
+}
+
+func (a *Action) Run(prNumber int, actionType string) error {
+ a.prNumber = prNumber
+
+ switch actionType {
+ case "opened", "edited", "labeled", "unlabeled":
+ return a.labeling()
+ }
+ return nil
+}
+
+func (a *Action) labeling() error {
+ pr, _, err := a.client.PullRequests.Get(a.globalContext, a.config.GetOwner(), a.config.GetRepo(), a.prNumber)
+ if err != nil {
+ return fmt.Errorf("get PR: %v", err)
+ }
+
+ var bodyLabels map[string]bool
+ if pr.Body != nil {
+ bodyLabels = a.extractLabels(*pr.Body)
+ }
+
+ logger.Infoln("@List repo labels")
+ repoLabels, err := a.getRepoLabels()
+ if err != nil {
+ return fmt.Errorf("list repo labels: %v", err)
+ }
+ logger.Infof("Repo labels: %v\n", repoLabels)
+
+ prLabels := a.labelsToMap(pr.Labels)
+ logger.Infof("PR labels: %v\n", prLabels)
+
+ // Get expected labels
+ // Only handle labels already exist in repo
+ expectedLabelsMap := make(map[string]bool)
+ checkedCount := 0
+ for label, checked := range bodyLabels {
+ if _, exist := repoLabels[label]; !exist {
+ logger.Infof("Found label %v not exist int repo\n", label)
+ continue
+ }
+ expectedLabelsMap[label] = checked
+ if checked {
+ checkedCount++
+ }
+ }
+ logger.Infof("Expected labels: %v\n", expectedLabelsMap)
+
+ labelsToRemove := make(map[string]struct{}, 0)
+ labelsToAdd := make(map[string]struct{}, 0)
+
+ if checkedCount == 0 {
+ logger.Infoln("Label missing")
+ for label := range a.config.labelWatchSet {
+ _, found := prLabels[label]
+ if found {
+ labelsToRemove[label] = struct{}{}
+ }
+ }
+ _, found := prLabels[a.config.GetLabelMissing()]
+ if !found {
+ labelsToAdd[a.config.GetLabelMissing()] = struct{}{}
+ } else {
+ logger.Infoln("Already added missing label.")
+ return errors.New(MessageLabelMissing)
+ }
+ } else {
+ if !a.config.GetEnableLabelMultiple() && checkedCount > 1 {
+ logger.Infoln("Multiple labels not enabled")
+ err = a.AddAndCleanupHelpComment(pr.User.GetLogin(), MessageLabelMultiple)
+ if err != nil {
+ return err
+ }
+ return errors.New(MessageLabelMultiple)
+ }
+
+ _, found := prLabels[a.config.GetLabelMissing()]
+ if found {
+ labelsToRemove[a.config.GetLabelMissing()] = struct{}{}
+ }
+
+ for label, checked := range expectedLabelsMap {
+ _, found := prLabels[label]
+ if found {
+ continue
+ }
+ if checked {
+ labelsToAdd[label] = struct{}{}
+ }
+ }
+ }
+
+ if len(labelsToAdd) == 0 {
+ logger.Infoln("No labels to add.")
+ } else {
+ labels := a.labelsSetToString(labelsToAdd)
+ logger.Infof("Labels to add: %v\n", labels)
+ err = a.AddLabels(labels)
+ if err != nil {
+ logger.Errorf("Failed add labels %v: %v\n", labelsToAdd, err)
+ return err
+ }
+ }
+
+ if len(labelsToRemove) == 0 {
+ logger.Infoln("No labels to remove.")
+ } else {
+ labels := a.labelsSetToString(labelsToRemove)
+ logger.Infof("Labels to remove: %v\n", labels)
+ for _, label := range labels {
+ err = a.RemoveLabel(label)
+ if err != nil {
+ logger.Errorf("Failed remove labels %v: %v\n", labelsToRemove, err)
+ return err
+ }
+ }
+ }
+
+ if checkedCount == 0 {
+ err := a.AddAndCleanupHelpComment(pr.User.GetLogin(), MessageLabelMissing)
+ if err != nil {
+ return err
+ }
+ return errors.New(MessageLabelMissing)
+ }
+
+ return nil
+}
+
+func (a *Action) extractLabels(prBody string) map[string]bool {
+ r := regexp.MustCompile(a.config.GetLabelPattern())
+ targets := r.FindAllStringSubmatch(prBody, -1)
+ labels := make(map[string]bool)
+
+ //// Init labels from watch list
+ //for label := range a.config.labelWatchSet {
+ // labels[label] = false
+ //}
+
+ for _, v := range targets {
+ checked := strings.ToLower(strings.TrimSpace(v[1])) == "x"
+ name := strings.TrimSpace(v[2])
+
+ // Filter uninterested labels
+ if _, exist := a.config.labelWatchSet[name]; !exist {
+ continue
+ }
+
+ labels[name] = checked
+ }
+
+ return labels
+}
+
+func (a *Action) getRepoLabels() (map[string]struct{}, error) {
+ ctx := context.Background()
+ listOptions := &github.ListOptions{PerPage: 100}
+ repoLabels := make(map[string]struct{}, 0)
+ for {
+ rLabels, resp, err := a.client.Issues.ListLabels(ctx, a.config.GetOwner(), a.config.GetRepo(), listOptions)
+ if err != nil {
+ return nil, err
+ }
+
+ for _, label := range rLabels {
+ repoLabels[label.GetName()] = struct{}{}
+ }
+ if resp.NextPage == 0 {
+ break
+ }
+ listOptions.Page = resp.NextPage
+ }
+ return repoLabels, nil
+}
+
+func (a *Action) getPRLabels() ([]*github.Label, error) {
+ ctx := context.Background()
+ listOptions := &github.ListOptions{PerPage: 100}
+ issueLabels := make([]*github.Label, 0)
+ for {
+ iLabels, resp, err := a.client.Issues.ListLabelsByIssue(ctx, a.config.GetOwner(), a.config.GetRepo(),
+ a.prNumber, listOptions)
+ if err != nil {
+ return nil, err
+ }
+ issueLabels = append(issueLabels, iLabels...)
+ if resp.NextPage == 0 {
+ break
+ }
+ listOptions.Page = resp.NextPage
+ }
+ return issueLabels, nil
+}
+
+func (a *Action) labelsToMap(labels []*github.Label) map[string]struct{} {
+ result := make(map[string]struct{}, 0)
+ for _, label := range labels {
+ result[label.GetName()] = struct{}{}
+ }
+ return result
+}
+
+func (a *Action) labelsSetToString(labels map[string]struct{}) []string {
+ result := []string{}
+ for label := range labels {
+ result = append(result, label)
+ }
+ return result
+}
+
+func (a *Action) GetLabelInvalidCommentIDs(body string) ([]int64, error) {
Review Comment:
```suggestion
func (a *Action) getLabelInvalidCommentIDs(body string) ([]int64, error) {
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar-test-infra] nodece commented on a diff in pull request #69: Refactor docbot and add tests
Posted by GitBox <gi...@apache.org>.
nodece commented on code in PR #69:
URL: https://github.com/apache/pulsar-test-infra/pull/69#discussion_r962750783
##########
docbot/action_test.go:
##########
@@ -0,0 +1,320 @@
+package main
+
+import (
Review Comment:
Yes, it is simple. To reduce external dependencies, I did not use this.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar-test-infra] nodece commented on pull request #69: Refactor docbot and add tests
Posted by GitBox <gi...@apache.org>.
nodece commented on PR #69:
URL: https://github.com/apache/pulsar-test-infra/pull/69#issuecomment-1230164873
Hi @tisonkun and @maxsxu, could help review this PR?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar-test-infra] nodece commented on a diff in pull request #69: Refactor docbot and add tests
Posted by GitBox <gi...@apache.org>.
nodece commented on code in PR #69:
URL: https://github.com/apache/pulsar-test-infra/pull/69#discussion_r957210713
##########
docbot/action.yml:
##########
@@ -11,12 +11,12 @@ runs:
- name: Checkout
uses: actions/checkout@v3
with:
- repository: apache/pulsar-test-infra
+ repository: nodece/pulsar-test-infra
Review Comment:
When this PR is ready for review, I will reset this config.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar-test-infra] tisonkun commented on a diff in pull request #69: Refactor docbot and add tests
Posted by GitBox <gi...@apache.org>.
tisonkun commented on code in PR #69:
URL: https://github.com/apache/pulsar-test-infra/pull/69#discussion_r962755814
##########
docbot/action_test.go:
##########
@@ -0,0 +1,326 @@
+package main
+
+import (
+ "context"
+ "fmt"
+ "os"
+ "strings"
+ "testing"
+
+ "github.com/google/go-github/v45/github"
+ "github.com/migueleliasweb/go-github-mock/src/mock"
+)
+
+func repoLabels() []*github.Label {
+ labels := []string{"doc-required", "doc-not-needed", "doc", "doc-complete", "doc-label-missing"}
+
+ result := make([]*github.Label, 0)
+ for _, label := range labels {
+ name := label
+ result = append(result, &github.Label{Name: &name})
+ }
+
+ return result
+}
+
+func mustNewActionConfig() *ActionConfig {
+ _ = os.Setenv("GITHUB_REPOSITORY", "apache/pulsar")
+ _ = os.Setenv("LABEL_WATCH_LIST", "doc,doc-required,doc-not-needed,doc-complete")
+ _ = os.Setenv("LABEL_MISSING", "doc-label-missing")
+
+ config, err := NewActionConfig()
+ if err != nil {
+ panic(err)
+ }
+
+ return config
+}
+
+func assertMessageLabel(t *testing.T, err error, message string) {
+ t.Helper()
+
+ if err == nil {
+ t.Fatal("Expect err not nil")
+ }
+
+ if err.Error() != message {
+ t.Fatal("Expect err equals " + message)
+ }
+}
+
+func TestSingleChecked(t *testing.T) {
+ id := int64(1)
+ body := fmt.Sprintf(`
+Check the box below or label this PR directly.
+
+Need to update docs?
+
+- [ ] %s
+(Your PR needs to update docs and you will update later)
+
+- [x] %s
+(Please explain why)
+
+- [ ] %s
+(Your PR contains doc changes)
+
+- [ ] %s
+(Docs have been already added)
+`, "`doc-required`", "`doc-not-needed`", "`doc`", "`doc-complete`")
+
+ mockedHTTPClient := mock.NewMockedHTTPClient(
+ mock.WithRequestMatch(
+ mock.GetReposPullsByOwnerByRepoByPullNumber,
+ github.PullRequest{
+ ID: &id,
+ Body: &body,
+ Labels: nil,
+ },
+ ), mock.WithRequestMatch(
+ mock.GetReposLabelsByOwnerByRepo,
+ repoLabels(),
+ ),
+ mock.WithRequestMatch(mock.PostReposIssuesLabelsByOwnerByRepoByIssueNumber, nil),
+ )
+
+ config := mustNewActionConfig()
+ action := NewActionWithClient(context.Background(), config, github.NewClient(mockedHTTPClient))
+
+ err := action.Run(1, "opened")
+ if err != nil {
+ t.Fatal(err)
+ }
+}
+
+func TestMultipleChecked(t *testing.T) {
+ id := int64(1)
+ body := fmt.Sprintf(`
+Check the box below or label this PR directly.
+
+Need to update docs?
+
+- [ ] %s
+(Your PR needs to update docs and you will update later)
+
+- [x] %s
+(Please explain why)
+
+- [x] %s
+(Your PR contains doc changes)
+
+- [ ] %s
+(Docs have been already added)
+`, "`doc-required`", "`doc-not-needed`", "`doc`", "`doc-complete`")
+
+ mockedHTTPClient := mock.NewMockedHTTPClient(
+ mock.WithRequestMatch(
+ mock.GetReposPullsByOwnerByRepoByPullNumber,
+ github.PullRequest{
+ ID: &id,
+ Body: &body,
+ Labels: nil,
+ },
+ ), mock.WithRequestMatch(
+ mock.GetReposLabelsByOwnerByRepo,
+ repoLabels(),
+ ),
+ mock.WithRequestMatch(mock.PostReposIssuesLabelsByOwnerByRepoByIssueNumber, nil),
+ )
+
+ const key = "ENABLE_LABEL_MULTIPLE"
+ value := os.Getenv(key)
+ defer func() {
+ // reset
+ _ = os.Setenv(key, value)
+ }()
+ _ = os.Setenv("ENABLE_LABEL_MULTIPLE", "true")
+
+ config := mustNewActionConfig()
+ action := NewActionWithClient(context.Background(), config, github.NewClient(mockedHTTPClient))
+
+ err := action.Run(1, "opened")
+ if err != nil {
+ t.Fatal(err)
+ }
+}
+
+func TestUnchecked(t *testing.T) {
+ id := int64(1)
+ body := fmt.Sprintf(`
+Check the box below or label this PR directly.
+
+Need to update docs?
+
+- [ ] %s
+(Your PR needs to update docs and you will update later)
+
+- [ ] %s
+(Please explain why)
+
+- [ ] %s
+(Your PR contains doc changes)
+
+- [ ] %s
+(Docs have been already added)
+`, "`doc-required`", "`doc-not-needed`", "`doc`", "`doc-complete`")
+
+ mockedHTTPClient := mock.NewMockedHTTPClient(
+ mock.WithRequestMatch(
+ mock.GetReposPullsByOwnerByRepoByPullNumber,
+ github.PullRequest{
+ ID: &id,
+ Body: &body,
+ Labels: nil,
+ },
+ ), mock.WithRequestMatch(
+ mock.GetReposLabelsByOwnerByRepo,
+ repoLabels(),
+ ),
+ mock.WithRequestMatch(mock.PostReposIssuesLabelsByOwnerByRepoByIssueNumber, nil),
+ mock.WithRequestMatch(mock.GetReposIssuesCommentsByOwnerByRepoByIssueNumber, nil),
+ mock.WithRequestMatch(mock.PostReposIssuesCommentsByOwnerByRepoByIssueNumber, nil),
+ )
+
+ config := mustNewActionConfig()
+ action := NewActionWithClient(context.Background(), config, github.NewClient(mockedHTTPClient))
+
+ err := action.Run(1, "opened")
+ assertMessageLabel(t, err, MessageLabelMissing)
+}
+
+func TestMultipleChecked_WhenMultipleLabelsNotEnabled(t *testing.T) {
+ id := int64(1)
+ body := fmt.Sprintf(`
+Check the box below or label this PR directly.
+
+Need to update docs?
+
+- [ ] %s
+(Your PR needs to update docs and you will update later)
+
+- [x] %s
+(Please explain why)
+
+- [x] %s
+(Your PR contains doc changes)
+
+- [ ] %s
+(Docs have been already added)
+`, "`doc-required`", "`doc-not-needed`", "`doc`", "`doc-complete`")
+
+ mockedHTTPClient := mock.NewMockedHTTPClient(
+ mock.WithRequestMatch(
+ mock.GetReposPullsByOwnerByRepoByPullNumber,
+ github.PullRequest{
+ ID: &id,
+ Body: &body,
+ Labels: nil,
+ },
+ ), mock.WithRequestMatch(
+ mock.GetReposLabelsByOwnerByRepo,
+ repoLabels(),
+ ),
+ mock.WithRequestMatch(mock.PostReposIssuesLabelsByOwnerByRepoByIssueNumber, nil),
+ mock.WithRequestMatch(mock.GetReposIssuesCommentsByOwnerByRepoByIssueNumber, nil),
+ mock.WithRequestMatch(mock.PostReposIssuesCommentsByOwnerByRepoByIssueNumber, nil),
+ )
+
+ config := mustNewActionConfig()
+ action := NewActionWithClient(context.Background(), config, github.NewClient(mockedHTTPClient))
+
+ err := action.Run(1, "opened")
+ assertMessageLabel(t, err, MessageLabelMultiple)
+}
+
+func TestSingleChecked_WhenLabelMissingExist(t *testing.T) {
+ id := int64(1)
+ body := fmt.Sprintf(`
+Check the box below or label this PR directly.
+
+Need to update docs?
+
+- [ ] %s
+(Your PR needs to update docs and you will update later)
+
+- [x] %s
+(Please explain why)
+
+- [ ] %s
+(Your PR contains doc changes)
+
+- [ ] %s
+(Docs have been already added)
+`, "`doc-required`", "`doc-not-needed`", "`doc`", "`doc-complete`")
+
+ labelMissing := "doc-label-missing"
+ mockedHTTPClient := mock.NewMockedHTTPClient(
+ mock.WithRequestMatch(
+ mock.GetReposPullsByOwnerByRepoByPullNumber,
+ github.PullRequest{
+ ID: &id,
+ Body: &body,
+ Labels: []*github.Label{{Name: &labelMissing}},
+ },
+ ), mock.WithRequestMatch(
+ mock.GetReposLabelsByOwnerByRepo,
+ repoLabels(),
+ ),
+ mock.WithRequestMatch(mock.PostReposIssuesLabelsByOwnerByRepoByIssueNumber, nil),
+ mock.WithRequestMatch(mock.DeleteReposIssuesLabelsByOwnerByRepoByIssueNumberByName, nil),
+ )
+
+ config := mustNewActionConfig()
+ action := NewActionWithClient(context.Background(), config, github.NewClient(mockedHTTPClient))
+
+ err := action.Run(1, "opened")
+ if err != nil {
+ t.Fatal(err)
+ }
+}
+
+func TestUnchecked_WhenLabelMissingExist(t *testing.T) {
+ id := int64(1)
+ body := fmt.Sprintf(`
+Check the box below or label this PR directly.
+
+Need to update docs?
+
+- [ ] %s
+(Your PR needs to update docs and you will update later)
+
+- [ ] %s
+(Please explain why)
+
+- [ ] %s
+(Your PR contains doc changes)
+
+- [ ] %s
+(Docs have been already added)
+`, "`doc-required`", "`doc-not-needed`", "`doc`", "`doc-complete`")
+
+ labelMissing := "doc-label-missing"
+ mockedHTTPClient := mock.NewMockedHTTPClient(
+ mock.WithRequestMatch(
+ mock.GetReposPullsByOwnerByRepoByPullNumber,
+ github.PullRequest{
+ ID: &id,
+ Body: &body,
+ Labels: []*github.Label{{Name: &labelMissing}},
+ },
+ ), mock.WithRequestMatch(
+ mock.GetReposLabelsByOwnerByRepo,
+ repoLabels(),
+ ),
+ )
+
+ config := mustNewActionConfig()
+ action := NewActionWithClient(context.Background(), config, github.NewClient(mockedHTTPClient))
+
+ err := action.Run(1, "opened")
+ assertMessageLabel(t, err, MessageLabelMissing)
+}
+
+func TestName(t *testing.T) {
+ split := strings.Split("docs, docs-required,", ",")
+ t.Log(split)
+}
Review Comment:
This test should test the functionality of `NewActionConfig` directly with assertions.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar-test-infra] nodece commented on pull request #69: Refactor docbot and add tests
Posted by GitBox <gi...@apache.org>.
nodece commented on PR #69:
URL: https://github.com/apache/pulsar-test-infra/pull/69#issuecomment-1237888481
I can split this PR because this PR is complex for reviewer.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar-test-infra] tisonkun commented on a diff in pull request #69: Refactor docbot and add tests
Posted by GitBox <gi...@apache.org>.
tisonkun commented on code in PR #69:
URL: https://github.com/apache/pulsar-test-infra/pull/69#discussion_r962603706
##########
docbot/main.go:
##########
@@ -607,26 +31,8 @@ func main() {
logger.Fatalln("Action type is not string")
Review Comment:
Perhaps we can remove the `case` branch above:
```go
case "issues":
logger.Infoln("@EventName is issues")
```
Since this action is only intended to be used for PRs.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar-test-infra] nodece commented on pull request #69: Refactor docbot and add tests
Posted by GitBox <gi...@apache.org>.
nodece commented on PR #69:
URL: https://github.com/apache/pulsar-test-infra/pull/69#issuecomment-1236821960
Hi @tisonkun, Thanks for your review, these request has been fixed.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar-test-infra] lhotari merged pull request #69: Refactor docbot and add tests
Posted by GitBox <gi...@apache.org>.
lhotari merged PR #69:
URL: https://github.com/apache/pulsar-test-infra/pull/69
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org