ijuma commented on code in PR #18296:
URL: https://github.com/apache/kafka/pull/18296#discussion_r1899687406

##########
clients/src/test/java/org/apache/kafka/test/TestUtils.java:
##########
@@ -583,19 +583,6 @@ public static <T extends Throwable> void 
assertFutureThrows(
         assertEquals(expectedMessage, receivedException.getMessage());
     }
 
-    public static void assertFutureError(Future<?> future, Class<? extends 
Throwable> exceptionClass)
-        throws InterruptedException {
-        try {
-            future.get();
-            fail("Expected a " + exceptionClass.getSimpleName() + " exception, 
but got success.");
-        } catch (ExecutionException ee) {
-            Throwable cause = ee.getCause();
-            assertEquals(exceptionClass, cause.getClass(),

Review Comment:
   I was referring to the `assertInstanceOf` part. I tested and this is what I 
see:
   
   Before:
   ```org.opentest4j.AssertionFailedError: Expected a IllegalArgumentException 
exception, but got TimeoutException ==> 
   Expected :class java.lang.IllegalArgumentException
   Actual   :class org.apache.kafka.common.errors.TimeoutException
   ```
   
   After:
   ```
   org.opentest4j.AssertionFailedError: Unexpected exception cause 
org.apache.kafka.common.errors.TimeoutException: Timed out waiting for a node 
assignment. Call: createTopics ==> Unexpected type, 
   Expected :class java.lang.IllegalArgumentException
   Actual   :class org.apache.kafka.common.errors.TimeoutException
   ```
   
   I think the new message is ok, but we should probably say something like 
"ExecutionException thrown by Future has unexpected cause ".
   
   One additional detail, the new check is less strict - it allows the class or 
subclass while the old one only allowed the exact class. We should probably 
make the behavior configurable - if code expects an exact exception to be 
thrown, we should check for that.



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