RFR: 8262108: SimpleDateFormat formatting broken for sq_MK Locale [v3]

[Jaikiran Pai]

On Wed, 14 Apr 2021 18:01:10 GMT, Naoto Sato <naoto at openjdk.org> wrote:

>> Jaikiran Pai has updated the pull request incrementally with two additional commits since the last revision:
>> 
>>  - Update existing NarrowNamesTest to match expectations
>>  - Naoto's review suggestion
>
> src/java.base/share/classes/sun/util/locale/provider/CalendarNameProviderImpl.java line 193:
> 
>> 191:                             }
>> 192:                             if (field == AM_PM && !javatime && i > PM) {
>> 193:                                 // when dealing with calendar fields, don't set AM_PM field value
> 
> Make it explicit that this block only serves for `java.util.Calendar`, not `java.time.format.DateTimeFormatter(Builder)`.

Hello Naoto, I've now updated the PR to be more explicit in this code comment.

> test/jdk/java/util/Calendar/CalendarDisplayNamesTest.java line 53:
> 
>> 51:             for (final int style : styles) {
>> 52:                 final Calendar cal = Calendar.getInstance();
>> 53:                 cal.setLenient(false);
> 
> Don't think leniency is relevant here.

Removed in the latest update of this PR. This was a leftover from my experimental testing.

> test/jdk/java/util/Calendar/NarrowNamesTest.java line 119:
> 
>> 117:             expectedFieldValues.put("a", Calendar.AM);
>> 118:             expectedFieldValues.put("p", Calendar.PM);
>> 119:             testAM_PM(US, ALL_STYLES, expectedFieldValues);
> 
> I believe this can be reverted to the original, as the original code compares the exact map (including the length of the map).  , i.e., 
> 
>            testMap(US, AM_PM, ALL_STYLES,
>                     "AM", "PM",
>                     RESET_INDEX,
>                     "a", "p");

Done. Reverted this specific part to what it was originally. Test continues to pass with this change and the latest PR update.

-------------

PR: https://git.openjdk.java.net/jdk/pull/3463