On Tue, 25 Apr 2023 07:02:47 GMT, Ambarish Rapte <ara...@openjdk.org> wrote:
> The expected result was changed due to an enhancement in JDK: > [JDK-8284840](https://bugs.openjdk.org/browse/JDK-8284840): Update CLDR to > Version 42.0 > Similar tests were corrected in JDK along with the enhancement: [check this > for > example](https://github.com/openjdk/jdk/blob/4900517479f12b59cd8f1c31ad94ad7487c522f7/test/jdk/java/time/test/java/time/format/TestUnicodeExtension.java#L130) > > We need to make similar change. But the expected result is different before > JDK20 and with JDK20. The fix checks JDK major version to change expected > result accordingly. The fix looks good. I left a (minor) comment suggesting that the initialization of the static `JAPANESE_DATE_STRING` field only needs to be done once for the class and not for each instance. I'll reapprove if you want to change it. modules/javafx.base/src/test/java/test/javafx/util/converter/LocalDateTimeStringConverterTest.java line 115: > 113: } else { > 114: JAPANESE_DATE_STRING = "Saturday, January 12, 60 Shōwa, > 12:34:56\u202fPM"; > 115: } Minor: I think this could move to the (static) `@BeforeClass` method rather than being recomputed for each instance. ------------- Marked as reviewed by kcr (Lead). PR Review: https://git.openjdk.org/jfx/pull/1114#pullrequestreview-1399797901 PR Review Comment: https://git.openjdk.org/jfx/pull/1114#discussion_r1176434655
