showuon commented on code in PR #16264:
URL: https://github.com/apache/kafka/pull/16264#discussion_r1663413976


##########
clients/src/test/java/org/apache/kafka/test/TestUtils.java:
##########
@@ -558,14 +558,13 @@ public static Set<TopicPartition> 
generateRandomTopicPartitions(int numTopic, in
      */
     public static <T extends Throwable> T assertFutureThrows(Future<?> future, 
Class<T> exceptionCauseClass) {
         try {
-            future.get(DEFAULT_MAX_WAIT_MS, TimeUnit.SECONDS);
+            future.get(DEFAULT_MAX_WAIT_MS, TimeUnit.MILLISECONDS);
             fail("Future should throw expected exception " + 
exceptionCauseClass.getSimpleName() + " but succeed.");
         } catch (TimeoutException | InterruptedException | ExecutionException 
e) {
-            assertInstanceOf(exceptionCauseClass, e.getCause(),
-                    "Unexpected exception cause " + e.getCause());
+            assertInstanceOf(exceptionCauseClass, e.getCause(), "Expected a" + 
exceptionCauseClass.getSimpleName() + "but got" + e.getCause());
             return exceptionCauseClass.cast(e.getCause());
         }
-        return null;
+        throw new RuntimeException("Future should throw expected exception but 
unexpected error happened.");

Review Comment:
   I saw you addressed 2 of my 3 comments:
   
   > 1. DEFAULT_MAX_WAIT_MS is a millisecond time unit, not seconds. Same 
comment applied to L583.
   
   It's fixed now. Thanks.
   > 2. Now, if the future throws TimeoutException or InterruptedException, it 
will fail as expected. But what error message will we get? In your opinion, do 
you think that's helpful for troubleshooting?
   
   Could you first let me know what error message we will get when 
`TimeoutException or InterruptedException` thrown?
       
   > 3. What happened if the future throws CancellationException? return null? 
Is there any better way to handle it?
   
   Now, it will throw RuntimeException, which works, too. But I think we can 
directly `fail` the test, like L 562 did.
   



-- 
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: jira-unsubscr...@kafka.apache.org

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

Reply via email to