RFR: 8310923: Refactor Currency tests to use JUnit [v4]
Justin Lu
jlu at openjdk.org
Fri Jun 30 07:43:58 UTC 2023
On Thu, 29 Jun 2023 22:38:21 GMT, Naoto Sato <naoto at 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
More information about the core-libs-dev
mailing list