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

[Naoto Sato]

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 51:

> 49:  * @summary tests the order in which the Properties.store() method writes out the properties
> 50:  * @bug 8231640 8282023
> 51:  * @modules jdk.localedata

OK, you can remove `@modules jdk.localedata` so that other tests would run. (it'd be unusual setup, but still possible with jlink)

test/jdk/java/util/Properties/PropertiesStoreTest.java line 60:

> 58:     // it internally calls the Date.toString() which always writes in a locale insensitive manner
> 59:     private static final DateTimeFormatter formatter = DateTimeFormatter.ofPattern(DATE_FORMAT_PATTERN)
> 60:             .withLocale(Locale.ROOT);

Should have noticed before, but you can call the convenient overload of `ofPattern(DATE_FORMAT_PATTERN, Locale.ROOT)` here.

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

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