Skip to content

Add a custom test listener for usable JUnit XML reports #6313

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

Merged
merged 1 commit into from
Mar 6, 2018

Conversation

retronym
Copy link
Member

@retronym retronym commented Feb 7, 2018

  • Update to partest that emits more detailed TestEvents
  • Group partest JUnit XML reports in, e.g, test.files.pos.xml
  • workaround Jenkins dislike of the work "run"
    Requires a new version of partest to provide some missing metadata.

Sample files generated:

> ;partest --srcpath scaladoc --grep t7876; partest --grep default
...
⚡ (cd target/test/test-reports/partest && find . )
.
./test.files.jvm.xml
./test.files.neg.xml
./test.files.pos.xml
./test.files.presentation.xml
./test.files.run_.xml
./test.files.scalap.xml
./test.files.specialized.xml
./test.scaladoc.run_.xml

@retronym
Copy link
Member Author

retronym commented Feb 7, 2018

Together with scala/scala-partest#95, this enables output like:

gist target/test/test-reports/partest/run/run-bug4840.scala.xml

https://gist.github.com/ee32b258501b3c8fc698ddf74aa8d615

Which should, in turn, help us get a nice unified output from the Jenkins Junit Test report .

@retronym
Copy link
Member Author

retronym commented Feb 7, 2018

@SethTisue I half remember discussing this goal with you on a ticket, but I can't seem to find it to cross reference.

@scala-jenkins scala-jenkins added this to the 2.13.0-M4 milestone Feb 7, 2018
@SethTisue
Copy link
Member

doesn't ring a bell...

@SethTisue
Copy link
Member

needs rebase

build.sbt Outdated
val trace: String = if (e.throwable.isDefined) {
val stringWriter = new StringWriter()
val writer = new PrintWriter(stringWriter)
e.throwable.get.printStackTrace(writer)
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this be processed further (as in https://github.com/sbt/junit-interface/blob/master/src/main/java/com/novocode/junit/RichLogger.java) or does the consumer of the log take care of that?

build.sbt Outdated
// file on group of tests run by `testAll`, and the test names in the XML file don't seem to show the path to the
// test for tests defined in a single file.
//
// Let's roll our own to try to enable the Jenkins JUnit test reports.
Copy link
Contributor

Choose a reason for hiding this comment

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

Will this still work on Travis?

Copy link
Member Author

@retronym retronym Mar 1, 2018

Choose a reason for hiding this comment

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

No, it looks like does not support anything other than plain text reports, which seems like a step backwards to me. We can install our own tooling on the Travis VM (Jenkins perhaps! Only 28% joking...) to render HTML reports and upload them to S3 and add a link to the text output.

/cc @adriaanm

Copy link
Member

Choose a reason for hiding this comment

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

it's not coming soon it seems travis-ci/travis-ci#239 - in any case, we're keeping CI validation on jenkins for now, travis only for releases.

build.sbt Outdated
} else {
""
}
val result = <testsuite hostname={ delegate.hostname } name={ e.fullyQualifiedName()}
Copy link
Contributor

Choose a reason for hiding this comment

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

Wouldn't it make more sense to make pos, neg, etc. the test suites rather than reporting each individual test as one?

Copy link
Member Author

Choose a reason for hiding this comment

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

Agreed, that would be an improvement. I'll see if I can rig that up.

@retronym retronym force-pushed the topic/partest-junit-xml branch from d25ae2b to 6d21300 Compare March 2, 2018 04:29
@retronym
Copy link
Member Author

retronym commented Mar 2, 2018

@Stefan: I've had to make another change to partest to pass through the --srcpath (so we don't clobber the XML generated by partest --run when we later run partest --srcpath scaladoc --run.

scala/scala-partest#98

With that, and the changes just pushed here, I end up a file structure like:

⚡ (cd target/test/test-reports/partest; find .)
/code/scala/target/test/test-reports/partest
.
./test
./test/files
./test/files/jvm.xml
./test/files/neg.xml
./test/files/pos.xml
./test/files/presentation.xml
./test/files/run.xml
./test/files/scalap.xml
./test/files/specialized.xml
./test/scaladoc
./test/scaladoc/run.xml

Inside the files, I've added durations (although read them with a grain of salt as things are running in parallel), and stripped ANSI escape sequence from places it could show up.

Failed tests get the transcript of partests steps, the checkfile diff and the exception stacktraces.

I'm going to run the latest files through the Jenkins reporter to see how they look.

  - Update to partest that emits more detailed TestEvents
  - Group partest JUnit XML reports in, e.g, test.files.pos.xml
  - workaround Jenkins dislike of the work "run"
Requires a new version of partest to provide some missing metadata.

Sample files generated:

```
> ;partest --srcpath scaladoc --grep t7876; partest --grep default
...
```

```
⚡ (cd target/test/test-reports/partest && find . )
.
./test.files.jvm.xml
./test.files.neg.xml
./test.files.pos.xml
./test.files.presentation.xml
./test.files.run_.xml
./test.files.scalap.xml
./test.files.specialized.xml
./test.scaladoc.run_.xml
```
@retronym retronym force-pushed the topic/partest-junit-xml branch from 32b1d5f to 8f96e5e Compare March 2, 2018 06:21
@retronym
Copy link
Member Author

retronym commented Mar 2, 2018

I've just released partest 1.1.5 via Sonatype, this should be buildable in an hour or two.

@retronym retronym removed the WIP label Mar 2, 2018
@retronym
Copy link
Member Author

retronym commented Mar 2, 2018

/rebuild

@retronym
Copy link
Member Author

retronym commented Mar 6, 2018

This is ready to go now.

Results are in https://scala-ci.typesafe.com/job/scala-2.13.x-validate-main/260/testReport/

under "test.files" and "test.scaladoc"

The remaining problem is that rendering of these pages is horrendously slow. Using https://scala-ci.typesafe.com/threadDump, I noticed:

at com.thoughtworks.xstream.XStream.unmarshal(XStream.java:1189)
	at hudson.util.XStream2.unmarshal(XStream2.java:160)
	at hudson.util.XStream2.unmarshal(XStream2.java:131)
	at com.thoughtworks.xstream.XStream.unmarshal(XStream.java:1173)
	at com.thoughtworks.xstream.XStream.fromXML(XStream.java:1053)
	at hudson.XmlFile.read(XmlFile.java:147)
	at hudson.tasks.junit.TestResultAction.load(TestResultAction.java:208)
	at hudson.tasks.junit.TestResultAction.getResult(TestResultAction.java:145)
	-  locked hudson.tasks.junit.TestResultAction@75de8d15
	at hudson.tasks.junit.TestResultAction.getResult(TestResultAction.java:63)
	at hudson.tasks.test.AbstractTestResultAction.findCorrespondingResult(AbstractTestResultAction.java:252)
	at hudson.tasks.test.TestResult.getPreviousResult(TestResult.java:146)
	at hudson.tasks.junit.ClassResult.getPreviousResult(ClassResult.java:74)
	at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
	at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.lang.reflect.Method.invoke(Method.java:498)
	at org.apache.commons.jexl.util.PropertyExecutor.execute(PropertyExecutor.java:125)

I think this is looking back through the archives of previous builds to find the last result of each given test. This might become faster after we have our first build with the new partest XML test results in the build history. I'm testing this theory with a rebuild: https://scala-ci.typesafe.com/job/scala-2.13.x-validate-main/320/console

Otherwise, there might be something unusual in the way we're mapping partest test cases to pseudo class name and method names that trigger an inefficiency here.

@retronym retronym merged commit f3aaecf into scala:2.13.x Mar 6, 2018
@retronym
Copy link
Member Author

retronym commented Mar 6, 2018

The test result page rendering still isn't great but seems < 5s, so let's book the progress.

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.

5 participants