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

Wed Feb 23 19:21:57 UTC 2022

On Wed, 23 Feb 2022 01:08:21 GMT, Jaikiran Pai <jpai at openjdk.org> wrote:

>> Can I please get a review of this test only change which fixes the issue noted in https://bugs.openjdk.java.net/browse/JDK-8282023?
>> 
>> As noted in that JBS issue these tests fail when the default locale under which those tests are run isn't `en_US`. Both these tests were introduced as part of https://bugs.openjdk.java.net/browse/JDK-8231640. At a high level, these test do the following:
>> - Use Properties.store(...) APIs to write out a properties file. This internally ends up writing a date comment via a call to `java.util.Date.toString()` method.
>> - The test then runs a few checks to make sure that the written out `Date` is an expected one. To run these checks it uses the `java.time.format.DateTimeFormatter` to parse the date comment out of the properties file.
>> 
>> All this works fine when the default locale is (for example) `en_US`. However, when the default locale is (for example `en_CA` or even `hi_IN`) the tests fail with an exception in the latter step above while parsing the date comment using the `DateTimeFormatter` instance. The exception looks like:
>> 
>> 
>> Using locale: he for Properties#store(OutputStream) test
>> test PropertiesStoreTest.testStoreOutputStreamDateComment(he): failure
>> java.lang.AssertionError: Unexpected date comment: Mon Feb 21 19:10:31 IST 2022
>> 	at org.testng.Assert.fail(Assert.java:87)
>> 	at PropertiesStoreTest.testDateComment(PropertiesStoreTest.java:255)
>> 	at PropertiesStoreTest.testStoreOutputStreamDateComment(PropertiesStoreTest.java:223)
>> 	at java.base/jdk.internal.reflect.DirectMethodHandleAccessor.invoke(DirectMethodHandleAccessor.java:104)
>> 	at java.base/java.lang.reflect.Method.invoke(Method.java:577)
>> 	at org.testng.internal.MethodInvocationHelper.invokeMethod(MethodInvocationHelper.java:132)
>> 	at org.testng.internal.TestInvoker.invokeMethod(TestInvoker.java:599)
>> 	at org.testng.internal.TestInvoker.invokeTestMethod(TestInvoker.java:174)
>> 	at org.testng.internal.MethodRunner.runInSequence(MethodRunner.java:46)
>> 	at org.testng.internal.TestInvoker$MethodInvocationAgent.invoke(TestInvoker.java:822)
>> 	at org.testng.internal.TestInvoker.invokeTestMethods(TestInvoker.java:147)
>> 	at org.testng.internal.TestMethodWorker.invokeTestMethods(TestMethodWorker.java:146)
>> 	at org.testng.internal.TestMethodWorker.run(TestMethodWorker.java:128)
>> 	at java.base/java.util.ArrayList.forEach(ArrayList.java:1511)
>> 	at org.testng.TestRunner.privateRun(TestRunner.java:764)
>> 	at org.testng.TestRunner.run(TestRunner.java:585)
>> 	at org.testng.SuiteRunner.runTest(SuiteRunner.java:384)
>> 	at org.testng.SuiteRunner.runSequentially(SuiteRunner.java:378)
>> 	at org.testng.SuiteRunner.privateRun(SuiteRunner.java:337)
>> 	at org.testng.SuiteRunner.run(SuiteRunner.java:286)
>> 	at org.testng.SuiteRunnerWorker.runSuite(SuiteRunnerWorker.java:53)
>> 	at org.testng.SuiteRunnerWorker.run(SuiteRunnerWorker.java:96)
>> 	at org.testng.TestNG.runSuitesSequentially(TestNG.java:1218)
>> 	at org.testng.TestNG.runSuitesLocally(TestNG.java:1140)
>> 	at org.testng.TestNG.runSuites(TestNG.java:1069)
>> 	at org.testng.TestNG.run(TestNG.java:1037)
>> 	at com.sun.javatest.regtest.agent.TestNGRunner.main(TestNGRunner.java:94)
>> 	at com.sun.javatest.regtest.agent.TestNGRunner.main(TestNGRunner.java:54)
>> 	at java.base/jdk.internal.reflect.DirectMethodHandleAccessor.invoke(DirectMethodHandleAccessor.java:104)
>> 	at java.base/java.lang.reflect.Method.invoke(Method.java:577)
>> 	at com.sun.javatest.regtest.agent.MainWrapper$MainThread.run(MainWrapper.java:127)
>> 	at java.base/java.lang.Thread.run(Thread.java:828)
>> Caused by: java.time.format.DateTimeParseException: Text 'Mon Feb 21 19:10:31 IST 2022' could not be parsed at index 0
>> 	at java.base/java.time.format.DateTimeFormatter.parseResolved0(DateTimeFormatter.java:2106)
>> 	at java.base/java.time.format.DateTimeFormatter.parse(DateTimeFormatter.java:1934)
>> 	at PropertiesStoreTest.testDateComment(PropertiesStoreTest.java:253)
>> 	... 30 more
>> 
>> (in the exception above a locale with language `he` is being used)
>> 
>> The root cause of this failure lies (only) within the tests - the `DateTimeFormatter` used in the tests is locale sensitive and uses the current (`Locale.getDefault()`) locale by default for parsing the date text. This parsing fails because, although `Date.toString()` javadoc states nothing about locales, it does mention the exact characters that will be used to write out the date comment. In other words, this date comment written out is locale insensitive and as such when parsing using `DateTimeFormatter` a neutral locale is appropriate on the `DateTimeFormatter` instance. This PR thus changes the tests to use `Locale.ROOT` while parsing this date comment.  Additionally, while I was at it, I updated the `PropertiesStoreTest` to automate the use of multiple different locales to reproduce this issue (automatically) and verify the fix.
>
> 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.

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.

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]);
    }

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

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