From f993e745535bd577ac503df2b9b1466946bde0f0 Mon Sep 17 00:00:00 2001 From: Romain Gilles Date: Fri, 24 May 2024 09:07:17 +0200 Subject: [PATCH 1/3] Add unit test to the JUnit xml report generation --- scalalib/src/mill/scalalib/TestModule.scala | 62 ++-- .../src/mill/scalalib/TestModuleTests.scala | 302 ++++++++++++++++++ .../src/mill/scalalib/TestRunnerTests.scala | 43 ++- 3 files changed, 364 insertions(+), 43 deletions(-) create mode 100644 scalalib/test/src/mill/scalalib/TestModuleTests.scala diff --git a/scalalib/src/mill/scalalib/TestModule.scala b/scalalib/src/mill/scalalib/TestModule.scala index 8b58e4c64df..2cb6f6dea3e 100644 --- a/scalalib/src/mill/scalalib/TestModule.scala +++ b/scalalib/src/mill/scalalib/TestModule.scala @@ -206,6 +206,10 @@ trait TestModule object TestModule { private val FailedTestReportCount = 5 + private val ErrorStatus = Status.Error.name() + private val FailureStatus = Status.Failure.name() + private val SkippedStatus = + Set(Status.Ignored.name(), Status.Skipped.name(), Status.Pending.name()) /** * TestModule using TestNG Framework to run tests. @@ -334,9 +338,12 @@ object TestModule { 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)) - ) + for { + fileName <- testReportXml + path = ctx.dest / fileName + xml <- genTestXmlReport(results, Instant.now(), props.getOrElse(Map.empty)) + _ = scala.xml.XML.save(path.toString(), xml, xmlDecl = true) + } yield () handleResults(doneMsg, results, Some(ctx)) } @@ -348,17 +355,11 @@ object TestModule { def scalacOptions: T[Seq[String]] = Seq.empty[String] } - private def genTestXmlReport( + private[scalalib] def genTestXmlReport( results0: Seq[TestResult], - out: os.Path, + timestamp: Instant, props: Map[String, String] - ): Unit = { - val timestamp = DateTimeFormatter.ISO_LOCAL_DATE_TIME.format( - LocalDateTime.ofInstant( - Instant.now.truncatedTo(ChronoUnit.SECONDS), - ZoneId.systemDefault() - ) - ) + ): Option[Elem] = { def durationAsString(value: Long) = (value / 1000d).toString def testcaseName(testResult: TestResult) = testResult.selector.replace(s"${testResult.fullyQualifiedName}.", "") @@ -386,9 +387,11 @@ object TestModule { 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} + skipped={ + testResults.count(testResult => SkippedStatus.contains(testResult.status)).toString + } time={(testResults.map(_.duration).sum / 1000.0).toString} - timestamp={timestamp}> + timestamp={formatTimestamp(timestamp)}> {properties} {cases} @@ -398,20 +401,25 @@ object TestModule { SkippedStatus.contains(testResult.status)).toString + } time={durationAsString(results0.map(_.duration).sum)}> {suites} - if (results0.nonEmpty) scala.xml.XML.save(out.toString(), xml, xmlDecl = true) + if (results0.nonEmpty) Some(xml) else None } - 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 formatTimestamp(timestamp: Instant): String = { + DateTimeFormatter.ISO_LOCAL_DATE_TIME.format( + LocalDateTime.ofInstant( + timestamp.truncatedTo(ChronoUnit.SECONDS), + ZoneId.of("UTC") + ) + ) + } + private def testCaseStatus(e: TestResult): Option[Elem] = { val trace: String = e.exceptionTrace.map(stackTraceTrace => stackTraceTrace.map(t => s"${t.getClassName}.${t.getMethodName}(${t.getFileName}:${t.getLineNumber})" @@ -423,17 +431,17 @@ object TestModule { ) ).getOrElse("") e.status match { - case Error if (e.exceptionMsg.isDefined && e.exceptionName.isDefined) => + case ErrorStatus if (e.exceptionMsg.isDefined && e.exceptionName.isDefined) => Some( {trace} ) - case Error => Some() - case Failure if (e.exceptionMsg.isDefined && e.exceptionName.isDefined) => + case ErrorStatus => Some() + case FailureStatus if (e.exceptionMsg.isDefined && e.exceptionName.isDefined) => Some( {trace} ) - case Failure => Some() - case Ignored | Skipped | Pending => Some() + case FailureStatus => Some() + case s if SkippedStatus.contains(s) => Some() case _ => None } } diff --git a/scalalib/test/src/mill/scalalib/TestModuleTests.scala b/scalalib/test/src/mill/scalalib/TestModuleTests.scala new file mode 100644 index 00000000000..d5d40b681d3 --- /dev/null +++ b/scalalib/test/src/mill/scalalib/TestModuleTests.scala @@ -0,0 +1,302 @@ +package mill.scalalib + +import mill.testrunner.TestResult +import utest._ + +import java.time.Instant +import scala.xml._ + +object TestModuleTests extends TestSuite { + + override def tests: Tests = Tests { + val timestamp = "2024-05-23T12:20:04" + val instant = Instant.parse(timestamp + "Z") + + val succeededTestResult: TestResult = TestResult( + "mill.scalalib.TestModuleTests", + "mill.scalalib.TestModuleTests.success", + 1, + "Success", + None, + None, + None + ) + + val succeededTestCaseElement = + + + test("success") - { + val expectedReport = + + + + {succeededTestCaseElement} + + + assertEquals( + expectedReport, + TestModule.genTestXmlReport(Seq(succeededTestResult), instant, Map.empty) + ) + } + val failedTestResult: TestResult = TestResult( + "mill.scalalib.TestModuleTests", + "mill.scalalib.TestModuleTests.fail", + 12, + "Failure", + Some("utest.AssertionError"), + Some("assert(1 == 2)"), + Some(Seq( + new StackTraceElement( + "utest.asserts.Util$", + "$anonfun$makeAssertError$1", + "Util.scala", + 26 + ), + new StackTraceElement( + "utest.framework.StackMarker$", + "dropInside", + "StackMarker.scala", + 11 + ) + )) + ) + val failedTestCaseElement = + + utest.AssertionError: assert(1 == 2) at utest.asserts.Util$.$anonfun$makeAssertError$1(Util.scala:26) at utest.framework.StackMarker$.dropInside(StackMarker.scala:11) + + test("fail") - { + val expectedReport = + + + + + {failedTestCaseElement} + + + assertEquals( + expectedReport, + TestModule.genTestXmlReport(Seq(failedTestResult), instant, Map.empty) + ) + } + test("fail - empty message") - { + val expectedReport = + + + + + + + + + + val testResult: TestResult = TestResult( + "mill.scalalib.TestModuleTests", + "mill.scalalib.TestModuleTests.fail", + 12, + "Failure", + None, + None, + None + ) + assertEquals( + expectedReport, + TestModule.genTestXmlReport(Seq(testResult), instant, Map.empty) + ) + } + val errorTestResult: TestResult = TestResult( + "mill.scalalib.TestModuleTests", + "mill.scalalib.TestModuleTests.error", + 2400, + "Error", + Some("java.lang.RuntimeException"), + Some("throw new RuntimeException()"), + Some(Seq( + new StackTraceElement( + "utest.asserts.Util$", + "$anonfun$makeAssertError$1", + "Util.scala", + 26 + ), + new StackTraceElement( + "utest.framework.StackMarker$", + "dropInside", + "StackMarker.scala", + 11 + ) + )) + ) + val errorTestCaseElement = + + java.lang.RuntimeException: throw new RuntimeException() at utest.asserts.Util$.$anonfun$makeAssertError$1(Util.scala:26) at utest.framework.StackMarker$.dropInside(StackMarker.scala:11) + + test("error") - { + val expectedReport = + + + + + {errorTestCaseElement} + + + assertEquals( + expectedReport, + TestModule.genTestXmlReport(Seq(errorTestResult), instant, Map.empty) + ) + } + test("error - empty message") - { + val expectedReport = + + + + + + + + + + val testResult: TestResult = TestResult( + "mill.scalalib.TestModuleTests", + "mill.scalalib.TestModuleTests.error", + 2400, + "Error", + None, + None, + None + ) + assertEquals( + expectedReport, + TestModule.genTestXmlReport(Seq(testResult), instant, Map.empty) + ) + } + test("skipped") - { + val expectedReport = + + + + + + + + + + + + + + + + val skippedTestResult: TestResult = TestResult( + "mill.scalalib.TestModuleTests", + "mill.scalalib.TestModuleTests.skipped", + 100, + "Skipped", + None, + None, + None + ) + val ignoredTestResult: TestResult = TestResult( + "mill.scalalib.TestModuleTests", + "mill.scalalib.TestModuleTests.ignored", + 100, + "Ignored", + None, + None, + None + ) + val pendingTestResult: TestResult = TestResult( + "mill.scalalib.TestModuleTests", + "mill.scalalib.TestModuleTests.pending", + 100, + "Pending", + None, + None, + None + ) + + assertEquals( + expectedReport, + TestModule.genTestXmlReport( + Seq(skippedTestResult, ignoredTestResult, pendingTestResult), + instant, + Map.empty + ) + ) + } + test("multi test cases") - { + val expectedReport = + + + + {succeededTestCaseElement} + {failedTestCaseElement} + + + assertEquals( + expectedReport, + TestModule.genTestXmlReport(Seq(succeededTestResult, failedTestResult), instant, Map.empty) + ) + } + test("multi test suites") - { + val expectedReport = + + + + + {succeededTestCaseElement} + {failedTestCaseElement} + + + + + + + + + val testSuite2TestResult: TestResult = TestResult( + "mill.scalalib.TestModuleTests2", + "mill.scalalib.TestModuleTests2.success", + 1, + "Success", + None, + None, + None + ) + assertEquals( + expectedReport, + TestModule.genTestXmlReport( + Seq(succeededTestResult, testSuite2TestResult, failedTestResult), + instant, + Map.empty + ) + ) + } + } + + private def assertEquals(expected: Elem, actual: Option[Elem]): Unit = { + val actElem = actual.get + val act = asString(actElem) + val exp = asString(expected) + // extracted variables to get a usable display + assert(exp == act) + } + + private def asString(elem: Elem) = new PrettyPrinter(Int.MaxValue, 0, true).format(elem) +} diff --git a/scalalib/test/src/mill/scalalib/TestRunnerTests.scala b/scalalib/test/src/mill/scalalib/TestRunnerTests.scala index 5e8d4479884..fde5271ec78 100644 --- a/scalalib/test/src/mill/scalalib/TestRunnerTests.scala +++ b/scalalib/test/src/mill/scalalib/TestRunnerTests.scala @@ -86,7 +86,7 @@ object TestRunnerTests extends TestSuite { assert( test._2.size == 3 ) - junitReportIn(eval.outPath, "utest").shouldHave(3, Status.Success) + junitReportIn(eval.outPath, "utest").shouldHave(3 -> Status.Success) } "testOnly" - { def testOnly(eval: TestEvaluator, args: Seq[String], size: Int) = { @@ -119,7 +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) + junitReportIn(eval.outPath, "doneMessageFailure").shouldHave(1 -> Status.Failure) } } test("success") { @@ -141,7 +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) + junitReportIn(eval.outPath, "scalatest").shouldHave(2 -> Status.Success) } } } @@ -151,7 +151,7 @@ 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) + junitReportIn(eval.outPath, "ziotest").shouldHave(1 -> Status.Success) } } } @@ -159,7 +159,7 @@ object TestRunnerTests extends TestSuite { } trait JUnitReportMatch { - def shouldHave(quantity: Int, status: Status): Unit + def shouldHave(statuses: (Int, Status)*): Unit } private def junitReportIn( outPath: Path, @@ -168,18 +168,29 @@ object TestRunnerTests extends TestSuite { ): 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) + new JUnitReportMatch { + override def shouldHave(statuses: (Int, Status)*): Unit = { + def getValue(attribute: String): Int = + reportXML.attribute(attribute).map(_.toString).getOrElse("0").toInt + statuses.foreach { case (expectedQuantity: 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(expectedQuantity == actualSucceededTestCases) + case _ => + val statusXML = reportXML \\ status.name().toLowerCase + val nbSpecificStatusElement = statusXML.size + assert(expectedQuantity == nbSpecificStatusElement) + val specificStatusAttributeValue = getValue(s"${status.name().toLowerCase}s") + assert(expectedQuantity == specificStatusAttributeValue) + } + } + val expectedNbTests = statuses.map(_._1).sum + val actualNbTests = getValue("tests") + assert(expectedNbTests == actualNbTests) } - () } } } From 0ee897e1f92344e90e1d59838d99a36303fe143d Mon Sep 17 00:00:00 2001 From: Romain Gilles Date: Fri, 24 May 2024 12:10:25 +0200 Subject: [PATCH 2/3] Unify usage of constants and helper function --- scalalib/src/mill/scalalib/TestModule.scala | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/scalalib/src/mill/scalalib/TestModule.scala b/scalalib/src/mill/scalalib/TestModule.scala index 2cb6f6dea3e..3e18bcef3f7 100644 --- a/scalalib/src/mill/scalalib/TestModule.scala +++ b/scalalib/src/mill/scalalib/TestModule.scala @@ -385,12 +385,12 @@ object TestModule { SkippedStatus.contains(testResult.status)).toString } - time={(testResults.map(_.duration).sum / 1000.0).toString} + time={durationAsString(testResults.map(_.duration).sum)} timestamp={formatTimestamp(timestamp)}> {properties} {cases} @@ -399,8 +399,8 @@ object TestModule { // todo add the parent module name val xml = SkippedStatus.contains(testResult.status)).toString } From 50ff508c981bb1db5524d58f1f71b5f5ca3ea83f Mon Sep 17 00:00:00 2001 From: Romain Gilles Date: Fri, 24 May 2024 16:00:14 +0200 Subject: [PATCH 3/3] Rename the set of the skipped test status to SkippedStates --- scalalib/src/mill/scalalib/TestModule.scala | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/scalalib/src/mill/scalalib/TestModule.scala b/scalalib/src/mill/scalalib/TestModule.scala index 3e18bcef3f7..4a323ced7cf 100644 --- a/scalalib/src/mill/scalalib/TestModule.scala +++ b/scalalib/src/mill/scalalib/TestModule.scala @@ -208,7 +208,7 @@ object TestModule { private val FailedTestReportCount = 5 private val ErrorStatus = Status.Error.name() private val FailureStatus = Status.Failure.name() - private val SkippedStatus = + private val SkippedStates = Set(Status.Ignored.name(), Status.Skipped.name(), Status.Pending.name()) /** @@ -388,7 +388,7 @@ object TestModule { failures={testResults.count(_.status == FailureStatus).toString} errors={testResults.count(_.status == ErrorStatus).toString} skipped={ - testResults.count(testResult => SkippedStatus.contains(testResult.status)).toString + testResults.count(testResult => SkippedStates.contains(testResult.status)).toString } time={durationAsString(testResults.map(_.duration).sum)} timestamp={formatTimestamp(timestamp)}> @@ -402,7 +402,7 @@ object TestModule { failures={results0.count(_.status == FailureStatus).toString} errors={results0.count(_.status == ErrorStatus).toString} skipped={ - results0.count(testResult => SkippedStatus.contains(testResult.status)).toString + results0.count(testResult => SkippedStates.contains(testResult.status)).toString } time={durationAsString(results0.map(_.duration).sum)}> {suites} @@ -441,7 +441,7 @@ object TestModule { {trace} ) case FailureStatus => Some() - case s if SkippedStatus.contains(s) => Some() + case s if SkippedStates.contains(s) => Some() case _ => None } }