Avoid skipping the last error for @RetryingTest (#820 / #821)

boris-faniuk-n26 · web-flow · commit 98cef28462c8 · 2024-07-16T06:31:40.000+02:00
Fixes @RetryingTest "eating" the last exception for a test that always fails. Closes: #820
diff --git a/README.adoc b/README.adoc
@@ -124,6 +124,7 @@ The least we can do is to thank them and list some of their accomplishments here
 ==== 2024
 
 * https://github.com/TWiStErRob[Papp Róbert (TWiStErRob)] updated Gradle and GitHub Actions tooling to latest. (#803 / #804 / #805)
+* https://github.com/boris-faniuk-n26[Boris Faniuk] contributed to `@RetryingTest` extension (#821)
 
 ==== 2023
 
diff --git a/src/main/java/org/junitpioneer/jupiter/RetryingTestExtension.java b/src/main/java/org/junitpioneer/jupiter/RetryingTestExtension.java
@@ -163,10 +163,16 @@ <E extends Throwable> void failed(E exception) throws E {
 					exception);
 				seenExceptions.add(testAbortedException);
 				throw testAbortedException;
-			} else
+			} else {
+				var testAbortedException = new TestAbortedException(
+					format("%s%nTest execution #%d (of up to %d) failed ~> will not retry any more",
+						exception.getMessage(), retriesSoFar, maxRetries),
+					exception);
+				seenExceptions.add(testAbortedException);
 				throw new MultipleFailuresError(format(
 					"Test execution #%d (of up to %d with at least %d successes) failed ~> test fails - see cause for details",
 					retriesSoFar, maxRetries, minSuccess), seenExceptions);
+			}
 		}
 
 		private boolean expectedException(Throwable exception) {
diff --git a/src/test/java/org/junitpioneer/jupiter/RetryingTestExtensionTests.java b/src/test/java/org/junitpioneer/jupiter/RetryingTestExtensionTests.java
@@ -23,6 +23,7 @@
 import java.util.List;
 
 import org.assertj.core.api.Assertions;
+import org.assertj.core.api.InstanceOfAssertFactories;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.TestInfo;
 import org.junit.jupiter.api.TestInstance;
@@ -31,6 +32,8 @@
 import org.junit.platform.testkit.engine.Execution;
 import org.junitpioneer.testkit.ExecutionResults;
 import org.junitpioneer.testkit.PioneerTestKit;
+import org.opentest4j.MultipleFailuresError;
+import org.opentest4j.TestAbortedException;
 
 class RetryingTestExtensionTests {
 
@@ -111,6 +114,7 @@ void failsOnlyOnFirstInvocationWithUnexpectedException_executedOnce_fails() {
 				.executeTestMethod(RetryingTestTestCases.class, "failsOnlyOnFirstInvocationWithUnexpectedException");
 
 		assertThat(results).hasNumberOfDynamicallyRegisteredTests(1).hasNumberOfFailedTests(1);
+		assertThat(results).hasSingleFailedTest().withExceptionInstanceOf(NullPointerException.class);
 	}
 
 	@Test
@@ -122,6 +126,8 @@ void failsOnlyOnFirstInvocationWithUnexpectedException_executedTwice_fails() {
 				.hasNumberOfDynamicallyRegisteredTests(2)
 				.hasNumberOfAbortedTests(1)
 				.hasNumberOfFailedTests(1);
+
+		assertThat(results).hasSingleFailedTest().withExceptionInstanceOf(NullPointerException.class);
 	}
 
 	@Test
@@ -132,6 +138,8 @@ void failsAlways_executedThreeTimes_fails() {
 				.hasNumberOfDynamicallyRegisteredTests(3)
 				.hasNumberOfAbortedTests(2)
 				.hasNumberOfFailedTests(1);
+
+		assertFailedTest(results);
 	}
 
 	@Test
@@ -184,6 +192,9 @@ void executesOnceWithThreeFails_fails() {
 				.hasNumberOfAbortedTests(2)
 				.hasNumberOfFailedTests(1)
 				.hasNumberOfSucceededTests(1);
+
+		assertFailedTest(results);
+
 	}
 
 	@Test
@@ -195,6 +206,8 @@ void failsThreeTimes_fails() {
 				.hasNumberOfAbortedTests(2)
 				.hasNumberOfFailedTests(1)
 				.hasNumberOfSucceededTests(0);
+
+		assertFailedTest(results);
 	}
 
 	@Test
@@ -279,6 +292,7 @@ void failThreeTimesWithSuspend() {
 				.hasNumberOfSucceededTests(0);
 
 		assertSuspendedFor(results, SUSPEND_FOR);
+		assertFailedTest(results);
 	}
 
 	@Test
@@ -293,6 +307,7 @@ void failThreeTimesWithoutSuspend() {
 				.hasNumberOfSucceededTests(0);
 
 		assertSuspendedFor(results, 0);
+		assertFailedTest(results);
 	}
 
 	private void assertSuspendedFor(ExecutionResults results, long greaterThanOrEqualTo) {
@@ -315,6 +330,17 @@ private void assertSuspendedFor(ExecutionResults results, long greaterThanOrEqua
 		}
 	}
 
+	private void assertFailedTest(ExecutionResults results) {
+		assertThat(results)
+				.hasSingleFailedTest()
+				.withExceptionInstanceOf(MultipleFailuresError.class)
+				.extracting(MultipleFailuresError::getFailures, InstanceOfAssertFactories.list(Throwable.class))
+				.hasSize(3)
+				.hasOnlyElementsOfType(TestAbortedException.class)
+				.extracting(Throwable::getCause)
+				.hasOnlyElementsOfType(IllegalArgumentException.class);
+	}
+
 	// TEST CASES -------------------------------------------------------------------
 
 	// Some tests require state to keep track of the number of test executions.
