Bug #76485 for Test-Harness: TAP::Formatter::Base is too strict about filehandles

Tue Apr 10 18:42:32 2012 ether [...] cpan.org - Ticket created

Subject:

TAP::Formatter::Base is too strict about filehandles

This should work: my $output; open my $fh, '>', \$output; my @results = Test::Harness::execute_tests(tests => [$test_file], out => \$fh); But it does not: option 'stdout' needs a filehandle at /home/perlbrew/perls/perl-5.14.2/lib/5.14.2/TAP/Object.pm line 92 TAP::Object::_croak('TAP::Formatter::File=HASH(0x41b10b8)', 'option \'stdout\' needs a filehandle') called at /home/perlbrew/perls/perl-5.14.2/lib/5.14.2/TAP/Formatter/Base.pm line 30 TAP::Formatter::Base::__ANON__('TAP::Formatter::File=HASH(0x41b10b8)', 'REF(0x30fdf90)') called at /home/perlbrew/perls/perl-5.14.2/lib/5.14.2/TAP/Formatter/Base.pm line 80 TAP::Formatter::Base::_initialize('TAP::Formatter::File=HASH(0x41b10b8)', 'HASH(0x2fcfd38)') called at /home/perlbrew/perls/perl-5.14.2/lib/5.14.2/TAP/Object.pm line 58 ... The filehandle needn't be a glob - it just needs to be something that IO::Handle can understand.

Tue Apr 10 18:47:36 2012 ether [...] cpan.org - Status changed from 'new' to 'resolved'

Tue Apr 10 18:47:51 2012 ether [...] cpan.org - Correspondence added

never mind. please delete.

Tue Apr 10 18:47:52 2012 The RT System itself - Status changed from 'resolved' to 'open'

Tue Apr 10 18:48:07 2012 ether [...] cpan.org - Correspondence added

sigh.

Tue Apr 10 18:48:08 2012 ether [...] cpan.org - Status changed from 'open' to 'resolved'

Wed Apr 11 11:25:25 2012 schwern [...] pobox.com - Correspondence added

Subject:	Re: [rt.cpan.org #76485] TAP::Formatter::Base is too strict about filehandles
Date:	Wed, 11 Apr 2012 08:25:10 -0700
To:	bug-Test-Harness [...] rt.cpan.org
From:	Michael G Schwern <schwern [...] pobox.com>

Looks like a valid issue to me. Test::Builder goes through some gyrations to ensure it will accept non-glob filehandles and that code could be borrowed. What made you decide it wasn't an issue? We'd rather hear about more issues and deal with the few that turn out to be a non-issue. :-) As a side issue, the error message is incorrect referring to the 'stdout' option. This is because of the translation of the args from what Test::Harness wants to what TAP::Harness wants. A little tricky to fix without getting into error message hacking.

Wed Apr 11 11:25:26 2012 The RT System itself - Status changed from 'resolved' to 'open'

Wed Apr 11 12:19:47 2012 ether [...] cpan.org - Correspondence added

On Wed Apr 11 08:25:25 2012, schwern@pobox.com wrote: Show quoted text

> Looks like a valid issue to me. Test::Builder goes through some > gyrations to > ensure it will accept non-glob filehandles and that code could be > borrowed. > What made you decide it wasn't an issue?

Because this works: my $output; my @results = Test::Harness::execute_tests( tests => [$file], out => IO::Scalar->new(\$output), ); It's reasonable to check for an object that $obj->can('print'), since the print method is used later. It is true though that any scalarref can be converted to an object that isa IO::Handle, so Test::Harness could do that itself too, rather than having the caller do it. But I filed the bug before closely inspecting the code and seeing what the requirements were, so that was my bad :)

Wed Apr 11 13:30:57 2012 schwern [...] pobox.com - Correspondence added

Subject:	Re: [rt.cpan.org #76485] TAP::Formatter::Base is too strict about filehandles
Date:	Wed, 11 Apr 2012 10:30:37 -0700
To:	bug-Test-Harness [...] rt.cpan.org
From:	Michael G Schwern <schwern [...] pobox.com>

On 2012.4.11 9:19 AM, Karen Etheridge via RT wrote: Show quoted text

>> What made you decide it wasn't an issue?

> > Because this works: > > my $output; > my @results = Test::Harness::execute_tests( > tests => [$file], > out => IO::Scalar->new(\$output), > ); > > It's reasonable to check for an object that $obj->can('print'), since > the print method is used later. It is true though that any scalarref > can be converted to an object that isa IO::Handle, so Test::Harness > could do that itself too, rather than having the caller do it. But I > filed the bug before closely inspecting the code and seeing what the > requirements were, so that was my bad :)

Just because there's a less convenient hack around doesn't make it any better. Nor should you be writing your code to match the internal validation code or need to have any knowledge of the quirks of that validation code. I don't think ->can('print') is a hard requirement, it's just a convenient way to check if a thing is usable as a filehandle, Test::Harness doesn't actually try to call a print method. And it's lying anyway. You can call print() on a scalar filehandle if IO::Handle is loaded, but can() insists you can't. I think the real problem is you passed a lexical filehandle in as a reference. Globs have confusing magic when you take a reference to them, but lexical filehandles do not. To add to the confusion, you must pass in a reference to a glob, just because of that check. Test::Builder has extra logic to handle this case and I'll put it into Test::Harness. https://github.com/AndyA/Test-Harness/commit/98815ad19b00544caea70dcc6d771dcfbd487f58 So, passing in a lexical filehandle attached to a string is just fine. But when I try it, there's a new problem no matter what's passed into 'out'. PANIC: could not determine iterator for input at lib/Test/Harness.pm line 152 No idea what that's about. Do you not see that?

Wed Apr 11 14:20:53 2012 ether [...] cpan.org - Correspondence added

On Wed Apr 11 10:30:57 2012, schwern@pobox.com wrote: Show quoted text

> Just because there's a less convenient hack around doesn't make it any > better. > Nor should you be writing your code to match the internal validation > code or > need to have any knowledge of the quirks of that validation code.

Agreed. I just didn't want to make a fuss about it, but I'm happy you want to fix it. :) Show quoted text

> I think the real problem is you passed a lexical filehandle in as a > reference.

Right, that should have been just out => $fh, because: my $output; open my $fh, '>', \$output print ref $fh; ...prints "GLOB", and indeed Test::Formatter::Base is happy with $fh as an argument. Show quoted text

> No idea what that's about. Do you not see that?

I'm afraid not; after applying that change to Test::Formatter::Base, I can successfully pass out => *STDOUT.

Wed Apr 11 15:21:34 2012 schwern [...] pobox.com - Correspondence added

Subject:	Re: [rt.cpan.org #76485] TAP::Formatter::Base is too strict about filehandles
Date:	Wed, 11 Apr 2012 12:21:23 -0700
To:	bug-Test-Harness [...] rt.cpan.org
From:	Michael G Schwern <schwern [...] pobox.com>

On 2012.4.11 11:20 AM, Karen Etheridge via RT wrote: Show quoted text

> Agreed. I just didn't want to make a fuss about it, but I'm happy you > want to fix it. :)

Can't decide if it should be fixed if there's no report. Show quoted text

>> I think the real problem is you passed a lexical filehandle in as a >> reference.

> > Right, that should have been just out => $fh, because: > > my $output; open my $fh, '>', \$output > print ref $fh; > > ...prints "GLOB", and indeed Test::Formatter::Base is happy with $fh as > an argument.

More importantly, a reference to a lexical filehandle doesn't work as a filehandle. my $output = ''; open my $fh, ">", \$output; my $ref = \$fh; print $ref "Foo"; # "Not a GLOB reference" error Show quoted text

>> No idea what that's about. Do you not see that?

> > I'm afraid not; after applying that change to Test::Formatter::Base, I > can successfully pass out => *STDOUT.

Hmmm. What version of Test::Harness and Perl please?

Wed Apr 11 16:21:44 2012 ether [...] cpan.org - Correspondence added

Subject:	Re: [rt.cpan.org #76485] TAP::Formatter::Base is too strict about filehandles
Date:	Wed, 11 Apr 2012 13:21:31 -0700
To:	Michael G Schwern via RT <bug-Test-Harness [...] rt.cpan.org>
From:	Karen Etheridge <ether [...] cpan.org>

On Wed, Apr 11, 2012 at 03:21:36PM -0400, Michael G Schwern via RT wrote: Show quoted text

> >> No idea what that's about. Do you not see that?

> > > > I'm afraid not; after applying that change to Test::Formatter::Base, I > > can successfully pass out => *STDOUT.

> > Hmmm. What version of Test::Harness and Perl please?

$ perl -MTest::Harness\ 999 -e1 Test::Harness version 999 required--this is only version 3.23. BEGIN failed--compilation aborted. $ perl -V Summary of my perl5 (revision 5 version 14 subversion 2) configuration: Platform: osname=linux, osvers=2.6.32-220.el6.x86_64, archname=x86_64-linux uname='linux h192-168-2-139.airg.us 2.6.32-220.el6.x86_64 #1 smp tue dec 6 19:48:22 gmt 2011 x86_64 x86_64 x86_64 gnulinux ' config_args='-de -Dprefix=/home/perlbrew/perls/perl-5.14.2' 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-strict-aliasing -pipe -fstack-protector -I/usr/local/include -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64', optimize='-O2', cppflags='-fno-strict-aliasing -pipe -fstack-protector -I/usr/local/include' ccversion='', gccversion='4.4.6 20110731 (Red Hat 4.4.6-3)', 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='cc', ldflags =' -fstack-protector -L/usr/local/lib' libpth=/usr/local/lib /lib/../lib64 /usr/lib/../lib64 /lib /usr/lib /lib64 /usr/lib64 /usr/local/lib64 libs=-lnsl -lgdbm -ldl -lm -lcrypt -lutil -lc perllibs=-lnsl -ldl -lm -lcrypt -lutil -lc libc=, so=so, useshrplib=false, libperl=libperl.a gnulibc_version='2.12' Dynamic Linking: dlsrc=dl_dlopen.xs, dlext=so, d_dlsymun=undef, ccdlflags='-Wl,-E' cccdlflags='-fPIC', lddlflags='-shared -O2 -L/usr/local/lib -fstack-protector' Characteristics of this binary (from libperl): Compile-time options: PERL_DONT_CREATE_GVSV PERL_MALLOC_WRAP PERL_PRESERVE_IVUV USE_64_BIT_ALL USE_64_BIT_INT USE_LARGE_FILES USE_PERLIO USE_PERL_ATOF Built under linux Compiled at Jan 18 2012 19:37:06 %ENV: PERLBREW_BASHRC_VERSION="0.39" PERLBREW_HOME="/home/ether/.perlbrew" PERLBREW_MANPATH="/home/perlbrew/perls/perl-5.14.2/man" PERLBREW_PATH="/home/perlbrew/bin:/home/perlbrew/perls/perl-5.14.2/bin" PERLBREW_PERL="perl-5.14.2" PERLBREW_ROOT="/home/perlbrew" PERLBREW_VERSION="0.39" @INC: /home/perlbrew/perls/perl-5.14.2/lib/site_perl/5.14.2/x86_64-linux /home/perlbrew/perls/perl-5.14.2/lib/site_perl/5.14.2 /home/perlbrew/perls/perl-5.14.2/lib/5.14.2/x86_64-linux /home/perlbrew/perls/perl-5.14.2/lib/5.14.2 .

Wed Apr 11 18:57:40 2012 schwern [...] pobox.com - Correspondence added

Subject:	Re: [rt.cpan.org #76485] TAP::Formatter::Base is too strict about filehandles
Date:	Wed, 11 Apr 2012 15:57:25 -0700
To:	bug-Test-Harness [...] rt.cpan.org
From:	Michael G Schwern <schwern [...] pobox.com>

Ahh, apparently that is Test::Harness' way of reacting to passing [undef] to the 'tests' option. I will report that separately and close this up.

Wed Apr 11 18:57:45 2012 mschwern [...] cpan.org - Status changed from 'open' to 'resolved'