On Mon, 26 Feb 2024 22:38:50 GMT, Justin Lu <j...@openjdk.org> wrote:

>> Naoto Sato has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   Update make/jdk/src/classes/build/tools/cldrconverter/CLDRConverter.java
>>   
>>   Co-authored-by: Andrey Turbanov <turban...@gmail.com>
>
> test/jdk/java/util/Locale/ExpectedAdapterTypes.java line 27:
> 
>> 25:  * @test
>> 26:  * @bug 8008577 8138613 8174269
>> 27:  * @summary Check whether CLDR locale provider adapter is enabled by 
>> default
> 
> Unless I'm mistaken, there aren't any other dedicated tests to ensuring the 
> FALLBACK adapter is included, it might be worth updating the summary to make 
> it apparent that while the default preference list has CLDR, _FALLBACK_ is 
> always appended, since that is new behavior.

I believe some tests will break if `FALLBACK` is not included in the preferred 
list. I am kind of hesitant to specify it in the command line for the sake of 
testing, as it is not a public constant.

> test/jdk/java/util/Locale/LocaleProvidersFormat.java line 80:
> 
>> 78:     @Test
>> 79:     @EnabledOnOs(WINDOWS)
>> 80:     @EnabledIfSystemProperty(named = "user.language", matches = "ja")
> 
> Thanks for fixing, as it is `HOST` the locale value should be based on the 
> machine at startup. Although, I'm wondering how the test passed previously 
> then.

Right, it only works if the underlying machine is configured with ja locale as 
the default.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/17991#discussion_r1503478795
PR Review Comment: https://git.openjdk.org/jdk/pull/17991#discussion_r1503478741

Reply via email to