Show failed containers in JUnit XML report #1689

[fedejeanne]

What id does

• Change the LegacyXmlResultFormatter so it shows failed containers too (e.g. test classes)
• Refactor the class: extract 1 new method called writeIfPresentInMap and call it from 3 existing methods

Fixes #1689

How to test

• See Wrong XML report generated when running JUnit(4) tests with EclipseTestRunner #1689 for information on how to run the EclipseTestRunner and run it
• The generated XML report should have 1 <testcase> node showing the failed test class ErrorOnTearDownClass, like this:

	<testcase classname="ErrorOnTearDownClass" name="" time="0.014">
		<error type="java.lang.RuntimeException">java.lang.RuntimeException
	at org.eclipse.test.tests.snippets.resources.ErrorOnTearDownClass.tearDownClass(ErrorOnTearDownClass.java:13)
	at java.base/java.lang.reflect.Method.invoke(Method.java:568)
</error>
	</testcase>

[iloveeclipse]

@fedejeanne : I'm actually on vacation already, so I will be not able to revert this if that breaks the build.

If that fixes eclipse-jdt/eclipse.jdt.ui#999, it is nice, but it can also break entire SDK reporting.

So if you plan to merge it, be sure that you will be available on the next day to check SDK build results and if anything would break, make sure to fix it or revert this change.

[fedejeanne]

@fedejeanne : I'm actually on vacation already, so I will be not able to revert this if that breaks the build.

If that fixes eclipse-jdt/eclipse.jdt.ui#999, it is nice, but it can also break entire SDK reporting.

So if you plan to merge it, be sure that you will be available on the next day to check SDK build results and if anything would break, make sure to fix it or revert this change.

I'm actually going on holidays in like 5 minutes so I'd say we can wait to merge this :-)

In fact I'm going to be on parental leave for 3 months so I'll only be checking my e-mails/notifications occasionally. I'd say we leave this PR open for a few days and see if someone has a big objection to it before merging it since I won't be able to react quickly to requested changes. Would that be ok for you?

[iloveeclipse]

In fact I'm going to be on parental leave for 3 months

Congrats :)

I'd say we leave this PR open for a few days and see if someone has a big objection to it before merging it since I won't be able to react quickly to requested changes. Would that be ok for you?

Sure. Simeon will take over, he wanted to work next week.

@trancexpress : could you please review & merge that on your next working day next week, and in case SDK test reports would be broken in some unexpected way, revert it again?

This should fix the "wrong" error reported for JDT UI tests, see eclipse-jdt/eclipse.jdt.ui#999 .

[fedejeanne]

In fact I'm going to be on parental leave for 3 months

Congrats :)

Thank you :)

This should fix the "wrong" error reported for JDT UI tests, see eclipse-jdt/eclipse.jdt.ui#999 .

Just to make sure we're on the same page: this PR will not get rid of the "error" shown in the report mentioned in eclipse-jdt/eclipse.jdt.ui#999, it will simply add one <testcase> entry to the resulting XML report (and therefore to the HTML report generated from it) and that entry will show the failing container. Whether or not that entry is related to the Aborted test introduced by me in eclipse-jdt/eclipse.jdt.ui#953 remains to be seen.

[trancexpress]

Merging this, I'll monitor build results for a few days to see if we want to revert the change. Code-wise it looks fine.

[trancexpress]

The JDT UI test error is now shown in the xml/html report:

org.eclipse.jdt.junit.tests.JUnit5TestFinderJupiterTest		Error	testresources/testClasses/JupiterTests.java

java.nio.file.NoSuchFileException: testresources/testClasses/JupiterTests.java
at java.base/sun.nio.fs.UnixException.translateToIOException(UnixException.java:92)
at java.base/sun.nio.fs.UnixException.rethrowAsIOException(UnixException.java:106)
at java.base/sun.nio.fs.UnixException.rethrowAsIOException(UnixException.java:111)
at java.base/sun.nio.fs.UnixFileSystemProvider.newByteChannel(UnixFileSystemProvider.java:218)
at java.base/java.nio.file.Files.newByteChannel(Files.java:380)
at java.base/java.nio.file.Files.newByteChannel(Files.java:432)
at java.base/java.nio.file.Files.readAllBytes(Files.java:3288)
at java.base/java.nio.file.Files.readString(Files.java:3366)
at java.base/java.nio.file.Files.readString(Files.java:3325)
at org.eclipse.jdt.junit.tests.JUnit5TestFinderJupiterTest.createCompilationUnit(JUnit5TestFinderJupiterTest.java:144)
at org.eclipse.jdt.junit.tests.JUnit5TestFinderJupiterTest.beforeClass(JUnit5TestFinderJupiterTest.java:107)
at java.base/java.lang.reflect.Method.invoke(Method.java:568) 

https://download.eclipse.org/eclipse/downloads/drops4/I20231226-1800/testresults/html/org.eclipse.jdt.ui.tests_ep431I-unit-cen64-gtk3-java17_linux.gtk.x86_64_17.html#org.eclipse.jdt.ui.tests

The build results prior to merging this PR don't show this error (though there is an error listed, without any details):

https://download.eclipse.org/eclipse/downloads/drops4/I20231225-1800/testresults/html/org.eclipse.jdt.ui.tests_ep431I-unit-cen64-gtk3-java17_linux.gtk.x86_64_17.html#org.eclipse.jdt.ui.tests

I'm not noticing anything else changed in the test reports for the latest build (I20231226-1800). So from my POV the change is fine and doesn't need a revert. I'll continue checking test results from integration builds for a few days, just in case.

[fedejeanne]

@trancexpress thank you very much for reviewing and merging!