Fix container failure XML reporting

documentation/src/docs/asciidoc/release-notes/release-notes-5.7.1.adoc

@@ -27,6 +27,8 @@ GitHub.
   of this change may be witnessed by end users and test engine or extension authors.
 * Method `scanForClassesInPackage(String)` in `ClasspathScanner` now returns a valid list
   of class names when the package name is equal to the name of a module on the module path.
+* The legacy XML report now always includes container-level failures (e.g. from
+  `@BeforeAll` Jupiter lifecycle methods).
 
 ==== Deprecations and Breaking Changes
 

junit-platform-engine/src/testFixtures/java/org/junit/platform/engine/support/hierarchical/DemoEngineExecutionContext.java

@@ -13,5 +13,5 @@
 /**
  * @since 1.0
  */
-class DemoEngineExecutionContext implements EngineExecutionContext {
+public class DemoEngineExecutionContext implements EngineExecutionContext {
 }

junit-platform-engine/src/testFixtures/java/org/junit/platform/engine/support/hierarchical/DemoHierarchicalTestDescriptor.java

@@ -26,7 +26,7 @@ public class DemoHierarchicalTestDescriptor extends AbstractTestDescriptor imple
 	private String skippedReason;
 	private boolean skipped;
 
-	DemoHierarchicalTestDescriptor(UniqueId uniqueId, String displayName, Runnable executeBlock) {
+	public DemoHierarchicalTestDescriptor(UniqueId uniqueId, String displayName, Runnable executeBlock) {
 		this(uniqueId, displayName, null, executeBlock);
 	}
 

junit-platform-engine/src/testFixtures/java/org/junit/platform/engine/support/hierarchical/DemoHierarchicalTestEngine.java

@@ -10,6 +10,8 @@
 
 package org.junit.platform.engine.support.hierarchical;
 
+import java.util.function.Function;
+
 import org.junit.platform.engine.EngineDiscoveryRequest;
 import org.junit.platform.engine.ExecutionRequest;
 import org.junit.platform.engine.TestDescriptor;
@@ -47,10 +49,8 @@ public DemoHierarchicalTestDescriptor addTest(String uniqueName, Runnable execut
 	}
 
 	public DemoHierarchicalTestDescriptor addTest(String uniqueName, String displayName, Runnable executeBlock) {
-		var uniqueId = engineDescriptor.getUniqueId().append("test", uniqueName);
-		var child = new DemoHierarchicalTestDescriptor(uniqueId, displayName, executeBlock);
-		engineDescriptor.addChild(child);
-		return child;
+		return addChild(uniqueName, uniqueId -> new DemoHierarchicalTestDescriptor(uniqueId, displayName, executeBlock),
+			"test");
 	}
 
 	public DemoHierarchicalContainerDescriptor addContainer(String uniqueName, String displayName, TestSource source) {
@@ -64,10 +64,17 @@ public DemoHierarchicalContainerDescriptor addContainer(String uniqueName, Runna
 	public DemoHierarchicalContainerDescriptor addContainer(String uniqueName, String displayName, TestSource source,
 			Runnable beforeBlock) {
 
-		var uniqueId = engineDescriptor.getUniqueId().append("container", uniqueName);
-		var container = new DemoHierarchicalContainerDescriptor(uniqueId, displayName, source, beforeBlock);
-		engineDescriptor.addChild(container);
-		return container;
+		return addChild(uniqueName,
+			uniqueId -> new DemoHierarchicalContainerDescriptor(uniqueId, displayName, source, beforeBlock),
+			"container");
+	}
+
+	public <T extends TestDescriptor & Node<DemoEngineExecutionContext>> T addChild(String uniqueName,
+			Function<UniqueId, T> creator, String segmentType) {
+		var uniqueId = engineDescriptor.getUniqueId().append(segmentType, uniqueName);
+		var child = creator.apply(uniqueId);
+		engineDescriptor.addChild(child);
+		return child;
 	}
 
 	@Override

junit-platform-reporting/src/main/java/org/junit/platform/reporting/legacy/xml/XmlReportData.java

@@ -11,15 +11,16 @@
 package org.junit.platform.reporting.legacy.xml;
 
 import static java.util.Collections.emptyList;
+import static java.util.stream.Collectors.toList;
 import static org.junit.platform.engine.TestExecutionResult.Status.ABORTED;
-import static org.junit.platform.engine.TestExecutionResult.Status.SUCCESSFUL;
 
 import java.time.Clock;
 import java.time.Duration;
 import java.time.Instant;
 import java.util.ArrayList;
 import java.util.List;
 import java.util.Map;
+import java.util.Objects;
 import java.util.Optional;
 import java.util.concurrent.ConcurrentHashMap;
 import java.util.function.Predicate;
@@ -103,32 +104,23 @@ String getSkipReason(TestIdentifier testIdentifier) {
 		}).orElse(null);
 	}
 
-	Optional<TestExecutionResult> getResult(TestIdentifier testIdentifier) {
-		if (this.finishedTests.containsKey(testIdentifier)) {
-			return Optional.of(this.finishedTests.get(testIdentifier));
-		}
-		Optional<TestIdentifier> parent = this.testPlan.getParent(testIdentifier);
-		Optional<TestIdentifier> ancestor = findAncestor(parent, this.finishedTests::containsKey);
-		if (ancestor.isPresent()) {
-			TestExecutionResult result = this.finishedTests.get(ancestor.get());
-			if (result.getStatus() != SUCCESSFUL) {
-				return Optional.of(result);
-			}
-		}
-		return Optional.empty();
+	List<TestExecutionResult> getResults(TestIdentifier testIdentifier) {
+		return getAncestors(testIdentifier).stream() //
+				.map(this.finishedTests::get) //
+				.filter(Objects::nonNull) //
+				.collect(toList());
 	}
 
 	List<ReportEntry> getReportEntries(TestIdentifier testIdentifier) {
 		return this.reportEntries.getOrDefault(testIdentifier, emptyList());
 	}
 
 	private Optional<TestIdentifier> findSkippedAncestor(TestIdentifier testIdentifier) {
-		return findAncestor(Optional.of(testIdentifier), this.skippedTests::containsKey);
+		return findAncestor(testIdentifier, this.skippedTests::containsKey);
 	}
 
-	private Optional<TestIdentifier> findAncestor(Optional<TestIdentifier> testIdentifier,
-			Predicate<TestIdentifier> predicate) {
-		Optional<TestIdentifier> current = testIdentifier;
+	private Optional<TestIdentifier> findAncestor(TestIdentifier testIdentifier, Predicate<TestIdentifier> predicate) {
+		Optional<TestIdentifier> current = Optional.of(testIdentifier);
 		while (current.isPresent()) {
 			if (predicate.test(current.get())) {
 				return current;
@@ -138,4 +130,14 @@ private Optional<TestIdentifier> findAncestor(Optional<TestIdentifier> testIdent
 		return Optional.empty();
 	}
 
+	private List<TestIdentifier> getAncestors(TestIdentifier testIdentifier) {
+		TestIdentifier current = testIdentifier;
+		List<TestIdentifier> ancestors = new ArrayList<>();
+		while (current != null) {
+			ancestors.add(current);
+			current = this.testPlan.getParent(current).orElse(null);
+		}
+		return ancestors;
+	}
+
 }