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.
openshift · Nov 8, 2024 · dd6ad66 · dd6ad66
1 parent e273349
commit dd6ad66
Show file tree

Hide file tree

Showing 2 changed files with 36 additions and 4 deletions.
diff --git 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
@@ -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>`