fix: BazelJUnitOutputListener logging xml output on suite-level failures

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

@@ -5,6 +5,7 @@
 import java.io.BufferedWriter;
 import java.io.Closeable;
 import java.io.IOException;
+import java.io.Writer;
 import java.nio.file.Files;
 import java.nio.file.Path;
 import java.util.ArrayList;
@@ -50,8 +51,12 @@ public class BazelJUnitOutputListener implements TestExecutionListener, Closeabl
   public BazelJUnitOutputListener(Path xmlOut) {
     try {
       Files.createDirectories(xmlOut.getParent());
-      BufferedWriter writer = Files.newBufferedWriter(xmlOut, UTF_8);
-      xml = XMLOutputFactory.newFactory().createXMLStreamWriter(writer);
+      // Use LazyFileWriter to defer file creation until the first write operation.
+      // This prevents premature file creation in cases where the JVM terminates abruptly
+      // before any content is written. If no output is generated, Bazel has custom logic
+      // to create the XML file from test.log, but this logic only activates if the
+      // output file does not already exist.
+      xml = XMLOutputFactory.newFactory().createXMLStreamWriter(new LazyFileWriter(xmlOut));
       xml.writeStartDocument("UTF-8", "1.0");
     } catch (IOException | XMLStreamException e) {
       throw new IllegalStateException("Unable to create output file", e);
@@ -140,8 +145,9 @@ private Map<TestData, List<TestData>> matchTestCasesToSuites_locked(
         throw new IllegalStateException(
             "Unexpected test organization for test Case: " + testCase.getId());
       }
-      if (includeIncompleteTests || testCase.getDuration() != null) {
-        knownSuites.computeIfAbsent(parent, id -> new ArrayList<>()).add(testCase);
+      knownSuites.computeIfAbsent(parent, id -> new ArrayList<>());
+      if (testCase.getId().isTest() && (includeIncompleteTests || testCase.getDuration() != null)) {
+        knownSuites.get(parent).add(testCase);
       }
     }
 
@@ -153,18 +159,19 @@ private Map<TestData, List<TestData>> matchTestCasesToSuites_locked(
   // This is really just documentation until someone actually turns on a static analyser.
   // If they do, we can decide whether we want to pick up the dependency.
   // @GuardedBy("resultsLock")
-  private List<TestData> findTestCases_locked() {
+  private List<TestData> findTestCases_locked(String engineId) {
     return results.values().stream()
         // Ignore test plan roots. These are always the engine being used.
         .filter(result -> !testPlan.getRoots().contains(result.getId()))
         .filter(
-            result -> {
-              // Find the test results we will convert to `testcase` entries. These
-              // are identified by the fact that they have no child test cases in the
-              // test plan, or they are marked as tests.
-              TestIdentifier id = result.getId();
-              return id.getSource() != null || id.isTest() || testPlan.getChildren(id).isEmpty();
-            })
+            result ->
+                engineId == null
+                    || result
+                        .getId()
+                        .getUniqueIdObject()
+                        .getEngineId()
+                        .map(engineId::equals)
+                        .orElse(false))
         .collect(Collectors.toList());
   }
 
@@ -196,20 +203,38 @@ private void outputIfTestRootIsComplete(TestIdentifier testIdentifier) {
       return;
     }
 
-    output(false);
+    output(false, testIdentifier.getUniqueIdObject().getEngineId().orElse(null));
   }
 
-  private void output(boolean includeIncompleteTests) {
+  private void output(boolean includeIncompleteTests, /*@Nullable*/ String engineId) {
     synchronized (this.resultsLock) {
-      List<TestData> testCases = findTestCases_locked();
+      List<TestData> testCases = findTestCases_locked(engineId);
       Map<TestData, List<TestData>> testSuites =
           matchTestCasesToSuites_locked(testCases, includeIncompleteTests);
 
       // Write the results
       try {
         for (Map.Entry<TestData, List<TestData>> suiteAndTests : testSuites.entrySet()) {
-          new TestSuiteXmlRenderer(testPlan)
-              .toXml(xml, suiteAndTests.getKey(), suiteAndTests.getValue());
+          TestData suite = suiteAndTests.getKey();
+          List<TestData> tests = suiteAndTests.getValue();
+          if (suite.getResult() != null
+              && suite.getResult().getStatus() != TestExecutionResult.Status.SUCCESSFUL) {
+            // If a test suite fails or is skipped, all its tests must be included in the XML output
+            // with the same result as the suite, since the XML format does not support marking a
+            // suite as failed or skipped. This aligns with Bazel's XmlWriter for JUnitRunner.
+            getTestsFromSuite(suite.getId())
+                .forEach(
+                    testIdentifier -> {
+                      TestData test = results.get(testIdentifier.getUniqueIdObject());
+                      if (test == null) {
+                        // add test to results.
+                        test = getResult(testIdentifier);
+                        tests.add(test);
+                      }
+                      test.mark(suite.getResult()).skipReason(suite.getSkipReason());
+                    });
+          }
+          new TestSuiteXmlRenderer(testPlan).toXml(xml, suite, tests);
         }
       } catch (XMLStreamException e) {
         throw new RuntimeException(e);
@@ -227,6 +252,18 @@ private void output(boolean includeIncompleteTests) {
     }
   }
 
+  private List<TestIdentifier> getTestsFromSuite(TestIdentifier suiteIdentifier) {
+    return testPlan.getChildren(suiteIdentifier).stream()
+        .flatMap(
+            testIdentifier -> {
+              if (testIdentifier.isContainer()) {
+                return getTestsFromSuite(testIdentifier).stream();
+              }
+              return Stream.of(testIdentifier);
+            })
+        .collect(Collectors.toList());
+  }
+
   @Override
   public void reportingEntryPublished(TestIdentifier testIdentifier, ReportEntry entry) {
     getResult(testIdentifier).addReportEntry(entry);
@@ -248,7 +285,7 @@ public void close() {
       return;
     }
     if (wasInterrupted.get()) {
-      output(true);
+      output(true, null);
     }
     try {
       xml.writeEndDocument();
@@ -257,4 +294,41 @@ public void close() {
       LOG.log(Level.SEVERE, "Unable to close xml output", e);
     }
   }
+
+  static class LazyFileWriter extends Writer {
+    private final Path path;
+    private BufferedWriter delegate;
+    private boolean isCreated = false;
+
+    public LazyFileWriter(Path path) {
+      this.path = path;
+    }
+
+    private void createWriterIfNeeded() throws IOException {
+      if (!isCreated) {
+        delegate = Files.newBufferedWriter(path, UTF_8);
+        isCreated = true;
+      }
+    }
+
+    @Override
+    public void write(char[] cbuf, int off, int len) throws IOException {
+      createWriterIfNeeded();
+      delegate.write(cbuf, off, len);
+    }
+
+    @Override
+    public void flush() throws IOException {
+      if (isCreated) {
+        delegate.flush();
+      }
+    }
+
+    @Override
+    public void close() throws IOException {
+      if (isCreated) {
+        delegate.close();
+      }
+    }
+  }
 }

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

@@ -11,9 +11,15 @@
 public class StubbedTestDescriptor implements TestDescriptor {
 
   private final UniqueId uniqueId;
+  private final Type type;
 
   public StubbedTestDescriptor(UniqueId uniqueId) {
+    this(uniqueId, Type.TEST);
+  }
+
+  public StubbedTestDescriptor(UniqueId uniqueId, Type type) {
     this.uniqueId = uniqueId;
+    this.type = type;
   }
 
   @Override
@@ -68,7 +74,7 @@ public void removeFromHierarchy() {
 
   @Override
   public Type getType() {
-    return Type.TEST;
+    return type;
   }
 
   @Override

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

@@ -8,6 +8,7 @@
 import static org.junit.platform.engine.discovery.DiscoverySelectors.selectClass;
 import static org.junit.platform.launcher.LauncherConstants.STDERR_REPORT_ENTRY_KEY;
 import static org.junit.platform.launcher.LauncherConstants.STDOUT_REPORT_ENTRY_KEY;
+import static org.mockito.Mockito.when;
 
 import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
 import java.io.File;
@@ -17,8 +18,10 @@
 import java.io.StringWriter;
 import java.io.Writer;
 import java.nio.file.Files;
+import java.nio.file.Path;
 import java.util.Collection;
 import java.util.List;
+import java.util.Set;
 import java.util.concurrent.atomic.AtomicBoolean;
 import javax.xml.parsers.DocumentBuilder;
 import javax.xml.parsers.DocumentBuilderFactory;
@@ -28,6 +31,7 @@
 import javax.xml.stream.XMLStreamWriter;
 import org.junit.jupiter.api.AfterAll;
 import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.io.TempDir;
 import org.junit.platform.engine.TestDescriptor;
 import org.junit.platform.engine.TestExecutionResult;
 import org.junit.platform.engine.UniqueId;
@@ -258,6 +262,57 @@ public void interruptedTestsAreMarkedAsFailed() {
     assertEquals("Test timed out and was interrupted", failure.getTextContent());
   }
 
+  @Test
+  void testSuiteError(@TempDir Path tempDir)
+      throws ParserConfigurationException, IOException, SAXException {
+    TestIdentifier root =
+        TestIdentifier.from(new StubbedTestDescriptor(UniqueId.parse("[engine:mocked]")));
+    TestIdentifier suiteIdentifier =
+        TestIdentifier.from(
+            new StubbedTestDescriptor(
+                UniqueId.parse("[engine:mocked]/[class:ExampleTest]"),
+                TestDescriptor.Type.CONTAINER));
+    TestData suite =
+        new TestData(suiteIdentifier)
+            .started()
+            .mark(TestExecutionResult.failed(new RuntimeException("test suite error")));
+    TestPlan testPlan = Mockito.mock(TestPlan.class);
+
+    TestIdentifier child1 = TestIdentifier.from(new StubbedTestDescriptor(createId("child1")));
+    TestIdentifier child2 = TestIdentifier.from(new StubbedTestDescriptor(createId("child2")));
+
+    when(testPlan.getChildren(suite.getId())).thenReturn(Set.of(child1, child2));
+    when(testPlan.getRoots()).thenReturn(Set.of(root));
+
+    Path xmlPath = tempDir.resolve("test.xml");
+    try (BazelJUnitOutputListener listener = new BazelJUnitOutputListener(xmlPath)) {
+      listener.testPlanExecutionStarted(testPlan);
+      listener.executionStarted(root);
+      listener.executionStarted(suiteIdentifier);
+      listener.executionFinished(
+          suiteIdentifier, TestExecutionResult.failed(new RuntimeException("test suite error")));
+      listener.executionFinished(root, TestExecutionResult.successful());
+    }
+
+    Document document =
+        DocumentBuilderFactory.newInstance().newDocumentBuilder().parse(xmlPath.toFile());
+    document.getDocumentElement().normalize();
+
+    NodeList testsuiteList = document.getDocumentElement().getElementsByTagName("testsuite");
+    assertEquals(1, testsuiteList.getLength());
+    Element testsuite = (Element) testsuiteList.item(0);
+    assertEquals("2", testsuite.getAttribute("tests"));
+    assertEquals("2", testsuite.getAttribute("failures"));
+
+    NodeList testcaseList = testsuite.getElementsByTagName("testcase");
+    assertEquals(2, testcaseList.getLength());
+
+    for (int i = 0; i < testcaseList.getLength(); i++) {
+      Element testcase = (Element) testcaseList.item(i);
+      assertNotNull(getChild("failure", testcase));
+    }
+  }
+
   @Test
   void throwablesWithNullMessageAreSerialized() {
     var test = new TestData(identifier).mark(TestExecutionResult.failed(new Throwable()));