On Wed, 23 Feb 2022 18:37:03 GMT, Roger Riggs <[email protected]> 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
