<i18n dev> RFR: JDK-8317372: Refactor some NumberFormat tests to use JUnit [v2]

[Justin Lu]

On Mon, 9 Oct 2023 20:13:39 GMT, Naoto Sato <naoto at openjdk.org> wrote:

>> Justin Lu has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Reflect review comments
>
> test/jdk/java/text/Format/NumberFormat/BigDecimalCompatibilityTest.java line 162:
> 
>> 160:         // Check parse and returned value
>> 161:         Number parsedValue = assertDoesNotThrow(()-> df.parse(longString),
>> 162:                 "Should not throw a ParseException");
> 
> It could catch any exception, so the message could be broader. Maybe just `a ParseException` -> `an Exception`.

Thank you for the review. I Adjusted the message, but clarified in the comment that the original intention was checking for a `ParseException`; fixed in the other instances as well.

> test/jdk/java/text/Format/NumberFormat/Bug4838107.java line 254:
> 
>> 252:                     "\n\tre-parsed number: " + parsed2 +
>> 253:                     "  (" + parsed2.getClass().getName() + ")" +
>> 254:                     "\n\tminus sign: " + df.getDecimalFormatSymbols().getMinusSign());
> 
> Could use text block to get rid of `\n`/`\t`s, with `.formatted()`. Same for the following one.

Adjusted.

> test/jdk/java/text/Format/NumberFormat/Bug8132125.java line 47:
> 
>> 45:         NumberFormat nf = NumberFormat.getInstance(deCH);
>> 46: 
>> 47:         // i.e. "\u2019" as decimal separator, "\u2019" as grouping separator
> 
> This is incorrect (sorry, I am to be blamed as I wrote this 😄). Should be "`\u002E` as decimal separator, " also remove `i.e.` if you move the comment on top of the variable declaration

Fixed.

> test/jdk/java/text/Format/NumberFormat/CurrencyFormat.java line 62:
> 
>> 60:     // currencySymbolsTest() is only ran for COMPAT
>> 61:     private static final boolean isCompat =
>> 62:             "COMPAT".equals(System.getProperty("java.locale.providers"));
> 
> This check could be moved into `currencySymbolsTest()`

I left it as a static declaration as although it is used in `currencySymbolsTest()`, it is also used in the data provider of `CurrencyFormatTest()`.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/16035#discussion_r1355790743
PR Review Comment: https://git.openjdk.org/jdk/pull/16035#discussion_r1355790850
PR Review Comment: https://git.openjdk.org/jdk/pull/16035#discussion_r1355790950
PR Review Comment: https://git.openjdk.org/jdk/pull/16035#discussion_r1355791032