Re: RFR: 8282023: PropertiesStoreTest and StoreReproducibilityTest jtreg failures due to en_CA locale [v3]
On Fri, 25 Feb 2022 04:44:45 GMT, Naoto Sato wrote: >> That's a very good point. I've updated the PR to now explicitly use a >> mutable `Set` instead of using `Collectors.toSet()` > > This is ok, although `Collectors.toCollection(HashSet::new)` is a bit concise. Hello Naoto, I've updated the PR to use `HashSet::new`. - PR: https://git.openjdk.java.net/jdk/pull/7558
Re: RFR: 8282023: PropertiesStoreTest and StoreReproducibilityTest jtreg failures due to en_CA locale [v3]
On Fri, 25 Feb 2022 03:51:09 GMT, Jaikiran Pai wrote: >> 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` is mutable? `Collectors.toSet()` >> javadoc reads no guarantee for mutability. > > That's a very good point. I've updated the PR to now explicitly use a mutable > `Set` instead of using `Collectors.toSet()` This is ok, although `Collectors.toCollection(HashSet::new)` is a bit concise. - PR: https://git.openjdk.java.net/jdk/pull/7558
Re: RFR: 8282023: PropertiesStoreTest and StoreReproducibilityTest jtreg failures due to en_CA locale [v3]
On Thu, 24 Feb 2022 17:15:16 GMT, Naoto Sato wrote: >> 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` is mutable? `Collectors.toSet()` > javadoc reads no guarantee for mutability. That's a very good point. I've updated the PR to now explicitly use a mutable `Set` instead of using `Collectors.toSet()` - PR: https://git.openjdk.java.net/jdk/pull/7558
Re: RFR: 8282023: PropertiesStoreTest and StoreReproducibilityTest jtreg failures due to en_CA locale [v3]
On Thu, 24 Feb 2022 05:02:47 GMT, Jaikiran Pai 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 >>
Re: RFR: 8282023: PropertiesStoreTest and StoreReproducibilityTest jtreg failures due to en_CA locale [v3]
> 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. >
