On Sat, 4 Mar 2023 01:36:11 GMT, Jaikiran Pai <[email protected]> wrote:
>> Naoto Sato has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> now() -> of() to avoid DST
>
> test/jdk/java/time/test/java/time/format/TestUTCParse.java line 27:
>
>> 25: * @modules jdk.localedata
>> 26: * @bug 8303440
>> 27: * @summary Test parsing "UTC-XX:XX" text works correctly
>
> Hello Naoto, I haven't tried it, but without a `@run testng` instruction,
> does jtreg have the ability to auto infer this as a testng testcase?
Thanks for the review, Jai!
The default test behavior under `java/time` directory is defined in
`java/time/test/TEST.properties` file, where it directs they are run using
`TestNG`.
> test/jdk/java/time/test/java/time/format/TestUTCParse.java line 48:
>
>> 46: // Assuming CLDR's SHORT name for "America/Los_Angeles"
>> 47: // produces "UTC\u212208:00"
>> 48: System.setProperty("java.locale.providers", "CLDR");
>
> Since this updates a system property (that's recognized by code even outside
> of this test), perhaps we should use `@run testng/othervm` to prevent
> interfering with other tests?
And the `TEST.properties` directs tests to be run under `othervm` mode.
> test/jdk/java/time/test/java/time/format/TestUTCParse.java line 63:
>
>> 61: public void testUTCShortNameRoundTrip() {
>> 62: var fmt = DateTimeFormatter.ofPattern("z", Locale.FRANCE);
>> 63: var now = ZonedDateTime.of(2023, 3, 3, 0, 0, 0, 0,
>> ZoneId.of("America/Los_Angeles"));
>
> Nit, this is no longer `now`. Should it renamed to avoid confusion?
Good catch. Modified.
-------------
PR: https://git.openjdk.org/jdk/pull/12868
