On Mon, 21 Feb 2022 14:09:50 GMT, Jaikiran Pai <j...@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. Hello Naoto, thank you for the review. I've updated the PR with the suggested changes. Tests continue to pass with these changes. ------------- PR: https://git.openjdk.java.net/jdk/pull/7558
