On Wed, 3 Jan 2024 23:30:41 GMT, Justin Lu <j...@openjdk.org> wrote: > Please review this PR which splits up the _LocaleProvidersRun_ test file for > performance and maintenance reasons. > > _LocaleProvidersRun_ which tests against the various Locale Providers (CLDR, > HOST, SPI, etc.) was getting rather long, as all related bugs were added to > the single test file. To improve maintainability, this change splits up the > single test file into separate test files focused on specific areas (ex: > _j.text.Format_). The original _LocaleProvidersRun_ test file remains and is > used for more general Locale Provider testing such as the adapter loading. > > In addition, the previously single test file used to suffer from performance > issues, as each test method had to launch a new JVM (since Locale Providers > are set at Java startup time). With this change, these tests files can be ran > with VM flags and not cause timeout, thus `@requires vm.flagless` is no > longer needed (Tiers 6-8 tested). > > Other updates > - For OS/locale specific tests, the OS/locale is now checked before (not > after) launching a JVM > - For tests that are meant to be tested against specific locales, additional > run invocations were added with the appropiate locale to guarantee a run (ex: > `@run junit/othervm -Duser.language=en -Duser.country=GB`) > - Added comments for each test method
Great to see this refactoring! Much cleaner now. test/jdk/java/util/Locale/LocaleProvidersFormat.java line 30: > 28: * @library /test/lib > 29: * @build LocaleProviders > 30: * providersrc.spi.src.tznp8013086 Although it is not needed in this test, I would list `tznp` to be built here, as the provider's meta-info includes both classes for completeness. Applies to `LocaleProvidersTimeZone` test too. ------------- PR Review: https://git.openjdk.org/jdk/pull/17257#pullrequestreview-1804804618 PR Review Comment: https://git.openjdk.org/jdk/pull/17257#discussion_r1442145501
