diff --git a/Changes b/Changes index d6ca2cb..3203cb5 100644 --- a/Changes +++ b/Changes @@ -2,6 +2,14 @@ Revision history for Test-DiagINC {{$NEXT}} + - More robust error condition detection - now triggers on failures + in END-block-based test modules (e.g. Test::NoWarnings) + - Add diagnostic of "Attempted-but-failed-to-load" on perl >= 5.10 + - Minimize amount of reported modules: anything Test::DiagINC loads + in order to compile the report is excluded + - No longr chdir() during the local-library heuristic + - No non-core runtime dependencies, add 5.6 support + 0.002 2014-02-04 22:17:58-05:00 America/New_York [CHANGED] diff --git a/cpanfile b/cpanfile index 598cea7..7c73ddc 100644 --- a/cpanfile +++ b/cpanfile @@ -1,14 +1,13 @@ -requires "Path::Tiny" => "0"; -requires "perl" => "5.008001"; +requires "perl" => "5.006"; requires "strict" => "0"; requires "warnings" => "0"; on 'test' => sub { - requires "Capture::Tiny" => "0.23"; + requires "Capture::Tiny" => "0.21"; requires "ExtUtils::MakeMaker" => "0"; requires "File::Spec::Functions" => "0"; requires "List::Util" => "0"; - requires "Test::More" => "0.96"; + requires "Test::More" => "0"; requires "version" => "0"; }; diff --git a/examples/dies.t b/examples/dies.t index 795a02b..b81c9a0 100644 --- a/examples/dies.t +++ b/examples/dies.t @@ -3,6 +3,7 @@ use warnings; use if $ENV{AUTOMATED_TESTING}, 'Test::DiagINC'; use ExtUtils::MakeMaker; use examples::lib::Foo; +eval { require examples::lib::SyntaxErr }; chdir ".." or die "$!"; # try to mess up relative entry in %INC diff --git a/examples/fails.t b/examples/fails.t index d98a26e..7ceac1b 100644 --- a/examples/fails.t +++ b/examples/fails.t @@ -1,8 +1,10 @@ use strict; use warnings; -use if $ENV{AUTOMATED_TESTING}, 'Test::DiagINC'; use Test::More tests => 1; +use if $ENV{AUTOMATED_TESTING}, 'Test::DiagINC'; +use ExtUtils::MakeMaker (); use lib 'examples/lib'; use Foo; +eval { require SyntaxErr }; fail("this failed"); diff --git a/examples/fails_in_end.t b/examples/fails_in_end.t new file mode 100644 index 0000000..33f47ae --- /dev/null +++ b/examples/fails_in_end.t @@ -0,0 +1,14 @@ +use strict; +use warnings; + +# simulate what a Test::NoWarnings invocation looks like +use Test::More tests => 1; +END { fail("this failed") } + +use if $ENV{AUTOMATED_TESTING}, 'Test::DiagINC'; +use ExtUtils::MakeMaker (); +use lib 'examples/lib'; +use Foo; +eval { require SyntaxErr }; + + diff --git a/examples/lib/SyntaxErr.pm b/examples/lib/SyntaxErr.pm new file mode 100644 index 0000000..390f793 --- /dev/null +++ b/examples/lib/SyntaxErr.pm @@ -0,0 +1,3 @@ +asphinctersayswhat + +0; diff --git a/lib/Test/DiagINC.pm b/lib/Test/DiagINC.pm index d51c564..ef31c4d 100644 --- a/lib/Test/DiagINC.pm +++ b/lib/Test/DiagINC.pm @@ -1,12 +1,16 @@ -use 5.008001; -use strict; -use warnings; +use 5.006; package Test::DiagINC; # ABSTRACT: List modules and versions loaded if tests fail # VERSION -use Path::Tiny; +# If the tested module did not load strict/warnings we do not want +# to load them either. On the other hand we would like to know our +# code is at least somewhat ok. Therefore this madness ;) +BEGIN { if($ENV{RELEASE_TESTING}) { + require warnings && warnings->import; + require strict && strict->import; +} } sub _max_length { my $max = 0; @@ -15,41 +19,127 @@ sub _max_length { return $max; } +# Get our CWD *without* loading anything. The original idea by xdg++ was to +# invoke a with an -e snippet, and not worry about quoting, but it seemed too +# fragile. doing this trick instead (we know we can load ourselves): +BEGIN { + if ($^C) { + require Cwd; + require POSIX; + $|++; + print Cwd::getcwd() . "\n"; + POSIX::_exit(0); + } +} +chomp( my $REALPATH_CWD = do { + local $ENV{PATH}; + my ($perl) = $^X =~ /(.+)/; # $^X is internal how could it be tainted?! + `$perl -c @{[ __FILE__ ]}`; +}); my $ORIGINAL_PID = $$; -my $CWD = path(".")->absolute; END { - # Dump %INC if in the main process and have a non-zero exit code - if ( $$ == $ORIGINAL_PID && $? ) { - chdir $CWD; # improve resolution of relative path names - my @packages; - for my $p ( sort keys %INC ) { - next unless defined( $INC{$p} ) && !$CWD->subsumes( $INC{$p} ); - next unless $p =~ s/\.pm\z//; - $p =~ s{[\\/]}{::}g; - push @packages, $p if $p =~ /\A[A-Z_a-z][0-9A-Z_a-z]*(?:::[0-9A-Z_a-z]+)*\Z/; + if ( $$ == $ORIGINAL_PID ) { + # make sure we report only on stuff that was loaded by the test, + # nothing more + # get a snapshot early in order to not misreport B.pm and friends + # below - this *will* skip any extra modules loaded in END, it was + # deemed an acceptable compromise by ribasushi and xdg + my @INC_list = keys %INC; + + # If we meet the "fail" criteria - no need to load B and fire + # an extra check in an extra END (also doesn't work on 5.6) + if ( _assert_no_fail(@INC_list) and $] >= 5.008) { + + # we did not report anything yet - add an extra END to catch + # possible future-fails + require B; + push @{ B::end_av()->object_2svref }, sub { + _assert_no_fail(@INC_list) + }; + } + } +} + +# Dump %INC IFF in the main process and test is failing or exit is non-zero +# return true if no failure or if PID mismatches, return false otherwise +sub _assert_no_fail { + + return 1 if $$ != $ORIGINAL_PID; + + if ( $? or ( + $INC{'Test/Builder.pm'} + and + Test::Builder->can('is_passing') + and + ! Test::Builder->new->is_passing + ) ) { + + require Cwd; + require File::Spec; + require Cwd; + + my %results; + + for my $pkg_as_path ( @_ ) { + next unless (my $p = $pkg_as_path) =~ s/\.pm\z//; + $p =~ s{/}{::}g; + next unless $p =~ /\A[A-Z_a-z][0-9A-Z_a-z]*(?:::[0-9A-Z_a-z]+)*\Z/; + + # a module we recorded as INCed disappeared... + if (not exists $INC{$pkg_as_path}) { + $results{$p} = 'Module unloaded in END...?'; + next; + } + + if (not defined $INC{$pkg_as_path} ) { + $results{$p} = 'Found and failed to load'; + next; + } + + next if ( + # rel2abs on an absolute path is a noop + # https://metacpan.org/source/SMUELLER/PathTools-3.40/lib/File/Spec/Unix.pm#L474 + # https://metacpan.org/source/SMUELLER/PathTools-3.40/lib/File/Spec/Win32.pm#L324 + Cwd::realpath( File::Spec->rel2abs( $INC{$pkg_as_path}, $REALPATH_CWD ) ) + =~ + m| \A \Q$REALPATH_CWD\E [\\\/] |x + ); + + my $ver = do { + local $@; + my $v = eval { $p->VERSION }; + $@ ? '->VERSION call failed' : $v + }; + $ver = 'n/a' unless defined $ver; + $results{$p} = $ver; } - my %versions = map { - my $v = eval { $_->VERSION }; - $_ => defined($v) ? $v : "undef" - } @packages; + my $diag = "Listing modules from %INC\n"; + + my $ml = _max_length( keys %results ); + my $vl = _max_length( values %results ); - my $header = "Listing modules from %INC\n"; - my $format = " %*s %*s\n"; - my $ml = _max_length(@packages); - my $vl = _max_length( values %versions ); + for (sort keys %results) { + $diag .= sprintf( " %*s %*s\n", + # pairs of [ padding-length => content ] + $vl => $results{$_}, + -$ml => $_ + ) + } if ( $INC{"Test/Builder.pm"} ) { - my $tb = Test::Builder->new; - $tb->diag($header); - $tb->diag( sprintf( $format, $vl, $versions{$_}, -$ml, $_ ) ) for @packages; + Test::Builder->new->diag($diag); } else { - print STDERR "# $header"; - printf( STDERR "#$format", $vl, $versions{$_}, -$ml, $_ ) for @packages; + $diag =~ s/^/# /mg; + print STDERR $diag; } + + return 0; } + + return 1; } 1; @@ -58,7 +148,7 @@ END { =head1 SYNOPSIS - # Load *BEFORE* Test::More + # preferably load before anything else use if $ENV{AUTOMATED_TESTING}, 'Test::DiagINC'; use Test::More; @@ -71,9 +161,11 @@ diagnose deep dependency problems by showing you exactly what modules and versions were loaded during a test run. When this module is loaded, it sets up an C block that will take action if -a program exits with a non-zero exit code. If that happens, this module prints -out the names and version numbers of non-local modules appearing in C<%INC> at -the end of the test. +the program is about to exit with a non-zero exit code or if +L<< $test_builder->is_passing|Test::Builder/is_passing >> +is false by the time the C block is reached. If that happens, this module +prints out the names and version numbers of non-local modules appearing in +L<%INC|perlvar/%INC> at the end of the test. For example: @@ -88,10 +180,10 @@ For example: # 5.68 Exporter # 5.68 Exporter::Heavy # 1.07 PerlIO - # 1.001002 Test::Builder - # 1.001002 Test::Builder::Module - # 0.001 Test::DiagINC - # 1.001002 Test::More + # 0.98 Test::Builder + # 0.98 Test::Builder::Module + # 0.003 Test::DiagINC + # 0.98 Test::More # 1.22 overload # 0.02 overloading # 1.07 strict @@ -99,18 +191,31 @@ For example: # 1.18 warnings # 1.02 warnings::register -B: Because this module uses an C block, it must be loaded B -C so that the C's C block has a chance to set -the exit code first. If you're not using C, then it's up to you to -ensure your code generates the non-zero exit code (e.g. C or C). +This module deliberately does not load B during runtime, +instead delaying all loads until it needs to generate a failure report in its +C block. The only exception is loading L and L for +self-check B C is true. Therefore an empty +invocation will look like this: + + $ perl -MTest::DiagINC -e 'exit(1)' + # Listing modules from %INC + # 0.003 Test::DiagINC + +B: Because this module uses an C block, it is a good idea to load +it as early as possible, so the C block it installs will execute as +B as possible (see L for details on how this works). While +this module does employ some cleverness to work around load order, it is +still a heuristic and is no substitute to loading this module early. A notable +side-effect is when a module is loaded in an C block executing B +the one installed by this library: such modules will be "invisible" to us and +will not be reported as part of the diagnostic report. Modules that appear to be sourced from below the current directory when C was loaded will be excluded from the report (e.g. excludes -local modules from C, C, and so on). +local modules from C<./>, C, C, and so on). The heuristic of searching C<%INC> for loaded modules may fail if the module -path loaded does not map to a package within the module file or if that package -does not define a C<$VERSION>. +path loaded does not map to a package within the module file. If C is loaded, the output will go via the C function. Otherwise, it will just be sent to STDERR. diff --git a/t/basic.t b/t/basic.t index a78a539..3eb21dc 100644 --- a/t/basic.t +++ b/t/basic.t @@ -1,20 +1,39 @@ -use 5.008001; use strict; use warnings; -use Test::More 0.96; -use Capture::Tiny 0.23 qw/capture/; +use Test::More; +use Capture::Tiny 0.21 qw/capture/; -for my $file (qw/fails.t dies.t/) { - $ENV{AUTOMATED_TESTING} = 1; +my @testfiles = qw/fails.t dies.t/; +push @testfiles, 'fails_in_end.t' unless $] < 5.008; + +plan tests => @testfiles * ($] < 5.010 ? 3 : 4); + +my $tainted_run = ! eval { $ENV{PATH} . kill(0) and 1 } + and diag ( __FILE__ . ' running under taint mode' ); + +local $ENV{AUTOMATED_TESTING} = 1; + +# untaint PATH but do not unset it so we test that $^X will +# run with it just fine +local ( $ENV{PATH} ) = $ENV{PATH} =~ /(.*)/ if $tainted_run; + +for my $file (@testfiles) { my ( $stdout, $stderr ) = capture { - system( $^X, "examples/$file" ); + system( + ($^X =~ /(.+)/), # $^X is internal how can it be tainted?! + ( $tainted_run ? (qw( -I . -I lib -T )) : () ), + "examples/$file" + ); }; + like( $stderr, qr/\QListing modules from %INC/, "$file: Saw diagnostic header" ); - like( $stderr, qr/[0-9.]+\s+Path::Tiny/, "$file: Saw Path::Tiny in module list" ); + like( $stderr, qr/[0-9.]+\s+ExtUtils::MakeMaker/, "$file: Saw EUMM in module list" ); unlike( $stderr, qr/Foo/, "$file: Did not see local module Foo in module list", ); + + like( $stderr, qr/Found and failed to load\s+[\w\:]*SyntaxErr/, + "$file: Saw failed load attempt of SyntaxErr" ) unless $] < 5.010; } -done_testing; # COPYRIGHT # vim: ts=4 sts=4 sw=4 et: diff --git a/t/leanload.t b/t/leanload.t new file mode 100644 index 0000000..158e755 --- /dev/null +++ b/t/leanload.t @@ -0,0 +1,74 @@ +# +# WE ARE DOING %INC EXAMS IN THIS TEST +# No Test::More loaded, all TAP output by hand +# + +BEGIN { + if (keys %INC) { + print "1..0 # SKIP Your %INC is already populated, perhaps PERL5OPTS is set?\n"; + exit 0; + } +} + +# madness explanation at the top of Test::DiagInc +BEGIN { + if ($ENV{RELEASE_TESTING}) { + require warnings && warnings->import; + require strict && strict->import; + } + + @::initial_INC = keys %INC; + + unless ($] < 5.008) { + @::B_inc = split /\0/, `$^X -Mt::lib::B_laced_INC_dump`; + } +} + +my $nongreat_success; +END { + cmp_inc_contents( @::initial_INC, 'Test/DiagINC.pm', @::B_inc ); + print "1..4\n"; + $? ||= ( $nongreat_success || 0 ); +} + +sub cmp_inc_contents { + my %current_inc = %INC; + + my ($seen, @leftover_keys); + for (@_) { + next if $seen->{$_}++; + if (exists $current_inc{$_}) { + delete $current_inc{$_} + } + else { + push @leftover_keys, $_ + } + } + + my $fail = 0; + if (my @mods = sort keys %current_inc) { + $_ =~ s|/|::|g for @mods; + $_ =~ s|\.pm$|| for @mods; + print "not ok - the following modules were unexpectedly found in %INC: @mods\n"; + $fail++; + } + else { + print "ok - %INC does not contain anything extra\n"; + } + + if (@leftover_keys) { + $_ =~ s|/|::|g for @leftover_keys; + $_ =~ s|\.pm$|| for @leftover_keys; + print "not ok - the following modules were expected but not found in %INC: @leftover_keys\n"; + $fail++; + } + else { + print "ok - %INC contents as expected\n"; + } + + $nongreat_success += $fail; +} + +use Test::DiagINC; + +BEGIN { cmp_inc_contents( @::initial_INC, 'Test/DiagINC.pm' ) } diff --git a/t/lib/B_laced_INC_dump.pm b/t/lib/B_laced_INC_dump.pm new file mode 100644 index 0000000..41974a8 --- /dev/null +++ b/t/lib/B_laced_INC_dump.pm @@ -0,0 +1,13 @@ +my %inc_copy = %INC; +delete $inc_copy{+__FILE__}; + +if (keys %inc_copy) { + print STDERR "%INC can not be populated when loading @{[ __FILE__ ]}\n"; + exit 255; +} + +require B; + +print join "\0", sort grep { $_ ne __FILE__ } keys %INC; + +exit 0; diff --git a/t/taint.t b/t/taint.t new file mode 100644 index 0000000..4882dfd --- /dev/null +++ b/t/taint.t @@ -0,0 +1,17 @@ +use warnings; +use strict; +use File::Spec; + +# there is talk of possible perl compilations where -T is a fatal +# we don't want to have the user deal with that +system( $^X => -T => -e => 'use warnings; use strict; exit 0' ); +if ($?) { + print "1..0 # SKIP Your perl does not seem to like -T...\n"; + exit 0; +} + +# all is well - just rerun the basic test +exec( $^X => -T => File::Spec->catpath( + (File::Spec->splitpath( __FILE__ ))[0,1], + 'basic.t' +) );