Merge pull request #3686 from npolshakova/fix-empty-sig-label-map-rep…

…lacement

Fix empty sig list render for markdown

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"

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

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)

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,23 +402,68 @@ 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() {
 				actualSlice = append(actualSlice, actualVal.Index(i).String())
 			}
 			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()