Ensure release-notes tool doesn't skip automated cherry picks

pkg/notes/notes.go

@@ -619,29 +619,17 @@ var noteExclusionFilters = []*regexp.Regexp{
 	// 'none','n/a','na' case insensitive with optional trailing
 	// whitespace, wrapped in ``` with/without release-note identifier
 	// the 'none','n/a','na' can also optionally be wrapped in quotes ' or "
-	regexp.MustCompile("(?i)```release-notes?\\s*('\")?(none|n/a|na)?('\")?\\s*```"),
+	regexp.MustCompile("(?i)```release-notes?\\s*('\")?(none|n/a|na)('\")?\\s*```"),
 
 	// simple '/release-note-none' tag
 	regexp.MustCompile("/release-note-none"),
 }
 
-// Similarly, now that the known not-release-notes are filtered out, we can
-// use some patterns to find actual release notes.
-var noteInclusionFilters = []*regexp.Regexp{
-	regexp.MustCompile("release-note"),
-	regexp.MustCompile("Does this PR introduce a user-facing change?"),
-}
-
 // MatchesExcludeFilter returns true if the string matches an excluded release note
 func MatchesExcludeFilter(msg string) bool {
 	return matchesFilter(msg, noteExclusionFilters)
 }
 
-// MatchesIncludeFilter returns true if the string matches an included release note
-func MatchesIncludeFilter(msg string) bool {
-	return matchesFilter(msg, noteInclusionFilters)
-}
-
 func matchesFilter(msg string, filters []*regexp.Regexp) bool {
 	for _, filter := range filters {
 		if filter.MatchString(msg) {
@@ -732,12 +720,34 @@ func (g *Gatherer) notesForCommit(commit *gogithub.RepositoryCommit) (*Result, e
 			"Got PR #%d for commit: %s", pr.GetNumber(), commit.GetSHA(),
 		)
 
-		if MatchesIncludeFilter(prBody) {
+		// If we match exclusion filter (release-note-none), we don't look further,
+		// instead we return that PR. We return that PR because it might have a map,
+		// which is checked after this function returns
+		if MatchesExcludeFilter(prBody) {
+			res := &Result{commit: commit, pullRequest: pr}
+			logrus.Infof("PR #%d contains exclusion (release-note-none)", pr.GetNumber())
+
+			return res, nil
+		}
+
+		// If we didn't match the exclusion filter, try to extract the release note from the PR.
+		// If we can't extract the release note, consider that the PR is invalid and take the next one
+		s, err := noteTextFromString(prBody)
+		if err != nil {
+			logrus.Infof("PR #%d does not seem to contain a valid release note, skipping", pr.GetNumber())
+
+			continue
+		}
+
+		// If we found a valid release note, return the PR, otherwise, take the next one
+		if len(s) > 0 {
 			res := &Result{commit: commit, pullRequest: pr}
 			logrus.Infof("PR #%d seems to contain a release note", pr.GetNumber())
 			// Do not test further PRs for this commit as soon as one PR matched
 			return res, nil
 		}
+
+		logrus.Infof("PR #%d does not seem to contain a valid release note, skipping", pr.GetNumber())
 	}
 
 	return nil, nil

pkg/notes/notes_gatherer_test.go

@@ -335,26 +335,23 @@ func TestGatherNotes(t *testing.T) {
 			listPullRequestsWithCommitStubber: func(t *testing.T) listPullRequestsWithCommitStub {
 				prsPerCall := [][]*github.PullRequest{
 					// explicitly excluded
-					{pullRequest(1, "something ```release-note N/a``` something")},
-					{pullRequest(2, "something ```release-note Na``` something")},
-					{pullRequest(3, "something ```release-note None ``` something")},
-					{pullRequest(4, "something ```release-note 'None' ``` something")},
+					{pullRequest(1, "something ```release-note\nN/a\n``` something")},
+					{pullRequest(2, "something ```release-note\nNa\n``` something")},
+					{pullRequest(3, "something ```release-note\nNone \n``` something")},
+					{pullRequest(4, "something ```release-note\n'None' \n``` something")},
 					{pullRequest(5, "something /release-note-none something")},
-					// explicitly included
-					{pullRequest(6, "something release-note something")},
-					{pullRequest(7, "something Does this PR introduce a user-facing change something")},
 					// multiple PRs
 					{ // first does no match, second one matches, rest is ignored
-						pullRequest(8, ""),
-						pullRequest(9, " Does this PR introduce a user-facing change"),
-						pullRequest(10, "does-not-matter--is-not-considered"),
+						pullRequest(6, ""),
+						pullRequest(7, " something ```release-note\nTest\n``` something"),
+						pullRequest(8, "does-not-matter--is-not-considered"),
 					},
 					// some other strange things
-					{pullRequest(11, "Does this PR introduce a user-facing chang")}, // matches despite the missing 'e'
-					{pullRequest(12, "release-note /release-note-none")},            // excluded, the exclusion filters take precedence
-					{pullRequest(13, "```release-note NAAAAAAAAAA```")},             // included, does not match the N/A filter, but the 'release-note' check
-					{pullRequest(14, "```release-note none something ```")},         // included, does not match the N/A filter, but the 'release-note' check
-					{pullRequest(15, "release-noteNOOOO")},                          // included
+					{pullRequest(9, "release-note /release-note-none")},       // excluded, the exclusion filters take precedence
+					{pullRequest(10, "```release-note\nNAAAAAAAAAA\n```")},    // included, does not match the N/A filter, but the 'release-note' check
+					{pullRequest(11, "```release-note\nnone something\n```")}, // included, does not match the N/A filter, but the 'release-note' check
+					// empty release note block shouldn't be matched
+					{pullRequest(12, "```release-note\n\n```")},
 				}
 				var callCount int64 = -1
 
@@ -370,7 +367,7 @@ func TestGatherNotes(t *testing.T) {
 			resultsChecker: func(t *testing.T, results []*Result) {
 				// there is not much we can check on the Result, as all the fields are
 				// unexported
-				expectedResultSize := 13
+				expectedResultSize := 9
 				if e, a := expectedResultSize, len(results); e != a {
 					t.Errorf("Expected the result to be of size %d, got %d", e, a)
 				}