[GitHub] [logging-log4j2] avandeursen commented on a change in pull request #721: Expand test suite for DateLookup

[GitBox <gi...@apache.org>]

avandeursen commented on a change in pull request #721:
URL: https://github.com/apache/logging-log4j2/pull/721#discussion_r789731643



##########
File path: log4j-core/src/test/java/org/apache/logging/log4j/core/lookup/DateLookupTest.java
##########
@@ -23,31 +23,42 @@
 import org.junit.jupiter.api.Test;
 
 import static org.junit.jupiter.api.Assertions.*;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
 
 public class DateLookupTest {
 
-
     @Test
-    public void testLookup() {
-        final StrLookup lookup = new DateLookup();
-        final LogEvent event = new MyLogEvent();
-        final String value = lookup.lookup(event, "MM/dd/yyyy");
-        assertNotNull(value);
-        assertEquals("12/30/2011", value);
+    public void testDateLookupInEvent() {
+        final LogEvent mockedEvent = mock(LogEvent.class);
+        final Calendar cal = Calendar.getInstance();
+        cal.set(2011, Calendar.DECEMBER, 30, 10, 56, 35);
+        when(mockedEvent.getTimeMillis()).thenReturn(cal.getTimeInMillis());
+
+        final String actualDate = new DateLookup().lookup(mockedEvent, "MM/dd/yyyy");
+        assertEquals("12/30/2011", actualDate);
     }
 
-    private static class MyLogEvent extends AbstractLogEvent {
-        /**
-         * Generated serial version ID.
-         */
-        private static final long serialVersionUID = -2663819677970643109L;
+    @Test
+    public void testDateLookUpNoEvent() {
+        final String dateFormat = "MM/dd/yyyy";
+        final String actualDate = new DateLookup().lookup(null, dateFormat);
+        // we don't know current time, so check length of format instead.
+        assertEquals(actualDate.length(), dateFormat.length());
+    }
 
-        @Override
-        public long getTimeMillis() {
-            final Calendar cal = Calendar.getInstance();
-            cal.set(2011, Calendar.DECEMBER, 30, 10, 56, 35);
-            return cal.getTimeInMillis();
-        }
+    @Test
+    public void testNullDateLookupYieldsCurrentTime() {
+        final String currentTime = new DateLookup().lookup(null, null);
+        assertNotNull(currentTime);
+        // we don't know current time nor format, so just check length of date string.

Review comment:
       This would typically be at the time level of minutes -- with quite a chance of becoming flaky. So instead I better documented the rationale.

##########
File path: log4j-core/src/test/java/org/apache/logging/log4j/core/lookup/DateLookupTest.java
##########
@@ -23,31 +23,42 @@
 import org.junit.jupiter.api.Test;
 
 import static org.junit.jupiter.api.Assertions.*;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
 
 public class DateLookupTest {
 
-
     @Test
-    public void testLookup() {
-        final StrLookup lookup = new DateLookup();
-        final LogEvent event = new MyLogEvent();
-        final String value = lookup.lookup(event, "MM/dd/yyyy");
-        assertNotNull(value);
-        assertEquals("12/30/2011", value);
+    public void testDateLookupInEvent() {
+        final LogEvent mockedEvent = mock(LogEvent.class);
+        final Calendar cal = Calendar.getInstance();
+        cal.set(2011, Calendar.DECEMBER, 30, 10, 56, 35);
+        when(mockedEvent.getTimeMillis()).thenReturn(cal.getTimeInMillis());
+
+        final String actualDate = new DateLookup().lookup(mockedEvent, "MM/dd/yyyy");
+        assertEquals("12/30/2011", actualDate);
     }
 
-    private static class MyLogEvent extends AbstractLogEvent {
-        /**
-         * Generated serial version ID.
-         */
-        private static final long serialVersionUID = -2663819677970643109L;
+    @Test
+    public void testDateLookUpNoEvent() {
+        final String dateFormat = "MM/dd/yyyy";
+        final String actualDate = new DateLookup().lookup(null, dateFormat);
+        // we don't know current time, so check length of format instead.
+        assertEquals(actualDate.length(), dateFormat.length());
+    }
 
-        @Override
-        public long getTimeMillis() {
-            final Calendar cal = Calendar.getInstance();
-            cal.set(2011, Calendar.DECEMBER, 30, 10, 56, 35);
-            return cal.getTimeInMillis();
-        }
+    @Test
+    public void testNullDateLookupYieldsCurrentTime() {
+        final String currentTime = new DateLookup().lookup(null, null);
+        assertNotNull(currentTime);
+        // we don't know current time nor format, so just check length of date string.
+        assertTrue(currentTime.length() > 0);
+    }
 
+    @Test
+    public void testInvalidFormatYieldsDefaultFormat() {
+        final String currentTime = new DateLookup().lookup(null, "bananas");
+        // we don't know current time nor format, so just check length of date string.

Review comment:
       See above.




-- 
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: notifications-unsubscribe@logging.apache.org

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