On Wed, 8 May 2024 11:36:07 GMT, serhiysachkov <d...@openjdk.org> wrote:
>> Calendar.add() tests that describe its behavior.
>
> serhiysachkov has updated the pull request incrementally with one additional
> commit since the last revision:
>
> JDK-8331646 consolidating test methods into ParameterizedTests
Thanks for the changes. Also, match this PR title to the JBS title (remove that
`Task - P4`)
test/jdk/java/util/Calendar/CalendarLeapYearAddTest.java line 41:
> 39:
> 40: import static java.util.Calendar.*;
> 41: import static java.util.Calendar.MONTH;
It's better to explicitly declare which fields are statically imported from
`Calendar`.
test/jdk/java/util/Calendar/CalendarLeapYearAddTest.java line 54:
> 52: int value, int calendarField, int
> expectedDate, int expectedMonth,
> 53: int expectedYear) {
> 54: Calendar calendar = Calendar.getInstance();
Instead of using a factory on `Calendar`, this should explicitly construct a
`GregorianCalendar` instance which guarantees the leap-year behavior.
test/jdk/java/util/Calendar/CalendarLeapYearAddTest.java line 69:
> 67: public void testMonthYearAddSubtractNonLeapYear() {
> 68: Calendar calendar = Calendar.getInstance();
> 69: calendar.set(2024, 1, 29, 15, 0, 0);
It's easier to understand using `FEBRUARY`, instead of the immediate value `1`.
test/jdk/java/util/Calendar/CalendarLeapYearAddTest.java line 72:
> 70: calendar.add(Calendar.MONTH, 1);
> 71: calendar.add(YEAR, -1);
> 72: calendar.add(Calendar.MONTH, -1);
`Calendar.MONTH` -> `MONTH`
test/jdk/java/util/Calendar/CalendarLeapYearAddTest.java line 118:
> 116: Arguments.of("testMonthAddLeapYear", 29, 1, 2024, 1,
> MONTH, 29, MARCH, 2024),
> 117: Arguments.of("testOneMonthSubtractLeapYear", 31, 2,
> 2024, -1, MONTH, 29, FEBRUARY, 2024),
> 118: Arguments.of("testTwoMonthSubtractLeapYear", 30, 3,
> 2024, -2, MONTH, 29, FEBRUARY, 2024)
Please use those month fields in those arguments.
-------------
PR Review: https://git.openjdk.org/jdk/pull/19082#pullrequestreview-2046786753
PR Review Comment: https://git.openjdk.org/jdk/pull/19082#discussion_r1594729131
PR Review Comment: https://git.openjdk.org/jdk/pull/19082#discussion_r1594732962
PR Review Comment: https://git.openjdk.org/jdk/pull/19082#discussion_r1594730699
PR Review Comment: https://git.openjdk.org/jdk/pull/19082#discussion_r1594731131
PR Review Comment: https://git.openjdk.org/jdk/pull/19082#discussion_r1594742243
