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

Naoto Sato naoto at openjdk.org
Mon Oct 9 22:47:16 UTC 2023


On Tue, 3 Oct 2023 22:23:11 GMT, Justin Lu <jlu at openjdk.org> wrote:

> Please review this PR which refactors a number of tests under `test/text/NumberFormat` to use JUnit.
> 
> During the switch to JUnit, the tests had the following updates (to improve readability)
> - separate the test data generation from the actual execution of the test
> - create distinct test methods so that all the tests aren't just run under one big method (e.g. main)

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`.

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.

test/jdk/java/text/Format/NumberFormat/Bug4944439.java line 71:

> 69:     public void parseLongTest(String s) {
> 70:         Number parsedNumber = assertDoesNotThrow(() -> df.parse(s),
> 71:                 "DecimalFormat.parse(\"%s\") should not throw ParseException");

`ParseException` -> `Exception`. Applies to other locations

test/jdk/java/text/Format/NumberFormat/Bug4990596.java line 45:

> 43:     public void formatSubclassedNumberTest() {
> 44:         assertDoesNotThrow(() -> new DecimalFormat().format(new MutableInteger(0)),
> 45:                 "DecimalFormat.parse() should support subclasses of Number");

DecimalFormat.format()?

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

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()`

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

PR Review Comment: https://git.openjdk.org/jdk/pull/16035#discussion_r1350752425
PR Review Comment: https://git.openjdk.org/jdk/pull/16035#discussion_r1350835828
PR Review Comment: https://git.openjdk.org/jdk/pull/16035#discussion_r1350845412
PR Review Comment: https://git.openjdk.org/jdk/pull/16035#discussion_r1350862874
PR Review Comment: https://git.openjdk.org/jdk/pull/16035#discussion_r1350883202
PR Review Comment: https://git.openjdk.org/jdk/pull/16035#discussion_r1350899278


More information about the i18n-dev mailing list