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.

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

Commit messages:
 - fix StoreReproducibilityTest
 - fix PropertiesStoreTest

Changes: https://git.openjdk.java.net/jdk/pull/7558/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=7558&range=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8282023
  Stats: 76 lines in 2 files changed: 56 ins; 2 del; 18 mod
  Patch: https://git.openjdk.java.net/jdk/pull/7558.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/7558/head:pull/7558

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

Reply via email to