Hi, if it is only about parsing floats, why not use NumberFormat.getInstance().parse(value)?
I did 8205108: [testbug] Fix pattern matching in jstatd tests. http://hg.openjdk.java.net/jdk-updates/jdk11u/rev/1637a4e50fc9?revcount=80 some while ago, because we had wrong float parsing on mac in our tests. ... just a hint to think about, you don't need to redo the change ... Best regards, Goetz. > -----Original Message----- > From: core-libs-dev <core-libs-dev-boun...@openjdk.java.net> On Behalf Of > Langer, Christoph > Sent: Mittwoch, 20. März 2019 10:10 > To: Stuart Marks <stuart.ma...@oracle.com> > Cc: core-libs-dev <core-libs-dev@openjdk.java.net> > Subject: [CAUTION] RE: RFR(S): 8172695: (scanner) > java/util/Scanner/ScanTest.java fails > > Hi Stuart, > > thanks for consulting with Naoto and providing the update. > > So I'll run the fix through jdk-submit and our test system at SAP with > Locale.US. > Provided I don't see issues, I'll push it thereafter... > > Best regards > Christoph > > > -----Original Message----- > > From: Stuart Marks <stuart.ma...@oracle.com> > > Sent: Dienstag, 19. März 2019 23:22 > > To: Langer, Christoph <christoph.lan...@sap.com> > > Cc: Brian Burkhalter <brian.burkhal...@oracle.com>; core-libs-dev <core- > > libs-...@openjdk.java.net> > > Subject: Re: RFR(S): 8172695: (scanner) java/util/Scanner/ScanTest.java > > fails > > > > Hi Christoph, > > > > I spoke with Naoto Sato (internationalization) and he recommends using > > Locale.US > > instead of Locale.ROOT. (Sorry for having misdirected you.) > > > > The test case in question, parsing float strings, relies on the decimal > > separator, which isn't defined in the ROOT locale. Of course it's "." in > > the US > > locale. Also, the US locale is the only one that's guaranteed to be > > available; > > see Locale::getAvailableLocales. > > > > So I think your current changeset will be fine if Locale.ROOT is replaced > > with > > Locale.US. Assuming it passes test runs. :-) > > > > s'marks > > > > > > On 3/19/19 1:46 AM, Langer, Christoph wrote: > > > Hi Stuart, > > > > > > thanks for your analysis of this. > > > > > > So you are suggesting to use Locale.ROOT for this test unconditionally? I > > agree 😊 > > > > > > The test coding as it is right now is not ready to support arbitrary > > > locales, as > > the comment already suggests. Generally, the test probably needs some > > overhaul. The scanned Strings should for instance use the Locale's decimal > > separators instead of hard coding ".". Maybe there are other shortcomings. > > But as long as this is not done, using Locale ROOT seems to be safe for all > > possible scenarios. > > > > > > I've updated the webrev: > > http://cr.openjdk.java.net/~clanger/webrevs/8172695.1/ > > > > > > Are you ok with that? Brian also? > > > > > > Thanks > > > Christoph > > > > > > > > >> -----Original Message----- > > >> From: Stuart Marks <stuart.ma...@oracle.com> > > >> Sent: Dienstag, 19. März 2019 00:44 > > >> To: Langer, Christoph <christoph.lan...@sap.com> > > >> Cc: Brian Burkhalter <brian.burkhal...@oracle.com>; core-libs-dev <core- > > >> libs-...@openjdk.java.net> > > >> Subject: Re: RFR(S): 8172695: (scanner) java/util/Scanner/ScanTest.java > > fails > > >> > > >> Hi Christoph, > > >> > > >> After looking at this for a little bit I realized that it's not > > >> necessary to > > >> have a system particular default locale in order to get the test case to > > >> fail. > > >> It's possible create a locale that causes test case to fail, like so: > > >> > > >> Locale.setDefault(new Locale("en", "DE")) > > >> Scanner sc = new Scanner("\u0f23.\u0f27"); > > >> sc.nextFloat(); > > >> ^ throws InputMismatchException > > >> > > >> (This is because using "DE" as the locale's country causes texpects , > > instead of > > >> . as the decimal separator.) > > >> > > >> The test already saves the current default locale and attempts to find a > > >> known > > >> good locale for running the tests. The difficulty seems to be finding a > > "known > > >> good" locale. The current check for the language of English doesn't work > > >> if > > >> the > > >> default is en_DE. > > >> > > >> Instead, I'd suggest we use the root locale Locale.ROOT as the known > > good > > >> locale > > >> for running the tests. We could then do something like this: > > >> > > >> Locale reservedLocale = Locale.getDefault(); > > >> try { > > >> Locale.setDefault(Locale.ROOT); > > >> // run all the tests > > >> } finally { > > >> // restore the default locale > > >> Locale.setDefault(reservedLocale); > > >> } > > >> > > >> and dispense with the logic of searching the existing locales for one > > >> that > > we > > >> think is suitable. > > >> > > >> (Actually, since the test is being run in /othervm mode, it's not clear > > >> to me > > >> that restoring the previous default locale is necessary. But that's kind > > >> of a > > >> separate discussion. I'm ok with leaving the save/restore in place.) > > >> > > >> s'marks > > >> > > >> On 3/18/19 10:57 AM, Brian Burkhalter wrote: > > >>> Hi Christoph, > > >>> > > >>> Not a locale expert here so you probably need to hear from someone > > else, > > >> but this looks like a reasonable approach to me. One minor comment is to > > >> perhaps use format() at lines 67-68 instead of println(). > > >>> > > >>> Thanks, > > >>> > > >>> Brian > > >>> > > >>>> On Mar 18, 2019, at 7:51 AM, Langer, Christoph > > >> <christoph.lan...@sap.com> wrote: > > >>>> > > >>>> please review a fix for 8172695, where java/util/Scanner/ScanTest.java > > >> fails. It reproducibly fails on one of our Mac servers, where the locale > > >> is > > set to > > >> en_DE. > > >>>> > > >>>> The test in its current form checks if the test VM is run with a > > supported > > >> default Locale by just checking the language to be one of “en”, “zh”, > > >> “ko”, > > >> “ja”. But it turns out, that also the country part seems to be > > >> important. I > > >> suggest to change the test case to test whether the VM’s default locale > > >> is > > >> one of a restricted set of locales and if it isn’t, it shall try to run > > >> with > > >> “ENGLISH”. > > >>>> > > >>>> I tested the set of US, UK, CHINA, KOREA, JAPAN and GERMANY to > > work > > >> on my machine. Before pushing, I’d run it through the submit repo. > > >>>> > > >>>> bug: https://bugs.openjdk.java.net/browse/JDK-8172695 > > >> <https://bugs.openjdk.java.net/browse/JDK-8172695> > > >>>> webrev: http://cr.openjdk.java.net/~clanger/webrevs/8172695.0/ > > >> <http://cr.openjdk.java.net/~clanger/webrevs/8172695.0/>
