From b1826016d1f464747508233c7b1c9a9236663e86 Mon Sep 17 00:00:00 2001 From: npolshakova Date: Thu, 18 Jul 2024 08:36:02 -0700 Subject: [PATCH] fix empty list render --- .../{unit => applymap-unit-test}/maps.yaml | 0 .../maps/testdata/no-sigs-unit-test/map.yaml | 8 ++ pkg/notes/notes.go | 4 +- pkg/notes/notes_map_test.go | 6 +- pkg/notes/notes_test.go | 79 ++++++++++++------- 5 files changed, 65 insertions(+), 32 deletions(-) rename pkg/notes/maps/testdata/{unit => applymap-unit-test}/maps.yaml (100%) create mode 100644 pkg/notes/maps/testdata/no-sigs-unit-test/map.yaml diff --git a/pkg/notes/maps/testdata/unit/maps.yaml b/pkg/notes/maps/testdata/applymap-unit-test/maps.yaml similarity index 100% rename from pkg/notes/maps/testdata/unit/maps.yaml rename to pkg/notes/maps/testdata/applymap-unit-test/maps.yaml diff --git a/pkg/notes/maps/testdata/no-sigs-unit-test/map.yaml b/pkg/notes/maps/testdata/no-sigs-unit-test/map.yaml new file mode 100644 index 00000000000..d12baa42a74 --- /dev/null +++ b/pkg/notes/maps/testdata/no-sigs-unit-test/map.yaml @@ -0,0 +1,8 @@ +pr: 95000 +releasenote: + text: "This test string has been modified" +datafields: + testcase: + property: "Markdown" + expected: "This test string has been modified (#95000, @arghya88)" + name: "Modifying release note markdown" \ No newline at end of file diff --git a/pkg/notes/notes.go b/pkg/notes/notes.go index 605b7f5ed08..b0782563076 100644 --- a/pkg/notes/notes.go +++ b/pkg/notes/notes.go @@ -863,7 +863,7 @@ func (g *Gatherer) prsFromCommit(commit *gogithub.RepositoryCommit) ( // advantage of this to contextualize release note generation with the kind, sig, // area, etc labels. func labelsWithPrefix(pr *gogithub.PullRequest, prefix string) []string { - labels := []string{} + var labels []string for _, label := range pr.Labels { if strings.HasPrefix(*label.Name, prefix) { labels = append(labels, strings.TrimPrefix(*label.Name, prefix+"/")) @@ -1226,7 +1226,7 @@ func (rn *ReleaseNote) ApplyMap(noteMap *ReleaseNotesMap, markdownLinks bool) er indented, rn.PrNumber, rn.PrURL, rn.Author, rn.AuthorURL) } // Add sig labels to markdown - if rn.SIGs != nil { + if len(rn.SIGs) > 1 { markdown = fmt.Sprintf("%s [%s]", markdown, prettifySIGList(rn.SIGs)) } // Uppercase the first character of the markdown to make it look uniform diff --git a/pkg/notes/notes_map_test.go b/pkg/notes/notes_map_test.go index e6234a488fc..8e13cfb9229 100644 --- a/pkg/notes/notes_map_test.go +++ b/pkg/notes/notes_map_test.go @@ -27,7 +27,7 @@ func TestNewProviderFromInitString(t *testing.T) { initString string returnsError bool }{ - {initString: "maps/testdata/unit/", returnsError: false}, + {initString: "maps/testdata/applymap-unit-test/", returnsError: false}, {initString: "/this/shoud/not/really.exist/as/a/d33rect0ree", returnsError: true}, {initString: "gs://bucket-name/map/path/", returnsError: true}, {initString: "github://kubernetes/sig-release/maps", returnsError: true}, @@ -44,7 +44,7 @@ func TestNewProviderFromInitString(t *testing.T) { } func TestParseReleaseNotesMap(t *testing.T) { - maps, err := ParseReleaseNotesMap("maps/testdata/unit/maps.yaml") + maps, err := ParseReleaseNotesMap("maps/testdata/applymap-unit-test/maps.yaml") require.Nil(t, err) require.GreaterOrEqual(t, 6, len(*maps)) @@ -59,7 +59,7 @@ func TestGetMapsForPR(t *testing.T) { maps, err := provider.GetMapsForPR(95000) require.Nil(t, err) - require.GreaterOrEqual(t, 6, len(maps)) + require.GreaterOrEqual(t, 7, len(maps)) maps, err = provider.GetMapsForPR(123) require.Nil(t, err) diff --git a/pkg/notes/notes_test.go b/pkg/notes/notes_test.go index 69c3bb125bc..f514d5d1784 100644 --- a/pkg/notes/notes_test.go +++ b/pkg/notes/notes_test.go @@ -354,30 +354,10 @@ https://github.com/kubernetes/website/pull/19630 } } -func TestApplyMap(t *testing.T) { - makeNewNote := func() ReleaseNote { - return ReleaseNote{ - Commit: "078b355da3cf56668ca1a8a5e36f2b3b52ff1bd8", - Text: "[ACTION REQUIRED] scheduler alpha metrics binding_duration_seconds and scheduling_algorithm_preemption_evaluation_seconds are deprecated, Both of those metrics are now covered as part of framework_extension_point_duration_seconds, the former as a PostFilter the latter and a Bind plugin. The plan is to remove both in 1.21", - Markdown: "[ACTION REQUIRED] scheduler alpha metrics binding_duration_seconds and scheduling_algorithm_preemption_evaluation_seconds are deprecated, Both of those metrics are now covered as part of framework_extension_point_duration_seconds, the former as a PostFilter the latter and a Bind plugin. The plan is to remove both in 1.21", - // Documentation: documentation, - Author: "arghya88", - AuthorURL: "https://github.com/arghya88", - PrURL: "https://github.com/kubernetes/kubernetes/pull/95001", - PrNumber: 95000, - SIGs: []string{"instrumentation", "scheduling"}, - Kinds: []string{"deprecation", "feature"}, - Areas: []string{}, - Feature: true, - Duplicate: false, - DuplicateKind: false, - ActionRequired: true, - } - } - +func testApplyMapHelper(t *testing.T, testDir string, makeNewNote func() *ReleaseNote) { reflectedOriginalNote := reflect.ValueOf(makeNewNote()) - mp, err := NewProviderFromInitString("maps/testdata/unit/") + mp, err := NewProviderFromInitString(testDir) require.Nil(t, err) // Read the maps from out test directory @@ -422,8 +402,8 @@ func TestApplyMap(t *testing.T) { actualVal := reflect.Indirect(reflectedNote).FieldByName(property).String() require.Equalf(t, expectedValue, actualVal, "Failed %s", testName) require.NotEqualf(t, expectedValue, originalVal.String(), "Failed %s", testName) - // Handle string slice cases case []interface{}: + // Handle string slice cases actualVal := reflect.Indirect(reflectedNote).FieldByName(property) actualSlice := make([]string, 0) for i := range actualVal.Len() { @@ -431,14 +411,59 @@ func TestApplyMap(t *testing.T) { } require.ElementsMatchf(t, expectedValue, actualSlice, "Failed %s", testName) default: - require.FailNowf( - t, "Unknown case", "Unable to handle case for %s %T", - property, testcase.(map[interface{}]interface{})["expected"], - ) + require.FailNowf(t, "Unknown case", "Unable to handle case for %s %T", property, expectedValue) } } } +func TestApplyMap(t *testing.T) { + makeNewNote := func() *ReleaseNote { + return &ReleaseNote{ + Commit: "078b355da3cf56668ca1a8a5e36f2b3b52ff1bd8", + Text: "[ACTION REQUIRED] scheduler alpha metrics binding_duration_seconds and scheduling_algorithm_preemption_evaluation_seconds are deprecated, Both of those metrics are now covered as part of framework_extension_point_duration_seconds, the former as a PostFilter the latter and a Bind plugin. The plan is to remove both in 1.21", + Markdown: "[ACTION REQUIRED] scheduler alpha metrics binding_duration_seconds and scheduling_algorithm_preemption_evaluation_seconds are deprecated, Both of those metrics are now covered as part of framework_extension_point_duration_seconds, the former as a PostFilter the latter and a Bind plugin. The plan is to remove both in 1.21", + // Documentation: documentation, + Author: "arghya88", + AuthorURL: "https://github.com/arghya88", + PrURL: "https://github.com/kubernetes/kubernetes/pull/95001", + PrNumber: 95000, + SIGs: []string{"instrumentation", "scheduling"}, + Kinds: []string{"deprecation", "feature"}, + Areas: []string{}, + Feature: true, + Duplicate: false, + DuplicateKind: false, + ActionRequired: true, + } + } + + testApplyMapHelper(t, "maps/testdata/applymap-unit-test/", makeNewNote) +} + +func TestApplyMapNoSigs(t *testing.T) { + makeNewNote := func() *ReleaseNote { + return &ReleaseNote{ + Commit: "078b355da3cf56668ca1a8a5e36f2b3b52ff1bd8", + Text: "[ACTION REQUIRED] scheduler alpha metrics binding_duration_seconds and scheduling_algorithm_preemption_evaluation_seconds are deprecated, Both of those metrics are now covered as part of framework_extension_point_duration_seconds, the former as a PostFilter the latter and a Bind plugin. The plan is to remove both in 1.21", + Markdown: "[ACTION REQUIRED] scheduler alpha metrics binding_duration_seconds and scheduling_algorithm_preemption_evaluation_seconds are deprecated, Both of those metrics are now covered as part of framework_extension_point_duration_seconds, the former as a PostFilter the latter and a Bind plugin. The plan is to remove both in 1.21", + // Documentation: documentation, + Author: "arghya88", + AuthorURL: "https://github.com/arghya88", + PrURL: "https://github.com/kubernetes/kubernetes/pull/95001", + PrNumber: 95000, + SIGs: []string{}, + Kinds: []string{"deprecation", "feature"}, + Areas: []string{}, + Feature: true, + Duplicate: false, + DuplicateKind: false, + ActionRequired: true, + } + } + + testApplyMapHelper(t, "maps/testdata/no-sigs-unit-test/", makeNewNote) +} + func TestDashify(t *testing.T) { t.Parallel()