> 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
-------------
Changes:
- all: https://git.openjdk.java.net/jdk/pull/7558/files
- new: https://git.openjdk.java.net/jdk/pull/7558/files/c5dd7f79..97c9afd5
Webrevs:
- full: https://webrevs.openjdk.java.net/?repo=jdk&pr=7558&range=02
- incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=7558&range=01-02
Stats: 36 lines in 2 files changed: 1 ins; 12 del; 23 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
