From 1c3321afb725cd89830d7adac2cfd4ca4b921cb4 Mon Sep 17 00:00:00 2001
From: Jonathan Grynspan <jgrynspan@apple.com>
Date: Mon, 30 Sep 2024 10:56:17 -0400
Subject: [PATCH] Do not emit a `time=` attribute in JUnit output for skipped
 tests.

Resolves #740.
---
 .../Recorder/Event.JUnitXMLRecorder.swift     | 15 +++++++++----
 Tests/TestingTests/EventRecorderTests.swift   | 22 +++++++++++++++++++
 2 files changed, 33 insertions(+), 4 deletions(-)

diff --git a/Sources/Testing/Events/Recorder/Event.JUnitXMLRecorder.swift b/Sources/Testing/Events/Recorder/Event.JUnitXMLRecorder.swift
index f8d088f8b..0b9b283e3 100644
--- a/Sources/Testing/Events/Recorder/Event.JUnitXMLRecorder.swift
+++ b/Sources/Testing/Events/Recorder/Event.JUnitXMLRecorder.swift
@@ -176,8 +176,15 @@ extension Event.JUnitXMLRecorder {
       let classNameComponents = CollectionOfOne(id.moduleName) + id.nameComponents.dropLast()
       let className = classNameComponents.joined(separator: ".")
       let name = id.nameComponents.last!
-      let durationNanoseconds = testData.startInstant.nanoseconds(until: testData.endInstant ?? .now)
-      let durationSeconds = Double(durationNanoseconds) / 1_000_000_000
+
+      // Tests that are skipped or for some reason never completed will not have
+      // an end instant; don't report timing for such tests.
+      var timeClause = ""
+      if let endInstant = testData.endInstant {
+        let durationNanoseconds = testData.startInstant.nanoseconds(until: endInstant)
+        let durationSeconds = Double(durationNanoseconds) / 1_000_000_000
+        timeClause = #"time="\#(durationSeconds)" "#
+      }
 
       // Build out any child nodes contained within this <testcase> node.
       var minutiae = [String]()
@@ -193,9 +200,9 @@ extension Event.JUnitXMLRecorder {
       }
 
       if minutiae.isEmpty {
-        result.append(#"    <testcase classname="\#(className)" name="\#(name)" time="\#(durationSeconds)" />"#)
+        result.append(#"    <testcase classname="\#(className)" name="\#(name)" \#(timeClause)/>"#)
       } else {
-        result.append(#"    <testcase classname="\#(className)" name="\#(name)" time="\#(durationSeconds)">"#)
+        result.append(#"    <testcase classname="\#(className)" name="\#(name)" \#(timeClause)>"#)
         result += minutiae
         result.append(#"    </testcase>"#)
       }
diff --git a/Tests/TestingTests/EventRecorderTests.swift b/Tests/TestingTests/EventRecorderTests.swift
index 525e9d252..97619b755 100644
--- a/Tests/TestingTests/EventRecorderTests.swift
+++ b/Tests/TestingTests/EventRecorderTests.swift
@@ -354,6 +354,28 @@ struct EventRecorderTests {
       throw caughtError
     }
   }
+
+  @Test(
+    "JUnit XML omits time for skipped tests",
+    .bug("https://github.com/swiftlang/swift-testing/issues/740")
+  )
+  func junitXMLWithTimelessSkippedTest() async throws {
+    let stream = Stream()
+
+    let eventRecorder = Event.JUnitXMLRecorder(writingUsing: stream.write)
+    eventRecorder.record(Event(.runStarted, testID: nil, testCaseID: nil), in: Event.Context(test: nil, testCase: nil, configuration: nil))
+    let test = Test {}
+    eventRecorder.record(Event(.testSkipped(.init(sourceContext: .init())), testID: test.id, testCaseID: nil), in: Event.Context(test: test, testCase: nil, configuration: nil))
+    eventRecorder.record(Event(.runEnded, testID: nil, testCaseID: nil), in: Event.Context(test: nil, testCase: nil, configuration: nil))
+
+    let xmlString = stream.buffer.rawValue
+    #expect(xmlString.hasPrefix("<?xml"))
+    let testCaseLines = xmlString
+      .split(whereSeparator: \.isNewline)
+      .filter { $0.contains("<testcase") }
+    #expect(!testCaseLines.isEmpty)
+    #expect(!testCaseLines.contains { $0.contains("time=") })
+  }
 #endif
 
   @Test("HumanReadableOutputRecorder counts issues without associated tests")