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