raboof commented on code in PR #1596:
URL: https://github.com/apache/commons-lang/pull/1596#discussion_r2802918553
##########
src/test/java/org/apache/commons/lang3/time/DurationFormatUtilsTest.java:
##########
@@ -473,6 +473,53 @@ void testFormatPeriod() {
assertEquals("helloworld", DurationFormatUtils.formatPeriod(time1970,
time, "'hello''world'"));
}
+ /** Covers negative-day borrowing across month boundary */
+ @Test
+ void testFormatPeriodISONegativeDaysBorrowAcrossMonth() {
+ final TimeZone tz = TimeZones.getTimeZone("GMT-3");
+
+ final Calendar start = Calendar.getInstance(tz);
+ start.set(2020, Calendar.JANUARY, 31, 0, 0, 0);
+ start.set(Calendar.MILLISECOND, 0);
Review Comment:
I think these are not needed, right?
##########
src/test/java/org/apache/commons/lang3/time/DurationFormatUtilsTest.java:
##########
@@ -473,6 +473,53 @@ void testFormatPeriod() {
assertEquals("helloworld", DurationFormatUtils.formatPeriod(time1970,
time, "'hello''world'"));
}
+ /** Covers negative-day borrowing across month boundary */
+ @Test
+ void testFormatPeriodISONegativeDaysBorrowAcrossMonth() {
+ final TimeZone tz = TimeZones.getTimeZone("GMT-3");
Review Comment:
I'm not a huge fan of having "negative borrowed days" so front-and-center in
the test name, as that whole concept is part of the implementation details
rather than part of the more abstract 'desired functionality'. I think I would
prefer something like `testFormatPeriodISOAcrossMonths` for the test name, and
only mentioning "this test covers the code path where the internal 'borrowed
days' go negative" only in a comment.
--
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]
