Add support for @AfterAll in XML report

[fzakaria]

Writing a simple test like

package com.library;

import org.junit.jupiter.api.AfterAll;
import org.junit.jupiter.api.Test;

public class AlwaysFailTest {
    @Test
    public void doNothingTest() {
        System.out.println("Hi there!");
    }

    @AfterAll
    public static void alwaysFail() {
        throw new RuntimeException("Always failing.");
    }
}

Produces an XML that seems to look like it succeeds.

<?xml version="1.0" encoding="UTF-8"?>
<testsuites>
  <testsuite name="com.library.AlwaysFailTest" timestamp="2024-10-10T21:19:11.522176Z" hostname="KHK9NLVQGN" tests="2" failures="0" errors="0" disabled="0" skipped="0" package="">
    <properties/>
    <testcase name="doNothingTest" classname="com.library.AlwaysFailTest" time="0.03">
      <system-out><![CDATA[Hi there!
]]></system-out>
    </testcase>
  </testsuite>
</testsuites>

Bazel itself correctly identifies it fails. The Junit4 reporter also included with Bazel natively correclty reports the failure in the XML output.

Augment the Junit listener to add support for static methods and add them to the JUnit output.

Doing so produces the following XML.

<?xml version="1.0" encoding="UTF-8"?>
<testsuites>
  <testsuite name="com.library.AlwaysFailTest" timestamp="2024-10-10T21:49:02.096648Z" hostname="KHK9NLVQGN" tests="3" failures="1" errors="0" disabled="0" skipped="0" package="">
    <properties/>
    <testcase name="com.library.AlwaysFailTest" classname="com.library.AlwaysFailTest" time="0.05">
      <failure message="Always failing." type="java.lang.RuntimeException"><![CDATA[java.lang.RuntimeException: Always failing.
	at com.library.AlwaysFailTest.alwaysFail(AlwaysFailTest.java:20)
...
	at com.github.bazel_contrib.contrib_rules_jvm.junit5.JUnit5Runner.main(JUnit5Runner.java:39)
]]></failure>
    </testcase>
    <testcase name="doNothingTest" classname="com.library.AlwaysFailTest" time="0.03">
      <system-out><![CDATA[Hi there!
]]></system-out>
    </testcase>
  </testsuite>
</testsuites>

Co-authored-by: Vince Rose [email protected]

[shs96c]

For completeness, we should also modify the comparative test cases to try and capture how this will work with (eg) a parameterised test. However, this is already better than what we have, so if you kindly rework the test in BazelJUnitOuputListenerTest to be more like the others in that class, I'll happily merge this!

[fzakaria]

@shs96c I'm not sure what tests to add to mimic the other ones that will provide value.
The main "bug" was that the method findTestCases_locked was over-filtering the TestData results;

Nothing about the creation of the TestData results are incorrect itself and the method itself is private.
While I can see such a heavy test that I introduced clunky; it seems appropriate here.

I can make findTestCases_locked and test against a mock version; but to be honest, I'd rather than TestIdentifier as produced by JUnit to validate against.
(I couldn't find a cleaner way to load a JUnit test without the AtomicBoolean and execute it, happy to take knowledge on this angle)

Feel free to chat to me on Slack about this as well.
This PR has been working well for us for our CI system that consume the JUnit output.

[shs96c]

OK. Let's land this, and if we find issues we can always iterate. Thank you for your patience with the review!

Could you please rebase, and I'll merge this in?

[fzakaria]

@shs96c rebased;

[fzakaria]

I'm not sure about the 1 failure; there are no logs.

[shs96c]

Raw logs look like it timed out downloading apple_rules_lint

[amishra-u]

This change started logging testsuite also as test case. Now incorrectly logs the number of tests to number of tests + number of test classes.

[fzakaria]

@amishra-u Can you give an example?
As part of the PR I added "real" tests that apply to a test class (see TestAfterAllFails) rather than just mock'd unit test.
So it should be straightforward to validate & fix.

[amishra-u]

Fixed in this pr #328.

here is verification of the bug
#330