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/>
