[GitHub] [jackrabbit-oak] nfsantos commented on a diff in pull request #838: OAK-10095 | Fix NPE in FieldFactory.dateToLong

["nfsantos (via GitHub)" <gi...@apache.org>]

nfsantos commented on code in PR #838:
URL: https://github.com/apache/jackrabbit-oak/pull/838#discussion_r1091568799


##########
oak-lucene/src/test/java/org/apache/jackrabbit/oak/plugins/index/lucene/LuceneIndexEditorTest.java:
##########
@@ -233,6 +233,16 @@ public void testLuceneWithNonFullText() throws Exception {
         //Date
         assertEquals("/test", getPath(NumericRangeQuery.newLongRange("creationTime",
                 dateToTime("05/05/2014"), dateToTime("05/07/2014"), true, true)));
+
+        // Call FieldFactory.dateToLong with an unsupported Date format - this should throw a RuntimeException
+        try {
+            getPath(NumericRangeQuery.newLongRange("creationTime",
+                    FieldFactory.dateToLong("05/05/2014"), FieldFactory.dateToLong("05/07/2014"), true, true));
+        } catch (Exception e) {

Review Comment:
   The date parsing method is throwing a `RuntimeException`, while the test is catching the more generic `Exception`. The testing code should match the exact exception.
   
   I was also looking if there is any builtin function to assert that a method throws an exception. JUnit 5 has the `assertThrows`, but I think JUnit 4 doesn't have that method, it only has a parameter for the Test annotation, which I'm not sure if it allows testing for the message inside: `@Test(expected=IndexOutOfBoundsException.class)`



-- 
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: dev-unsubscribe@jackrabbit.apache.org

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