diff --git a/pkg/notes/notes.go b/pkg/notes/notes.go index 54a6cb046b17..fc78defa485b 100644 --- a/pkg/notes/notes.go +++ b/pkg/notes/notes.go @@ -28,6 +28,7 @@ import ( "net/http" "net/url" "regexp" + "slices" "sort" "strconv" "strings" @@ -1031,25 +1032,26 @@ func (g *Gatherer) prsForCommitFromSHA(sha string) (prs []*gogithub.PullRequest, } func (g *Gatherer) prsForCommitFromMessage(commitMessage string) (prs []*gogithub.PullRequest, err error) { - mainPrNum, err := prNumForCommitFromMessage(commitMessage) + prsNum, err := prsNumForCommitFromMessage(commitMessage) if err != nil { return nil, err } - // Given the PR number that we've now converted to an integer, get the PR from - // the API - mainPr, err := g.getPr(mainPrNum) - if err != nil { - return prs, err - } - - prs = append(prs, mainPr) + for _, prNum := range prsNum { + // Given the PR number that we've now converted to an integer, get the PR from + // the API + pr, err := g.getPr(prNum) + if err != nil { + return prs, err + } + prs = append(prs, pr) - originPrNum, origPrErr := originPrNumFromPr(mainPr) - if origPrErr == nil { - originPr, err := g.getPr(originPrNum) - if err == nil { - prs = append(prs, originPr) + originPrNum, origPrErr := originPrNumFromPr(pr) + if origPrErr == nil && slices.Index(prsNum, originPrNum) == -1 { + originPr, err := g.getPr(originPrNum) + if err == nil { + prs = append(prs, originPr) + } } } @@ -1074,25 +1076,34 @@ var ( // stops being true, this definitely won't work anymore. regexMergedPR = regexp.MustCompile(`Merge pull request #(?P\d+)`) - // If the PR was squash merged, the regexp is different + // If the PR was squash merged, the regexp is different. regexSquashPR = regexp.MustCompile(`\(#(?P\d+)\)`) - // The branch name created by "hack/cherry_pick_pull.sh" + // The branch name created by "hack/cherry_pick_pull.sh". regexHackBranch = regexp.MustCompile(`automated-cherry-pick-of-#(?P\d+)`) - // The branch name created by k8s-infra-cherrypick-robot + // The branch name created by k8s-infra-cherrypick-robot. regexProwBranch = regexp.MustCompile(`cherry-pick-(?P\d+)-to`) ) -func prNumForCommitFromMessage(commitMessage string) (prs int, err error) { +func prsNumForCommitFromMessage(commitMessage string) (prs []int, err error) { if pr := prForRegex(regexMergedPR, commitMessage); pr != 0 { - return pr, nil + prs = append(prs, pr) } if pr := prForRegex(regexSquashPR, commitMessage); pr != 0 { - return pr, nil + prs = append(prs, pr) + } + + if pr := prForRegex(regexHackBranch, commitMessage); pr != 0 { + prs = append(prs, pr) + } + + if len(prs) == 0 { + return nil, errNoPRIDFoundInCommitMessage + } else { + return prs, nil } - return 0, errNoPRIDFoundInCommitMessage } func originPrNumFromPr(pr *gogithub.PullRequest) (origPRNum int, err error) { diff --git a/pkg/notes/notes_gatherer_test.go b/pkg/notes/notes_gatherer_test.go index 5f42628a50a2..d06abafdb2a4 100644 --- a/pkg/notes/notes_gatherer_test.go +++ b/pkg/notes/notes_gatherer_test.go @@ -291,10 +291,15 @@ func TestGatherNotes(t *testing.T) { "when commit messages hold PR numbers, we use those and don't query to get a list of PRs for a SHA": { commitList: []*github.RepositoryCommit{ repoCommit("123", "there is the message Merge pull request #123 somewhere in the middle"), - repoCommit("124", "and lastly in parens (#124) yeah"), + repoCommit("124", "some automated-cherry-pick-of-#124 can be found too"), + repoCommit("125", "and lastly in parens (#125) yeah"), + repoCommit("126", `all three together + some Merge pull request #126 and + another automated-cherry-pick-of-#127 with + a thing (#128) in parens`), }, getPullRequestStubber: func(t *testing.T) getPullRequestStub { - seenPRs := newIntsRecorder(123, 124) + seenPRs := newIntsRecorder(123, 124, 125, 126, 127, 128) return func(_ context.Context, org, repo string, prID int) (*github.PullRequest, *github.Response, error) { checkOrgRepo(t, org, repo) @@ -304,7 +309,7 @@ func TestGatherNotes(t *testing.T) { return nil, nil, nil } }, - expectedGetPullRequestCallCount: 2, + expectedGetPullRequestCallCount: 6, }, "when the PR is a cherry pick": { diff --git a/pkg/notes/notes_test.go b/pkg/notes/notes_test.go index 56ae939cc239..f514d5d1784d 100644 --- a/pkg/notes/notes_test.go +++ b/pkg/notes/notes_test.go @@ -223,12 +223,12 @@ func TestGetPRNumberFromCommitMessage(t *testing.T) { for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { - pr, err := prNumForCommitFromMessage(tc.commitMessage) + prs, err := prsNumForCommitFromMessage(tc.commitMessage) if err != nil { t.Fatalf("Expected no error to occur but got %v", err) } - if pr != tc.expectedPRNumber { - t.Errorf("Expected PR number to be %d but was %d", tc.expectedPRNumber, pr) + if prs[0] != tc.expectedPRNumber { + t.Errorf("Expected PR number to be %d but was %d", tc.expectedPRNumber, prs[0]) } }) } diff --git a/pkg/notes/notes_v2.go b/pkg/notes/notes_v2.go index c6ac8ede875c..c99d6cd66d3c 100644 --- a/pkg/notes/notes_v2.go +++ b/pkg/notes/notes_v2.go @@ -280,7 +280,7 @@ func (g *Gatherer) listLeftParentCommits(opts *options.Options) ([]*commitPrPair } // Find and collect PR number from commit message - prNum, err := prNumForCommitFromMessage(commitPointer.Message) + prNums, err := prsNumForCommitFromMessage(commitPointer.Message) if err == errNoPRIDFoundInCommitMessage { logrus.WithFields(logrus.Fields{ "sha": hashString, @@ -301,11 +301,11 @@ func (g *Gatherer) listLeftParentCommits(opts *options.Options) ([]*commitPrPair } logrus.WithFields(logrus.Fields{ "sha": hashString, - "pr": prNum, + "prs": prNums, }).Debug("found PR from commit") // Only taking the first one, assuming they are merged by Prow - pairs = append(pairs, &commitPrPair{Commit: commitPointer, PrNum: prNum}) + pairs = append(pairs, &commitPrPair{Commit: commitPointer, PrNum: prNums[0]}) // Advance pointer based on left parent hashPointer = commitPointer.ParentHashes[0]