[perl #41529] [BUG]: t/perl/Parrot_Distribution.t test failure

[James Keenan]

# New Ticket Created by James Keenan
# Please include the string: [perl #41529]
# in the subject line of all future correspondence about this issue.
# <URL: http://rt.perl.org/rt3/Ticket/Display.html?id=41529 >

Running make test, this morning, one failing test was fixed, one (the
shootout) remained unfixed, and one new failure appeared in t/perl/
Parrot_Distribution.t.

Failed Test Stat Wstat Total Fail List of Failed
------------------------------------------------------------------------
-------
t/examples/shootout.t 1 256 20 1 16
t/perl/Parrot_Distribution.t 1 256 31 1 31
11 tests and 602 subtests skipped.
Failed 2/261 test scripts. 2/6558 subtests failed.
Files=261, Tests=6558, 1431 wallclock secs (396.89 cusr + 235.21 csys
= 632.10 CPU)
Failed 2/261 test programs. 2/6558 subtests failed.
make: *** [test] Error 25

Here's the output of prove -v on the failing test file:

[parrot] 513 $ vi t/perl/Parrot_Distribution.t
[parrot] 514 $ prove -v t/perl/Parrot_Distribution.t
t/perl/Parrot_Distribution....1..31
ok 1 - use Parrot::Distribution;
ok 2 - The object isa Parrot::Docs::Directory
ok 3 - C source file
WARNING: lib/Parrot/Distribution.pm:262 File not found: moomoo.c
ok 4 - C source file not there
ok 5 - C header file
WARNING: lib/Parrot/Distribution.pm:262 File not found: moomoo.h
ok 6 - C header file not there
ok 7 - PMC code
WARNING: lib/Parrot/Distribution.pm:262 File not found: moomoo.pmc
ok 8 - PMC code file not there
ok 9 - Yacc code
WARNING: lib/Parrot/Distribution.pm:262 File not found: moomoo.y
ok 10 - Yacc code file not there
ok 11 - Lex code
WARNING: lib/Parrot/Distribution.pm:262 File not found: moomoo.l
ok 12 - Lex code file not there
ok 13 - Perl module file
ok 14 - Perl module file not there
ok 15 - Directory from hardcoded list compilers/bcg/src/pmc found
through MANIFEST
ok 16 - Directory from hardcoded list languages/APL/src/pmc found
through MANIFEST
ok 17 - Directory from hardcoded list languages/WMLScript/pmc found
through MANIFEST
ok 18 - Directory from hardcoded list languages/amber/lib/kernel/pmc
found through MANIFEST
ok 19 - Directory from hardcoded list languages/cardinal/src/pmc
found through MANIFEST
ok 20 - Directory from hardcoded list languages/dotnet/pmc found
through MANIFEST
ok 21 - Directory from hardcoded list languages/lua/pmc found through
MANIFEST
ok 22 - Directory from hardcoded list languages/perl6/src/pmc found
through MANIFEST
ok 23 - Directory from hardcoded list languages/pugs/pmc found
through MANIFEST
ok 24 - Directory from hardcoded list languages/tcl/src/pmc found
through MANIFEST
ok 25 - Directory from hardcoded list src/dynpmc found through MANIFEST
ok 26 - Directory from hardcoded list src/pmc found through MANIFEST
ok 27 - Got perl files
ok 28 - Perl source file (.pl)
ok 29 - Perl source file (.pm)
ok 30 - Perl source file (.t)
not ok 31 - Not a Perl source file (.t)

# Failed test (t/perl/Parrot_Distribution.t at line 75)
# Looks like you failed 1 test of 31.
dubious
Test returned status 1 (wstat 256, 0x100)
DIED. FAILED test 31
Failed 1/31 tests, 96.77% okay
Failed Test Stat Wstat Total Fail List of Failed
------------------------------------------------------------------------
-------
t/perl/Parrot_Distribution.t 1 256 31 1 31
Failed 1/1 test scripts. 1/31 subtests failed.
Files=1, Tests=31, 4 wallclock secs ( 2.47 cusr + 0.39 csys = 2.86
CPU)
Failed 1/1 test programs. 1/31 subtests failed.

Here's the relevant section of t/perl/Parrot_Distribution.t:

[parrot] 519 $ ack perl_source_file_with_name
t/perl/Parrot_Distribution.t
72: ok( $d->perl_source_file_with_name('ops2c.pl'), 'Perl source
file (.pl)' );
73: ok( $d->perl_source_file_with_name('Distribution.pm'), 'Perl
source file (.pm)' );
74: ok( $d->perl_source_file_with_name('perlcritic.t'), 'Perl
source file (.t)' );
75: ok( !$d->perl_source_file_with_name('p5rx.t'), 'Not a Perl
source file (.t)' );

Note that acking for 'perl_source_file_with_name' turned up no
instances *except* in the test file. This has so far impeded my
attempt to diagnose the test failure, as I can't be certain as to how
that function works.

The file which is failing the test is a Parrot source file (which
happens to be a Parrot test file) -- not a Perl source file (which
happens to be a Perl test file). So the test at line 75 is correctly
formatted.

particle: You recently added this test. Was it passing for you?

kid51

[Jerry Gay]

On 2/17/07, via RT James Keenan <parrotbug...@parrotcode.org> wrote:
> The file which is failing the test is a Parrot source file (which
> happens to be a Parrot test file) -- not a Perl source file (which
> happens to be a Perl test file). So the test at line 75 is correctly
> formatted.
>
> particle: You recently added this test. Was it passing for you?
>

as noted in my svn log at time of checkin, this is a failing test
which exposes a bug in Parrot::Distribution. '*.t' files are only perl
files if the shebang says they are, but Parrot::Distribution is
classifying all '*.t' files as perl files, which is incorrect. the fix
belongs in lib/Parrot/Distribution.pm.

~jerry

[James E Keenan]

Two points in follow-up:

1. This bug has been difficult to diagnose because Parrot::Distribution
-- which is what it is ultimately testing -- names some variables in
ways that, IMHO, are misleading. Take, for example, this subroutine:

sub get_perl_language_files {
my $self = shift;

my @files = (
$self->perl_source_files,
);

my @perl_language_files = ();
foreach my $file ( @files ) {
next if $self->is_perl_exemption($file);
print STDERR "$file\n";
push @perl_language_files, $file;
}

return @perl_language_files;
}

I would expect that @perl_language_files would contain a list of files.
Instead, it contains a list of objects, with the actual filenames
buried within the objects' data structures.

$VAR1 = [
bless( {
'NAME' => 'Parrot.pm',
'PARENT_PATH' =>
'/Users/jimk/work/parrot/lib/Parrot/Docs/Section',
'PATH' =>
'/Users/jimk/work/parrot/lib/Parrot/Docs/Section/Parrot.pm'
}, 'Parrot::Docs::File' ),
bless( {
'NAME' => 'memalign.pm',
'PARENT_PATH' => '/Users/jimk/work/parrot/config/auto',
'PATH' =>
'/Users/jimk/work/parrot/config/auto/memalign.pm'
}, 'Parrot::Docs::File' ),
...
];

2. Parrot::Distribution contains a subroutine called is_perl() which
presumably tests whether a file in @perl_language_files actually *is* a
Perl file.

sub is_perl {
my $self = shift;
my $filename = shift;

if ( !-f $filename ) {
return 0;
}

# modules and perl scripts should always be tested..
if ( $filename =~ /\.(?:pm|pl)$/ ) {
return 1;
}

# test files (.t) and configure (.in) files might need testing.
# ignore everything else.
if ( $filename !~ /\.(?:t|in)$/ ) {
return 0;
}

# Now let's check to see if there's a perl shebang.

open my $file_handle, '<', $filename
or $self->_croak( "Could not open $filename for reading" );
my $line = <$file_handle>;
close $file_handle;

if ( $line && $line =~ /^#!.*perl/ ) {
return 1;
}

return 0;
}

However, I cannot locate any location where is_perl() is called.

[parrot] 542 $ grep -n is_perl lib/Parrot/Distribution.pm
388: next if $self->is_perl_exemption($file);
390:# next unless $self->is_perl($file);
398:=item C<is_perl_exemption()>
409: sub is_perl_exemption {
425:=item C<is_perl()>
433:sub is_perl {

kid51

[Paul Cochrane]

> 2. Parrot::Distribution contains a subroutine called is_perl() which
> presumably tests whether a file in @perl_language_files actually *is* a
> Perl file.
>

> However, I cannot locate any location where is_perl() is called.

It used to be called deep in the bowels of Parrot::Distribution before
particle++ did his fantastic job of refactoring P::D, however doesn't
seem to be used anymore. This, I'm guessing, is related to ticket
#41387 which, unfortunately, I don't have the tuits to get to atm.
Basically all files which P::D picks up as being "perl" by using the
file suffix also need to be checked to see if they have a perl shebang
line, and this is what is_perl() was used for.

I agree that the variable names can be confusing. IIRC the variable
is some kind of special parrot file object, such that one can call the
path() method on it and one obtains the full path to the file; in some
sense this was the reasoning for the name.

I don't think that helps solve the problem, but hopefully has cleared
up some confusion.

Thanks for your hard work!

Paul

[Jerry Gay]

On 2/17/07, Paul Cochrane <paultc...@gmail.com> wrote:
> > 2. Parrot::Distribution contains a subroutine called is_perl() which
> > presumably tests whether a file in @perl_language_files actually *is* a
> > Perl file.
> >
> > However, I cannot locate any location where is_perl() is called.
>
> It used to be called deep in the bowels of Parrot::Distribution before
> particle++ did his fantastic job of refactoring P::D, however doesn't
> seem to be used anymore. This, I'm guessing, is related to ticket
> #41387 which, unfortunately, I don't have the tuits to get to atm.
> Basically all files which P::D picks up as being "perl" by using the
> file suffix also need to be checked to see if they have a perl shebang
> line, and this is what is_perl() was used for.
>

well, i *half* refactored Parrot::Distribution. there's plenty more to
do--and i haven't gotten around to it yet. after my first pass at
refactoring, i ran across the failing test case, so i added the test
to the suite. i had expected to have a quick follow-up patch to fix
it, but i haven't had the time. i hope to have it fixed before this
week's release.

~jerry

[Jerry Gay]

On 2/17/07, James E Keenan <jk...@verizon.net> wrote:

> jerry gay wrote:
> > On 2/17/07, via RT James Keenan <parrotbug...@parrotcode.org> wrote:
> >
> >> The file which is failing the test is a Parrot source file (which
> >> happens to be a Parrot test file) -- not a Perl source file (which
> >> happens to be a Perl test file). So the test at line 75 is correctly
> >> formatted.
> >>
> >> particle: You recently added this test. Was it passing for you?
> >>
> > as noted in my svn log at time of checkin, this is a failing test
> > which exposes a bug in Parrot::Distribution.
>

> If this were a CPAN module, I would recommend adding that test inside a
> TODO block. It's a test I currently expect to fail but want to know
> when it's fixed and "unexpectedly passes."
>
since parrot's constantly under development, we generally check in
failing tests to trunk as a reminder, since todo tests largely go
unnoticed. these tests are converted to todo tests before a release,
so they won't fail for those users.

for a long time, the canonical reference for TODO items has been the
source tree, whether code comments or tests. we're slowly migrating
towards using RT as canon, but we're not there yet.

> Are TODO blocks options for Parrot tests?
>
yes, and there are plenty in the suite already.
~jerry

[James E Keenan]

jerry gay wrote:
> On 2/17/07, via RT James Keenan <parrotbug...@parrotcode.org> wrote:
>
>> The file which is failing the test is a Parrot source file (which
>> happens to be a Parrot test file) -- not a Perl source file (which
>> happens to be a Perl test file). So the test at line 75 is correctly
>> formatted.
>>
>> particle: You recently added this test. Was it passing for you?
>>
> as noted in my svn log at time of checkin, this is a failing test
> which exposes a bug in Parrot::Distribution.

If this were a CPAN module, I would recommend adding that test inside a

TODO block. It's a test I currently expect to fail but want to know
when it's fixed and "unexpectedly passes."

Are TODO blocks options for Parrot tests?

kid51

[Chromatic]

On Saturday 17 February 2007 08:27, jerry gay wrote:

> as noted in my svn log at time of checkin, this is a failing test
> which exposes a bug in Parrot::Distribution. '*.t' files are only perl
> files if the shebang says they are, but Parrot::Distribution is
> classifying all '*.t' files as perl files, which is incorrect. the fix
> belongs in lib/Parrot/Distribution.pm.

Fixed in r17069.

-- c

[Paul Cochrane via RT]

And the issue of P::D picking up non-perl files by not using the
is_perl() method was corrected in r17434. Resolving the ticket.