[GitHub] [commons-geometry] singhbaljit commented on a change in pull request #112: [GEOMETRY-105] - Replace the try-catch pattern with assertThrows()

[GitBox]

singhbaljit commented on a change in pull request #112:
URL: https://github.com/apache/commons-geometry/pull/112#discussion_r540224420



##########
File path: 
commons-geometry-euclidean/src/test/java/org/apache/commons/geometry/euclidean/threed/Bounds3DTest.java
##########
@@ -125,29 +125,12 @@ public void testFrom_invalidBounds() {
         final Vector3D negInf = Vector3D.of(1, 1, Double.NEGATIVE_INFINITY);
 
         // act/assert
-        GeometryTestUtils.assertThrows(() -> {
-            Bounds3D.from(Vector3D.NaN);
-        }, IllegalStateException.class, INVALID_BOUNDS_PATTERN);
-
-        GeometryTestUtils.assertThrows(() -> {
-            Bounds3D.from(Vector3D.POSITIVE_INFINITY);
-        }, IllegalStateException.class, INVALID_BOUNDS_PATTERN);
-
-        GeometryTestUtils.assertThrows(() -> {
-            Bounds3D.from(Vector3D.NEGATIVE_INFINITY);
-        }, IllegalStateException.class, INVALID_BOUNDS_PATTERN);
-
-        GeometryTestUtils.assertThrows(() -> {
-            Bounds3D.from(good, nan);
-        }, IllegalStateException.class, INVALID_BOUNDS_PATTERN);
-
-        GeometryTestUtils.assertThrows(() -> {
-            Bounds3D.from(posInf, good);
-        }, IllegalStateException.class, INVALID_BOUNDS_PATTERN);
-
-        GeometryTestUtils.assertThrows(() -> {
-            Bounds3D.from(good, negInf, good);
-        }, IllegalStateException.class, INVALID_BOUNDS_PATTERN);
+        assertThrows(IllegalStateException.class, () -> 
Bounds3D.from(Vector3D.NaN));

Review comment:
       I agree with @darkma773r's two suggestions. I would also suggest the 
following:
   1. Remove the `message != null` check for the message assertion in 
`GeometryTestUtils. assertThrowsWithMessage(Runnable, Throwable, String)`. 
Instead of using `null` to skip the test (which I assume was done to make 
`GeometryTestUtils.assertThrows(Runnable, Throwable)` work), it is a validate 
test to check if the message is in fact `null`.
   2. Similarly, I also don't think the pattern parameter in 
`GeometryTestUtils.assertThrowsWithMessage(Runnable, Throwable, Pattern)` 
should be optional/nullable.




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

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org