Perl / perl5

🐪 The Perl programming language
https://dev.perl.org/perl5/
Other
1.85k stars 527 forks source link

Test::Builder does not handle overloaded "name" values #7433

Closed p5pRT closed 19 years ago

p5pRT commented 19 years ago

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

Searchable as RT30783$

p5pRT commented 19 years ago

From @audreyt

Created by autrijus@autrijus.org

This happens on ithread 5.8.x perls​:

% perl -Moverload -MTest​::Simple=no_plan -e'BEGIN{overload​::constant(q=>sub{[]})}ok(1\,"x")' Invalid value for shared scalar at /usr/share/perl/5.8/Test/Builder.pm line 319. WHOA! Somehow you got a different number of results than tests ran! This should never happen! Please contact the author immediately! END failed--call queue aborted.

This bug manifested in i18n-0.04's tests\, which I worked around in i18n-0.05 by appending |'' to names to force stringification.

Somehow the shared %result hash does not like being assigned a {name} that contains a reference produced by overloading constants.

Perl Info ``` Flags: category=core severity=low Site configuration information for perl v5.8.2: Configured by root at Wed Feb 18 21:22:17 PST 2004. Summary of my perl5 (revision 5.0 version 8 subversion 2) configuration: Platform: osname=linux, osvers=2.4.25_pre6-gss, archname=i686-linux uname='linux ttul.org 2.4.25_pre6-gss #1 smp wed feb 18 09:59:29 local time zone must be set--see zic i686 mobile amd athlon(tm) xp-m 1400+ authenticamd gnulinux ' config_args='-des -Darchname=i686-linux -Dcccdlflags=-fPIC -Dccdlflags=-rdynamic -Dcc=gcc -Dprefix=/usr -Dvendorprefix=/usr -Dsiteprefix=/usr -Dlocincpth= -Doptimize=-O3 -march=i686 -funroll-loops -pipe -fomit-frame-pointer -Duselargefiles -Dd_dosuid -Dd_semctl_semun -Dscriptdir=/usr/bin -Dman3ext=3pm -Dcf_by=Gentoo -Ud_csh -Di_gdbm -Di_db -Di_ndbm' hint=recommended, useposix=true, d_sigaction=define usethreads=undef use5005threads=undef useithreads=undef usemultiplicity=undef useperlio=define d_sfio=undef uselargefiles=define usesocks=undef use64bitint=undef use64bitall=undef uselongdouble=undef usemymalloc=n, bincompat5005=undef Compiler: cc='gcc', ccflags ='-fno-strict-aliasing -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64', optimize='-O3 -march=i686 -funroll-loops -pipe -fomit-frame-pointer', cppflags='-DPERL5 -fno-strict-aliasing' ccversion='', gccversion='3.2.3 20030422 (Gentoo Linux 1.4 3.2.3-r1, propolice)', gccosandvers='' intsize=4, longsize=4, ptrsize=4, doublesize=8, byteorder=1234 d_longlong=define, longlongsize=8, d_longdbl=define, longdblsize=12 ivtype='long', ivsize=4, nvtype='double', nvsize=8, Off_t='off_t', lseeksize=8 alignbytes=4, prototype=define Linker and Libraries: ld='gcc', ldflags =' -L/usr/local/lib' libpth=/usr/local/lib /lib /usr/lib libs=-lpthread -lnsl -lndbm -lgdbm -ldb -ldl -lm -lcrypt -lutil -lc perllibs=-lpthread -lnsl -ldl -lm -lcrypt -lutil -lc libc=/lib/libc-2.3.2.so, so=so, useshrplib=false, libperl=libperl.a gnulibc_version='2.3.2' Dynamic Linking: dlsrc=dl_dlopen.xs, dlext=so, d_dlsymun=undef, ccdlflags='-rdynamic' cccdlflags='-fPIC', lddlflags='-shared -L/usr/local/lib' Locally applied patches: @INC for perl v5.8.2: /etc/perl /usr/lib/perl5/site_perl/5.8.2/i686-linux /usr/lib/perl5/site_perl/5.8.2 /usr/lib/perl5/site_perl/5.8.0/i686-linux /usr/lib/perl5/site_perl/5.8.0 /usr/lib/perl5/site_perl /usr/lib/perl5/vendor_perl/5.8.2/i686-linux /usr/lib/perl5/vendor_perl/5.8.2 /usr/lib/perl5/vendor_perl /usr/lib/perl5/5.8.2/i686-linux /usr/lib/perl5/5.8.2 /usr/local/lib/site_perl /usr/lib/perl5/site_perl/5.8.0/i686-linux /usr/lib/perl5/site_perl/5.8.0 . Environment for perl v5.8.2: HOME=/home/autrijus LANG (unset) LANGUAGE (unset) LD_LIBRARY_PATH (unset) LOGDIR (unset) PATH=/bin:/usr/bin:/usr/local/bin:/opt/bin:/usr/i686-pc-linux-gnu/gcc-bin/3.3:/usr/X11R6/bin:/opt/blackdown-jdk-1.4.1/bin:/opt/blackdown-jdk-1.4.1/jre/bin:/usr/qt/3/bin PERL_BADLANG (unset) SHELL=/bin/bash ```
p5pRT commented 19 years ago

From @iabyn

On Thu\, Jul 22\, 2004 at 09​:12​:21PM -0000\, Autrijus Tang wrote​:

This happens on ithread 5.8.x perls​:

% perl -Moverload -MTest​::Simple=no_plan -e'BEGIN{overload​::constant(q=>sub{[]})}ok(1\,"x")' Invalid value for shared scalar at /usr/share/perl/5.8/Test/Builder.pm line 319. WHOA! Somehow you got a different number of results than tests ran! This should never happen! Please contact the author immediately! END failed--call queue aborted.

This bug manifested in i18n-0.04's tests\, which I worked around in i18n-0.05 by appending |'' to names to force stringification.

Somehow the shared %result hash does not like being assigned a {name} that contains a reference produced by overloading constants.

Its simpler than that. You can't store an unshared ref in a shared structure; eg

  use threads;   use threads​::shared;   my $h = {};   share($h);   $h->{foo} = [];

outputs

  Invalid value for shared scalar at /tmp/p line 7.

I don't know enough about Test​::Builder etc to know whether it should be responible for stringifying any values before assigning them to $result->{name}\, or whether overloading q() to return a ref is a case of 'doctor it hurts when I do this'.

Dave.

-- You never really learn to swear until you learn to drive.

p5pRT commented 19 years ago

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

p5pRT commented 19 years ago

From fergal@esatclear.ie

On Fri\, Jul 23\, 2004 at 12​:05​:01AM +0100\, Dave Mitchell wrote​:

I don't know enough about Test​::Builder etc to know whether it should be responible for stringifying any values before assigning them to $result->{name}\, or whether overloading q() to return a ref is a case of 'doctor it hurts when I do this'.

from Test-Simple's Changes file​:

0.48_02 Mon Jul 19 02​:07​:23 EDT 2004   * Overloaded objects as names now won't blow up under threads   [rt.cpan.org 4218 and 4232]

I don't it's the case here but using overloaded objects for the names would not be so wise if those objects are (or depend on) the things you are testing. The right bug would leave with test failures and the wrong or no names on your tests\,

F

p5pRT commented 19 years ago

From @audreyt

On 22 Jul 2004 23​:04​:53 -0000\, Dave Mitchell via RT \perlbug\-followup@​perl\.org wrote​:

I don't know enough about Test​::Builder etc to know whether it should be responible for stringifying any values before assigning them to $result->{name}\, or whether overloading q() to return a ref is a case of 'doctor it hurts when I do this'.

Well\, overloading q() to return a ref is how i18n.pm works. :-/

I think stringifying $todo and $name is the way to go. Attached is a patch that implements this solution.

Thanks\, /Autrijus/

p5pRT commented 19 years ago

From @audreyt

Builder.pm.diff

p5pRT commented 19 years ago

From @rgs

Autrijus Tang wrote​:

I don't know enough about Test​::Builder etc to know whether it should be responible for stringifying any values before assigning them to $result->{name}\, or whether overloading q() to return a ref is a case of 'doctor it hurts when I do this'.

Well\, overloading q() to return a ref is how i18n.pm works. :-/

I think stringifying $todo and $name is the way to go. Attached is a patch that implements this solution.

Thanks\, applied as #23167 to bleadperl\, where I've also bumped the Test​::Builder version to 0.17_01. Michael\, please integrate in your aegis repository :)

p5pRT commented 19 years ago

From @schwern

On Fri\, 23 Jul 2004 00​:23​:39 +0100\, Fergal Daly \fergal@​esatclear\.ie wrote​:

I don't it's the case here but using overloaded objects for the names would not be so wise if those objects are (or depend on) the things you are testing. The right bug would leave with test failures and the wrong or no names on your tests\,

As a common reason to overload operators is to hide the fact that there's an object there at all\, the test author often will not even know that they're using an overloaded object as the name. As far as they know\, its just a string.

In the cases where they do know its not such a big deal to use the stringified version of the object you're testing as the name. Being left with no names or wrong names on tests is not very critical and you might just notice that its not working. Besides\, you're always working under the assumption that other parts of your system work while testing. This is nothing new.

p5pRT commented 19 years ago

From @schwern

On Thu\, 22 Jul 2004 20​:17​:58 -0700\, Autrijus Tang \autrijus@​gmail\.com wrote​:

On 22 Jul 2004 23​:04​:53 -0000\, Dave Mitchell via RT \perlbug\-followup@​perl\.org wrote​:

I don't know enough about Test​::Builder etc to know whether it should be responible for stringifying any values before assigning them to $result->{name}\, or whether overloading q() to return a ref is a case of 'doctor it hurts when I do this'.

Well\, overloading q() to return a ref is how i18n.pm works. :-/

I think stringifying $todo and $name is the way to go. Attached is a patch that implements this solution.

Does 0.48_02 solve your problem?

p5pRT commented 19 years ago

From @rgs

Michael G Schwern wrote​:

On Thu\, 22 Jul 2004 20​:17​:58 -0700\, Autrijus Tang \autrijus@​gmail\.com wrote​:

On 22 Jul 2004 23​:04​:53 -0000\, Dave Mitchell via RT \perlbug\-followup@​perl\.org wrote​:

I don't know enough about Test​::Builder etc to know whether it should be responible for stringifying any values before assigning them to $result->{name}\, or whether overloading q() to return a ref is a case of 'doctor it hurts when I do this'.

Well\, overloading q() to return a ref is how i18n.pm works. :-/

I think stringifying $todo and $name is the way to go. Attached is a patch that implements this solution.

Does 0.48_02 solve your problem?

According to : http​://mungus.schwern.org/cgi-bin/aeget/Test-Simple.1.49/lib/Test/Builder.pm it does. (although I don't know why offhand) BTW\, sync patches against blead welcome :)

(hey\, does everybody have a gmail account those days ?)

p5pRT commented 19 years ago

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

p5pRT commented 19 years ago

From @schwern

On Wed\, 28 Jul 2004 12​:09​:21 +0200\, Rafael Garcia-Suarez \rgarciasuarez@​mandrakesoft\.com wrote​:

Does 0.48_02 solve your problem?

According to : http​://mungus.schwern.org/cgi-bin/aeget/Test-Simple.1.49/lib/Test/Builder.pm it does. (although I don't know why offhand)

In the list of changes completed http​://mungus.schwern.org/cgi-bin/aeget/Test-Simple.1.49/@​@​changes@​completed

Look at change 14 http​://mungus.schwern.org/cgi-bin/aeget/Test-Simple.1.49.C14/@​@​menu

And the code change is in Download -> patch (no metadata) http​://mungus.schwern.org/cgi-bin/aeget/Test-Simple.1.49.C14/@​@​aepatch@​compat=4.16