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