RFR(S): 8172695: (scanner) java/util/Scanner/ScanTest.java fails

[Langer, Christoph]

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.marks at oracle.com>
> Sent: Dienstag, 19. März 2019 00:44
> To: Langer, Christoph <christoph.langer at sap.com>
> Cc: Brian Burkhalter <brian.burkhalter at oracle.com>; core-libs-dev <core-
> libs-dev at 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.langer at 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/>