<i18n dev> RFR: 8311663: Additional refactoring of Locale tests to JUnit [v2]

Justin Lu jlu at openjdk.org
Mon Jul 17 21:31:33 UTC 2023


On Fri, 14 Jul 2023 21:59:40 GMT, Naoto Sato <naoto at openjdk.org> wrote:

>> Justin Lu has updated the pull request incrementally with six additional commits since the last revision:
>> 
>>  - Review: Explicitly run via othervm and pass locale args in cmdline
>>  - Review: ws removal in Bug8179071.java
>>  - Review: remove redudant 'throws'
>>  - Review: Use assertTrue() in Bug8179071.java
>>  - Review: Revert "Same as prev commit"
>>    
>>    This reverts commit 933c0312ed2fa0f40a750e950c166b7e820df7ab.
>>  - Review: Revert "Remove try catch in Bug8135061.java"
>>    
>>    This reverts commit f213c74e37ed257a05a535b2c077af327343acb5.
>
> test/jdk/java/util/Locale/Bug8135061.java line 56:
> 
>> 54:         assertNull(match, "[Locale.lookup failed on language"
>> 55:                 + " range: " + ranges + " and language tags "
>> 56:                 + locales + "]");
> 
> Removing try-catch will lose the information on what kind of exception was thrown

I did a second take before pushing and mistakenly removed those blocks, I knew I had left them in there originally for a reason. Fixed.

> test/jdk/java/util/Locale/ThaiGov.java line 51:
> 
>> 49:     // Test number formatting for thai
>> 50:     @Test
>> 51:     public void numberTest() throws RuntimeException {
> 
> Not your change, but this `throws` is redundant

Thanks for the review, addressed this and your other suggestions as well.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/14881#discussion_r1265919717
PR Review Comment: https://git.openjdk.org/jdk/pull/14881#discussion_r1265919129


More information about the i18n-dev mailing list