Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve the support of JUnit XML report #3135

Merged
merged 5 commits into from
May 14, 2024

Conversation

romain-gilles-ultra
Copy link
Contributor

@romain-gilles-ultra romain-gilles-ultra commented Apr 29, 2024

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

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
Copy link
Member

lefou commented Apr 30, 2024

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
Copy link
Contributor Author

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

@romain-gilles-ultra
Copy link
Contributor Author

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

@lefou
Copy link
Member

lefou commented May 2, 2024

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
Copy link
Contributor Author

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

@lefou
Copy link
Member

lefou commented May 2, 2024

@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
Copy link
Contributor Author

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

@romain-gilles-ultra
Copy link
Contributor Author

romain-gilles-ultra commented May 3, 2024

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
Copy link
Member

lefou commented May 7, 2024

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
Copy link
Contributor Author

romain-gilles-ultra commented May 13, 2024

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
Copy link
Member

lefou commented May 13, 2024

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

Sound good to me.

Copy link
Member

@lefou lefou left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me. Thank you!

@lefou lefou merged commit 10086ad into com-lihaoyi:main May 14, 2024
38 checks passed
@lefou lefou added this to the 0.11.8 milestone May 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants