search

com-lihaoyi / mill

Your shiny new Java/Scala build tool!
https://mill-build.com/
MIT License
2k stars 308 forks source link

Improve the support of JUnit XML report #3135

Closed romain-gilles-ultra closed 2 months ago

romain-gilles-ultra commented 2 months ago

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:

Resources

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 &gt; 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>
lefou commented 2 months ago

If including explicitly set environment variables in the report is required/wanted, we could use forkEnv which is what all non-local test targets use.

romain-gilles-ultra commented 2 months ago

Hi @lefou I'm sorry but I don't get it :cry: What do you mean should I update/fix something in my PR? Sorry

romain-gilles-ultra commented 2 months ago

Do you mean mapping the forkEnv values into the <properties> section

lefou commented 2 months ago

Yeah, IIUC, you tried to put as much as possible information into the JUnit report. And since it can contain env variable and the sbt report also adds these env variable, I assumed, you might want to have these variable in the report. So I was suggesting a way to access them. But it's not an request from my side.

romain-gilles-ultra commented 2 months ago

Hi @lefou I give it a try tell me what you think about it :hugs:

lefou commented 2 months ago

@romain-gilles-ultra I just realized, that the JUnit XML contains "properties" but I thought of "env vars", which is a completely different thing. So I guess we should revert to your previous solution. Sorry about that.

romain-gilles-ultra commented 2 months ago

I think I can keep the system props as the <properties> section is dedicated to that

romain-gilles-ultra commented 2 months ago

Hi @lefou I fact after checking the doc we can use properties to capture env variables values: https://github.com/testmoapp/junitxml?tab=readme-ov-file#properties-for-suites-and-cases

Properties can be included for suites and cases to record additional information, such as version numbers, environment variables or links to CI pipelines.

I think we can keep this version as it joins the environment variables with the system properties no?

lefou commented 2 months ago

We could/should keep the Java system properties, although I'm not 100 percent certain, that they will be accurate. Since we spawn a new Java process to run the tests, there is a change some system properties are different. It would be cool, if we could either return the actual used properties from the test run or write the report file from the test runner process. Both requires more work, so we can keep it for another PR.

Regarding the environment variables, I we want include them in the report, we should prefix them (e.g. with sys.env. or something like that). Otherwise, collisions or shadowing can occur. We can defer to some later PR too, until there is a demand for it.

romain-gilles-ultra commented 2 months ago

Hi @lefou Thanks for the feedback. What about I remove the property management and we keep it for another PR? I can prepare the code to use properties like:

  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))
  }

But keep the way the properties are fed for another iteration

lefou commented 2 months ago

I can prepare the code to use properties ... But keep the way the properties are fed for another iteration

Sound good to me.