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