Use suite events for more accurate reporting

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

@@ -56,4 +56,5 @@ GitHub.
 
 ==== New Features and Improvements
 
-* ❓
+* More accurate reporting of intermediate start/finish events, e.g. iterations of the
+  `Parameterized` runner and classes executed indirectly via the `Suite` runner.

junit-vintage-engine/src/main/java/org/junit/vintage/engine/JUnit4VersionCheck.java

@@ -51,7 +51,7 @@ static void checkSupported(Supplier<String> versionSupplier) {
 		}
 	}
 
-	private static BigDecimal parseVersion(String versionString) {
+	static BigDecimal parseVersion(String versionString) {
 		try {
 			Matcher matcher = versionPattern.matcher(versionString);
 			if (matcher.matches()) {

junit-vintage-engine/src/main/java/org/junit/vintage/engine/execution/RunListenerAdapter.java

@@ -54,6 +54,15 @@ public void testRunStarted(Description description) {
 		}
 	}
 
+	@Override
+	public void testSuiteStarted(Description description) {
+		RunnerTestDescriptor runnerTestDescriptor = testRun.getRunnerTestDescriptor();
+		// runnerTestDescriptor is reported in testRunStarted
+		if (!runnerTestDescriptor.getDescription().equals(description)) {
+			testStarted(lookupOrRegisterNextTestDescriptor(description), EventType.REPORTED);
+		}
+	}
+
 	@Override
 	public void testIgnored(Description description) {
 		testIgnored(lookupOrRegisterNextTestDescriptor(description), determineReasonForIgnoredTest(description));
@@ -80,17 +89,29 @@ public void testFinished(Description description) {
 	}
 
 	@Override
-	public void testRunFinished(Result result) {
+	public void testSuiteFinished(Description description) {
 		RunnerTestDescriptor runnerTestDescriptor = testRun.getRunnerTestDescriptor();
-		if (testRun.isNotSkipped(runnerTestDescriptor)) {
-			if (testRun.isNotStarted(runnerTestDescriptor)) {
-				fireExecutionStarted(runnerTestDescriptor, EventType.SYNTHETIC);
+		// runnerTestDescriptor is reported in testRunFinished
+		if (!runnerTestDescriptor.getDescription().equals(description)) {
+			reportContainerFinished(lookupOrRegisterNextTestDescriptor(description));
+		}
+	}
+
+	@Override
+	public void testRunFinished(Result result) {
+		reportContainerFinished(testRun.getRunnerTestDescriptor());
+	}
+
+	private void reportContainerFinished(TestDescriptor containerTestDescriptor) {
+		if (testRun.isNotSkipped(containerTestDescriptor)) {
+			if (testRun.isNotStarted(containerTestDescriptor)) {
+				fireExecutionStarted(containerTestDescriptor, EventType.SYNTHETIC);
 			}
 			testRun.getInProgressTestDescriptorsWithSyntheticStartEvents().stream() //
 					.filter(this::canFinish) //
 					.forEach(this::fireExecutionFinished);
-			if (testRun.isNotFinished(runnerTestDescriptor)) {
-				fireExecutionFinished(runnerTestDescriptor);
+			if (testRun.isNotFinished(containerTestDescriptor)) {
+				fireExecutionFinished(containerTestDescriptor);
 			}
 		}
 	}
@@ -143,6 +164,7 @@ private void handleFailure(Failure failure, Function<Throwable, TestExecutionRes
 			testStarted(testDescriptor, EventType.SYNTHETIC);
 		}
 		if (testRun.isNotFinished(testDescriptor) && testDescriptor.isContainer()
+				&& testRun.hasSyntheticStartEvent(testDescriptor)
 				&& testRun.isDescendantOfRunnerTestDescriptor(testDescriptor)) {
 			testFinished(testDescriptor);
 		}

junit-vintage-engine/src/main/java/org/junit/vintage/engine/execution/TestRun.java

@@ -91,6 +91,10 @@ boolean isDescendantOfRunnerTestDescriptor(TestDescriptor testDescriptor) {
 		return runnerDescendants.contains(testDescriptor);
 	}
 
+	boolean hasSyntheticStartEvent(TestDescriptor testDescriptor) {
+		return inProgressDescriptors.get(testDescriptor) == EventType.SYNTHETIC;
+	}
+
 	Optional<VintageTestDescriptor> lookupNextTestDescriptor(Description description) {
 		return lookupUnambiguouslyOrApplyFallback(description, VintageDescriptors::getNextUnstarted);
 	}

junit-vintage-engine/src/test/java/org/junit/vintage/engine/VintageTestEngineExecutionTests.java

@@ -11,6 +11,7 @@
 package org.junit.vintage.engine;
 
 import static org.assertj.core.api.Assertions.assertThat;
+import static org.junit.jupiter.api.Assumptions.assumeTrue;
 import static org.junit.platform.engine.discovery.DiscoverySelectors.selectClass;
 import static org.junit.platform.engine.discovery.DiscoverySelectors.selectMethod;
 import static org.junit.platform.engine.discovery.DiscoverySelectors.selectUniqueId;
@@ -31,6 +32,10 @@
 import static org.junit.runner.Description.createSuiteDescription;
 import static org.junit.runner.Description.createTestDescription;
 
+import java.math.BigDecimal;
+
+import junit.runner.Version;
+
 import org.assertj.core.api.Condition;
 import org.junit.AssumptionViolatedException;
 import org.junit.jupiter.api.Test;
@@ -80,6 +85,9 @@
 import org.junit.vintage.engine.samples.junit4.JUnit4TestCaseWithRunnerWithDuplicateChangingChildDescriptions;
 import org.junit.vintage.engine.samples.junit4.MalformedJUnit4TestCase;
 import org.junit.vintage.engine.samples.junit4.ParameterizedTestCase;
+import org.junit.vintage.engine.samples.junit4.ParameterizedTimingTestCase;
+import org.junit.vintage.engine.samples.junit4.ParameterizedWithAfterParamFailureTestCase;
+import org.junit.vintage.engine.samples.junit4.ParameterizedWithBeforeParamFailureTestCase;
 import org.junit.vintage.engine.samples.junit4.PlainJUnit4TestCaseWithFiveTestMethods;
 import org.junit.vintage.engine.samples.junit4.PlainJUnit4TestCaseWithLifecycleMethods;
 import org.junit.vintage.engine.samples.junit4.PlainJUnit4TestCaseWithSingleTestWhichFails;
@@ -455,6 +463,74 @@ void executesIgnoredParameterizedTestCase() {
 			event(engine(), finishedSuccessfully()));
 	}
 
+	@Test
+	void executesParameterizedTimingTestCase() {
+		assumeTrue(atLeastJUnit4_13(), "@BeforeParam and @AfterParam were introduced in JUnit 4.13");
+
+		Class<?> testClass = ParameterizedTimingTestCase.class;
+
+		var events = execute(testClass).allEvents().debug();
+
+		var firstParamStartedEvent = events.filter(event(container("[foo]"), started())::matches).findFirst() //
+				.orElseThrow(() -> new AssertionError("No start event for [foo]"));
+		var firstParamFinishedEvent = events.filter(
+			event(container("[foo]"), finishedSuccessfully())::matches).findFirst() //
+				.orElseThrow(() -> new AssertionError("No finish event for [foo]"));
+		var secondParamStartedEvent = events.filter(event(container("[bar]"), started())::matches).findFirst() //
+				.orElseThrow(() -> new AssertionError("No start event for [bar]"));
+		var secondParamFinishedEvent = events.filter(
+			event(container("[bar]"), finishedSuccessfully())::matches).findFirst() //
+				.orElseThrow(() -> new AssertionError("No finish event for [bar]"));
+
+		assertThat(ParameterizedTimingTestCase.EVENTS.get("beforeParam(foo)")).isAfterOrEqualTo(
+			firstParamStartedEvent.getTimestamp());
+		assertThat(ParameterizedTimingTestCase.EVENTS.get("afterParam(foo)")).isBeforeOrEqualTo(
+			firstParamFinishedEvent.getTimestamp());
+		assertThat(ParameterizedTimingTestCase.EVENTS.get("beforeParam(bar)")).isAfterOrEqualTo(
+			secondParamStartedEvent.getTimestamp());
+		assertThat(ParameterizedTimingTestCase.EVENTS.get("afterParam(bar)")).isBeforeOrEqualTo(
+			secondParamFinishedEvent.getTimestamp());
+	}
+
+	@Test
+	void executesParameterizedWithAfterParamFailureTestCase() {
+		assumeTrue(atLeastJUnit4_13(), "@AfterParam was introduced in JUnit 4.13");
+
+		Class<?> testClass = ParameterizedWithAfterParamFailureTestCase.class;
+
+		execute(testClass).allEvents().assertEventsMatchExactly( //
+			event(engine(), started()), //
+			event(container(testClass), started()), //
+			event(container("[foo]"), started()), //
+			event(test("test[foo]"), started()), //
+			event(test("test[foo]"), finishedSuccessfully()), //
+			event(container("[foo]"), finishedWithFailure(instanceOf(AssertionError.class))), //
+			event(container("[bar]"), started()), //
+			event(test("test[bar]"), started()), //
+			event(test("test[bar]"),
+				finishedWithFailure(instanceOf(AssertionError.class), message("expected:<[foo]> but was:<[bar]>"))), //
+			event(container("[bar]"), finishedWithFailure(instanceOf(AssertionError.class))), //
+			event(container(testClass), finishedSuccessfully()), //
+			event(engine(), finishedSuccessfully()));
+	}
+
+	@Test
+	void executesParameterizedWithBeforeParamFailureTestCase() {
+		assumeTrue(atLeastJUnit4_13(), "@BeforeParam was introduced in JUnit 4.13");
+
+		Class<?> testClass = ParameterizedWithBeforeParamFailureTestCase.class;
+
+		execute(testClass).allEvents().assertEventsMatchExactly( //
+			event(engine(), started()), //
+			event(container(testClass), started()), //
+			event(container("[foo]"), started()), //
+			event(container("[foo]"), finishedWithFailure(instanceOf(AssertionError.class))), //
+			event(container("[bar]"), started()), //
+			event(container("[bar]"), finishedWithFailure(instanceOf(AssertionError.class))), //
+			event(container(testClass), finishedSuccessfully()), //
+			event(engine(), finishedSuccessfully()));
+	}
+
 	@Test
 	void executesJUnit4TestCaseWithExceptionThrowingRunner() {
 		Class<?> testClass = JUnit4TestCaseWithExceptionThrowingRunner.class;
@@ -806,4 +882,8 @@ private static LauncherDiscoveryRequest request(Class<?> testClass) {
 		return LauncherDiscoveryRequestBuilder.request().selectors(selectClass(testClass)).build();
 	}
 
+	private static boolean atLeastJUnit4_13() {
+		return JUnit4VersionCheck.parseVersion(Version.id()).compareTo(new BigDecimal("4.13")) >= 0;
+	}
+
 }

junit-vintage-engine/src/testFixtures/java/org/junit/vintage/engine/samples/junit4/ParameterizedTimingTestCase.java

@@ -0,0 +1,68 @@
+/*
+ * Copyright 2015-2022 the original author or authors.
+ *
+ * All rights reserved. This program and the accompanying materials are
+ * made available under the terms of the Eclipse Public License v2.0 which
+ * accompanies this distribution and is available at
+ *
+ * https://www.eclipse.org/legal/epl-v20.html
+ */
+
+package org.junit.vintage.engine.samples.junit4;
+
+import static org.junit.Assert.assertEquals;
+
+import java.time.Instant;
+import java.util.LinkedHashMap;
+import java.util.List;
+import java.util.Map;
+
+import org.junit.BeforeClass;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+import org.junit.runners.Parameterized.AfterParam;
+import org.junit.runners.Parameterized.BeforeParam;
+import org.junit.runners.Parameterized.Parameter;
+import org.junit.runners.Parameterized.Parameters;
+
+/**
+ * @since 5.9
+ */
+@RunWith(Parameterized.class)
+public class ParameterizedTimingTestCase {
+
+	public static Map<String, Instant> EVENTS = new LinkedHashMap<>();
+
+	@BeforeClass
+	public static void beforeClass() throws Exception {
+		EVENTS.clear();
+	}
+
+	@BeforeParam
+	public static void beforeParam(String param) throws Exception {
+		EVENTS.put("beforeParam(" + param + ")", Instant.now());
+		Thread.sleep(100);
+	}
+
+	@AfterParam
+	public static void afterParam(String param) throws Exception {
+		Thread.sleep(100);
+		System.out.println("ParameterizedTimingTestCase.afterParam");
+		EVENTS.put("afterParam(" + param + ")", Instant.now());
+	}
+
+	@Parameters(name = "{0}")
+	public static Iterable<String> parameters() {
+		return List.of("foo", "bar");
+	}
+
+	@Parameter
+	public String value;
+
+	@Test
+	public void test() {
+		assertEquals("foo", value);
+	}
+
+}

junit-vintage-engine/src/testFixtures/java/org/junit/vintage/engine/samples/junit4/ParameterizedWithAfterParamFailureTestCase.java

@@ -0,0 +1,49 @@
+/*
+ * Copyright 2015-2022 the original author or authors.
+ *
+ * All rights reserved. This program and the accompanying materials are
+ * made available under the terms of the Eclipse Public License v2.0 which
+ * accompanies this distribution and is available at
+ *
+ * https://www.eclipse.org/legal/epl-v20.html
+ */
+
+package org.junit.vintage.engine.samples.junit4;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.fail;
+
+import java.util.List;
+
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+import org.junit.runners.Parameterized.AfterParam;
+import org.junit.runners.Parameterized.Parameter;
+import org.junit.runners.Parameterized.Parameters;
+
+/**
+ * @since 5.9
+ */
+@RunWith(Parameterized.class)
+public class ParameterizedWithAfterParamFailureTestCase {
+
+	@AfterParam
+	public static void afterParam() {
+		fail();
+	}
+
+	@Parameters(name = "{0}")
+	public static Iterable<String> parameters() {
+		return List.of("foo", "bar");
+	}
+
+	@Parameter
+	public String value;
+
+	@Test
+	public void test() {
+		assertEquals("foo", value);
+	}
+
+}

junit-vintage-engine/src/testFixtures/java/org/junit/vintage/engine/samples/junit4/ParameterizedWithBeforeParamFailureTestCase.java

@@ -0,0 +1,49 @@
+/*
+ * Copyright 2015-2022 the original author or authors.
+ *
+ * All rights reserved. This program and the accompanying materials are
+ * made available under the terms of the Eclipse Public License v2.0 which
+ * accompanies this distribution and is available at
+ *
+ * https://www.eclipse.org/legal/epl-v20.html
+ */
+
+package org.junit.vintage.engine.samples.junit4;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.fail;
+
+import java.util.List;
+
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+import org.junit.runners.Parameterized.BeforeParam;
+import org.junit.runners.Parameterized.Parameter;
+import org.junit.runners.Parameterized.Parameters;
+
+/**
+ * @since 5.9
+ */
+@RunWith(Parameterized.class)
+public class ParameterizedWithBeforeParamFailureTestCase {
+
+	@BeforeParam
+	public static void beforeParam() {
+		fail();
+	}
+
+	@Parameters(name = "{0}")
+	public static Iterable<String> parameters() {
+		return List.of("foo", "bar");
+	}
+
+	@Parameter
+	public String value;
+
+	@Test
+	public void test() {
+		assertEquals("foo", value);
+	}
+
+}