XComp commented on code in PR #21289:
URL: https://github.com/apache/flink/pull/21289#discussion_r1297093524
##########
flink-test-utils-parent/flink-test-utils-junit/src/test/java/org/apache/flink/testutils/junit/RetryOnFailureExtensionTest.java:
##########
@@ -21,56 +21,69 @@
import org.apache.flink.testutils.junit.extensions.retry.RetryExtension;
import org.junit.jupiter.api.AfterAll;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.TestInfo;
import org.junit.jupiter.api.TestTemplate;
import org.junit.jupiter.api.extension.ExtendWith;
-import static org.junit.jupiter.api.Assertions.assertEquals;
+import java.util.HashMap;
+
+import static org.assertj.core.api.Assertions.assertThat;
/** Tests for the {@link RetryOnFailure} annotation on JUnit5 {@link
RetryExtension}. */
@ExtendWith(RetryExtension.class)
class RetryOnFailureExtensionTest {
- private static final int NUMBER_OF_RUNS = 5;
-
- private static int numberOfFailedRuns;
+ private static final int NUMBER_OF_RETRIES = 5;
- private static int numberOfSuccessfulRuns;
+ private static final HashMap<String, Integer> methodRunCount = new
HashMap<>();
- private static boolean firstRun = true;
+ @BeforeEach
+ void incrementMethodRunCount(TestInfo testInfo) {
+ // Set or increment the run count for the unit test method, by the
method short name.
+ // This starts at 1 and is incremented before the test starts.
+ testInfo.getTestMethod()
+ .ifPresent(
+ method ->
+ methodRunCount.compute(
+ method.getName(), (k, v) -> (v ==
null) ? 1 : v + 1));
+ }
@AfterAll
static void verify() {
- assertEquals(NUMBER_OF_RUNS + 1, numberOfFailedRuns);
- assertEquals(3, numberOfSuccessfulRuns);
+ assertThat(methodRunCount.get("testRetryOnFailure"))
Review Comment:
I think we can apply the same approach that I described above here.
##########
flink-test-utils-parent/flink-test-utils-junit/src/test/java/org/apache/flink/testutils/junit/RetryOnExceptionExtensionTest.java:
##########
@@ -37,53 +40,66 @@
@ExtendWith(RetryExtension.class)
class RetryOnExceptionExtensionTest {
- private static final int NUMBER_OF_RUNS = 3;
+ private static final int NUMBER_OF_RETRIES = 3;
- private static int runsForSuccessfulTest = 0;
+ private static final HashMap<String, Integer> methodRunCount = new
HashMap<>();
- private static int runsForTestWithMatchingException = 0;
-
- private static int runsForTestWithSubclassException = 0;
-
- private static int runsForPassAfterOneFailure = 0;
+ @BeforeEach
+ void incrementMethodRunCount(TestInfo testInfo) {
+ // Set or increment the run count for the unit test method, by the
method short name.
+ // This starts at 1 and is incremented before the test starts.
+ testInfo.getTestMethod()
+ .ifPresent(
+ method ->
+ methodRunCount.compute(
+ method.getName(), (k, v) -> (v ==
null) ? 1 : v + 1));
+ }
@AfterAll
static void verify() {
- assertThat(runsForTestWithMatchingException).isEqualTo(NUMBER_OF_RUNS
+ 1);
- assertThat(runsForTestWithSubclassException).isEqualTo(NUMBER_OF_RUNS
+ 1);
- assertThat(runsForSuccessfulTest).isOne();
- assertThat(runsForPassAfterOneFailure).isEqualTo(2);
+ assertThat(methodRunCount.get("testSuccessfulTest"))
Review Comment:
I'm wondering whether we could also implement this test without having plain
method names that we would have to add everywhere. We could do that by
utilizing the `TestInfo` parameter in every test method and register the
callbacks for the corresponding test method. That has the advantage that we
have the test code in one location (the verification callback is defined within
the test method instead of the `@AfterAll` annotated method). WDYT?
We would have something like a `verificationCallbackRegistry` next to the
`methodRunCount` and utilize methods like the following ones:
```
private static int assertAndReturnRunCount(TestInfo testInfo) {
return methodRunCount.get(assertAndReturnTestMethodName(testInfo));
}
private static void registerCallbackForTest(TestInfo testInfo,
Consumer<Integer> verification) {
verificationCallbackRegistry.putIfAbsent(
assertAndReturnTestMethodName(testInfo),
() ->
verification.accept(assertAndReturnRunCount(testInfo)));
}
private static String assertAndReturnTestMethodName(TestInfo testInfo) {
return testInfo.getTestMethod()
.orElseThrow(() -> new AssertionError("No test method is
provided."))
.getName();
}
```
The testInfo comes from the test method's parameters. All test handling can
happen within the test method. WDYT?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
