<i18n dev> RFR: JDK-8331646: Add specific regression leap year tests (Task - P4) [v3]

[Naoto Sato]

On Wed, 8 May 2024 11:36:07 GMT, serhiysachkov <duke at 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