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

[Naoto Sato]

On Thu, 24 Feb 2022 05:02:47 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 four additional commits since the last revision:
> 
>  - use Roger's suggestion of using Stream and Collection based APIs to avoid code duplication in the data provider method of the test
>  - no need for system.out.println since testng add the chosen params to the output logs
>  - review comments:
>      - upper case static final fields in test
>      - use DateTimeFormatter.ofPattern(DATE_FORMAT_PATTERN, Locale.ROOT)
>  - remove @modules declaration on the jtreg test

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

> 110:         locales.add(Locale.getDefault()); // always test the default locale
> 111:         locales.add(Locale.US); // guaranteed to be present
> 112:         locales.add(Locale.ROOT); // guaranteed to be present

Can we assume the returned `Set<Locale>` is mutable? `Collectors.toSet()` javadoc reads no guarantee for mutability.

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

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