garydgregory commented on code in PR #261:
URL: https://github.com/apache/commons-pool/pull/261#discussion_r2041947822
##########
src/test/java/org/apache/commons/pool3/proxy/AbstractTestProxiedObjectPool.java:
##########
@@ -193,4 +232,38 @@ public void testUsageTracking() throws
InterruptedException {
assertTrue(logOutput.contains("The last code to use this object was"));
}
+ @Test
+ public void testUnwrapInvocationTargetExceptionTrue() {
+ @SuppressWarnings("resource")
+ ObjectPool<TestObject, RuntimeException> pool =
createProxiedObjectPool(true, new MyException());
+
+ TestObject object = pool.borrowObject();
+ try {
Review Comment:
Hello @reda-alaoui
Please use `assertThrows()` where you can.
##########
src/test/java/org/apache/commons/pool3/proxy/AbstractTestProxiedObjectPool.java:
##########
@@ -193,4 +232,38 @@ public void testUsageTracking() throws
InterruptedException {
assertTrue(logOutput.contains("The last code to use this object was"));
}
+ @Test
+ public void testUnwrapInvocationTargetExceptionTrue() {
+ @SuppressWarnings("resource")
+ ObjectPool<TestObject, RuntimeException> pool =
createProxiedObjectPool(true, new MyException());
+
+ TestObject object = pool.borrowObject();
+ try {
+ object.getData();
+ fail("Expected to throw %s".formatted(MyException.class));
+ } catch (MyException e) {
+ // As expected
+ }
+ }
+
+ @Test
+ public void testUnwrapInvocationTargetExceptionFalse() {
+ @SuppressWarnings("resource")
+ ObjectPool<TestObject, RuntimeException> pool =
createProxiedObjectPool(false, new MyException());
+
+ TestObject object = pool.borrowObject();
+ Exception caughtException = null;
+ try {
+ object.getData();
+ } catch (Exception e) {
+ caughtException = e;
+ }
+
+ if (caughtException instanceof UndeclaredThrowableException ||
caughtException instanceof InvocationTargetException) {
Review Comment:
It would be better to refactor the test to make the expected exception
unique. This will make what a subclass expects explicit, making each test class
easier to understand. You can then use `assertThrows()` to make the test
clearer.
--
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: issues-unsubscr...@commons.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
