From dd6ad66f5fde9bc5765c871d428b174040f21fd6 Mon Sep 17 00:00:00 2001
From: Patrick Ohly <patrick.ohly@intel.com>
Date: Fri, 8 Nov 2024 15:12:53 +0100
Subject: [PATCH] prune-junit-xml: avoid appending semicolon

Appending a semicolon after some text is unnecessary if it's the last
entry. This led to visually distracting extra semicolons in Spyglass which
looked like a bug in Spyglass.

Now the code checks if a semicolon is necessary before inserting it.
---
 cmd/prune-junit-xml/prunexml.go      | 18 +++++++++++++++---
 cmd/prune-junit-xml/prunexml_test.go | 22 +++++++++++++++++++++-
 2 files changed, 36 insertions(+), 4 deletions(-)

diff --git a/cmd/prune-junit-xml/prunexml.go b/cmd/prune-junit-xml/prunexml.go
index 26df0d06d28..5da06e3b235 100644
--- a/cmd/prune-junit-xml/prunexml.go
+++ b/cmd/prune-junit-xml/prunexml.go
@@ -139,9 +139,9 @@ func pruneTESTS(suites *junitxml.JUnitTestSuites) {
 			// The top level testcase element in a JUnit xml file does not have the / character.
 			if testcase.Failure != nil {
 				failflag = true
-				updatedTestcaseFailure.Message = updatedTestcaseFailure.Message + testcase.Failure.Message + ";"
-				updatedTestcaseFailure.Contents = updatedTestcaseFailure.Contents + testcase.Failure.Contents + ";"
-				updatedTestcaseFailure.Type = updatedTestcaseFailure.Type + testcase.Failure.Type
+				updatedTestcaseFailure.Message = joinTexts(updatedTestcaseFailure.Message, testcase.Failure.Message)
+				updatedTestcaseFailure.Contents = joinTexts(updatedTestcaseFailure.Contents, testcase.Failure.Contents)
+				updatedTestcaseFailure.Type = joinTexts(updatedTestcaseFailure.Type, testcase.Failure.Type)
 			}
 		}
 		if failflag {
@@ -153,6 +153,18 @@ func pruneTESTS(suites *junitxml.JUnitTestSuites) {
 	suites.Suites = updatedTestsuites
 }
 
+// joinTexts returns "<a>; <b>" if both are non-empty,
+// otherwise just the non-empty string, if there is one.
+func joinTexts(a, b string) string {
+	if a == "" {
+		return b
+	}
+	if b == "" {
+		return a
+	}
+	return a + "; " + b
+}
+
 func fetchXML(xmlReader io.Reader) (*junitxml.JUnitTestSuites, error) {
 	decoder := xml.NewDecoder(xmlReader)
 	var suites junitxml.JUnitTestSuites
diff --git a/cmd/prune-junit-xml/prunexml_test.go b/cmd/prune-junit-xml/prunexml_test.go
index 4737e3feb5a..6d40f5e8a9c 100644
--- a/cmd/prune-junit-xml/prunexml_test.go
+++ b/cmd/prune-junit-xml/prunexml_test.go
@@ -96,6 +96,18 @@ func TestPruneTESTS(t *testing.T) {
 			<failure message="Failed" type="">FailureContent</failure>
 		</testcase>
 	</testsuite>
+	<testsuite tests="3" failures="2" time="30.050000" name="k8s.io/kubernetes/test/integration/apimachinery2" timestamp="">
+		<properties>
+			<property name="go.version" value="go1.18 linux/amd64"></property>
+		</properties>
+		<testcase classname="k8s.io/kubernetes/test/integration/apimachinery2" name="TestWatchRestartsIfTimeoutNotReached/group/InformerWatcher_survives_closed_watches" time="30.050000"></testcase>
+		<testcase classname="k8s.io/kubernetes/test/integration/apimachinery2" name="TestSchedulerInformers" time="-0.000000">
+			<failure message="FailedA" type="">FailureContentA</failure>
+		</testcase>
+		<testcase classname="k8s.io/kubernetes/test/integration/apimachinery2" name="TestSchedulerInformers2" time="-0.000000">
+			<failure message="FailedB" type="">FailureContentB</failure>
+		</testcase>
+	</testsuite>
 </testsuites>`
 
 	outputXML := `<?xml version="1.0" encoding="UTF-8"?>
@@ -111,7 +123,15 @@ func TestPruneTESTS(t *testing.T) {
 			<property name="go.version" value="go1.18 linux/amd64"></property>
 		</properties>
 		<testcase classname="k8s.io/kubernetes/test/integration" name="apimachinery" time="30.050000">
-			<failure message="Failed;" type="">FailureContent;</failure>
+			<failure message="Failed" type="">FailureContent</failure>
+		</testcase>
+	</testsuite>
+	<testsuite tests="3" failures="2" time="30.050000" name="k8s.io/kubernetes/test/integration/apimachinery2" timestamp="">
+		<properties>
+			<property name="go.version" value="go1.18 linux/amd64"></property>
+		</properties>
+		<testcase classname="k8s.io/kubernetes/test/integration" name="apimachinery2" time="30.050000">
+			<failure message="FailedA; FailedB" type="">FailureContentA; FailureContentB</failure>
 		</testcase>
 	</testsuite>
 </testsuites>`