vy commented on a change in pull request #721:
URL: https://github.com/apache/logging-log4j2/pull/721#discussion_r788564237
##########
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:
By following the logic in `DateLookup`, we can guess both these – see my
earlier comments. Mind improving this test too, please?
##########
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.
Review comment:
`lookup(null, format)` simply delegates to
`formatDate(System.currentTimeMillis(), format)`, right? Hence, we know what is
expected – indeed second-precision values are not reliable, but we certainly
know the expected `MM/dd/yyyy` output. Mind updating this test accordingly,
please?
##########
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:
By following the logic in `DateLookup`, we know the current time (see my
comment above) and the format: `DateFormat.getInstance()`. Mind updating this
test accordingly, please?
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
