From 10086ad073158941aee84aa16ab58f867d2c9dbd Mon Sep 17 00:00:00 2001 From: Romain Gilles <58700468+romain-gilles-ultra@users.noreply.github.com> Date: Tue, 14 May 2024 23:09:29 +0200 Subject: [PATCH] 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 `` a.k.a. per test (spec...) class while this solution (original) is producing one `` 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` ``` ... 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) 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) java.lang.RuntimeException: big error at io.ultra.AppTest.testErrorApp(AppTest.java:51) ``` ## 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: ``` ... ``` ### 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: ``` ... ``` #### 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: ``` ... ``` Pull request: https://github.com/com-lihaoyi/mill/pull/3135 --- .../src/mill/scalajslib/ScalaJSModule.scala | 2 +- scalalib/src/mill/scalalib/TestModule.scala | 153 +++++++++++------- .../src/mill/scalalib/TestRunnerTests.scala | 35 +++- .../scalanativelib/ScalaNativeModule.scala | 2 +- 4 files changed, 132 insertions(+), 60 deletions(-) diff --git a/scalajslib/src/mill/scalajslib/ScalaJSModule.scala b/scalajslib/src/mill/scalajslib/ScalaJSModule.scala index 0772733cd92..9ee319bed21 100644 --- 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 index f3ccee241d5..c867a8115af 100644 --- 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) => + + } + + {ps} + } - 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( - - { - trace - .map(t => - s"${t.getClassName}.${t.getMethodName}(${t.getFileName}:${t.getLineNumber})" - ) - .mkString(s"${name}: ${msg}\n at ", "\n at ", "") - } - - ) - case _ => None - } - - {failure.orNull} + val suites = results0.groupBy(_.fullyQualifiedName).map { case (fqn, testResults) => + val cases = testResults.map { testResult => + val testName = testcaseName(testResult) + + {testCaseStatus(testResult).orNull} } - + + timestamp={timestamp} + {properties} {cases} } - + // todo add the parent module name val xml = - + {suites} - 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( + {trace} + ) + case Error => Some() + case Failure if (e.exceptionMsg.isDefined && e.exceptionName.isDefined) => + Some( + {trace} + ) + case Failure => Some() + case Ignored | Skipped | Pending => Some() + case _ => None } } } diff --git a/scalalib/test/src/mill/scalalib/TestRunnerTests.scala b/scalalib/test/src/mill/scalalib/TestRunnerTests.scala index 14c82dfb9ce..5e8d4479884 100644 --- 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 index d934ada850c..e76625ff5df 100644 --- 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.