<i18n dev> RFR: 8310234: Refactor Locale tests to use JUnit [v2]

[Naoto Sato]

On Thu, 22 Jun 2023 18:55:31 GMT, Justin Lu <jlu at openjdk.org> wrote:

>> Please review this PR as apart of [JDK-8307843](https://bugs.openjdk.org/browse/JDK-8307843) which refactors some tests in Locale to use JUnit. Other cleanup and small changes are included as well. More refactoring in Locale tests will be done in separate PRs.
>> 
>> If the test had a bugNNNNN.java name, it was also renamed to something more [descriptive](https://openjdk.org/jtreg/faq.html#how-should-i-name-a-test).
>> 
>> Below is a list of all the changes,
>> 
>> - Refactor Bug4316602.java as LocaleConstructors.java
>> - Refactor Bug4210525.java as CaseCheckVariant.java
>> - Refactor bug6277243.java as RootLocale.java
>> - Refactor bug6312358.java as GetInstanceCheck.java
>> - Refactor Bug8154797.java as CompareProviderFormats.java
>> - Refactor Bug8004240.java as GetAdapterPreference.java
>> - Refactor bug4122700.java into AvailableLocalesTest.java (and combined with StreamAvailableLocales.java)
>
> Justin Lu has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Review: Clarify LocaleConstructors.java with comment for each test method

test/jdk/java/util/Locale/AvailableLocalesTest.java line 46:

> 44:     /**
> 45:      * Test that Locale.getAvailableLocales() is non-empty.
> 46:      * Print out the locales.

If you combine two tests into a single one, I'd put the corresponding bug id into the method description.

test/jdk/java/util/Locale/AvailableLocalesTest.java line 61:

> 59:      */
> 60:     @Test
> 61:     public void StreamEqualsArrayTest() {

Method names should start with a lowercase character

test/jdk/java/util/Locale/CaseCheckVariant.java line 50:

> 48:     @ParameterizedTest
> 49:     @MethodSource("variants")
> 50:     public void variantNormalizationTest(String variant) {

"Normalization" -> "Case"? Locale does not normalize the variant.

test/jdk/java/util/Locale/GetInstanceCheck.java line 79:

> 77:         } catch (InvocationTargetException | IllegalAccessException exc) {
> 78:             Throwable cause = exc.getCause();
> 79:             if (exc.getCause() instanceof NullPointerException) {

Should this condition only be meaningful for `InvocationTargetException`?

test/jdk/java/util/Locale/GetInstanceCheck.java line 99:

> 97:         } catch (InvocationTargetException | IllegalAccessException exc) {
> 98:             Throwable cause = exc.getCause();
> 99:             if (cause instanceof NullPointerException) {

Same as above

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/14609#discussion_r1238910004
PR Review Comment: https://git.openjdk.org/jdk/pull/14609#discussion_r1238912059
PR Review Comment: https://git.openjdk.org/jdk/pull/14609#discussion_r1238915580
PR Review Comment: https://git.openjdk.org/jdk/pull/14609#discussion_r1238927882
PR Review Comment: https://git.openjdk.org/jdk/pull/14609#discussion_r1238927101