Improve the support of JUnit XML report (#3135)

# JUnit XML Rework the JUnit XML reporting feature. After a couple of tests, the XML report output is not compliant with the "standard" I try to make it more compliant without breaking the great first solution! I took inspiration from the pseudo specification and SBT implementation. One important point is that Maven and SBT are producing one output file per `<testsuite>` a.k.a. per test (spec...) class while this solution (original) is producing one `<testsuites>` output file for the entire module. The specification supports it. We should keep this approach. In this PR: * Follow the "specification" provided in https://github.com/testmoapp/junitxml * Fix the failure/error reporting as there is not necessarily an exception and error message provided (optional) * Call the junit report from `testLocal` * Make sure all the test module types call the same code for junit report: `TestModule`, `ScalaJSModule`, `ScalaNativeModule` ## Resources * Specification (more or less): https://github.com/testmoapp/junitxml * Sbt Inspiration: https://github.com/sbt/sbt/blob/72bfb3f45ab346ddf3558bc23853dfbb9392cc55/testing/src/main/scala/sbt/JUnitXmlTestsListener.scala ## Maven output One output per test class: `target/surefire-reports/TEST-io.ultra.AppTest.xml` ``` <?xml version="1.0" encoding="UTF-8"?> <testsuite xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:noNamespaceSchemaLocation="https://maven.apache.org/surefire/maven-surefire-plugin/xsd/surefire-test-report.xsd" name="io.ultra.AppTest" time="0.009" tests="4" errors="1" skipped="0" failures="2"> <properties> <property name="awt.toolkit" value="sun.awt.X11.XToolkit"/> ... <property name="java.class.version" value="55.0"/> </properties> <testcase name="testApp" classname="io.ultra.AppTest" time="0"/> <testcase name="testFailAssertionApp" classname="io.ultra.AppTest" time="0.003"> <failure type="junit.framework.AssertionFailedError">junit.framework.AssertionFailedError at junit.framework.Assert.fail(Assert.java:47) at junit.framework.Assert.assertTrue(Assert.java:20) at junit.framework.Assert.assertTrue(Assert.java:27) at io.ultra.AppTest.testFailAssertionApp(AppTest.java:41) </failure> </testcase> <testcase name="testFailApp" classname="io.ultra.AppTest" time="0"> <failure type="junit.framework.AssertionFailedError">junit.framework.AssertionFailedError at junit.framework.Assert.fail(Assert.java:47) at junit.framework.Assert.fail(Assert.java:53) at io.ultra.AppTest.testFailApp(AppTest.java:46) </failure> </testcase> <testcase name="testErrorApp" classname="io.ultra.AppTest" time="0.001"> <error message="big error" type="java.lang.RuntimeException">java.lang.RuntimeException: big error at io.ultra.AppTest.testErrorApp(AppTest.java:51) </error> </testcase> </testsuite> ``` ## mill test output Examples vs SBT ### UTest ``` { "fullyQualifiedName": "foo.FooTests", "selector": "foo.FooTests.simple", "duration": 26, "status": "Success" }, { "fullyQualifiedName": "foo.FooTests", "selector": "foo.FooTests.escaping", "duration": 0, "status": "Success" } ``` ### ZIO test ``` { "fullyQualifiedName": "io.ultra.uniq.indexer.ATestSuite", "selector": "root - test-case-1", "duration": 16, "status": "Success" }, { "fullyQualifiedName": "io.ultra.uniq.indexer.ATestSuite", "selector": "root - sub-suite - test-case-2.1", "duration": 17, "status": "Success" }, { "fullyQualifiedName": "io.ultra.uniq.indexer.ATestSuite", "selector": "root - test-case-2", "duration": 18, "status": "Success" }, { "fullyQualifiedName": "io.ultra.uniq.indexer.ATestSuite", "selector": "root - sub-suite - test-case-2.2", "duration": 17, "status": "Success" }, ``` sbt output: ``` <?xml version='1.0' encoding='UTF-8'?> <testsuite hostname="ultra-lapttop-roro" name="io.ultra.uniq.indexer.ATestSuite" tests="4" errors="0" failures="0" skipped="0" time="1.045" timestamp="2024-04-24T17:27:38"> <properties> <property name="awt.toolkit" value="sun.awt.X11.XToolkit"/> ... <property name="jna.nosys" value="true"/> <property name="java.runtime.name" value="OpenJDK Runtime Environment"/> <property name="java.vm.name" value="OpenJDK 64-Bit Server VM"/> <property name="jna.platform.library.path" value="/usr/lib64:/lib64:/usr/lib:/lib:/usr/lib32:/usr/lib/opencollada:/opt/intel/oneapi/tbb/latest/lib/intel64/gcc4.8:/opt/intel/oneapi/compiler/latest/linux/lib:/opt/intel/oneapi/compiler/latest/linux/compiler/lib/intel64_lin"/> <property name="java.vendor.url.bug" value="https://github.com/adoptium/adoptium-support/issues"/> <property name="user.dir" value="/home/rogilles/sandbox/ultra/platform"/> <property name="os.arch" value="amd64"/> <property name="grouping.with.qualified.names.enabled" value="true"/> <property name="idea.managed" value="true"/> <property name="java.vm.info" value="mixed mode"/> <property name="java.vm.version" value="11.0.23+9"/> <property name="java.class.version" value="55.0"/> </properties> <testcase classname="io.ultra.uniq.indexer.ATestSuite" name="root - test-case-1" time="0.256"> </testcase> <testcase classname="io.ultra.uniq.indexer.ATestSuite" name="root - test-case-2" time="0.272"> </testcase> <testcase classname="io.ultra.uniq.indexer.ATestSuite" name="root - sub-suite - test-case-2.1" time="0.259"> </testcase> <testcase classname="io.ultra.uniq.indexer.ATestSuite" name="root - sub-suite - test-case-2.2" time="0.258"> </testcase> <system-out><![CDATA[]]></system-out> <system-err><![CDATA[]]></system-err> </testsuite> ``` ### scalatest #### FreeSpec ``` { "fullyQualifiedName": "io.ultra.cloudevent.ATestSuiteFreeSpec", "selector": "A Set when empty should have size 0", "duration": 2, "status": "Success" }, { "fullyQualifiedName": "io.ultra.cloudevent.ATestSuiteFreeSpec", "selector": "A Set when empty should produce NoSuchElementException when head is invoked", "duration": 1, "status": "Success" }, { "fullyQualifiedName": "io.ultra.cloudevent.ATestSuiteFreeSpec", "selector": "A Set when non-empty should have size > 0", "duration": 0, "status": "Success" } ``` sbt output: ``` <?xml version='1.0' encoding='UTF-8'?> <testsuite hostname="ultra-lapttop-roro" name="io.ultra.cloudevent.ATestSuiteFreeSpec" tests="3" errors="0" failures="0" skipped="0" time="0.016" timestamp="2024-04-24T18:08:19"> <properties> <property name="awt.toolkit" value="sun.awt.X11.XToolkit"/> <property name="java.specification.version" value="11"/> ... <property name="java.class.version" value="55.0"/> </properties> <testcase classname="io.ultra.cloudevent.ATestSuiteFreeSpec" name="A Set when empty should have size 0" time="0.014"> </testcase> <testcase classname="io.ultra.cloudevent.ATestSuiteFreeSpec" name="A Set when empty should produce NoSuchElementException when head is invoked" time="0.0"> </testcase> <testcase classname="io.ultra.cloudevent.ATestSuiteFreeSpec" name="A Set when non-empty should have size > 0" time="0.002"> </testcase> <system-out><![CDATA[]]></system-out> <system-err><![CDATA[]]></system-err> </testsuite> ``` #### FlatSpec ``` { "fullyQualifiedName": "io.ultra.cloudevent.ATestSuiteFlatSpec", "selector": "root should work 1", "duration": 16, "status": "Success" }, { "fullyQualifiedName": "io.ultra.cloudevent.ATestSuiteFlatSpec", "selector": "root should work 2", "duration": 0, "status": "Success" }, ``` sbt output: ``` <?xml version='1.0' encoding='UTF-8'?> <testsuite hostname="ultra-lapttop-roro" name="io.ultra.cloudevent.ATestSuiteFlatSpec" tests="2" errors="0" failures="0" skipped="0" time="0.017" timestamp="2024-04-24T18:08:19"> <properties> <property name="awt.toolkit" value="sun.awt.X11.XToolkit"/> ... <property name="java.class.version" value="55.0"/> </properties> <testcase classname="io.ultra.cloudevent.ATestSuiteFlatSpec" name="root should work 1" time="0.017"> </testcase> <testcase classname="io.ultra.cloudevent.ATestSuiteFlatSpec" name="root should work 2" time="0.0"> </testcase> <system-out><![CDATA[]]></system-out> <system-err><![CDATA[]]></system-err> </testsuite> ``` Pull request: #3135
com-lihaoyi · May 14, 2024 · 10086ad · 10086ad
1 parent 25d975d
commit 10086ad
Show file tree

Hide file tree

Showing 4 changed files with 132 additions and 60 deletions.
diff --git a/scalajslib/src/mill/scalajslib/ScalaJSModule.scala b/scalajslib/src/mill/scalajslib/ScalaJSModule.scala
@@ -373,7 +373,7 @@ trait TestScalaJSModule extends ScalaJSModule with TestModule {
       T.testReporter,
       TestRunnerUtils.globFilter(globSelectors())
     )
-    val res = TestModule.handleResults(doneMsg, results, Some(T.ctx()))
+    val res = TestModule.handleResults(doneMsg, results, T.ctx(), testReportXml())
     // Hack to try and let the Node.js subprocess finish streaming it's stdout
     // to the JVM. Without this, the stdout can still be streaming when `close()`
     // is called, and some of the output is dropped onto the floor.

diff --git a/scalalib/src/mill/scalalib/TestModule.scala b/scalalib/src/mill/scalalib/TestModule.scala
@@ -1,13 +1,18 @@
 package mill.scalalib
 
-import mill.{Agg, T}
-import mill.define.{Command, Task, TaskModule}
 import mill.api.{Ctx, PathRef, Result}
-import mill.util.Jvm
+import mill.define.{Command, Task, TaskModule}
 import mill.scalalib.bsp.{BspBuildTarget, BspModule}
 import mill.testrunner.{Framework, TestArgs, TestResult, TestRunner}
+import mill.util.Jvm
+import mill.{Agg, T}
 import sbt.testing.Status
 
+import java.time.format.DateTimeFormatter
+import java.time.temporal.ChronoUnit
+import java.time.{Instant, LocalDateTime, ZoneId}
+import scala.xml.Elem
+
 trait TestModule
     extends TestModule.JavaModuleBase
     with WithZincWorker
@@ -108,8 +113,6 @@ trait TestModule
       globSelectors: Task[Seq[String]]
   ): Task[(String, Seq[TestResult])] =
     T.task {
-      testReportXml().foreach(file => os.remove(T.ctx().dest / file))
-
       val outputPath = T.dest / "out.json"
       val useArgsFile = testUseArgsFile()
 
@@ -167,10 +170,10 @@ trait TestModule
       else
         try {
           val jsonOutput = ujson.read(outputPath.toIO)
-          val (doneMsg, results) =
+          val (doneMsg, results) = {
             upickle.default.read[(String, Seq[TestResult])](jsonOutput)
-          testReportXml().foreach(file => TestModule.genTestXmlReport(results, T.ctx().dest / file))
-          TestModule.handleResults(doneMsg, results, Some(T.ctx()))
+          }
+          TestModule.handleResults(doneMsg, results, T.ctx(), testReportXml())
         } catch {
           case e: Throwable =>
             Result.Failure("Test reporting failed: " + e)
@@ -189,7 +192,7 @@ trait TestModule
       args,
       T.testReporter
     )
-    TestModule.handleResults(doneMsg, results, Some(T.ctx()))
+    TestModule.handleResults(doneMsg, results, T.ctx(), testReportXml())
   }
 
   override def bspBuildTarget: BspBuildTarget = {
@@ -324,6 +327,19 @@ object TestModule {
     }
   }
 
+  def handleResults(
+      doneMsg: String,
+      results: Seq[TestResult],
+      ctx: Ctx.Env with Ctx.Dest,
+      testReportXml: Option[String],
+      props: Option[Map[String, String]] = None
+  ): Result[(String, Seq[TestResult])] = {
+    testReportXml.foreach(fileName =>
+      genTestXmlReport(results, ctx.dest / fileName, props.getOrElse(Map.empty))
+    )
+    handleResults(doneMsg, results, Some(ctx))
+  }
+
   trait JavaModuleBase extends BspModule {
     def ivyDeps: T[Agg[Dep]] = Agg.empty[Dep]
   }
@@ -332,68 +348,93 @@ object TestModule {
     def scalacOptions: T[Seq[String]] = Seq.empty[String]
   }
 
-  case class TestResultExtra(suiteName: String, testName: String, result: TestResult)
+  private def genTestXmlReport(
+      results0: Seq[TestResult],
+      out: os.Path,
+      props: Map[String, String]
+  ): Unit = {
+    val timestamp = DateTimeFormatter.ISO_LOCAL_DATE_TIME.format(
+      LocalDateTime.ofInstant(
+        Instant.now.truncatedTo(ChronoUnit.SECONDS),
+        ZoneId.systemDefault()
+      )
+    )
+    def durationAsString(value: Long) = (value / 1000d).toString
+    def testcaseName(testResult: TestResult) =
+      testResult.selector.replace(s"${testResult.fullyQualifiedName}.", "")
 
-  def genTestXmlReport(results0: Seq[TestResult], out: os.Path): Unit = {
-    val results = results0.map { r =>
-      val (suiteName, testName) = splitFullyQualifiedName(r.selector)
-      TestResultExtra(suiteName, testName, r)
+    def properties: Elem = {
+      val ps = props.map { case (key, value) =>
+        <property name={key} value={value}/>
+      }
+      <properties>
+        {ps}
+      </properties>
     }
 
-    val suites = results.groupMap(_.suiteName)(identity).map { case (suiteName, tests) =>
-      val cases = tests.map { test =>
-        val failure =
-          (test.result.exceptionName, test.result.exceptionMsg, test.result.exceptionTrace) match {
-            case (Some(name), Some(msg), Some(trace)) =>
-              Some(
-                <failure message={msg} type={name}>
-                  {
-                  trace
-                    .map(t =>
-                      s"${t.getClassName}.${t.getMethodName}(${t.getFileName}:${t.getLineNumber})"
-                    )
-                    .mkString(s"${name}: ${msg}\n    at ", "\n    at ", "")
-                }
-                </failure>
-              )
-            case _ => None
-          }
-        <testcase id={test.result.fullyQualifiedName}
-                  classname={test.suiteName}
-                  name={test.testName}
-                  time={(test.result.duration / 1000.0).toString}>
-          {failure.orNull}
+    val suites = results0.groupBy(_.fullyQualifiedName).map { case (fqn, testResults) =>
+      val cases = testResults.map { testResult =>
+        val testName = testcaseName(testResult)
+        <testcase classname={testResult.fullyQualifiedName}
+                  name={testName}
+                  time={durationAsString(testResult.duration)}>
+          {testCaseStatus(testResult).orNull}
         </testcase>
       }
 
-      <testsuite id={suiteName}
-                 name={suiteName}
-                 tests={tests.length.toString}
-                 failures={tests.count(_.result.status == Status.Failure.toString).toString}
-                 errors={tests.count(_.result.status == Status.Error.toString).toString}
-                 skipped={tests.count(_.result.status == Status.Skipped.toString).toString}
-                 time={(tests.map(_.result.duration).sum / 1000.0).toString}>
+      <testsuite name={fqn}
+                 tests={testResults.length.toString}
+                 failures={testResults.count(_.status == Status.Failure.toString).toString}
+                 errors={testResults.count(_.status == Status.Error.toString).toString}
+                 skipped={testResults.count(_.status == Status.Skipped.toString).toString}
+                 time={(testResults.map(_.duration).sum / 1000.0).toString}>
+                 timestamp={timestamp}
+        {properties}
         {cases}
       </testsuite>
     }
-
+    // todo add the parent module name
     val xml =
-      <testsuites tests={results.size.toString}
-                  failures={results.count(_.result.status == Status.Failure.toString).toString}
-                  errors={results.count(_.result.status == Status.Error.toString).toString}
-                  skipped={results.count(_.result.status == Status.Skipped.toString).toString}
-                  time={(results.map(_.result.duration).sum / 1000.0).toString}>
+      <testsuites tests={results0.size.toString}
+                  failures={results0.count(_.status == Status.Failure.toString).toString}
+                  errors={results0.count(_.status == Status.Error.toString).toString}
+                  skipped={results0.count(_.status == Status.Skipped.toString).toString}
+                  time={durationAsString(results0.map(_.duration).sum)}>
         {suites}
       </testsuites>
-    if (results.nonEmpty) scala.xml.XML.save(out.toString(), xml, xmlDecl = true)
+    if (results0.nonEmpty) scala.xml.XML.save(out.toString(), xml, xmlDecl = true)
   }
 
-  private val RE_FQN = """^(([a-zA-Z_$][a-zA-Z\d_$]*\.)*[a-zA-Z_$][a-zA-Z\d_$]*)\.(.*)$""".r
+  private def testCaseStatus(e: TestResult): Option[Elem] = {
+    val Error = Status.Error.toString
+    val Failure = Status.Failure.toString
+    val Ignored = Status.Ignored.toString
+    val Skipped = Status.Skipped.toString
+    val Pending = Status.Pending.toString
 
-  private def splitFullyQualifiedName(fullyQualifiedName: String): (String, String) = {
-    RE_FQN.findFirstMatchIn(fullyQualifiedName) match {
-      case Some(m) => (m.group(1), m.group(3))
-      case None => ("", fullyQualifiedName)
+    val trace: String = e.exceptionTrace.map(stackTraceTrace =>
+      stackTraceTrace.map(t =>
+        s"${t.getClassName}.${t.getMethodName}(${t.getFileName}:${t.getLineNumber})"
+      )
+        .mkString(
+          s"${e.exceptionName.getOrElse("")}: ${e.exceptionMsg.getOrElse("")}\n    at ",
+          "\n    at ",
+          ""
+        )
+    ).getOrElse("")
+    e.status match {
+      case Error if (e.exceptionMsg.isDefined && e.exceptionName.isDefined) =>
+        Some(<error message={e.exceptionMsg.get} type={e.exceptionName.get}>
+          {trace}
+        </error>)
+      case Error => Some(<error message={"No Exception or message provided"}/>)
+      case Failure if (e.exceptionMsg.isDefined && e.exceptionName.isDefined) =>
+        Some(<failure message={e.exceptionMsg.get} type={e.exceptionName.get}>
+          {trace}
+        </failure>)
+      case Failure => Some(<failure message={"No Exception or message provided"}/>)
+      case Ignored | Skipped | Pending => Some(<skipped/>)
+      case _ => None
     }
   }
 }
diff --git a/scalalib/test/src/mill/scalalib/TestRunnerTests.scala b/scalalib/test/src/mill/scalalib/TestRunnerTests.scala
@@ -1,13 +1,15 @@
 package mill.scalalib
 
-import mill.{Agg, T}
-
 import mill.api.Result
 import mill.util.{TestEvaluator, TestUtil}
+import mill.{Agg, T}
+import os.Path
+import sbt.testing.Status
 import utest._
 import utest.framework.TestPath
 
 import java.io.{ByteArrayOutputStream, PrintStream}
+import scala.xml.{Elem, NodeSeq, XML}
 
 object TestRunnerTests extends TestSuite {
   object testrunner extends TestUtil.BaseModule with ScalaModule {
@@ -84,6 +86,7 @@ object TestRunnerTests extends TestSuite {
           assert(
             test._2.size == 3
           )
+          junitReportIn(eval.outPath, "utest").shouldHave(3, Status.Success)
         }
         "testOnly" - {
           def testOnly(eval: TestEvaluator, args: Seq[String], size: Int) = {
@@ -116,6 +119,7 @@ object TestRunnerTests extends TestSuite {
             val Left(Result.Failure(msg, _)) = eval(testrunner.doneMessageFailure.test())
             val stdout = new String(outStream.toByteArray)
             assert(stdout.contains("test failure done message"))
+            junitReportIn(eval.outPath, "doneMessageFailure").shouldHave(1, Status.Failure)
           }
         }
         test("success") {
@@ -137,6 +141,7 @@ object TestRunnerTests extends TestSuite {
           workspaceTest(testrunner) { eval =>
             val Right((testRes, count)) = eval(testrunner.scalatest.test())
             assert(testRes._2.size == 2)
+            junitReportIn(eval.outPath, "scalatest").shouldHave(2, Status.Success)
           }
         }
       }
@@ -146,9 +151,35 @@ object TestRunnerTests extends TestSuite {
           workspaceTest(testrunner) { eval =>
             val Right((testRes, count)) = eval(testrunner.ziotest.test())
             assert(testRes._2.size == 1)
+            junitReportIn(eval.outPath, "ziotest").shouldHave(1, Status.Success)
           }
         }
       }
     }
   }
+
+  trait JUnitReportMatch {
+    def shouldHave(quantity: Int, status: Status): Unit
+  }
+  private def junitReportIn(
+      outPath: Path,
+      moduleName: String,
+      action: String = "test"
+  ): JUnitReportMatch = {
+    val reportPath: Path = outPath / moduleName / s"$action.dest" / "test-report.xml"
+    val reportXML = XML.loadFile(reportPath.toIO)
+    (quantity: Int, status: Status) => {
+      status match {
+        case Status.Success =>
+          val testCases: NodeSeq = reportXML \\ "testcase"
+          val actualSucceededTestCases: Int =
+            testCases.count(tc => !tc.child.exists(n => n.isInstanceOf[Elem]))
+          assert(quantity == actualSucceededTestCases)
+        case _ =>
+          val statusXML = reportXML \\ status.name().toLowerCase
+          assert(quantity == statusXML.size)
+      }
+      ()
+    }
+  }
 }
diff --git a/scalanativelib/src/mill/scalanativelib/ScalaNativeModule.scala b/scalanativelib/src/mill/scalanativelib/ScalaNativeModule.scala
@@ -362,7 +362,7 @@ trait TestScalaNativeModule extends ScalaNativeModule with TestModule {
       T.testReporter,
       TestRunnerUtils.globFilter(globSeletors())
     )
-    val res = TestModule.handleResults(doneMsg, results, Some(T.ctx()))
+    val res = TestModule.handleResults(doneMsg, results, T.ctx(), testReportXml())
     // Hack to try and let the Scala Native subprocess finish streaming it's stdout
     // to the JVM. Without this, the stdout can still be streaming when `close()`
     // is called, and some of the output is dropped onto the floor.