ozlerhakan/poiji

Default system locale gives different behavior

bendh opened this issue · 2 comments

bendh commented

First, I enjoy working with your library, saves a lot of POI boilerplate code ;<). Thanks for the hard work.

I was recently investigating issues with the parsing of Integers. While the tests where succesfull on some developpers PC and jenkins they where failing on other PC's. We noticed differences in OS locales.

After pulling the library sources locally. I runned "mvn clean test" and all tests where green. However when using the test button in intelliJ, some tests failed. After analysis I found out that the Locale used bij Intelij was different then the one used by Maven.
The Locale used is especially important for the formatting of numbers (https://github.com/ozlerhakan/poiji/blob/master/src/main/java/com/poiji/parser/Parsers.java). There you use NumberFormat.getInstance(), This will return a NumberFormat instance with the default Locale causing different behavior on different systems

Solution would be to use a overloaded NumberFormat.getInstance(Locale.us_US) as default and add a option in Options to be able to pass a other Locale as desired. This is effectively what is done in the Maven surefire plugin config
-Duser.language=en -Duser.region=US

Also the test DeserializerCaseDateTest#shouldMapExcelToJava fails with:
java.lang.AssertionError:
Expected: is "12/31/2020 12.00 AM"
but: was "12/31/2020 12.00 a.m."
Expected :12/31/2020 12.00 AM
Actual :12/31/2020 12.00 a.m.

This is also a Locale issue but in Apache POI itself. Test is sucessfull when adding the line LocaleUtil.setUserLocale(Locale.ENGLISH). This LocaleUtil is a POI utility what can set the Locale for the running thread.

If you want I can do that change in a pull request but wanted to know if I'm missing something here before getting to work,

Hey @bendh,

Thank you first off :) and for bringing this up.

Solution would be to use a overloaded NumberFormat.getInstance(Locale.us_US) as default and add a option in Options to be able to pass a other Locale as desired.

Feel free to make a PR for that.

you can use 3.0.3 right now @bendh , thank your for your help 🙌