JavaMoney / jsr354-ri

JSR 354 - Moneta: Reference Implementation
Other
344 stars 101 forks source link

Build Failed on JDK11 and JDK12 EAP #286

Closed stokito closed 5 years ago

stokito commented 5 years ago

During a build on OpenJDK 11 there was failed for all tests which tries to format Bulgarian amounts. After investigation it turned out that this is absolutely not related to the recent bugs that we had. But tests that me made are detected a new bug. It turned out that internally we calling a DecimalFormat and it returns an amount that formatted without thousands separator at all. The simple test to see the result:

    @Test
    public void formatDecimal_BG() {
        DecimalFormat currencyFormat = (DecimalFormat)DecimalFormat.getCurrencyInstance(new Locale("bg", "BG"));
        Currency BGN = Currency.getInstance("BGN");
        currencyFormat.setCurrency(BGN);
        String formatted = currencyFormat.format(-1234567890);
        assertEquals(formatted, "-1 234 567 890,00 лв."); //AssertionError: expected [-1 234 567 890,00 лв.] but found [-1234567890,00 лв.]
    }

On JDK10 this test passed but on JDK11 it fails. JDK 11 updated CLDR to 11 release 33.

Not sure if this is a bug in JDK or Bulgarian locale really not uses a thousand separator but we should investigate better.

stokito commented 5 years ago

Ok I played a little bit, tried to run format numbers in different JDK versions, then compared results with CLDR and it looks like a lot of formats was changed but the JDK doesn't format correctly and have a lot bugs:

  1. The number formatted in bg_BG locale should definitely use the group separator.
  2. Minus sign is put before dollar sign for en_US locale. This looks similar to recently reported issue #282.
  3. For fr_CH recently changed and now it should used the comma for numbers but for the currency amount it should be used the dot as previous.

And so on. All the bugs we should report to OpenJDK.

So we just can't write a stable tests when the formats are changed between JDK releases. I hope we can at least chose more or least stable locales so their tests will working on all releases. Of we can take in account current JVM version inside tests. Or we can create different versions and say like JavaMoney v1.4 support only JDK8 and JDK9 but if you want to use JDK11 or JDK12 please use JavaMoney v1.5.

Bugs in JDK affects to the JavaMoney library and users will file bugs to us and they will be right because this is something that we should take care. And in fact there is some design decision:

  1. JavaMoney fully relies on JDK formatting as now. Then users of JDK8 will use an old and obsolete formats from CLRD 29 while the current version is 34.
  2. We can try to include CLDR into JavaMoney itself so if users wants to use latest CLDR they can update only JavaMoney itself without updating the whole JDK. This can be simplified to just using ICU4J library from Unicode authors which in fact always updated to latest CLDR version.

Personally I think that we should rely on JDK formats but where it's possible we should be able to parse numbers formatted for latter CLDR (like we did for the French numbers).

But for example in next major version v2.0 of JavaMoney we can try to include the latest CLDR. Or start to depend on ICU4J.

keilw commented 5 years ago

I think we may have to restrict upward-compatibility here if necessary. JavaMoney 2.0 will allow us to use e.g. the Multi-Release JAR for certain special cases and better address needs of Java 13, 15 or 20. Also the iterative mode of a new JSR as opposed to this one will make it easiser.

Looking at Glassfish 5 not supporting Java 9+ at all, I think we can afford to draw a line here and ask people to use the next version with a higher JDK.

keilw commented 5 years ago

Guess this can be closed? Please reopen if you see that something was missing.