fix(internal/pkg/externalplugins/cherrypicker): do not change commit …

…message add option for `git cherry-pick command`: `--cleanup=verbatim`. Signed-off-by: wuhuizuo <[email protected]>
ti-community-infra · Aug 6, 2024 · de79f85 · de79f85
1 parent 8d3686d
commit de79f85
Show file tree

Hide file tree

Showing 3 changed files with 162 additions and 71 deletions.
diff --git a/internal/pkg/externalplugins/cherrypicker/cherrypicker.go b/internal/pkg/externalplugins/cherrypicker/cherrypicker.go
@@ -646,7 +646,7 @@ func (s *Server) handle(logger *logrus.Entry, requestor string,
 			}
 
 			//  Try git cherry-pick.
-			cherrypick := ex.Command("git", "cherry-pick", "-m", "1", *pr.MergeSHA)
+			cherrypick := ex.Command("git", "cherry-pick", "-m", "1", "--cleanup=verbatim", *pr.MergeSHA)
 			cherrypick.SetDir(dir)
 			out, err = cherrypick.CombinedOutput()
 			if err != nil {

diff --git a/internal/pkg/externalplugins/formatchecker/formatchecker_test.go b/internal/pkg/externalplugins/formatchecker/formatchecker_test.go
@@ -13,11 +13,14 @@ import (
 	"k8s.io/test-infra/prow/config"
 	"k8s.io/test-infra/prow/github"
 	"k8s.io/test-infra/prow/github/fakegithub"
+
+	_ "embed"
 )
 
 const (
 	issueTitleRegex         = "^(\\[TI-(?P<issue_number>[1-9]\\d*)\\])+.+: .{10,160}$"
 	issueNumberRegex        = "#([1-9]\\d*)"
+	testTaskCheckedRegex    = `<!-- At least one of them must be included\. -->\s*[\n]{1,}(\- \[[xX ]\] .*\n)*(\- \[[xX]\] .*\n)(\- \[[xX ]\] .*\n)*\n`
 	issueNumberPrefixRegex  = "((https|http)://github\\.com/{{.Org}}/{{.Repo}}/issues/|{{.Org}}/{{.Repo}}#|#)"
 	keywordPrefixRegex      = "(ref|close[sd]?|resolve[sd]?|fix(e[sd])?)"
 	issueNumberLineTemplate = "(?im)^Issue Number:\\s*((,\\s*)?%s\\s*%s(?P<issue_number>[1-9]\\d*))+"
@@ -27,6 +30,9 @@ var (
 	issueNumberLineRegexp = fmt.Sprintf(issueNumberLineTemplate, keywordPrefixRegex, issueNumberPrefixRegex)
 )
 
+//go:embed test-task-body.md
+var testTaskBody string
+
 func TestHandlePullRequestEvent(t *testing.T) {
 	formattedLabel := func(label string) string {
 		return fmt.Sprintf("%s/%s#%d:%s", "org", "repo", 1, label)
@@ -871,6 +877,26 @@ func TestHandleIssueEvent(t *testing.T) {
 			expectDeletedLabels: []string{},
 			shouldComment:       false,
 		},
+		{
+			name:      "Issue body without test task checked",
+			action:    github.IssueActionOpened,
+			body:      testTaskBody,
+			createdAt: earlierCreatedAt,
+			labels:    []string{},
+			requiredMatchRules: []externalplugins.RequiredMatchRule{
+				{
+					Issue:        true,
+					Body:         true,
+					Regexp:       testTaskCheckedRegex,
+					MissingLabel: "do-not-merge/needs-finish-test-tasks",
+				},
+			},
+			expectAddedLabels: []string{
+				formattedLabel("do-not-merge/needs-finish-test-tasks"),
+			},
+			expectDeletedLabels: []string{},
+			shouldComment:       false,
+		},
 		{
 			name:   "Issue body with issue number",
 			action: github.IssueActionOpened,
@@ -1109,85 +1135,87 @@ func TestHandleIssueEvent(t *testing.T) {
 	}
 
 	for _, testcase := range testcases {
-		tc := testcase
-
-		labels := make([]github.Label, 0)
-		for _, l := range tc.labels {
-			labels = append(labels, github.Label{
-				Name: l,
-			})
-		}
+		t.Run(testcase.name, func(t *testing.T) {
+			tc := testcase
+
+			labels := make([]github.Label, 0)
+			for _, l := range tc.labels {
+				labels = append(labels, github.Label{
+					Name: l,
+				})
+			}
 
-		fc := &fakegithub.FakeClient{
-			Issues: map[int]*github.Issue{
-				12345: {
-					Number:      12345,
-					PullRequest: nil,
-				},
-				1234: {
-					Number:      1234,
-					PullRequest: &struct{}{},
+			fc := &fakegithub.FakeClient{
+				Issues: map[int]*github.Issue{
+					12345: {
+						Number:      12345,
+						PullRequest: nil,
+					},
+					1234: {
+						Number:      1234,
+						PullRequest: &struct{}{},
+					},
 				},
-			},
-			IssueComments:      make(map[int][]github.IssueComment),
-			IssueLabelsAdded:   []string{},
-			IssueLabelsRemoved: []string{},
-		}
+				IssueComments:      make(map[int][]github.IssueComment),
+				IssueLabelsAdded:   []string{},
+				IssueLabelsRemoved: []string{},
+			}
 
-		cfg := &externalplugins.Configuration{}
-		cfg.TiCommunityFormatChecker = []externalplugins.TiCommunityFormatChecker{
-			{
-				Repos:              []string{"org/repo"},
-				RequiredMatchRules: tc.requiredMatchRules,
-			},
-		}
+			cfg := &externalplugins.Configuration{}
+			cfg.TiCommunityFormatChecker = []externalplugins.TiCommunityFormatChecker{
+				{
+					Repos:              []string{"org/repo"},
+					RequiredMatchRules: tc.requiredMatchRules,
+				},
+			}
 
-		ie := &github.IssueEvent{
-			Action: tc.action,
-			Issue: github.Issue{
-				Number:    1,
-				Title:     tc.title,
-				Body:      tc.body,
-				User:      github.User{Login: "zhang-san"},
-				CreatedAt: tc.createdAt,
-				Labels:    labels,
-			},
-			Repo: github.Repo{
-				Owner: github.User{
-					Login: "org",
+			ie := &github.IssueEvent{
+				Action: tc.action,
+				Issue: github.Issue{
+					Number:    1,
+					Title:     tc.title,
+					Body:      tc.body,
+					User:      github.User{Login: "zhang-san"},
+					CreatedAt: tc.createdAt,
+					Labels:    labels,
 				},
-				Name: "repo",
-			},
-			Label: github.Label{
-				Name: tc.label,
-			},
-		}
-		err := HandleIssueEvent(fc, ie, cfg, logrus.WithField("plugin", PluginName))
-		if err != nil {
-			t.Errorf("For case %s, didn't expect error: %v", tc.name, err)
-		}
+				Repo: github.Repo{
+					Owner: github.User{
+						Login: "org",
+					},
+					Name: "repo",
+				},
+				Label: github.Label{
+					Name: tc.label,
+				},
+			}
+			err := HandleIssueEvent(fc, ie, cfg, logrus.WithField("plugin", PluginName))
+			if err != nil {
+				t.Errorf("For case %s, didn't expect error: %v", tc.name, err)
+			}
 
-		sort.Strings(tc.expectAddedLabels)
-		sort.Strings(fc.IssueLabelsAdded)
-		if !reflect.DeepEqual(tc.expectAddedLabels, fc.IssueLabelsAdded) {
-			t.Errorf("For case \"%s\", expected the labels %q to be added, but %q were added",
-				tc.name, tc.expectAddedLabels, fc.IssueLabelsAdded)
-		}
+			sort.Strings(tc.expectAddedLabels)
+			sort.Strings(fc.IssueLabelsAdded)
+			if !reflect.DeepEqual(tc.expectAddedLabels, fc.IssueLabelsAdded) {
+				t.Errorf("For case \"%s\", expected the labels %q to be added, but %q were added",
+					tc.name, tc.expectAddedLabels, fc.IssueLabelsAdded)
+			}
 
-		sort.Strings(tc.expectDeletedLabels)
-		sort.Strings(fc.IssueLabelsRemoved)
-		if !reflect.DeepEqual(tc.expectDeletedLabels, fc.IssueLabelsRemoved) {
-			t.Errorf("For case %s, expected the labels %q to be deleted, but %q were deleted",
-				tc.name, tc.expectDeletedLabels, fc.IssueLabelsRemoved)
-		}
+			sort.Strings(tc.expectDeletedLabels)
+			sort.Strings(fc.IssueLabelsRemoved)
+			if !reflect.DeepEqual(tc.expectDeletedLabels, fc.IssueLabelsRemoved) {
+				t.Errorf("For case %s, expected the labels %q to be deleted, but %q were deleted",
+					tc.name, tc.expectDeletedLabels, fc.IssueLabelsRemoved)
+			}
 
-		if !tc.shouldComment && len(fc.IssueCommentsAdded) != 0 {
-			t.Errorf("unexpected comment %v", fc.IssueCommentsAdded)
-		}
+			if !tc.shouldComment && len(fc.IssueCommentsAdded) != 0 {
+				t.Errorf("unexpected comment %v", fc.IssueCommentsAdded)
+			}
 
-		if tc.shouldComment && len(fc.IssueCommentsAdded) == 0 {
-			t.Fatalf("expected comments but got none")
-		}
+			if tc.shouldComment && len(fc.IssueCommentsAdded) == 0 {
+				t.Fatalf("expected comments but got none")
+			}
+		})
 	}
 }
 

diff --git a/internal/pkg/externalplugins/formatchecker/test-task-body.md b/internal/pkg/externalplugins/formatchecker/test-task-body.md
@@ -0,0 +1,63 @@
+manually cherry-pick https://github.com/pingcap/tidb/pull/45330
+
+---
+<!--
+
+Thank you for contributing to TiDB!
+
+PR Title Format:
+1. pkg [, pkg2, pkg3]: what's changed
+2. *: what's changed
+
+-->
+
+### What problem does this PR solve?
+<!--
+
+Please create an issue first to describe the problem.
+
+There MUST be one line starting with "Issue Number:  " and 
+linking the relevant issues via the "close" or "ref".
+
+For more info, check https://pingcap.github.io/tidb-dev-guide/contribute-to-tidb/contribute-code.html#referring-to-an-issue.
+
+-->
+
+Issue Number: close #46540
+
+Problem Summary:
+
+### What is changed and how it works?
+
+### Check List
+
+Tests <!-- At least one of them must be included. -->
+
+- [ ] Unit test
+- [ ] Integration test
+- [ ] Manual test (add detailed scripts or steps below)
+- [ ] No code
+
+Side effects
+
+- [x] Performance regression: Consumes more CPU
+- [ ] Performance regression: Consumes more Memory
+- [ ] Breaking backward compatibility
+
+Documentation
+
+- [ ] Affects user behaviors
+- [ ] Contains syntax changes
+- [ ] Contains variable changes
+- [ ] Contains experimental features
+- [ ] Changes MySQL compatibility
+
+### Release note
+
+<!-- compatibility change, improvement, bugfix, and new feature need a release note -->
+
+Please refer to [Release Notes Language Style Guide](https://pingcap.github.io/tidb-dev-guide/contribute-to-tidb/release-notes-style-guide.html) to write a quality release note.
+
+```release-note
+None
+```