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.lan...@sap.com>; Stuart Marks
<stuart.ma...@oracle.com>
Cc: core-libs-dev <core-libs-dev@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-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/>
