showuon commented on code in PR #18891:
URL: https://github.com/apache/kafka/pull/18891#discussion_r1959354955
##########
clients/src/test/java/org/apache/kafka/test/TestUtils.java:
##########
@@ -580,21 +582,32 @@ public static Set<TopicPartition>
generateRandomTopicPartitions(int numTopic, in
}
/**
- * Assert that a future raises an expected exception cause type. Return
the exception cause
- * if the assertion succeeds; otherwise raise AssertionError.
+ * Assert that a future raises an expected exception cause type.
+ * This method will wait for the future to complete or timeout(15000
milliseconds) after a default period.
Review Comment:
nit: This method will wait for the future to complete or timeout(15000
milliseconds).
##########
clients/src/test/java/org/apache/kafka/test/TestUtils.java:
##########
@@ -580,21 +582,32 @@ public static Set<TopicPartition>
generateRandomTopicPartitions(int numTopic, in
}
/**
- * Assert that a future raises an expected exception cause type. Return
the exception cause
- * if the assertion succeeds; otherwise raise AssertionError.
+ * Assert that a future raises an expected exception cause type.
+ * This method will wait for the future to complete or timeout(15000
milliseconds) after a default period.
*
* @param future The future to await
* @param exceptionCauseClass Class of the expected exception cause
* @param <T> Exception cause type parameter
* @return The caught exception cause
*/
public static <T extends Throwable> T assertFutureThrows(Future<?> future,
Class<T> exceptionCauseClass) {
- ExecutionException exception = assertThrows(ExecutionException.class,
future::get);
- Throwable cause = exception.getCause();
- assertEquals(exceptionCauseClass, cause.getClass(),
- "Expected a " + exceptionCauseClass.getSimpleName() + " exception,
but got " +
- cause.getClass().getSimpleName());
- return exceptionCauseClass.cast(exception.getCause());
+ try {
+ future.get(DEFAULT_MAX_WAIT_MS, TimeUnit.MILLISECONDS);
+ fail("Should throw expected exception " +
exceptionCauseClass.getSimpleName() + " but nothing was thrown.");
+ } catch (InterruptedException | ExecutionException |
CancellationException e) {
+ Throwable cause = e instanceof ExecutionException ? e.getCause() :
e;
+ assertEquals(
+ exceptionCauseClass,
+ cause.getClass(),
+ "Expected " + exceptionCauseClass.getSimpleName() + ", but got
" + cause.getClass().getSimpleName()
+ );
+ return exceptionCauseClass.cast(cause);
+ } catch (TimeoutException e) {
+ fail("Future is not completed within " + DEFAULT_MAX_WAIT_MS + "
milliseconds.");
+ } catch (Exception e) {
+ fail("Unexpected error happened: " + e);
Review Comment:
I think this case should be handled the same as L602, that we expect
`exceptionCauseClass.getSimpleName()`, but got `e.getClass().getSimpleName()`.
WDYT?
--
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]
