reswqa commented on code in PR #21137:
URL: https://github.com/apache/flink/pull/21137#discussion_r1013656229


##########
flink-runtime/src/test/java/org/apache/flink/runtime/jobmaster/JobMasterServiceLeadershipRunnerTest.java:
##########
@@ -93,48 +87,47 @@ public class JobMasterServiceLeadershipRunnerTest extends 
TestLogger {
 
     private JobResultStore jobResultStore;
 
-    @BeforeClass
-    public static void setupClass() {
+    @BeforeAll
+    static void setupClass() {
 
         final JobVertex jobVertex = new JobVertex("Test vertex");
         jobVertex.setInvokableClass(NoOpInvokable.class);
         jobGraph = JobGraphTestUtils.streamingJobGraph(jobVertex);
     }
 
-    @Before
-    public void setup() {
+    @BeforeEach
+    void setup() {
         leaderElectionService = new TestingLeaderElectionService();
         jobResultStore = new EmbeddedJobResultStore();
         fatalErrorHandler = new TestingFatalErrorHandler();
     }
 
-    @After
-    public void tearDown() throws Exception {
+    @AfterEach
+    void tearDown() throws Exception {
         fatalErrorHandler.rethrowError();
     }
 
     @Test
-    public void testShutDownSignalsJobAsNotFinished() throws Exception {
+    void testShutDownSignalsJobAsNotFinished() throws Exception {
         try (JobManagerRunner jobManagerRunner =
                 newJobMasterServiceLeadershipRunnerBuilder().build()) {
             jobManagerRunner.start();
 
             final CompletableFuture<JobManagerRunnerResult> resultFuture =
                     jobManagerRunner.getResultFuture();
 
-            assertThat(resultFuture.isDone(), is(false));
+            assertThat(resultFuture).isNotDone();
 
             jobManagerRunner.closeAsync();
 
             assertJobNotFinished(resultFuture);
-            assertThat(
-                    jobManagerRunner.getJobMasterGateway(),
-                    
FlinkMatchers.futureWillCompleteExceptionally(Duration.ofMillis(5L)));
+            assertThat(jobManagerRunner.getJobMasterGateway())
+                    .failsWithin(5L, TimeUnit.MILLISECONDS);

Review Comment:
   Thank you for reminding me about this. If I understand correctly, can I 
change this timeout assertion to the approach like `assertThatThrownBy(() -> 
jobManagerRunner.getJobMasterGateway().get())` to rely on Azure's global 
timeout in this way?
   As for the migration of JUnit5, I saw this prompt: "Unless there is a 
specific reason, make sure you use JUnit 5 and AssertJ when contributing to 
Flink with new tests and even when modifying existing tests."  in the `Apache 
Flink Code Style and Quality Guide`. From my perspective, it seems a bit bad to 
have JUnt4 and JUnit5 dependencies in the test class at the same time, so I 
think this may be necessary. 



-- 
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]

Reply via email to