Add test coverage for negative-day adjustment in DurationFormatUtils#1596
Add test coverage for negative-day adjustment in DurationFormatUtils#1596garydgregory merged 1 commit intoapache:masterfrom
Conversation
raboof
left a comment
raboof
left a comment
There was a problem hiding this comment.
I think this is valuable test coverage. Two small comments.
| /** Covers negative-day borrowing across month boundary */ | ||
| @Test | ||
| void testFormatPeriodISONegativeDaysBorrowAcrossMonth() { | ||
| final TimeZone tz = TimeZones.getTimeZone("GMT-3"); |
There was a problem hiding this 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.
There was a problem hiding this comment.
Good point, that makes sense. I’ve renamed the test and moved the implementation detail to a comment. Thanks for the suggestion!
|
|
||
| final Calendar start = Calendar.getInstance(tz); | ||
| start.set(2020, Calendar.JANUARY, 31, 0, 0, 0); | ||
| start.set(Calendar.MILLISECOND, 0); |
There was a problem hiding this comment.
I think these are not needed, right?
There was a problem hiding this comment.
I updated the test to use clear() before setting the date to avoid leftover time fields. Thanks for the suggestion.
Adds unit tests covering month-boundary and leap-year cases that trigger the internal negative-days borrowing logic in DurationFormatUtils.formatPeriod. Removes obsolete TODO about missing JaCoCo coverage.
nine0703
force-pushed
the
add-negative-days-test
branch
from
f68b952 to
269ecf9
Compare
Thanks a lot! |
|
@nine0703 |
DurationFormatUtils #1596 - Sort new methods - Remove empty lines, use longer lines
3 participants
Adds unit tests covering month-boundary and leap-year cases that trigger the internal negative-days borrowing logic in
DurationFormatUtils.formatPeriod.These tests improve coverage for scenarios previously noted by a TODO comment regarding missing JaCoCo coverage. The obsolete TODO comment has been removed since the relevant paths are now exercised by tests.
mvn).AI usage disclosure:
Yes. ChatGPT (OpenAI) was used to discuss testing strategy and help draft the test cases and PR description. All code and tests were reviewed and validated manually before submission.