coreamp.t can fail for rand #13005

p5pRT · 2013-05-29T13:18:52Z

Migrated from rt.perl.org#118237 (status was 'resolved')

Searchable as RT118237$

p5pRT · 2013-05-29T13:18:52Z

From @nwc10

Sending this to perlbug because I don't want it to get lost, and I don't
know what the right fix should be.

test 303 - &rand at op/coreamp.t line 640
# got '2.90736361456823e-05'
# expected /(?^:^0[.\d+-e]*\z)/

The test code is this:

test_proto 'rand';
$tests += 3;
like &CORE::rand, qr/^0[.\d+-e]*\z/, '&rand';
unlike join(" ", &CORE::rand), qr/ /, '&rand in list context';
&cmp_ok(&CORE::rand(78), qw '< 78', '&rand with 1 arg');

This is perl 5, version 19, subversion 1 (v5.19.1 (v5.19.0-320-g8ef446e*)) built for x86_64-linux
(with 1 registered patch, see perl -V for more detail)

not that I think that that part matters.

Nicholas Clark

p5pRT · 2013-06-02T07:40:37Z

From @cpansprout

On Wed May 29 06:18:52 2013, nicholas wrote:

Sending this to perlbug because I don't want it to get lost, and I
don't
know what the right fix should be.

test 303 - &rand at op/coreamp.t line 640
# got '2.90736361456823e-05'
# expected /(?^:^0[.\d+-e]*\z)/

The test code is this:

test_proto 'rand';
$tests += 3;
like &CORE::rand, qr/^0[.\d+-e]*\z/, '&rand';
unlike join(" ", &CORE::rand), qr/ /, '&rand in list context';
&cmp_ok(&CORE::rand(78), qw '< 78', '&rand with 1 arg');

This is perl 5, version 19, subversion 1 (v5.19.1 (v5.19.0-320-
g8ef446e*)) built for x86_64-linux
(with 1 registered patch, see perl -V for more detail)

not that I think that that part matters.

Fixed by fba93b2.

--

Father Chrysostomos

p5pRT · 2013-06-02T07:40:38Z

The RT System itself - Status changed from 'new' to 'open'

p5pRT · 2013-06-02T07:40:38Z

@cpansprout - Status changed from 'open' to 'resolved'

p5pRT · 2013-12-10T23:58:22Z

From dabe@dabe.com

Created by dabe@dabe.com

When building 5.18.1 (using perlbrew) I got the following:

# Failed test 303 - &rand at op/coreamp.t line 640
# got '6.55239719016265e-05'
# expected /(?^:^0[.\d+-e]*\z)/

Apparently the random number (0.00006552) happened to be so small -- a one in how many chance,
I know -- it came out in scientific notation. Unfortunately, the regexp looks for a return
value that specifically starts with '0', which will never be the case for numbers in
scientific notation (since "0.34e-05" would end up being written "3.4e-06").

Anyway, I don't know if this is the BEST solution, but here's a quick patch that extends the
search pattern to explicitly match just plain zero (it's possible, I suppose), a decimal
that starts with zero, or a very small number in scientific notation:

Inline Patch

--- t/op/coreamp.t_OLD	2013-12-10 18:24:46.000000000 -0500
+++ t/op/coreamp.t	2013-12-10 18:29:04.000000000 -0500
@@ -637,7 +637,7 @@

 test_proto 'rand';
 $tests += 3;
-like &CORE::rand, qr/^0[.\d+-e]*\z/, '&rand';
+like &CORE::rand, qr/^0(?:\.\d+)?\z|[1-9](?:\.\d+)?e-\d+\z/, '&rand';
 unlike join(" ", &CORE::rand), qr/ /, '&rand in list context';
 &cmp_ok(&CORE::rand(78), qw '< 78', '&rand with 1 arg');

NOTE: Even as written in the original test, I don't think it's right to match a plus $e\.g\.\, "3\.4e\+05"$ since CORE::rand with no arguments should never return a number greater than one\.\.\.

PS: And, looking at it again, the "-" in the "[]" character class isn't at the beginning,
so it's actually looking for any character from '+' (0x2B) through lowercase 'e' (0x65),
which includes the entire uppercase alphabet and a slew of other punctuation. Oops!

prompt% perl -E '$f = "0AQb_`,J"; say "$f: " . ($f =~ qr/^0[.\d+-e]*\z/ ? 'YES' : 'NO')'
0AQb_`,J: YES

Perl Info


Flags:
    category=install
    severity=medium

Site configuration information for perl 5.18.1:

Configured by dabe at Tue Dec 10 17:42:30 EST 2013.

Summary of my perl5 (revision 5 version 18 subversion 1) configuration:
   
  Platform:
    osname=darwin, osvers=13.0.0, archname=darwin-2level
    uname='darwin athena.local 13.0.0 darwin kernel version 13.0.0: thu sep 19 22:22:27 pdt 2013; root:xnu-2422.1.72~6release_x86_64 x86_64 '
    config_args='-de -Dprefix=/Users/dabe/perl5/perlbrew/perls/perl-5.18.1 -Aeval:scriptdir=/Users/dabe/perl5/perlbrew/perls/perl-5.18.1/bin'
    hint=recommended, useposix=true, d_sigaction=define
    useithreads=undef, usemultiplicity=undef
    useperlio=define, d_sfio=undef, uselargefiles=define, usesocks=undef
    use64bitint=define, use64bitall=define, uselongdouble=undef
    usemymalloc=n, bincompat5005=undef
  Compiler:
    cc='cc', ccflags ='-fno-common -DPERL_DARWIN -fno-strict-aliasing -pipe -fstack-protector -I/usr/local/include',
    optimize='-O3',
    cppflags='-fno-common -DPERL_DARWIN -fno-strict-aliasing -pipe -fstack-protector -I/usr/local/include'
    ccversion='', gccversion='4.2.1 Compatible Apple LLVM 5.0 (clang-500.2.79)', gccosandvers=''
    intsize=4, longsize=8, ptrsize=8, doublesize=8, byteorder=12345678
    d_longlong=define, longlongsize=8, d_longdbl=define, longdblsize=16
    ivtype='long', ivsize=8, nvtype='double', nvsize=8, Off_t='off_t', lseeksize=8
    alignbytes=8, prototype=define
  Linker and Libraries:
    ld='env MACOSX_DEPLOYMENT_TARGET=10.3 cc', ldflags =' -fstack-protector -L/usr/local/lib'
    libpth=/usr/local/lib /usr/lib
    libs=-ldbm -ldb -ldl -lm -lutil -lc
    perllibs=-ldl -lm -lutil -lc
    libc=, so=dylib, useshrplib=false, libperl=libperl.a
    gnulibc_version=''
  Dynamic Linking:
    dlsrc=dl_dlopen.xs, dlext=bundle, d_dlsymun=undef, ccdlflags=' '
    cccdlflags=' ', lddlflags=' -bundle -undefined dynamic_lookup -L/usr/local/lib -fstack-protector'

Locally applied patches:
    


@INC for perl 5.18.1:
    /Users/dabe/perl5/perlbrew/perls/perl-5.18.1/lib/site_perl/5.18.1/darwin-2level
    /Users/dabe/perl5/perlbrew/perls/perl-5.18.1/lib/site_perl/5.18.1
    /Users/dabe/perl5/perlbrew/perls/perl-5.18.1/lib/5.18.1/darwin-2level
    /Users/dabe/perl5/perlbrew/perls/perl-5.18.1/lib/5.18.1
    .


Environment for perl 5.18.1:
    DYLD_LIBRARY_PATH (unset)
    HOME=/Users/dabe
    LANG=en_US.UTF-8
    LANGUAGE (unset)
    LD_LIBRARY_PATH (unset)
    LOGDIR (unset)
    PATH=/Users/dabe/perl5/perlbrew/bin:/Users/dabe/perl5/perlbrew/perls/perl-5.18.1/bin:/Users/dabe/bin:/usr/local/sbin:/usr/sbin:/sbin:/usr/local/bin:/opt/X11/bin:/usr/bin:/bin:/usr/X11R6/bin
    PERLBREW_BASHRC_VERSION=0.67
    PERLBREW_HOME=/Users/dabe/.perlbrew
    PERLBREW_MANPATH=/Users/dabe/perl5/perlbrew/perls/perl-5.18.1/man
    PERLBREW_PATH=/Users/dabe/perl5/perlbrew/bin:/Users/dabe/perl5/perlbrew/perls/perl-5.18.1/bin
    PERLBREW_PERL=perl-5.18.1
    PERLBREW_ROOT=/Users/dabe/perl5/perlbrew
    PERLBREW_VERSION=0.67
    PERL_BADLANG (unset)
    SHELL=/usr/local/bin/bash

p5pRT · 2013-12-11T00:12:11Z

From @jkeenan

On Tue Dec 10 15:58:22 2013, dabe@dabe.com wrote:

This is a bug report for perl from dabe@dabe.com,
generated with the help of perlbug 1.39 running under perl 5.18.1.

-----------------------------------------------------------------
[Please describe your issue here]

When building 5.18.1 (using perlbrew) I got the following:

# Failed test 303 - &rand at op/coreamp.t line 640
# got '6.55239719016265e-05'
# expected /(?^:^0[.\d+-e]*\z)/

Apparently the random number (0.00006552) happened to be so small -- a
one in how many chance,
I know -- it came out in scientific notation. Unfortunately, the
regexp looks for a return
value that specifically starts with '0', which will never be the case
for numbers in
scientific notation (since "0.34e-05" would end up being written
"3.4e-06").

Anyway, I don't know if this is the BEST solution, but here's a quick
patch that extends the
search pattern to explicitly match just plain zero (it's possible, I
suppose), a decimal
that starts with zero, or a very small number in scientific notation:

--- t/op/coreamp.t_OLD 2013-12-10 18:24:46.000000000 -0500
+++ t/op/coreamp.t 2013-12-10 18:29:04.000000000 -0500
@@ -637,7 +637,7 @@

test_proto 'rand';
$tests += 3;
-like &CORE::rand, qr/^0[.\d+-e]*\z/, '&rand';
+like &CORE::rand, qr/^0(?:\.\d+)?\z|[1-9](?:\.\d+)?e-\d+\z/, '&rand';
unlike join(" ", &CORE::rand), qr/ /, '&rand in list context';
&cmp_ok(&CORE::rand(78), qw '< 78', '&rand with 1 arg');

NOTE: Even as written in the original test, I don't think it's right
to match a plus
(e.g., "3.4e+05") since CORE::rand() with no arguments should never
return a number
greater than one...

PS: And, looking at it again, the "-" in the "[]" character class
isn't at the beginning,
so it's actually looking for any character from '+' (0x2B) through
lowercase 'e' (0x65),
which includes the entire uppercase alphabet and a slew of other
punctuation. Oops!

prompt% perl -E '$f = "0AQb_`,J"; say "$f: " . ($f =~ qr/^0[.\d+-
e]*\z/ ? 'YES' : 'NO')'
0AQb_`,J: YES

cc-ing sprout.

Father C.

It looks like you've already dealt with issues like this.

#####
$ git show fba93b2
commit fba93b2
Author: Father Chrysostomos <sprout@cpan.org>
Date: Sun Jun 2 00:36:33 2013 -0700

[perl #118237] Fix coreamp.t’s rand test

when rand returns something really small that does not
begin with 0, such as 2.90736361456823e-05.

#####

Could you take a look at this one?

Thank you very much.
Jim Keenan

p5pRT · 2013-12-11T00:12:11Z

The RT System itself - Status changed from 'new' to 'open'

p5pRT · 2013-12-11T04:30:49Z

From @tonycoz

On Tue Dec 10 16:12:11 2013, jkeenan wrote:

On Tue Dec 10 15:58:22 2013, dabe@dabe.com wrote:

This is a bug report for perl from dabe@dabe.com,
generated with the help of perlbug 1.39 running under perl 5.18.1.

-----------------------------------------------------------------
[Please describe your issue here]

When building 5.18.1 (using perlbrew) I got the following:

# Failed test 303 - &rand at op/coreamp.t line 640
# got '6.55239719016265e-05'
# expected /(?^:^0[.\d+-e]*\z)/

Apparently the random number (0.00006552) happened to be so small -- a
one in how many chance,
I know -- it came out in scientific notation. Unfortunately, the
regexp looks for a return
value that specifically starts with '0', which will never be the case
for numbers in
scientific notation (since "0.34e-05" would end up being written
"3.4e-06").

Anyway, I don't know if this is the BEST solution, but here's a quick
patch that extends the
search pattern to explicitly match just plain zero (it's possible, I
suppose), a decimal
that starts with zero, or a very small number in scientific notation:

--- t/op/coreamp.t_OLD 2013-12-10 18:24:46.000000000 -0500
+++ t/op/coreamp.t 2013-12-10 18:29:04.000000000 -0500
@@ -637,7 +637,7 @@

test_proto 'rand';
$tests += 3;
-like &CORE::rand, qr/^0[.\d+-e]*\z/, '&rand';
+like &CORE::rand, qr/^0(?:\.\d+)?\z|[1-9](?:\.\d+)?e-\d+\z/, '&rand';
unlike join(" ", &CORE::rand), qr/ /, '&rand in list context';
&cmp_ok(&CORE::rand(78), qw '< 78', '&rand with 1 arg');

NOTE: Even as written in the original test, I don't think it's right
to match a plus
(e.g., "3.4e+05") since CORE::rand() with no arguments should never
return a number
greater than one...

PS: And, looking at it again, the "-" in the "[]" character class
isn't at the beginning,
so it's actually looking for any character from '+' (0x2B) through
lowercase 'e' (0x65),
which includes the entire uppercase alphabet and a slew of other
punctuation. Oops!

prompt% perl -E '$f = "0AQb_`,J"; say "$f: " . ($f =~ qr/^0[.\d+-
e]*\z/ ? 'YES' : 'NO')'
0AQb_`,J: YES

cc-ing sprout.

Father C.

It looks like you've already dealt with issues like this.

#####
$ git show fba93b2
commit fba93b2
Author: Father Chrysostomos <sprout@cpan.org>
Date: Sun Jun 2 00:36:33 2013 -0700
\[perl \#118237\] Fix coreamp\.t’s rand test

when rand returns something really small that does not
begin with 0\, such as 2\.90736361456823e\-05\.
#####

Could you take a look at this one?

It's the same bug and is fixed in blead, that line now reads:

like &CORE::rand, qr/^[.\d+-e]*\z/, '&rand';

I don't believe this qualifies as a critical issue, and so doesn't qualify for
backport to maint.

Tony

p5pRT · 2013-12-13T04:43:02Z

From dabe@dabe.com

Tony Cook via RT wrote:

It's the same bug and is fixed in blead, that line now reads:

like &CORE::rand, qr/^[.\d+-e]*\z/, '&rand';

I wouldn't exactly call that "fixed"...

It certainly quells the false negative – i.e., it no longer reports a
failure when it actually succeeds – but it doesn't do very much to
ensure a valid result (which, if I'm not mistaken, is the whole purpose
of testing, no?)

Now even an empty string will succeed! As will any of the following:

-1
6.6.6
ERROR
:-\
+,-./:;<=>?@[\]^_`
^LL_Y0UR_b^SE_aRE_BeL@NG_+O_US

I would argue the following is "more correct":

like &CORE::rand, qr/^(?:0(?:\.\d+)?|[1-9](?:\.\d+)?e-\d+)\z/, '&rand';

or, (I hope) more readably:

like&CORE::rand, qr/^(?: # match either
0 (?:\.\d+)? # 0, and maybe "dot numbers"
| # or
[1-9] (:\.\d+)? # non-zero and optional decimal
e-\d+ # followed by a negative exponent
)\z/x, '&rand';

On Tue Dec 10 15:58:22 2013, dabe@dabe.com wrote:

Here's a quick patch that extends the search pattern to explicitly match just plain zero (it's possible, I suppose), a decimal that starts with zero, or a very small number in scientific notation:

--- t/op/coreamp.t_OLD 2013-12-10 18:24:46.000000000 -0500
+++ t/op/coreamp.t 2013-12-10 18:29:04.000000000 -0500
@@ -637,7 +637,7 @@

test_proto 'rand';
$tests += 3;
-like&CORE::rand, qr/^0[.\d+-e]*\z/, '&rand';
+like&CORE::rand, qr/^(?:0(?:\.\d+)?|[1-9](?:\.\d+)?e-\d+)\z/, '&rand';
unlike join(" ",&CORE::rand), qr/ /, '&rand in list context';
&cmp_ok(&CORE::rand(78), qw '< 78', '&rand with 1 arg');

NOTE: Even as written in the original test, I don't think it's right
to match a plus
(e.g., "3.4e+05") since CORE::rand() with no arguments should never
return a number
greater than one...

PS: And, looking at it again, the "-" in the "[]" character class
isn't at the beginning,
so it's actually looking for any character from '+' (0x2B) through
lowercase 'e' (0x65),
which includes the entire uppercase alphabet and a slew of other
punctuation. Oops!

prompt% perl -E '$f = "0AQb_`,J"; say "$f: " . ($f =~ qr/^0[.\d+-e]*\z/ ? "YES" : "NO")'
0AQb_`,J: YES

--
:- Dabe

p5pRT · 2013-12-13T10:46:21Z

From zefram@fysh.org

Dabrien 'Dabe' Murphy wrote:

I would argue the following is "more correct":

How about a numeric comparison?

my $r = &CORE::rand;
ok $r >= 0 && $r < 1;

All this regexp business is entertaining, and I'm not one to shy away
from solving a problem with a regexp, but in this case it's firmly the
wrong tool for the job.

-zefram

p5pRT · 2013-12-13T13:32:14Z

From @demerphq

On 13 December 2013 11:45, Zefram <zefram@fysh.org> wrote:

Dabrien 'Dabe' Murphy wrote:

I would argue the following is "more correct":

How about a numeric comparison?
    my $r = &CORE&#8203;::rand;
    ok $r >= 0 && $r \< 1;
All this regexp business is entertaining, and I'm not one to shy away
from solving a problem with a regexp, but in this case it's firmly the
wrong tool for the job.

I like this test. But since we all use the same RNG now, cant we just
set srand() explicitly and test that it produces the right sequence of
random numbers?

Yves

--
perl -Mre=debug -e "/just|another|perl|hacker/"

p5pRT · 2013-12-14T20:22:51Z

From dabe@dabe.com

Tony Cook via RT wrote:

It's the same bug and is fixed in blead, that line now reads:

like &CORE::rand, qr/^[.\d+-e]*\z/, '&rand';

I wouldn't exactly call that "fixed"...

It certainly quells the false positive -- i.e., it no longer reports a
failure when the test actually succeeds -- but it doesn't do very much
to ensure a valid result (which, if I'm not mistaken, is the whole
purpose of TDD, no?)

As it's written now, even an empty string will succeed! As will any of
the following:

-1
6.6.6
ERROR
:-\
+,-./:;<=>?@[\]^_`
^LL_Y0UR_b^SE_aRE_BeL@NG_+O_US

I would argue the following is "more correct":

like &CORE::rand, qr/^(?:0(?:\.\d+)?|[1-9](?:\.\d+)?e-\d+)\z/, '&rand';

or, (I hope) more readably:

like&CORE::rand, qr/^(?: # match either
0 (?:\.\d+)? # 0, and maybe "dot numbers"
| # or
[1-9] (?:\.\d+)? # non-zero and optional decimal
e-\d+ # followed by a negative exponent
)\z/x, '&rand';

--
:- Dabe

p5pRT · 2013-12-15T06:38:10Z

From dabe@dabe.com

On 12/13/13, 5:46 AM, Zefram via RT wrote:

All this regexp business is entertaining, and I'm not one to shy away
from solving a problem with a regexp, but in this case it's firmly the
wrong tool for the job.

You're right, of course... Not that the test code does it, but I wasn't
even thinking about what would happen in a locale that uses "," as the
decimal separator.

How about a numeric comparison?
my $r =&CORE&#8203;::rand;
ok $r>= 0&&  $r\<  1;

My only issue there is that, as written, the empty string and
non-numeric text output (like, "ERROR: Frobnication Failure") would
still pass.

How 'bout:

my $r = &CORE::rand;
ok eval {
use warnings FATAL => qw{numeric uninitialized};
$r >= 0 && $r < 1;
}, '&rand returns a valid number';

?

--
:- Dabe

p5pRT · 2013-12-15T14:33:50Z

From @cpansprout

Dabrien 'Dabe' Murphy wrote:

How 'bout:

 my $r = &CORE&#8203;::rand;
 ok eval \{
     use warnings FATAL => qw\{numeric uninitialized\};
     $r >= 0 && $r \< 1;
 \}\, '&rand returns a valid number';

?

That looks good. Could you turn that into a patch?

p5pRT · 2013-12-16T05:31:49Z

From perl5-porters@perl.org

I wrote:

Dabrien 'Dabe' Murphy wrote:
How 'bout:
 my $r = &CORE&#8203;::rand;
 ok eval \{
     use warnings FATAL => qw\{numeric uninitialized\};
     $r >= 0 && $r \< 1;
 \}\, '&rand returns a valid number';
?
That looks good. Could you turn that into a patch?

Never mind. I have just applied it as d62c8fd and added you to
AUTHORS in f17522d. Thank you.

p5pRT closed this as completed Jun 2, 2013

p5pRT added the Severity Low label Oct 19, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

coreamp.t can fail for rand #13005

coreamp.t can fail for rand #13005

p5pRT commented May 29, 2013

p5pRT commented May 29, 2013

p5pRT commented Jun 2, 2013

p5pRT commented Jun 2, 2013

p5pRT commented Jun 2, 2013

p5pRT commented Dec 10, 2013

p5pRT commented Dec 11, 2013

p5pRT commented Dec 11, 2013

p5pRT commented Dec 11, 2013

p5pRT commented Dec 13, 2013

p5pRT commented Dec 13, 2013

p5pRT commented Dec 13, 2013

p5pRT commented Dec 14, 2013

p5pRT commented Dec 15, 2013

p5pRT commented Dec 15, 2013

p5pRT commented Dec 16, 2013

coreamp.t can fail for rand #13005

coreamp.t can fail for rand #13005

Comments

p5pRT commented May 29, 2013

p5pRT commented May 29, 2013

From @nwc10

p5pRT commented Jun 2, 2013

From @cpansprout

p5pRT commented Jun 2, 2013

p5pRT commented Jun 2, 2013

p5pRT commented Dec 10, 2013

From dabe@dabe.com

Created by dabe@dabe.com

p5pRT commented Dec 11, 2013

From @jkeenan

p5pRT commented Dec 11, 2013

p5pRT commented Dec 11, 2013

From @tonycoz

p5pRT commented Dec 13, 2013

From dabe@dabe.com

p5pRT commented Dec 13, 2013

From zefram@fysh.org

p5pRT commented Dec 13, 2013

From @demerphq

p5pRT commented Dec 14, 2013

From dabe@dabe.com

p5pRT commented Dec 15, 2013

From dabe@dabe.com

p5pRT commented Dec 15, 2013

From @cpansprout

p5pRT commented Dec 16, 2013

From perl5-porters@perl.org