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

Add support for @AfterAll in XML report #300

Merged
merged 1 commit into from
Nov 5, 2024

Conversation

fzakaria
Copy link
Contributor

@fzakaria fzakaria commented Oct 10, 2024

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]

@fzakaria fzakaria force-pushed the support-afterall branch 2 times, most recently from 28684ad to f71f699 Compare October 11, 2024 20:23
Copy link
Collaborator

@shs96c shs96c left a comment

Choose a reason for hiding this comment

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

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

@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.

Copy link
Collaborator

@shs96c shs96c left a comment

Choose a reason for hiding this comment

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

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?

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

fzakaria commented Nov 4, 2024

@shs96c rebased;

@fzakaria
Copy link
Contributor Author

fzakaria commented Nov 5, 2024

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

@shs96c
Copy link
Collaborator

shs96c commented Nov 5, 2024

Raw logs look like it timed out downloading apple_rules_lint

@shs96c shs96c merged commit 8d83528 into bazel-contrib:main Nov 5, 2024
8 checks passed
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