RFR: 8282023: PropertiesStoreTest and StoreReproducibilityTest jtreg failures due to en_CA locale [v2]

Jaikiran Pai jpai at openjdk.java.net
Thu Feb 24 05:02:50 UTC 2022


On Wed, 23 Feb 2022 18:37:03 GMT, Roger Riggs <rriggs at openjdk.org> wrote:

>> Jaikiran Pai has updated the pull request incrementally with two additional commits since the last revision:
>> 
>>  - implement review comments
>>  - copyright years
>
> test/jdk/java/util/Properties/PropertiesStoreTest.java line 61:
> 
>> 59:     private static final DateTimeFormatter formatter = DateTimeFormatter.ofPattern(DATE_FORMAT_PATTERN)
>> 60:             .withLocale(Locale.ROOT);
>> 61:     private static final Locale prevLocale = Locale.getDefault();
> 
> By convention, static final field names are uppercase.  It make the code using them easier to read.

Makes sense. Fixed in the latest version of the PR.

> test/jdk/java/util/Properties/PropertiesStoreTest.java line 112:
> 
>> 110:             if (!locale.getLanguage().isEmpty() && !locale.getLanguage().equals("en")) {
>> 111:                 nonEnglishLocale = locale;
>> 112:                 System.out.println("Selected non-english locale: " + nonEnglishLocale + " for tests");
> 
> TestNG includes the arguments in the output when the test is run.

Removed the System.out.println statement from the latest version of the PR.

> test/jdk/java/util/Properties/PropertiesStoreTest.java line 116:
> 
>> 114:             }
>> 115:         }
>> 116:         if (nonEnglishLocale == null) {
> 
> Alternatively, create a Set<Locale>, to avoid duplicates, adding the candidates as they are discovered
> and finally convert the Set to an Object[][]. It may be a bit easier to maintain.
> 
> private static Object[][] provideLocales() {
>         // pick a non-english locale for testing
>         Set<Locale> locales = Arrays.stream(Locale.getAvailableLocales())
>                 .filter(l -> !l.getLanguage().isEmpty() && !l.getLanguage().equals("en"))
>                 .limit(1)
>                 .collect(Collectors.toSet());
>         locales.add(Locale.getDefault());
>         locales.add(Locale.US);
>         locales.add(Locale.ROOT);
> 
>         return locales.stream()
>                 .map(m -> new Locale[] {m})
>                 .toArray(n -> new Object[n][0]);
>     }

Thank you Roger for this suggestion. This indeed is cleaner. I've updated the PR to use this suggested code. The tests continue to pass with the latest round of changes.

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

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


More information about the core-libs-dev mailing list