Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix empty release note parsing #3764

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 11 additions & 1 deletion pkg/notes/notes.go
Original file line number Diff line number Diff line change
Expand Up @@ -372,6 +372,16 @@ func (g *Gatherer) ListReleaseNotes() (*ReleaseNotes, error) {
// may contain the commit message, the PR description, etc.
// This is generally the content inside the ```release-note ``` stanza.
func noteTextFromString(s string) (string, error) {
// check release note is not empty
// Matches "release-notes" block with no meaningful content (ex. only whitespace, empty, just newlines)
emptyExps := []*regexp.Regexp{
regexp.MustCompile("(?i)```release-notes?\\s*```\\s*"),
}

if matchesFilter(s, emptyExps) {
return "", errors.New("empty release note")
}

exps := []*regexp.Regexp{
// (?s) is needed for '.' to be matching on newlines, by default that's disabled
// we need to match ungreedy 'U', because after the notes a `docs` block can occur
Expand Down Expand Up @@ -627,7 +637,7 @@ func (l *commitList) List() []*gogithub.RepositoryCommit {
// that do NOT contain release notes. Notably, this is all of the variations of
// "release note none" that appear in the commit log.
var noteExclusionFilters = []*regexp.Regexp{
// 'none','n/a','na' case insensitive with optional trailing
// '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*```"),
Expand Down
6 changes: 4 additions & 2 deletions pkg/notes/notes_gatherer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -351,9 +351,11 @@ func TestGatherNotes(t *testing.T) {
{pullRequest(9, "release-note /release-note-none", "closed")}, // excluded, the exclusion filters take precedence
{pullRequest(10, "```release-note\nNAAAAAAAAAA\n```", "closed")}, // included, does not match the N/A filter, but the 'release-note' check
{pullRequest(11, "```release-note\nnone something\n```", "closed")}, // included, does not match the N/A filter, but the 'release-note' check
// empty release note block shouldn't be matched
// empty release note block should skipped because noteTextFromString returns an error
{pullRequest(12, "```release-note\n\n```", "closed")},
{pullRequest(13, "```release-note\n\n```", "open")},
{pullRequest(13, "```release-note```", "closed")},
{pullRequest(14, "```release-note ```", "closed")},
{pullRequest(15, "```release-note\n\n```", "open")},
}
var callCount int64 = -1

Expand Down
78 changes: 63 additions & 15 deletions pkg/notes/notes_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -297,6 +297,16 @@ func TestNoteTextFromString(t *testing.T) {
require.Equal(t, "item\nitem\n- item\n item", res)
},
},
{
noteBlock(
"",
),
func(res string, err error) {
require.NotNil(t, err)
require.Contains(t, err.Error(), "empty release note")
require.Equal(t, "", res)
},
},
} {
tc.expect(noteTextFromString(tc.input))
}
Expand Down Expand Up @@ -328,25 +338,63 @@ func TestMatchesExcludeFilter(t *testing.T) {
shouldExclude: false,
},
{
input: `@kubernetes/sig-auth-pr-reviews
/milestone v1.19
/priority important-longterm
/kind cleanup
/kind deprecation

xref: #81126
xref: #81152
xref: https://github.com/kubernetes/website/pull/19630

input: `@kubernetes/sig-auth-pr-reviews
/milestone v1.19
/priority important-longterm
/kind cleanup
/kind deprecation

xref: #81126
xref: #81152
xref: https://github.com/kubernetes/website/pull/19630

` + mdSep + `release-note
Action Required: Support for basic authentication via the --basic-auth-file flag has been removed. Users should migrate to --token-auth-file for similar functionality.
` + mdSep + `

` + mdSep + `docs
Removed "Static Password File" section from https://kubernetes.io/docs/reference/access-authn-authz/authentication/#static-password-file
https://github.com/kubernetes/website/pull/19630
` + mdSep,
shouldExclude: false,
},
{
input: `
#### Does this PR introduce a user-facing change?
<!--
If no, just write "NONE" in the release-note block below.
If yes, a release note is required:
Enter your extended release note in the block below. If the PR requires additional action from users switching to the new release, include the string "action required".

For more information on release notes see: https://git.k8s.io/community/contributors/guide/release-notes.md
-->
` + mdSep + `release-note
Action Required: Support for basic authentication via the --basic-auth-file flag has been removed. Users should migrate to --token-auth-file for similar functionality.

` + mdSep + `

#### Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.:

<!--
This section can be blank if this pull request does not require a release note.

When adding links which point to resources within git repositories, like
KEPs or supporting documentation, please reference a specific commit and avoid
linking directly to the master branch. This ensures that links reference a
specific point in time, rather than a document that may change over time.

See here for guidance on getting permanent links to files: https://help.github.com/en/articles/getting-permanent-links-to-files

Please use the following format for linking documentation:
- [KEP]: <link>
- [Usage]: <link>
- [Other doc]: <link>
-->
` + mdSep + `docs
Removed "Static Password File" section from https://kubernetes.io/docs/reference/access-authn-authz/authentication/#static-password-file
https://github.com/kubernetes/website/pull/19630
` + mdSep,
shouldExclude: false,

` + mdSep + `

`,
shouldExclude: false, // noteTextFromString will catch empty release notes
},
} {
res := MatchesExcludeFilter(tc.input)
Expand Down