[GitHub] [flink] tillrohrmann commented on a change in pull request #9767: [FLINK-14120][API/Datastream] Fix testImmediateShutdown

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

tillrohrmann commented on a change in pull request #9767: [FLINK-14120][API/Datastream] Fix testImmediateShutdown
URL: https://github.com/apache/flink/pull/9767#discussion_r328470570
 
 

 ##########
 File path: flink-streaming-java/src/test/java/org/apache/flink/streaming/runtime/tasks/SystemProcessingTimeServiceTest.java
 ##########
 @@ -154,27 +152,25 @@ public void testImmediateShutdown() throws Exception {
 			// the task should trigger immediately and should block until terminated with interruption
 			timer.registerTimer(System.currentTimeMillis(), timestamp -> {
 				synchronized (lock) {
-					latch.trigger();
-					Thread.sleep(100000000);
+					try {
+						latch.trigger();
+						Thread.sleep(100000000);
+						fail("should be interrupted");
+					} catch (InterruptedException ignored) {
+					}
 				}
 			});
 
 			latch.await();
 			timer.shutdownService();
+
+			// can only enter this scope after the task is interrupted
 			synchronized (lock) {
 				assertTrue(timer.isTerminated());
-
-				// The shutdownService() may not necessary wait for active tasks to finish properly.
-				// From the ScheduledThreadPoolExecutor Java docs:
-				// 	There are no guarantees beyond best-effort attempts to stop processing actively executing tasks.
-				// 	This implementation cancels tasks via {@link Thread#interrupt}, so any task that
-				// 	fails to respond to interrupts may never terminate.
-				assertThat(errorRef.get(), is(anyOf(nullValue(), instanceOf(InterruptedException.class))));
 
 Review comment:
   I don't understand this change. Shouldn't the `InterruptedException` not be a problem here because we allow it to be contained in `errorRef` here? Moreover, according to the JIRA description the problem with the `InterruptedException` failing the assertion occurs in line 196.

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services