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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
