XComp commented on code in PR #23180:
URL: https://github.com/apache/flink/pull/23180#discussion_r1294432593


##########
flink-runtime/src/test/java/org/apache/flink/runtime/rest/RestClientTest.java:
##########
@@ -239,11 +230,12 @@ public void testCloseClientBeforeRequest() throws 
Exception {
             // Call get() on the future with a timeout of 0s so we can test 
that the exception
             // thrown is not a TimeoutException, which is what would be thrown 
if restClient were
             // not already closed
-            final ThrowingRunnable getFuture = () -> future.get(0, 
TimeUnit.SECONDS);
-
-            final Throwable cause = assertThrows(ExecutionException.class, 
getFuture).getCause();
-            assertThat(cause, instanceOf(IllegalStateException.class));
-            assertThat(cause.getMessage(), equalTo("RestClient is already 
closed"));
+            assertThat(future)
+                    .failsWithin(Duration.ZERO)

Review Comment:
   ok, I needed to take a step back and you're right. I misjudged this due to 
the [comment in the original 
code](https://github.com/apache/flink/pull/23180/files#diff-b4650458c2f425e466b0db62528c7725b17e9408b7b5915bdaf1c1cfc38cb4f2L239-L241).
 Anyway, it's not necessary to use 0ms timeout in any case because we're using 
`directExecutor`.



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