tillrohrmann commented on a change in pull request #9767:
[FLINK-14120][API/Datastream] Fix testImmediateShutdown
URL: https://github.com/apache/flink/pull/9767#discussion_r328597931
##########
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:
The thing I'm wondering is then what we are actually testing here. I think
the intention was to check that the timer task is interrupted. This is no
longer the case since we swallow the exception. Morever, checking whether
`errorRef` contains `null` is a bit weird. Maybe it would be simpler to pass in
a `CompletableFuture<Exception>` on which we wait and check that it contains an
`InterruptedException`.
----------------------------------------------------------------
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:
[email protected]
With regards,
Apache Git Services
