Bug #67149 for Test-Trap: Can't use an undefined value as an ARRAY reference at ... Test/Trap/Builder.pm line 97.

Fri Apr 01 11:26:26 2011 gshank [...] cpan.org - Ticket created

Subject:

Can't use an undefined value as an ARRAY reference at ... Test/Trap/Builder.pm line 97.

When there are certain types of failures in my test cases, I get a failure in Test::Trap instead. ============================== 10_tests/unit/panel/06_noemail.pl 1..1 Can't use an undefined value as an ARRAY reference at /home/intellisurvey/versions/6.4/isapps/lib/perl5/site_perl/5.12.3/Test/Trap/Builder.pm line 97. Can't use an undefined value as an ARRAY reference at /home/intellisurvey/versions/6.4/isapps/lib/perl5/site_perl/5.12.3/Test/Trap/Builder.pm line 97. ok 1 - no total for addresses ============================== If I modify line 97 to "my @return = eval { @{$trap->return || []} };", then I don't get the failure. I can reproduce easily, so I can provide more information if you need it. A piece of the debugger output follows. ============================== DB<1> x $trap 0 Test::Trap=HASH(0xc662e20) ' id ' => '1301609166/32165/1' 'die' => 'Can\'t locate IS/DataFile/Asciidelim.pm in @INC (@INC contains: /<snip>.pm line 36. ' 'leaveby' => 'die' 'stderr' => "\e[31mCan't locate IS/DataFile/Asciidelim.pm in \@INC (\@INC contains: <snip>.pm line 36.\cJ\e[0m" 'stdout' => '' 'wantarray' => 1 'warn' => ARRAY(0xbd86180) empty array DB<2> n Test::Trap::Builder::trap(/home/intellisurvey/versions/6.4/isapps/lib/perl5/site_perl/5.12.3/Test/Trap/Builder.pm:98): 98: my @return = eval { @{$trap->return || []} };

Fri Apr 08 17:40:34 2011 ebhanssen [...] cpan.org - Correspondence added

Subject:	Re: [rt.cpan.org #67149] Can't use an undefined value as an ARRAY reference at ... Test/Trap/Builder.pm line 97.
Date:	Fri, 8 Apr 2011 23:40:24 +0200
To:	bug-Test-Trap [...] rt.cpan.org
From:	Eirik Berg Hanssen <ebhanssen [...] cpan.org>

On Fri, Apr 1, 2011 at 5:26 PM, Gerda Shank via RT < bug-Test-Trap@rt.cpan.org> wrote: Show quoted text

> 10_tests/unit/panel/06_noemail.pl > 1..1 > Can't use an undefined value as an ARRAY reference at > > /home/intellisurvey/versions/6.4/isapps/lib/perl5/site_perl/5.12.3/Test/Trap/Builder.pm > line 97. > Can't use an undefined value as an ARRAY reference at > > /home/intellisurvey/versions/6.4/isapps/lib/perl5/site_perl/5.12.3/Test/Trap/Builder.pm > line 97. > ok 1 - no total for addresses > ============================== > > If I modify line 97 to "my @return = eval { @{$trap->return || []} };", > then I don't get the failure. I can reproduce easily, so I can provide > more information if you need it. >

That's kinda odd, isn't it? Why doesn't the eval catch that exception? ... I assume this is not an April Fools prank? ;-) Your modification makes sense, and I may well include it in my next release (a somewhat overdue collection of small fixes coming up), but I'd sure like to see code to reproduce it. (What are we talking – a "__DIE__" handler?) ... if nothing else, it is something I would like to have a regression test for. Eirik

Fri Apr 08 17:40:36 2011 The RT System itself - Status changed from 'new' to 'open'

Fri Apr 08 17:42:22 2011 ebhanssen [...] allverden.no - Taken

Mon Apr 11 10:10:19 2011 gerda.shank [...] gmail.com - Correspondence added

Subject:	Re: [rt.cpan.org #67149] Can't use an undefined value as an ARRAY reference at ... Test/Trap/Builder.pm line 97.
Date:	Mon, 11 Apr 2011 10:10:04 -0400
To:	bug-Test-Trap [...] rt.cpan.org
From:	Gerda Shank <gerda.shank [...] gmail.com>

On 4/8/11 5:40 PM, ebhanssen via RT wrote: Show quoted text

> That's kinda odd, isn't it? Why doesn't the eval catch that exception? > > ... I assume this is not an April Fools prank? ;-) > > Your modification makes sense, and I may well include it in my next > release (a somewhat overdue collection of small fixes coming up), but I'd > sure like to see code to reproduce it. (What are we talking – a "__DIE__" > handler?)

Yes, we do have a DIE handler during our tests: $SIG{__DIE__} = sub { if ( ref $_[0] and UNIVERSAL::can( $_[0], 'tostring' ) ) { return $_[0]->tostring(1); } return use_tracing() ? Carp::confess( RED, @_, RESET ) : print STDERR RED, @_, RESET; } } (use_tracing checks for a flag, which in these cases is not true, so the print STDERR thing happens). It doesn't seem very sensible of the DIE handler to do a return of a 'print' statement. I'll try to get that changed. Show quoted text

> ... if nothing else, it is something I would like to have a regression > test for. > > > Eirik >

The next couple of days will be kind of busy, but I could try to boil it down to a test case later this week... Gerda

Thu Jun 02 17:19:20 2011 ebhanssen [...] allverden.no - Correspondence added

On Mon Apr 11 10:10:19 2011, gerda.shank@gmail.com wrote: Show quoted text

> On 4/8/11 5:40 PM, ebhanssen via RT wrote:

> > Your modification makes sense, and I may well include it in my next > > release (a somewhat overdue collection of small fixes coming up),

but I'd Show quoted text

> > sure like to see code to reproduce it. (What are we talking – a

"__DIE__" Show quoted text

> > handler?)

> Yes, we do have a DIE handler during our tests: > > $SIG{__DIE__} = sub { > if ( ref $_[0] and UNIVERSAL::can( $_[0], 'tostring' ) ) { > return $_[0]->tostring(1); > } > return use_tracing() > ? Carp::confess( RED, @_, RESET ) > : print STDERR RED, @_, RESET; > } > }

Ah yes; that would do it. Sorry I'm late in replying. Aside from having other things on my mind lately, I have been wondering just how to deal with this. Your suggested patch handles this exact problem, but it fails to handle problems arising when the return() method is overridden, and something inside it dies – while there is a noisy __DIE__ handler about. And that also points out to me that I've been neglecting to document that the trap object should respond to a return() method call in the first place. A hole in the API, that is. Rather than calling arbitrary methods of the trap class from the builder class, there should be an API so that the layer trapping the return values (or even something else) could store them so that the builder knows how to retrieve them. I suppose. But I'm in doubt, and I'm not even sure why. Need to sleep on it some more, I fear. -- Eirik Berg Hanssen, ebhanssen@allverden.no Just this .sig then nothing more