nastra commented on code in PR #10428:
URL: https://github.com/apache/iceberg/pull/10428#discussion_r1624384991
##########
api/src/test/java/org/apache/iceberg/TestHelpers.java:
##########
@@ -173,6 +176,26 @@ public static void assertSameSchemaMap(Map<Integer,
Schema> map1, Map<Integer, S
});
}
+ /**
+ * A convenience method to avoid a large number of @Test(expected=...) tests
+ *
+ * @param message A String message to describe this assertion
+ * @param expected An Exception class that the Runnable should throw
+ * @param containedInMessage A String that should be contained by the thrown
exception's message
+ * @param callable A Callable that is expected to throw the exception
+ */
+ public static void assertThrows(
Review Comment:
+1 on removing `TestHelpers` from `parquet`. I'd suggest to use
`assertThatThrownBy(callable::call).as(message).isInstanceOf(expected).hasMessageContaining(..)`
directly in the few places where it's needed rather than introducing this
method. Code like this already exists in `AssertHelpers`, which is deprecated.
Thus, we don't want to introduce similar code in `TestHelpers`, because using
AssertJ assertions directly just provides greater flexibility with testing
--
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...@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@iceberg.apache.org
For additional commands, e-mail: issues-h...@iceberg.apache.org
