Add option to suspend between retrying tests (junit-pioneer#407 / jun…

…it-pioneer#604)

Retrying tests may be flaky because of issues with external services.
The chance that they recover and behave correctly on the next try may
improve if there's a (short) pause between the test runs. This change
adds an option to suspend for a fixed time span (in milliseconds)
betweet tries.

The implemenation simply lets the executing thread sleep, which
isn't ideal. It would be much better if there were a way to schedule
the next execution for later and use the thread to execute other
tests in between, but Jupiter doesn't offer an API for that.

Closes: junit-pioneer#407
PR: junit-pioneer#604

README.md

@@ -118,8 +118,9 @@ The least we can do is to thank them and list some of their accomplishments here
 
 #### 2022
 
-* [Pankaj Kumar](https://github.com/p1729) contributed towards improving GitHub actions (#587 / #611)
 * [Filip Hrisafov](https://github.com/filiphr) contributed the [JSON Argument Source](https://junit-pioneer.org/docs/json-argument-source/) support (#101 / #492)
+* [Mathieu Fortin](https://github.com/mathieufortin01) contributed the `suspendForMs` attribute in [retrying tests](https://junit-pioneer.org/docs/retrying-test/) (#407 / #604)
+* [Pankaj Kumar](https://github.com/p1729) contributed towards improving GitHub actions (#587 / #611)
 
 #### 2021
 

docs/retrying-test.adoc

@@ -27,6 +27,14 @@ The test function will be executed, up to `maxAttempts`, until it completes succ
 
 The default for `minSuccess` is 1. The value must be greater than or equal to 1.
 
+=== suspendForMs [optional]
+
+The `suspendForMs` attribute specifies the amount of time to pause between retries, in milliseconds.
+
+The thread executing this test is sleeping during that time and won't execute other tests, so long suspensions are discouraged.
+
+The default for `suspendForMs` is 0 (no pause). The value must be greater than or equal to 0.
+
 === onExceptions [optional]
 
 By default, a test annotated with `@RetryingTest` will be retried on all exceptions except https://ota4j-team.github.io/opentest4j/docs/current/api/org/opentest4j/TestAbortedException.html[`TestAbortedException`] (which will abort the test entirely).
@@ -91,6 +99,17 @@ include::{demo}[tag=retrying_configure_numbers_of_success]
 The test `requiresTwoSuccesses` must run at least two times without raising an exception.
 However, it will not run more than four times.
 
+=== Suspending between each Retry
+
+[source,java,indent=0]
+----
+include::{demo}[tag=suspending_between_each_retry]
+----
+
+Use `suspendForMs` to specify the number of milliseconds to wait between each retry.
+
+After failure, the test `suspendBetweenRetries` will wait for 100ms before retrying.
+
 === Configuring on which Exceptions to Retry
 
 Use `onExceptions` to only retry on the mentioned exception(s):

src/demo/java/org/junitpioneer/jupiter/RetryingTestExtensionDemo.java

@@ -49,6 +49,13 @@ void requiresTwoSuccesses() {
 	}
 	// end::retrying_configure_numbers_of_success[]
 
+	// tag::suspending_between_each_retry[]
+	@RetryingTest(maxAttempts = 3, suspendForMs = 100)
+	void suspendsBetweenRetries() {
+		// test code that will suspend/wait for 100 ms between retries
+	}
+	// end::suspending_between_each_retry[]
+
 	class TheseTestsWillFailIntentionally {
 
 		// tag::retrying_configure_exception_for_retry[]

src/main/java/org/junitpioneer/jupiter/RetryingTest.java

@@ -142,6 +142,16 @@
 	 */
 	int minSuccess() default 1;
 
+	/**
+	 * Specifies a pause (in milliseconds) between executions.
+	 *
+	 * <p>The thread executing this test is sleeping during that time
+	 * and won't execute other tests, so long suspensions are discouraged.</p>
+	 *
+	 * <p>Value must be greater than or equal to 0.</p>
+	 */
+	int suspendForMs() default 0;
+
 	/**
 	 * Specifies on which exceptions a failed test is retried.
 	 *

src/main/java/org/junitpioneer/jupiter/RetryingTestExtension.java

@@ -19,6 +19,7 @@
 import java.util.Arrays;
 import java.util.Iterator;
 import java.util.NoSuchElementException;
+import java.util.concurrent.TimeUnit;
 import java.util.stream.Stream;
 
 import org.junit.jupiter.api.extension.ExtensionConfigurationException;
@@ -73,6 +74,7 @@ private static class FailedTestRetrier implements Iterator<RetryingTestInvocatio
 
 		private final int maxRetries;
 		private final int minSuccess;
+		private final int suspendForMs;
 		private final Class<? extends Throwable>[] expectedExceptions;
 		private final TestNameFormatter formatter;
 
@@ -81,10 +83,11 @@ private static class FailedTestRetrier implements Iterator<RetryingTestInvocatio
 		private boolean seenFailedAssumption;
 		private boolean seenUnexpectedException;
 
-		private FailedTestRetrier(int maxRetries, int minSuccess, Class<? extends Throwable>[] expectedExceptions,
-				TestNameFormatter formatter) {
+		private FailedTestRetrier(int maxRetries, int minSuccess, int suspendForMs,
+				Class<? extends Throwable>[] expectedExceptions, TestNameFormatter formatter) {
 			this.maxRetries = maxRetries;
 			this.minSuccess = minSuccess;
+			this.suspendForMs = suspendForMs;
 			this.expectedExceptions = expectedExceptions;
 			this.retriesSoFar = 0;
 			this.exceptionsSoFar = 0;
@@ -101,19 +104,20 @@ static FailedTestRetrier createFor(Method test, ExtensionContext context) {
 			String pattern = retryingTest.name();
 
 			if (maxAttempts == 0)
-				throw new IllegalStateException("@RetryingTest requires that one of `value` or `maxAttempts` be set.");
+				throw new ExtensionConfigurationException(
+					"@RetryingTest requires that one of `value` or `maxAttempts` be set.");
 			if (retryingTest.value() != 0 && retryingTest.maxAttempts() != 0)
-				throw new IllegalStateException(
+				throw new ExtensionConfigurationException(
 					"@RetryingTest requires that one of `value` or `maxAttempts` be set, but not both.");
 
 			if (minSuccess < 1)
-				throw new IllegalStateException(
+				throw new ExtensionConfigurationException(
 					"@RetryingTest requires that `minSuccess` be greater than or equal to 1.");
 			else if (maxAttempts <= minSuccess) {
 				String additionalMessage = maxAttempts == minSuccess
 						? " Using @RepeatedTest is recommended as a replacement."
 						: "";
-				throw new IllegalStateException(
+				throw new ExtensionConfigurationException(
 					format("@RetryingTest requires that `maxAttempts` be greater than %s.%s",
 						minSuccess == 1 ? "1" : "`minSuccess`", additionalMessage));
 			}
@@ -122,7 +126,13 @@ else if (maxAttempts <= minSuccess) {
 			String displayName = context.getDisplayName();
 			TestNameFormatter formatter = new TestNameFormatter(pattern, displayName, RetryingTest.class);
 
-			return new FailedTestRetrier(maxAttempts, minSuccess, retryingTest.onExceptions(), formatter);
+			if (retryingTest.suspendForMs() < 0) {
+				throw new ExtensionConfigurationException(
+					"@RetryingTest requires that `suspendForMs` be greater than or equal to 0.");
+			}
+
+			return new FailedTestRetrier(maxAttempts, minSuccess, retryingTest.suspendForMs(),
+				retryingTest.onExceptions(), formatter);
 		}
 
 		<E extends Throwable> void failed(E exception) throws E {
@@ -141,7 +151,8 @@ <E extends Throwable> void failed(E exception) throws E {
 
 			if (hasNext())
 				throw new TestAbortedException(
-					format("Test execution #%d (of up to %d) failed ~> will retry...", retriesSoFar, maxRetries),
+					format("Test execution #%d (of up to %d) failed ~> will retry in %d ms...", retriesSoFar,
+						maxRetries, suspendForMs),
 					exception);
 			else
 				throw new AssertionFailedError(format(
@@ -157,10 +168,28 @@ private boolean expectedException(Throwable exception) {
 			return Arrays.stream(expectedExceptions).anyMatch(type -> type.isInstance(exception));
 		}
 
+		private void suspendFor(int millis) {
+			if (millis < 1) {
+				return;
+			}
+
+			try {
+				TimeUnit.MILLISECONDS.sleep(millis);
+			}
+			catch (InterruptedException ex) {
+				Thread.currentThread().interrupt();
+				throw new IllegalStateException("Thread interrupted during retry suspension.", ex);
+			}
+		}
+
+		private boolean isFirstExecution() {
+			return retriesSoFar == 0;
+		}
+
 		@Override
 		public boolean hasNext() {
 			// there's always at least one execution
-			if (retriesSoFar == 0)
+			if (isFirstExecution())
 				return true;
 			if (seenFailedAssumption || seenUnexpectedException)
 				return false;
@@ -176,7 +205,13 @@ public boolean hasNext() {
 		public RetryingTestInvocationContext next() {
 			if (!hasNext())
 				throw new NoSuchElementException();
+
+			if (!isFirstExecution()) {
+				suspendFor(suspendForMs);
+			}
+
 			retriesSoFar++;
+
 			return new RetryingTestInvocationContext(formatter);
 		}
 

src/test/java/org/junitpioneer/jupiter/RetryingTestExtensionTests.java

@@ -18,17 +18,24 @@
 import java.lang.annotation.Retention;
 import java.lang.annotation.RetentionPolicy;
 import java.lang.annotation.Target;
+import java.time.Duration;
+import java.time.Instant;
+import java.util.List;
 
+import org.assertj.core.api.Assertions;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.TestInfo;
 import org.junit.jupiter.api.TestInstance;
 import org.junit.jupiter.api.TestReporter;
 import org.junit.jupiter.api.TestTemplate;
+import org.junit.platform.testkit.engine.Execution;
 import org.junitpioneer.testkit.ExecutionResults;
 import org.junitpioneer.testkit.PioneerTestKit;
 
 class RetryingTestExtensionTests {
 
+	private static final int SUSPEND_FOR = 10;
+
 	@Test
 	void invalidConfigurationWithTest() {
 		ExecutionResults results = PioneerTestKit
@@ -252,6 +259,62 @@ void minSuccessLessThanOne_fails() {
 		assertThat(results).hasNumberOfDynamicallyRegisteredTests(0);
 	}
 
+	@Test
+	void suspendForLessThanZero_fails() {
+		ExecutionResults results = PioneerTestKit
+				.executeTestMethod(RetryingTestTestCases.class, "suspendForLessThanZero");
+
+		assertThat(results).hasNumberOfDynamicallyRegisteredTests(0);
+	}
+
+	@Test
+	void failThreeTimesWithSuspend() {
+		ExecutionResults results = PioneerTestKit
+				.executeTestMethod(RetryingTestTestCases.class, "failThreeTimesWithSuspend");
+
+		assertThat(results)
+				.hasNumberOfDynamicallyRegisteredTests(3)
+				.hasNumberOfAbortedTests(2)
+				.hasNumberOfFailedTests(1)
+				.hasNumberOfSucceededTests(0);
+
+		assertSuspendedFor(results, SUSPEND_FOR);
+	}
+
+	@Test
+	void failThreeTimesWithoutSuspend() {
+		ExecutionResults results = PioneerTestKit
+				.executeTestMethod(RetryingTestTestCases.class, "failThreeTimesWithoutSuspend");
+
+		assertThat(results)
+				.hasNumberOfDynamicallyRegisteredTests(3)
+				.hasNumberOfAbortedTests(2)
+				.hasNumberOfFailedTests(1)
+				.hasNumberOfSucceededTests(0);
+
+		assertSuspendedFor(results, 0);
+	}
+
+	private void assertSuspendedFor(ExecutionResults results, long greaterThanOrEqualTo) {
+		List<Execution> finishedExecutions = results.testEvents().executions().finished().list();
+		List<Execution> startedExecutions = results.testEvents().executions().started().list();
+
+		// Compare 'finished' execution timestamp with follow-up 'started' execution,
+		// skipping the 1st 'started' and final 'finished'.
+		// This duration should be equals to or greater than 'suspendFor'.
+		for (int i = 0; i < finishedExecutions.size() - 1; i++) {
+			Execution finished = finishedExecutions.get(i);
+			Execution nextStarted = startedExecutions.get(i + 1);
+
+			Instant finishedAt = finished.getEndInstant();
+			Instant nextStartedAt = nextStarted.getStartInstant();
+
+			long suspendedFor = Duration.between(finishedAt, nextStartedAt).toMillis();
+
+			Assertions.assertThat(suspendedFor).isGreaterThanOrEqualTo(greaterThanOrEqualTo);
+		}
+	}
+
 	// TEST CASES -------------------------------------------------------------------
 
 	// Some tests require state to keep track of the number of test executions.
@@ -414,6 +477,21 @@ void minSuccessLessThanOne() {
 			// Do nothing
 		}
 
+		@RetryingTest(suspendForMs = -1)
+		void suspendForLessThanZero() {
+			// Do nothing
+		}
+
+		@RetryingTest(maxAttempts = 3, suspendForMs = SUSPEND_FOR)
+		void failThreeTimesWithSuspend() {
+			throw new IllegalArgumentException();
+		}
+
+		@RetryingTest(maxAttempts = 3)
+		void failThreeTimesWithoutSuspend() {
+			throw new IllegalArgumentException();
+		}
+
 	}
 
 	@Target({ ElementType.METHOD, ElementType.ANNOTATION_TYPE })