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

Stuart Marks stuart.marks at oracle.com
Wed Mar 20 22:39:18 UTC 2019 

I agree, this is not a test of locale-sensitive number parsing, it's a test of 
Scanner. Thus I think it's reasonable to keep the focus on the Scanner methods.

To step back a bit, this bug is about fixing the "intermittent" failure of this 
test. This isn't case where the test fails randomly 0.5% of the time. The 
failures are caused by a dependency on the JVM's default locale, which is a 
*hidden global variable*. It's possible to demonstrate a 100% failure rate the 
default locale is set to one for which the test is unprepared. Given this, 
setting the default locale to a known good locale is the right way to fix this test.

(I'm reminded of tests and even production code that has a hidden dependency on 
the VM's default charset. Incorrect assumptions about the default charset can 
cause all kinds of hilarity.)

s'marks

On 3/20/19 2:23 AM, Langer, Christoph wrote:
> Hi Goetz,
> 
> this test is specifically for java.util.Scanner.
> 
> Generally, it could probably be overhauled to make it run with any Locale. However, then the input data for scanning will need to be written in the Locale that the scanner uses. This is a bit out of scope for me at this point...
> 
> But anyway, thanks for your hint ��
> 
> /christoph
> 
>> -----Original Message-----
>> From: Lindenmaier, Goetz
>> Sent: Mittwoch, 20. März 2019 10:21
>> To: Langer, Christoph <christoph.langer at sap.com>; Stuart Marks
>> <stuart.marks at oracle.com>
>> Cc: core-libs-dev <core-libs-dev at openjdk.java.net>
>> Subject: RE: RFR(S): 8172695: (scanner) java/util/Scanner/ScanTest.java fails
>>
>> 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-bounces at openjdk.java.net> On Behalf
>> Of
>>> Langer, Christoph
>>> Sent: Mittwoch, 20. März 2019 10:10
>>> To: Stuart Marks <stuart.marks at oracle.com>
>>> Cc: core-libs-dev <core-libs-dev at 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.marks at oracle.com>
>>>> Sent: Dienstag, 19. März 2019 23:22
>>>> 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,
>>>>
>>>> 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.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/>

More information about the core-libs-dev mailing list