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

Tue Feb 22 17:38:58 UTC 2022

On Mon, 21 Feb 2022 14:09:50 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.

Looks good overall. Some nits follow. Please change the copyright year too.

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

> 48:  * @test
> 49:  * @summary tests the order in which the Properties.store() method writes out the properties
> 50:  * @bug 8231640

Add the bug id, also `@modules jdk.localedata` needs to be added.

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

> 102:         for (Locale locale : Locale.getAvailableLocales()) {
> 103:             // skip ROOT locale and ENGLISH language ones
> 104:             if (!locale.getLanguage().isEmpty() && !locale.getLanguage().equals(Locale.ENGLISH.getLanguage())) {

Could simply compare with "en".

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

> 187:     @Test(dataProvider = "localeProvider")
> 188:     public void testStoreWriterDateComment(final Locale testLocale) throws Exception {
> 189:         var prevLocale = Locale.getDefault();

Could be done once at the class loading time.

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

> 210:     @Test(dataProvider = "localeProvider")
> 211:     public void testStoreOutputStreamDateComment(final Locale testLocale) throws Exception {
> 212:         var prevLocale = Locale.getDefault();

Same as the above method

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

> 251:             // use a neutral locale for parsing, since when the date comment was written by Properties.store(...),
> 252:             // it internally calls the Date.toString() which always writes in a locale insensitive manner
> 253:             DateTimeFormatter.ofPattern(DATE_FORMAT_PATTERN).withLocale(Locale.ROOT).parse(comment);

This formatter can also be statically initialized.

test/jdk/java/util/Properties/StoreReproducibilityTest.java line 430:

> 428:             // use a neutral locale for parsing, since when the date comment was written by Properties.store(...),
> 429:             // it internally calls the Date.toString() which always writes in a locale insensitive manner
> 430:             var df = DateTimeFormatter.ofPattern(DATE_FORMAT_PATTERN).withLocale(Locale.ROOT);

Same as the above.

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

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