On Fri, 2 May 2025 15:49:03 GMT, Justin Lu <[email protected]> wrote:
> Please review this PR which reduces the amount of combinations tested by
> `LocaleNameProviderTest`.
>
> This test was exhaustively testing over 1000x1000 `Locale` combinations which
> was expensive and caused a timeout in tier 5. Narrowing down the tested
> locales to Japanese language ones can reduce our tested input to 9x9 and
> provides most of the same value the test previously did. (Since our SPI
> provider has defined preferred Japanese Locale name values.) This change also
> refactors the test to use JUnit. The test now runs around ~20 times faster.
>
> Most of the diff is attributed to separating the method source from the test
> itself. But notice the filtering of `Locale`s done on `availloc` and
> `jreImplLoc`.
Looks good.
test/jdk/java/util/PluggableLocale/LocaleNameProviderTest.java line 61:
> 59: * This is not an exhaustive test. Such a test would require
> iterating (1000x1000)+
> 60: * inputs. Instead, we check against Japanese lang locales which
> guarantees
> 61: * we will run into cases where the CLDR is not preferred and the SPI
> has defined
It would be helpful to mention why CLDR is not the preferred provider (as the
SPI impl has variants of the Japanese locale)
test/jdk/java/util/PluggableLocale/LocaleNameProviderTest.java line 92:
> 90: String jresctry = null;
> 91: String jresvrnt = null;
> 92: if (!lang.equals("")) {
Can be replaced with `isEmpty()`
-------------
PR Review: https://git.openjdk.org/jdk/pull/25009#pullrequestreview-2812517069
PR Review Comment: https://git.openjdk.org/jdk/pull/25009#discussion_r2071912893
PR Review Comment: https://git.openjdk.org/jdk/pull/25009#discussion_r2071918059
