On Thu, 29 Jun 2023 22:38:21 GMT, Naoto Sato <na...@openjdk.org> wrote:
>> Justin Lu has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Remove unusued JUnit imports from ValidateISO4217.java
>
> test/jdk/java/util/Currency/CurrencyNameProviderTest.java line 43:
>
>> 41: import static org.junit.jupiter.api.Assertions.assertThrows;
>> 42:
>> 43: public class CurrencyNameProviderTest {
>
> Since this is not a generic CurrencyNameProvider test, but specific to the
> default impl of CNP.getDisplayName(), the class name could be more specific.
I have renamed the file to be more descriptive, but if acronyms are not
preferred in file names, I can rename it.
> test/jdk/java/util/Currency/CurrencyTest.java line 241:
>
>> 239: Arguments.of("JOD", 3), Arguments.of("KWD", 3),
>> 240: Arguments.of("LYD", 3), Arguments.of("OMR", 3),
>> 241: Arguments.of("TND", 3), Arguments.of("TRL", 0), //
>> Turkish Lira
>
> The turkish lira comment should apply to both TRL and TRY
Yes good point, I have clarified the comment
> test/jdk/java/util/Currency/CurrencyTest.java line 321:
>
>> 319: Currency currency2 = Currency.getInstance(currencyCode);
>> 320: assertEquals(currency1, currency2, "Didn't get same instance
>> for same currency code");
>> 321: assertEquals(currency1.getCurrencyCode(), currencyCode,
>> "Currency code changed");
>
> Error message more like "wrong currency code", than "changed"
Updated the err msg
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/14682#discussion_r1247544121
PR Review Comment: https://git.openjdk.org/jdk/pull/14682#discussion_r1247544370
PR Review Comment: https://git.openjdk.org/jdk/pull/14682#discussion_r1247544313
