[GitHub] [flink] RyanSkraba commented on a diff in pull request #21289: [FLINK-29452] Allow unit tests to be executed independently

[GitBox <gi...@apache.org>]

RyanSkraba commented on code in PR #21289:
URL: https://github.com/apache/flink/pull/21289#discussion_r1019196065


##########
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);

Review Comment:
   Note that I changed the strategy to the same as `RetryOnExceptionExtensionTest`.  Instead of counting all the failures and all the successes, we count the number of expected times each test method was run.
   
   In my opinion, this is quite a bit more clear.
   
   ```
   assertEquals(3, numberOfSuccessfulRuns)
   ```
   
   can only succeed if and only every method in the test class finished by succeeding once.  In other words, it can only not happen if JUnit is already reporting a complete, non-retryable failure in this class.



-- 
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: issues-unsubscribe@flink.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org