diff --git a/docs/en/plugins/issue-triage.md b/docs/en/plugins/issue-triage.md index 33919a1ce..6ca8cab19 100644 --- a/docs/en/plugins/issue-triage.md +++ b/docs/en/plugins/issue-triage.md @@ -26,7 +26,7 @@ For critical or major severity bug issues. For pull requests that fix related bug issues and linked with them, we add a check named `check-issue-triage-complete` that will ensure that the bug issues are triaged before the pull request is merged. -By default, the plugin will trigger the check at the right time, and if it doesn't, contributors can trigger it manually with the `/run-check-issue-triage-complete` command. +By default, the plugin will trigger the check at the right time, and if it doesn't, contributors can trigger it manually with the `/run-check-issue-triage-complete` or `/check-issue-triage-complete` command. The plugin determines whether a pull request is triaged according to the following rules. diff --git a/docs/plugins/issue-triage.md b/docs/plugins/issue-triage.md index d422837f9..3ae708efb 100644 --- a/docs/plugins/issue-triage.md +++ b/docs/plugins/issue-triage.md @@ -26,7 +26,7 @@ ti-community-issue-triage 插件将被设计来对以上流程进行管控和自 对于修复相关 bug issue 并与之关联的 pull request 而言，我们会添加一个名为 `check-issue-triage-complete` 的检查项，该检查将会确保 bug issue 在 pull request 合并之前完成 triage。 -默认情况下，插件会在合适的时机来触发该检查，如果没有触发成功，Contributor 可以通过 `/run-check-issue-triage-complete` 命令进行手动触发。 +默认情况下，插件会在合适的时机来触发该检查，如果没有触发成功，Contributor 可以通过 `/run-check-issue-triage-complete` 或 `/check-issue-triage-complete` 命令进行手动触发。 插件会根据如下规则来判断 pull request 是否完成 triaged： diff --git a/internal/pkg/externalplugins/issuetriage/issuetriage.go b/internal/pkg/externalplugins/issuetriage/issuetriage.go index d076d0758..06e4883cd 100644 --- a/internal/pkg/externalplugins/issuetriage/issuetriage.go +++ b/internal/pkg/externalplugins/issuetriage/issuetriage.go @@ -26,19 +26,30 @@ import ( const PluginName = "ti-community-issue-triage" const ( - bugTypeLabel = "type/bug" + typeLabelPrefix = "type/" + bugTypeLabel = "type/bug" + + severityLabelPrefix = "severity/" majorSeverityLabel = "severity/major" criticalSeverityLabel = "severity/critical" - severityLabelPrefix = "severity/" - issueNeedTriagedContextName = "check-issue-triage-complete" - issueTriageContextMessageSuccess = "All linked bug issues have been triaged complete." - issueTriageContextMessagePending = "Bug issues need to triage completed before merge." + maxStatusDescriptionLength = 140 +) + +const ( + issueNeedTriagedContextName = "check-issue-triage-complete" + + statusDescAllBugIssueTriaged = "All linked bug issues have been triaged complete." + statusDescNoLinkedIssue = "No linked issue found." + statusDescFailedToGetIssue = "Failed to get issue %s/%s#%d." + statusDescTypeLabelNotFound = "Can not find any type label on issue %s/%s#%d." + statusDescSeverityLabelNotFound = "Can not find any severity label on bug issue %s/%s#%d." + statusDescMayAffectsLabelBeFound = "Found may-affects label on bug issue %s/%s#%d." ) var ( IssueNumberLineRe = regexp.MustCompile("(?im)^Issue Number:.+") - checkIssueTriagedRe = regexp.MustCompile(`(?mi)^/(run-)*check-issue-triage-complete\s*$`) + checkIssueTriagedRe = regexp.MustCompile(`(?mi)^/(run-)?check-issue-triage-complete\s*$`) ) type githubClient interface { @@ -345,6 +356,11 @@ func (s *Server) handleIssueEvent(ie *github.IssueEvent, log *logrus.Entry) erro if err != nil { return err } + if len(prs) == 0 { + log.Infof("Can not find any reference PRs.") + return nil + } + log.Infof("Found %d reference PRs.", len(prs)) // Notice: wait a few seconds to reduce API consumption in case of batched label operations. time.Sleep(5 * time.Second) @@ -359,9 +375,6 @@ func (s *Server) handleIssueEvent(ie *github.IssueEvent, log *logrus.Entry) erro var errs []error for _, pr := range prs { - issueNumberLine := IssueNumberLineRe.FindString(string(pr.Body)) - linkedIssueNumbers := utils.NormalizeIssueNumbers(issueNumberLine, org, repo) - defaultBranch := string(pr.Repository.DefaultBranchRef.Name) prOrg := string(pr.Repository.Owner.Login) prRepo := string(pr.Repository.Name) @@ -369,17 +382,27 @@ func (s *Server) handleIssueEvent(ie *github.IssueEvent, log *logrus.Entry) erro prState := string(pr.State) prSHA := string(pr.HeadRefOid) prBranch := string(pr.BaseRefName) + log = log.WithFields(logrus.Fields{ + "org": prOrg, + "repo": prRepo, + "num": prNum, + }) if !isPRNeedToCheck(prState, prBranch, defaultBranch) { log.Debugf("Skipping the check, because it is not a open PR on the default branch.") continue } + log.Infof("Checking triggered by issue %s/%s#%d.", org, repo, num) prLabels := sets.NewString() for _, node := range pr.Labels.Nodes { prLabels.Insert(string(node.Name)) } + issueNumberLine := IssueNumberLineRe.FindString(string(pr.Body)) + linkedIssueNumbers := utils.NormalizeIssueNumbers(issueNumberLine, org, repo) + log.Infof("Found %d linked issues.", len(linkedIssueNumbers)) + err := s.handle(log, cfg, prOrg, prRepo, prSHA, prNum, prLabels, linkedIssueNumbers, issues) if err != nil { errs = append(errs, err) @@ -403,11 +426,17 @@ func (s *Server) handlePullRequestEvent(pe *github.PullRequestEvent, log *logrus prRepo := pe.Repo.Name prSHA := pr.Head.SHA prNum := pr.Number + log = log.WithFields(logrus.Fields{ + "org": prOrg, + "repo": prRepo, + "num": prNum, + }) if !isPRNeedToCheck(pr.State, pr.Base.Ref, pe.Repo.DefaultBranch) { - log.Debugf("Skipping the check, because it is not a open PR on the default branch.") + log.Debug("Skipping the check, because it is not a open PR on the default branch.") return nil } + log.Infof("Checking triggered by PR %s/%s#%d.", prOrg, prRepo, prNum) prLabels := sets.NewString() for _, label := range pr.Labels { @@ -417,6 +446,7 @@ func (s *Server) handlePullRequestEvent(pe *github.PullRequestEvent, log *logrus cfg := s.ConfigAgent.Config().IssueTriageFor(prOrg, prRepo) issueNumberLine := IssueNumberLineRe.FindString(pr.Body) linkedIssueNumbers := utils.NormalizeIssueNumbers(issueNumberLine, prOrg, prRepo) + log.Infof("Found %d linked issues.", len(linkedIssueNumbers)) return s.handle(log, cfg, prOrg, prRepo, prSHA, prNum, prLabels, linkedIssueNumbers, issueCache{}) } @@ -432,9 +462,15 @@ func (s *Server) handleIssueCommentEvent(ice *github.IssueCommentEvent, log *log prOrg := ice.Repo.Owner.Login prRepo := ice.Repo.Name prNum := issue.Number + log = log.WithFields(logrus.Fields{ + "org": prOrg, + "repo": prRepo, + "num": prNum, + "comment_id": ice.Comment.ID, + }) if !checkIssueTriagedRe.MatchString(comment) { - log.Debugf("Skipping the check, because no command comment.") + log.Debug("Skipping the check, because no command comment.") return nil } @@ -444,9 +480,10 @@ func (s *Server) handleIssueCommentEvent(ice *github.IssueCommentEvent, log *log } if !isPRNeedToCheck(pr.State, pr.Base.Ref, ice.Repo.DefaultBranch) { - log.Debugf("Skipping the check because it is not a open PR on the default branch.") + log.Debug("Skipping the check because it is not a open PR on the default branch.") return nil } + log.Infof("Checking triggered by comment %s/%s#%d:%d.", prOrg, prRepo, prNum, ice.Comment.ID) prSHA := pr.Head.SHA prLabels := sets.NewString() @@ -457,6 +494,7 @@ func (s *Server) handleIssueCommentEvent(ice *github.IssueCommentEvent, log *log cfg := s.ConfigAgent.Config().IssueTriageFor(prOrg, prRepo) issueNumberLine := IssueNumberLineRe.FindString(pr.Body) linkedIssueNumbers := utils.NormalizeIssueNumbers(issueNumberLine, prOrg, prRepo) + log.Infof("Found %d linked issues.", len(linkedIssueNumbers)) return s.handle(log, cfg, prOrg, prRepo, prSHA, prNum, prLabels, linkedIssueNumbers, issueCache{}) } @@ -480,19 +518,21 @@ func (s *Server) handle(log *logrus.Entry, cfg *tiexternalplugins.TiCommunityIss lock.Lock() defer lock.Unlock() - existingStatus, err := s.checkExistingStatus(log, prOrg, prRepo, prSHA) + existingStatus, existingDesc, existingURL, err := s.checkExistingStatus(log, prOrg, prRepo, prSHA) if err != nil { return err } - allTriaged, affectsVersionLabels, err := s.checkLinkedIssues(cfg, issueKeys, issueCache) + wantStatus, wantDesc, affectsVersionLabels, err := s.checkLinkedIssues(cfg, issueKeys, issueCache) if err != nil { return err } + log.Infof("Checking result: %s, found affects labels: %v", strings.ToLower(wantDesc), affectsVersionLabels) - // Notice: all triaged means all the linked issues needed to be triaged have triaged complete, if a PR - // has no issues that require triaged, the check will pass too. - if allTriaged { + // Notice: check pass when all the linked issues needed to be triaged have triaged complete, + // if a PR has no linked bug issues, the check will pass too. + if wantStatus == github.StatusSuccess { + // Remove the need triaged label. if len(cfg.NeedTriagedLabel) != 0 && prLabels.Has(cfg.NeedTriagedLabel) { err := s.GitHubClient.RemoveLabel(prOrg, prRepo, prNum, cfg.NeedTriagedLabel) if err != nil { @@ -500,6 +540,7 @@ func (s *Server) handle(log *logrus.Entry, cfg *tiexternalplugins.TiCommunityIss } } + // Add needs-cherry-pick label. labelsNeedToAdd := make([]string, 0) for _, affectsVersionLabel := range affectsVersionLabels.List() { affectVersion := strings.TrimPrefix(affectsVersionLabel, cfg.AffectsLabelPrefix) @@ -509,30 +550,26 @@ func (s *Server) handle(log *logrus.Entry, cfg *tiexternalplugins.TiCommunityIss } } - err := s.createStatus(log, prOrg, prRepo, prSHA, existingStatus, - github.StatusSuccess, issueTriageContextMessageSuccess, cfg.StatusTargetURL) - if err != nil { - return err - } - if len(labelsNeedToAdd) > 0 { err := s.GitHubClient.AddLabels(prOrg, prRepo, prNum, labelsNeedToAdd...) if err != nil { return err } } - return nil - } - - if len(cfg.NeedTriagedLabel) != 0 && !prLabels.Has(cfg.NeedTriagedLabel) { - err := s.GitHubClient.AddLabel(prOrg, prRepo, prNum, cfg.NeedTriagedLabel) - if err != nil { - return err + } else if wantStatus == github.StatusError || wantStatus == github.StatusFailure { + // Add the need triaged label. + if len(cfg.NeedTriagedLabel) != 0 && !prLabels.Has(cfg.NeedTriagedLabel) { + err := s.GitHubClient.AddLabel(prOrg, prRepo, prNum, cfg.NeedTriagedLabel) + if err != nil { + return err + } } } - return s.createStatus(log, prOrg, prRepo, prSHA, existingStatus, - github.StatusPending, issueTriageContextMessagePending, cfg.StatusTargetURL) + // Update status. + wantDesc = truncatedStatusDesc(log, wantDesc) + return s.createStatus(log, prOrg, prRepo, prSHA, existingStatus, existingDesc, existingURL, + wantStatus, wantDesc, cfg.StatusTargetURL) } // isPRNeedToCheck used to determine if PR needs to be checked. @@ -543,13 +580,22 @@ func isPRNeedToCheck(state, prBranch, defaultBranch string) bool { // checkLinkedIssues used to check if a given set of bug issues have all been triaged. func (s *Server) checkLinkedIssues(cfg *tiexternalplugins.TiCommunityIssueTriage, - issueKeys []utils.IssueNumberData, issueCache issueCache) (bool, sets.String, error) { + issueKeys []utils.IssueNumberData, issueCache issueCache) (string, string, sets.String, error) { affectsVersionLabels := sets.NewString() + if len(issueKeys) == 0 { + return github.StatusSuccess, statusDescNoLinkedIssue, affectsVersionLabels, nil + } + for _, issueKey := range issueKeys { - issue, err := s.getIssueWithCache(issueCache, issueKey.Org, issueKey.Repo, issueKey.Number) + org := issueKey.Org + repo := issueKey.Repo + num := issueKey.Number + + issue, err := s.getIssueWithCache(issueCache, org, repo, num) if err != nil { - return false, sets.String{}, err + desc := fmt.Sprintf(statusDescFailedToGetIssue, org, repo, num) + return github.StatusFailure, desc, sets.String{}, err } labels := sets.NewString() @@ -560,6 +606,12 @@ func (s *Server) checkLinkedIssues(cfg *tiexternalplugins.TiCommunityIssueTriage } } + // Issue without type label is not triaged. + if !hasAnyTypeLabel(issue) { + desc := fmt.Sprintf(statusDescTypeLabelNotFound, org, repo, num) + return github.StatusError, desc, sets.String{}, nil + } + // Only bug issues need to be checked. if !labels.Has(bugTypeLabel) { continue @@ -567,7 +619,8 @@ func (s *Server) checkLinkedIssues(cfg *tiexternalplugins.TiCommunityIssueTriage // Bug issue must have severity label, if not, it will be considered to triage. if !hasAnySeverityLabel(issue) { - return false, sets.String{}, nil + desc := fmt.Sprintf(statusDescSeverityLabelNotFound, org, repo, num) + return github.StatusError, desc, sets.String{}, nil } // Only major or critical bug issues need to be checked. @@ -575,48 +628,53 @@ func (s *Server) checkLinkedIssues(cfg *tiexternalplugins.TiCommunityIssueTriage continue } - // Check if issue has any may-affects labels or no affects label, if any issue have + // Check if issue has any may-affects labels, if any issue have // not yet been triaged, the checker will fail. - if !hasAnyAffectsLabel(issue, cfg.AffectsLabelPrefix) || - hasAnyMayAffectsLabel(issue, cfg.MayAffectsLabelPrefix) { - return false, sets.String{}, nil + if hasAnyMayAffectsLabel(issue, cfg.MayAffectsLabelPrefix) { + desc := fmt.Sprintf(statusDescMayAffectsLabelBeFound, org, repo, num) + return github.StatusError, desc, sets.String{}, nil } } - return true, affectsVersionLabels, nil + + return github.StatusSuccess, statusDescAllBugIssueTriaged, affectsVersionLabels, nil } // checkExistingStatus will retrieve the current status of the linked issue need triaged context // for the provided SHA. -func (s *Server) checkExistingStatus(l *logrus.Entry, org, repo, sha string) (string, error) { +func (s *Server) checkExistingStatus(l *logrus.Entry, org, repo, sha string) (string, string, string, error) { combinedStatus, err := s.GitHubClient.GetCombinedStatus(org, repo, sha) if err != nil { - return "", fmt.Errorf("error listing pull request combined statuses: %w", err) + return "", "", "", fmt.Errorf("error listing pull request combined statuses: %w", err) } existingStatus := "" + existingDesc := "" + existingURL := "" for _, status := range combinedStatus.Statuses { if status.Context != issueNeedTriagedContextName { continue } existingStatus = status.State + existingDesc = status.Description + existingURL = status.TargetURL break } l.Debugf("Existing linked issue need triaged status context status is %q", existingStatus) - return existingStatus, nil + return existingStatus, existingDesc, existingURL, nil } -func hasAnySeverityLabel(issue *github.Issue) bool { +func hasAnyTypeLabel(issue *github.Issue) bool { for _, label := range issue.Labels { - if strings.HasPrefix(label.Name, severityLabelPrefix) { + if strings.HasPrefix(label.Name, typeLabelPrefix) { return true } } return false } -func hasAnyAffectsLabel(issue *github.Issue, affectsLabelPrefix string) bool { +func hasAnySeverityLabel(issue *github.Issue) bool { for _, label := range issue.Labels { - if strings.HasPrefix(label.Name, affectsLabelPrefix) { + if strings.HasPrefix(label.Name, severityLabelPrefix) { return true } } @@ -673,21 +731,30 @@ func (s *Server) getReferencePRList(log *logrus.Entry, org, repo string, issueNu return ret, nil } -func (s *Server) createStatus(log *logrus.Entry, org, repo, sha, existingState, - targetState, message, targetURL string) error { - if existingState != targetState { - log.Debugf("Setting check-issue-triage-complete status context to %s.", targetState) +func (s *Server) createStatus(log *logrus.Entry, org, repo, sha, existingState, existingDesc, existingURL, + wantState, wantDesc, wantURL string) error { + if existingState != wantState || existingDesc != wantDesc || existingURL != wantURL { + log.Debugf("Setting check-issue-triage-complete status context to %s.", wantState) if err := s.GitHubClient.CreateStatus(org, repo, sha, github.Status{ Context: issueNeedTriagedContextName, - State: targetState, - TargetURL: targetURL, - Description: message, + State: wantState, + TargetURL: wantURL, + Description: wantDesc, }); err != nil { return fmt.Errorf("error setting pull request status: %w", err) } } else { - log.Debugf("The check-issue-triage-complete status context is already %s.", targetState) + log.Debugf("The check-issue-triage-complete status context is already %s.", wantState) } return nil } + +func truncatedStatusDesc(log *logrus.Entry, wantDesc string) string { + if len(wantDesc) > maxStatusDescriptionLength { + original := wantDesc + wantDesc = fmt.Sprintf("%s...", wantDesc[0:(maxStatusDescriptionLength-3)]) + log.WithField("original-desc", original).Warn("GitHub status description needed to be truncated to fit GH API limit") + } + return wantDesc +} diff --git a/internal/pkg/externalplugins/issuetriage/issuetriage_test.go b/internal/pkg/externalplugins/issuetriage/issuetriage_test.go index 213268a08..6047bfc42 100644 --- a/internal/pkg/externalplugins/issuetriage/issuetriage_test.go +++ b/internal/pkg/externalplugins/issuetriage/issuetriage_test.go @@ -190,6 +190,7 @@ func TestHandleIssueEvent(t *testing.T) { expectAddedLabels []string expectRemovedLabels []string expectCreatedStatusState string + expectCreatedStatusDesc string }{ { name: "add a security/major label to a type/bug issue", @@ -296,7 +297,8 @@ func TestHandleIssueEvent(t *testing.T) { "org/repo#2:do-not-merge/needs-triage-completed", }, expectRemovedLabels: []string{}, - expectCreatedStatusState: github.StatusPending, + expectCreatedStatusState: github.StatusError, + expectCreatedStatusDesc: "Found may-affects label on bug issue org/repo#1.", }, { name: "the type/bug issue removed a may-affects label and does not have any other may-affects labels", @@ -317,13 +319,14 @@ func TestHandleIssueEvent(t *testing.T) { }, }, }, - statusState: github.StatusPending, + statusState: github.StatusError, expectAddedLabels: []string{ "org/repo#2:needs-cherry-pick-release-5.2", }, expectRemovedLabels: []string{}, expectCreatedStatusState: github.StatusSuccess, + expectCreatedStatusDesc: statusDescAllBugIssueTriaged, }, } @@ -525,6 +528,17 @@ func TestHandleIssueEvent(t *testing.T) { tc.name, tc.expectCreatedStatusState, createdStatuses[0].State) } } + + if len(tc.expectCreatedStatusDesc) != 0 { + createdStatuses, ok := fc.CreatedStatuses["sha"] + if !ok || len(createdStatuses) != 1 { + t.Errorf("For case [%s], expect status description: %s, but got: none.\n", + tc.name, tc.expectCreatedStatusDesc) + } else if tc.expectCreatedStatusDesc != createdStatuses[0].Description { + t.Errorf("For case [%s], expect status description: %s, but got: %s.\n", + tc.name, tc.expectCreatedStatusDesc, createdStatuses[0].Description) + } + } } } @@ -546,6 +560,7 @@ func TestHandlePullRequestEvent(t *testing.T) { expectAddedLabels []string expectRemovedLabels []string expectCreatedStatusState string + expectCreatedStatusDesc string }{ { name: "open a pull request with empty body", @@ -557,6 +572,7 @@ func TestHandlePullRequestEvent(t *testing.T) { expectAddedLabels: []string{}, expectRemovedLabels: []string{}, expectCreatedStatusState: github.StatusSuccess, + expectCreatedStatusDesc: statusDescNoLinkedIssue, }, { name: "open a pull request linked to a feature issue", @@ -579,6 +595,7 @@ func TestHandlePullRequestEvent(t *testing.T) { expectAddedLabels: []string{}, expectRemovedLabels: []string{}, expectCreatedStatusState: github.StatusSuccess, + expectCreatedStatusDesc: statusDescAllBugIssueTriaged, }, { name: "open a pull request linked to a bug issue without severity label", @@ -602,7 +619,8 @@ func TestHandlePullRequestEvent(t *testing.T) { "org/repo#1:do-not-merge/needs-triage-completed", }, expectRemovedLabels: []string{}, - expectCreatedStatusState: github.StatusPending, + expectCreatedStatusState: github.StatusError, + expectCreatedStatusDesc: "Can not find any severity label on bug issue org/repo#2.", }, { name: "open a pull request linked to a bug issue with severity/moderate label", @@ -623,6 +641,7 @@ func TestHandlePullRequestEvent(t *testing.T) { expectAddedLabels: []string{}, expectRemovedLabels: []string{}, + expectCreatedStatusDesc: statusDescAllBugIssueTriaged, expectCreatedStatusState: github.StatusSuccess, }, { @@ -642,11 +661,11 @@ func TestHandlePullRequestEvent(t *testing.T) { }, }, - expectAddedLabels: []string{ - "org/repo#1:do-not-merge/needs-triage-completed", - }, + // Notice: a bug issue maybe only affect master branch. + expectAddedLabels: []string{}, expectRemovedLabels: []string{}, - expectCreatedStatusState: github.StatusPending, + expectCreatedStatusDesc: statusDescAllBugIssueTriaged, + expectCreatedStatusState: github.StatusSuccess, }, { name: "open a pull request linked to a bug issue with severity/major label and may-affects/* label", @@ -670,7 +689,8 @@ func TestHandlePullRequestEvent(t *testing.T) { "org/repo#1:do-not-merge/needs-triage-completed", }, expectRemovedLabels: []string{}, - expectCreatedStatusState: github.StatusPending, + expectCreatedStatusState: github.StatusError, + expectCreatedStatusDesc: "Found may-affects label on bug issue org/repo#2.", }, { name: "open a pull request linked to a bug issue with severity/major label and affects/* label", @@ -695,6 +715,7 @@ func TestHandlePullRequestEvent(t *testing.T) { }, expectRemovedLabels: []string{}, expectCreatedStatusState: github.StatusSuccess, + expectCreatedStatusDesc: statusDescAllBugIssueTriaged, }, { name: "open a pull request linked to a bug issue with severity/major, affects/* and may-affects/* labels", @@ -719,7 +740,8 @@ func TestHandlePullRequestEvent(t *testing.T) { "org/repo#1:do-not-merge/needs-triage-completed", }, expectRemovedLabels: []string{}, - expectCreatedStatusState: github.StatusPending, + expectCreatedStatusState: github.StatusError, + expectCreatedStatusDesc: "Found may-affects label on bug issue org/repo#2.", }, { name: "open a pull request linked to a non-triaged bug issue and a feature issue", @@ -749,7 +771,7 @@ func TestHandlePullRequestEvent(t *testing.T) { "org/repo#1:do-not-merge/needs-triage-completed", }, expectRemovedLabels: []string{}, - expectCreatedStatusState: github.StatusPending, + expectCreatedStatusState: github.StatusError, }, { name: "open a pull request linked to a triaged issue and a non-triaged issue", @@ -781,7 +803,8 @@ func TestHandlePullRequestEvent(t *testing.T) { "org/repo#1:do-not-merge/needs-triage-completed", }, expectRemovedLabels: []string{}, - expectCreatedStatusState: github.StatusPending, + expectCreatedStatusState: github.StatusError, + expectCreatedStatusDesc: "Found may-affects label on bug issue org/repo#3.", }, { name: "open a pull request linked to two triaged issue", @@ -815,6 +838,7 @@ func TestHandlePullRequestEvent(t *testing.T) { }, expectRemovedLabels: []string{}, expectCreatedStatusState: github.StatusSuccess, + expectCreatedStatusDesc: statusDescAllBugIssueTriaged, }, { name: "open a pull request on release branch and link to a bug issue with may-affects/* labels", @@ -886,6 +910,7 @@ func TestHandlePullRequestEvent(t *testing.T) { }, expectRemovedLabels: []string{}, expectCreatedStatusState: github.StatusSuccess, + expectCreatedStatusDesc: statusDescAllBugIssueTriaged, }, } @@ -1012,6 +1037,17 @@ func TestHandlePullRequestEvent(t *testing.T) { tc.name, tc.expectCreatedStatusState, createdStatuses[0].State) } } + + if len(tc.expectCreatedStatusDesc) != 0 { + createdStatuses, ok := fc.CreatedStatuses["sha"] + if !ok || len(createdStatuses) != 1 { + t.Errorf("For case [%s], expect status description: %s, but got: none.\n", + tc.name, tc.expectCreatedStatusDesc) + } else if tc.expectCreatedStatusDesc != createdStatuses[0].Description { + t.Errorf("For case [%s], expect status description: %s, but got: %s.\n", + tc.name, tc.expectCreatedStatusDesc, createdStatuses[0].Description) + } + } } } @@ -1034,6 +1070,7 @@ func TestHandleIssueCommentEvent(t *testing.T) { expectAddedLabels []string expectRemovedLabels []string expectCreatedStatusState string + expectCreatedStatusDesc string }{ { name: "comment to a pull request with empty body", @@ -1046,6 +1083,7 @@ func TestHandleIssueCommentEvent(t *testing.T) { expectAddedLabels: []string{}, expectRemovedLabels: []string{}, expectCreatedStatusState: github.StatusSuccess, + expectCreatedStatusDesc: statusDescNoLinkedIssue, }, { name: "comment shorten command to a pull request with empty body", @@ -1058,6 +1096,7 @@ func TestHandleIssueCommentEvent(t *testing.T) { expectAddedLabels: []string{}, expectRemovedLabels: []string{}, expectCreatedStatusState: github.StatusSuccess, + expectCreatedStatusDesc: statusDescNoLinkedIssue, }, { name: "comment non command to a pull request with empty body", @@ -1067,9 +1106,8 @@ func TestHandleIssueCommentEvent(t *testing.T) { state: "open", targetBranch: "master", - expectAddedLabels: []string{}, - expectRemovedLabels: []string{}, - expectCreatedStatusState: "", + expectAddedLabels: []string{}, + expectRemovedLabels: []string{}, }, { name: "comment to a pull request linked to a feature issue", @@ -1093,6 +1131,7 @@ func TestHandleIssueCommentEvent(t *testing.T) { expectAddedLabels: []string{}, expectRemovedLabels: []string{}, expectCreatedStatusState: github.StatusSuccess, + expectCreatedStatusDesc: statusDescAllBugIssueTriaged, }, { name: "comment to a pull request linked to a bug issue without severity label", @@ -1117,7 +1156,8 @@ func TestHandleIssueCommentEvent(t *testing.T) { "org/repo#1:do-not-merge/needs-triage-completed", }, expectRemovedLabels: []string{}, - expectCreatedStatusState: github.StatusPending, + expectCreatedStatusState: github.StatusError, + expectCreatedStatusDesc: "Can not find any severity label on bug issue org/repo#2.", }, { name: "comment to a pull request linked to a bug issue with severity/moderate label", @@ -1140,6 +1180,7 @@ func TestHandleIssueCommentEvent(t *testing.T) { expectAddedLabels: []string{}, expectRemovedLabels: []string{}, expectCreatedStatusState: github.StatusSuccess, + expectCreatedStatusDesc: statusDescAllBugIssueTriaged, }, { name: "comment to a pull request linked to a bug issue with severity/major label", @@ -1159,11 +1200,11 @@ func TestHandleIssueCommentEvent(t *testing.T) { }, }, - expectAddedLabels: []string{ - "org/repo#1:do-not-merge/needs-triage-completed", - }, + // Notice: a bug issue maybe only affect master branch. + expectAddedLabels: []string{}, expectRemovedLabels: []string{}, - expectCreatedStatusState: github.StatusPending, + expectCreatedStatusState: github.StatusSuccess, + expectCreatedStatusDesc: statusDescAllBugIssueTriaged, }, { name: "comment to a pull request linked to a bug issue with severity/major label and may-affects/* label", @@ -1188,7 +1229,8 @@ func TestHandleIssueCommentEvent(t *testing.T) { "org/repo#1:do-not-merge/needs-triage-completed", }, expectRemovedLabels: []string{}, - expectCreatedStatusState: github.StatusPending, + expectCreatedStatusState: github.StatusError, + expectCreatedStatusDesc: "Found may-affects label on bug issue org/repo#2.", }, { name: "comment to a pull request linked to a bug issue with severity/major label and affects/* label", @@ -1214,6 +1256,7 @@ func TestHandleIssueCommentEvent(t *testing.T) { }, expectRemovedLabels: []string{}, expectCreatedStatusState: github.StatusSuccess, + expectCreatedStatusDesc: statusDescAllBugIssueTriaged, }, { name: "comment to a pull request linked to a bug issue with severity/major label and *-affects/* label", @@ -1239,7 +1282,8 @@ func TestHandleIssueCommentEvent(t *testing.T) { "org/repo#1:do-not-merge/needs-triage-completed", }, expectRemovedLabels: []string{}, - expectCreatedStatusState: github.StatusPending, + expectCreatedStatusState: github.StatusError, + expectCreatedStatusDesc: "Found may-affects label on bug issue org/repo#2.", }, { name: "comment to a pull request linked to a non-triaged bug issue and a feature issue", @@ -1270,7 +1314,8 @@ func TestHandleIssueCommentEvent(t *testing.T) { "org/repo#1:do-not-merge/needs-triage-completed", }, expectRemovedLabels: []string{}, - expectCreatedStatusState: github.StatusPending, + expectCreatedStatusState: github.StatusError, + expectCreatedStatusDesc: "Found may-affects label on bug issue org/repo#2.", }, { name: "comment to a pull request linked to a triaged issue and a non-triaged issue", @@ -1303,7 +1348,8 @@ func TestHandleIssueCommentEvent(t *testing.T) { "org/repo#1:do-not-merge/needs-triage-completed", }, expectRemovedLabels: []string{}, - expectCreatedStatusState: github.StatusPending, + expectCreatedStatusState: github.StatusError, + expectCreatedStatusDesc: "Found may-affects label on bug issue org/repo#3.", }, { name: "comment to a pull request linked to two triaged issue", @@ -1338,6 +1384,7 @@ func TestHandleIssueCommentEvent(t *testing.T) { }, expectRemovedLabels: []string{}, expectCreatedStatusState: github.StatusSuccess, + expectCreatedStatusDesc: statusDescAllBugIssueTriaged, }, } @@ -1476,6 +1523,17 @@ func TestHandleIssueCommentEvent(t *testing.T) { tc.name, tc.expectCreatedStatusState, createdStatuses[0].State) } } + + if len(tc.expectCreatedStatusDesc) != 0 { + createdStatuses, ok := fc.CreatedStatuses["sha"] + if !ok || len(createdStatuses) != 1 { + t.Errorf("For case [%s], expect status description: %s, but got: none.\n", + tc.name, tc.expectCreatedStatusDesc) + } else if tc.expectCreatedStatusDesc != createdStatuses[0].Description { + t.Errorf("For case [%s], expect status description: %s, but got: %s.\n", + tc.name, tc.expectCreatedStatusDesc, createdStatuses[0].Description) + } + } } }