Skip to content

Commit df44bd9

Browse files
subbu171subraman
andauthored
report assertion errors as 'failures' and other runtime exceptions as 'errors', consistent with Junit terminology (#371)
Currently, assertion failures (e.g., `AssertionError`) are being reported as errors, and unexpected exceptions (e.g., `NullPointerException`, `IOException`, etc.) are being reported as failures. This is inverted relative to JUnit’s standard terminology. **JUnit convention** Failures → Assertion failures (AssertionError) indicating the test ran but produced an unexpected result. Errors → Unexpected exceptions, infrastructure issues, or anything other than an AssertionError. Reference: JUnit 5 Legacy Reporter Logs AssertionError as FAILURE and everything else as ERROR. Reference: [JUnit Legacy XMLReportWriter.java (line 444)](https://github.com/junit-team/junit-framework/blob/main/junit-platform-reporting/src/main/java/org/junit/platform/reporting/legacy/xml/XmlReportWriter.java#L444) **Fix** Update error classification logic to correctly map: AssertionError → failure Any other exception → error **Impact** Test reports will now align with JUnit expectations. Downstream tooling and dashboards that parse JUnit XML or test results will correctly distinguish between test failures (assertion mismatches) and infrastructure errors (unexpected exceptions). --------- Co-authored-by: Subramanian Rao <subraman@uber.com>
1 parent 32d5092 commit df44bd9

File tree

2 files changed

+24
-23
lines changed

2 files changed

+24
-23
lines changed

java/src/com/github/bazel_contrib/contrib_rules_jvm/junit5/TestData.java

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,7 @@ public boolean isError() {
8080
return false;
8181
}
8282

83-
return result.getThrowable().map(thr -> thr instanceof AssertionError).orElse(false);
83+
return result.getThrowable().map(thr -> (!(thr instanceof AssertionError))).orElse(false);
8484
}
8585

8686
public boolean isFailure() {
@@ -90,11 +90,8 @@ public boolean isFailure() {
9090
|| isSkipped()) {
9191
return false;
9292
}
93-
if (result.getStatus() == TestExecutionResult.Status.ABORTED) {
94-
return true;
95-
}
9693

97-
return result.getThrowable().map(thr -> (!(thr instanceof AssertionError))).orElse(false);
94+
return result.getThrowable().map(thr -> thr instanceof AssertionError).orElse(false);
9895
}
9996

10097
public boolean isDisabled() {
@@ -110,6 +107,10 @@ public boolean isSkipped() {
110107
return true;
111108
}
112109

110+
if (getResult().getStatus() == TestExecutionResult.Status.ABORTED) {
111+
return true;
112+
}
113+
113114
return getResult().getThrowable().map(JUnit4Utils::isReasonToSkipTest).orElse(false);
114115
}
115116

java/test/com/github/bazel_contrib/contrib_rules_jvm/junit5/BazelJUnitOutputListenerTest.java

Lines changed: 18 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -60,16 +60,16 @@ public class BazelJUnitOutputListenerTest {
6060
private TestDescriptor testDescriptor = new StubbedTestDescriptor(createId("descriptors"));
6161
private TestIdentifier identifier = TestIdentifier.from(testDescriptor);
6262

63-
/** This latch is used in TestAfterAllFails for testAfterAllFailuresAreReported */
64-
private static final AtomicBoolean causeFailure = new AtomicBoolean(false);
63+
/** This latch is used in TestAfterAllErrors for testAfterAllFailuresAreReported */
64+
private static final AtomicBoolean causeError = new AtomicBoolean(false);
6565

66-
static final class TestAfterAllFails {
66+
static final class TestAfterAllErrors {
6767

6868
@SuppressFBWarnings(value = "THROWS_METHOD_THROWS_RUNTIMEEXCEPTION")
6969
@AfterAll
7070
static void afterAll() {
71-
if (causeFailure.get()) {
72-
throw new RuntimeException("I always fail.");
71+
if (causeError.get()) {
72+
throw new RuntimeException("I always error.");
7373
}
7474
}
7575

@@ -79,11 +79,11 @@ void test() {}
7979

8080
@Test
8181
public void testAfterAllFailuresAreReported() throws IOException {
82-
causeFailure.set(true);
82+
causeError.set(true);
8383

8484
// First let's do a sanity test that we have the expected failures for the @AfterAll
8585
EngineTestKit.engine("junit-jupiter")
86-
.selectors(selectClass(TestAfterAllFails.class))
86+
.selectors(selectClass(TestAfterAllErrors.class))
8787
.execute()
8888
.containerEvents()
8989
.assertStatistics(stats -> stats.skipped(0).started(2).succeeded(1).aborted(0).failed(1));
@@ -97,7 +97,7 @@ public void testAfterAllFailuresAreReported() throws IOException {
9797
Launcher launcher = LauncherFactory.create(config);
9898

9999
LauncherDiscoveryRequestBuilder request =
100-
LauncherDiscoveryRequestBuilder.request().selectors(selectClass(TestAfterAllFails.class));
100+
LauncherDiscoveryRequestBuilder.request().selectors(selectClass(TestAfterAllErrors.class));
101101

102102
launcher.discover(request.build());
103103

@@ -106,7 +106,7 @@ public void testAfterAllFailuresAreReported() throws IOException {
106106

107107
// now write an assertion to validate the XML file has an error
108108
String[] expectedStrings = {
109-
"<failure message=\"I always fail.\" type=\"java.lang.RuntimeException\">", "failures=\"1\"",
109+
"<error message=\"I always error.\" type=\"java.lang.RuntimeException\">", "errors=\"1\"",
110110
};
111111

112112
// Useful for debugging the expected output
@@ -118,7 +118,7 @@ public void testAfterAllFailuresAreReported() throws IOException {
118118
"Expected to find " + expected + " in " + xmlFile);
119119
}
120120

121-
causeFailure.set(false);
121+
causeError.set(false);
122122
}
123123

124124
@Test
@@ -338,14 +338,14 @@ void testSuiteError(@TempDir Path tempDir)
338338
assertEquals(1, testsuiteList.getLength());
339339
Element testsuite = (Element) testsuiteList.item(0);
340340
assertEquals("4", testsuite.getAttribute("tests"));
341-
assertEquals("4", testsuite.getAttribute("failures"));
341+
assertEquals("4", testsuite.getAttribute("errors"));
342342

343343
NodeList testcaseList = testsuite.getElementsByTagName("testcase");
344344
assertEquals(4, testcaseList.getLength());
345345

346346
for (int i = 0; i < testcaseList.getLength(); i++) {
347347
Element testcase = (Element) testcaseList.item(i);
348-
assertNotNull(getChild("failure", testcase));
348+
assertNotNull(getChild("error", testcase));
349349
}
350350
}
351351

@@ -357,10 +357,10 @@ void throwablesWithNullMessageAreSerialized() {
357357
assertNotNull(root);
358358
assertEquals("testcase", root.getTagName());
359359

360-
var failures = root.getElementsByTagName("failure");
361-
assertEquals(1, failures.getLength());
360+
var errors = root.getElementsByTagName("error");
361+
assertEquals(1, errors.getLength());
362362

363-
var message = failures.item(0).getAttributes().getNamedItem("message");
363+
var message = errors.item(0).getAttributes().getNamedItem("message");
364364
assertNotNull(message);
365365
assertEquals("null", message.getTextContent());
366366
}
@@ -387,10 +387,10 @@ void throwablesWithIllegalXmlCharactersInMessageAreSerialized() {
387387
assertNotNull(root);
388388
assertEquals("testcase", root.getTagName());
389389

390-
var failures = root.getElementsByTagName("failure");
391-
assertEquals(1, failures.getLength());
390+
var errors = root.getElementsByTagName("error");
391+
assertEquals(1, errors.getLength());
392392

393-
var message = failures.item(0).getAttributes().getNamedItem("message");
393+
var message = errors.item(0).getAttributes().getNamedItem("message");
394394
assertNotNull(message);
395395
assertEquals(
396396
"legal: | | | [ -\uD7FF] | [\uE000-�]"

0 commit comments

Comments
 (0)