Copilot commented on code in PR #1801:
URL: https://github.com/apache/commons-lang/pull/1801#discussion_r4270784140
##########
src/test/java/org/apache/commons/lang3/time/DateUtilsTest.java:
##########
@@ -337,6 +342,46 @@ void testAddDays() throws Exception {
assertNullPointerException(() -> DateUtils.addDays(null, 0));
}
+ @DefaultTimeZone("Europe/Brussels")
+ @Test
+ void testAddDaysDaylightSavingTime() {
+ final TimeZone brusselsTz = TimeZones.getTimeZone("Europe/Brussels");
+
+ // 2012-03-25 00:00:00 CET: Brussels spring DST transition (23-hour
day)
+ final Calendar calSpring = Calendar.getInstance(brusselsTz);
+ calSpring.clear();
+ calSpring.set(2012, Calendar.MARCH, 25, 0, 0, 0);
+ final Date inputSpring = calSpring.getTime();
+
+ // In Europe/Brussels: calendar day advances to 2012-03-26 00:00:00
CEST (23 hours elapsed)
+ final Date outputSpring = DateUtils.addDays(inputSpring, 1);
+ assertEquals(23 * 3600 * 1000L, outputSpring.getTime() -
inputSpring.getTime());
Review Comment:
The millis calculation is correct, but using a standard utility (e.g.,
`TimeUnit.HOURS.toMillis(23)`) improves readability and avoids accidental int
math if the expression is later edited. Consider updating this assertion
accordingly.
##########
src/test/java/org/apache/commons/lang3/time/DateUtilsTest.java:
##########
@@ -189,7 +193,7 @@ private static Stream<Arguments>
testToLocalDateTimeTimeZone() {
Arguments.of(
LocalDateTime.ofInstant(
java.sql.Timestamp.valueOf("2023-03-12
02:30:00").toInstant(),
- TimeZone.getDefault().toZoneId()
+ TimeZones.GMT.toZoneId()
),
java.sql.Timestamp.valueOf("2023-03-12 02:30:00"),
null
Review Comment:
This test case now hard-codes `GMT` for the expected `LocalDateTime` while
still passing `null` for the time zone argument (which typically means 'use the
JVM default time zone'). Unless the test method (or class) explicitly fixes the
default time zone to GMT for this case, expected and actual can diverge when
the default zone is not GMT. Suggestion (mandatory): either (a) compute the
expected `LocalDateTime` using the same zone the production code uses when the
argument is `null` (e.g., default zone), or (b) pass an explicit GMT `TimeZone`
as the third argument instead of `null`, so the expectation matches the input.
##########
src/test/java/org/apache/commons/lang3/time/DateUtilsTest.java:
##########
@@ -337,6 +342,46 @@ void testAddDays() throws Exception {
assertNullPointerException(() -> DateUtils.addDays(null, 0));
}
+ @DefaultTimeZone("Europe/Brussels")
+ @Test
+ void testAddDaysDaylightSavingTime() {
+ final TimeZone brusselsTz = TimeZones.getTimeZone("Europe/Brussels");
+
+ // 2012-03-25 00:00:00 CET: Brussels spring DST transition (23-hour
day)
+ final Calendar calSpring = Calendar.getInstance(brusselsTz);
+ calSpring.clear();
+ calSpring.set(2012, Calendar.MARCH, 25, 0, 0, 0);
+ final Date inputSpring = calSpring.getTime();
+
+ // In Europe/Brussels: calendar day advances to 2012-03-26 00:00:00
CEST (23 hours elapsed)
+ final Date outputSpring = DateUtils.addDays(inputSpring, 1);
+ assertEquals(23 * 3600 * 1000L, outputSpring.getTime() -
inputSpring.getTime());
+
+ final Calendar calSpringResult = Calendar.getInstance(brusselsTz);
+ calSpringResult.setTime(outputSpring);
+ assertEquals(2012, calSpringResult.get(Calendar.YEAR));
+ assertEquals(Calendar.MARCH, calSpringResult.get(Calendar.MONTH));
+ assertEquals(26, calSpringResult.get(Calendar.DAY_OF_MONTH));
+ assertEquals(0, calSpringResult.get(Calendar.HOUR_OF_DAY));
+
+ // 2012-10-28 00:00:00 CEST: Brussels autumn DST transition (25-hour
day)
+ final Calendar calAutumn = Calendar.getInstance(brusselsTz);
+ calAutumn.clear();
+ calAutumn.set(2012, Calendar.OCTOBER, 28, 0, 0, 0);
+ final Date inputAutumn = calAutumn.getTime();
+
+ // In Europe/Brussels: calendar day advances to 2012-10-29 00:00:00
CET (25 hours elapsed)
+ final Date outputAutumn = DateUtils.addDays(inputAutumn, 1);
+ assertEquals(25 * 3600 * 1000L, outputAutumn.getTime() -
inputAutumn.getTime());
Review Comment:
Same readability concern as the spring assertion: consider replacing the
manual multiplication with a clearer millis conversion helper (e.g.,
`TimeUnit.HOURS.toMillis(25)`).
--
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]
