Skip to content

Commit 8f426c4

Browse files
demerphqpjacklam
authored andcommitted
t/harness - die if t/harness wont run a test listed in MANIFEST
Historically we used to parse out the tests that we ran in t/harness from the MANIFEST file. At some point this changed and we started consulting the disk using globs. However because we do not use a recursive search over the t/ directory it is quite possible that a new directory of tests is added which actually never runs. In Perl#20637 (comment) Tony C noticed that I had added a new test file t/op/hook/require.t which is in a new subdirectory t/op/hook/ which was unknown to t/harness and thus not actually being run by make test_harness. (This patch does NOT add t/op/hoop to the list of directories to scan, I will do that in the PR.) I then took the time to add code to detect if any other test files are not being run, and it turns out that it is also the case for the new t/class/ directory of tests and it is also the case for the tests for test.pl itself, found in the t/test_pl directory. This patch adds logic to detect if this happens and make t/harness die if it finds a test file in the manifest which will not be detected by the custom rules for finding test files that is used in t/harness. It does not die if t/harness finds tests that are not in MANIFEST, that should be detected by a different test. The level of complexity in finding and deciding the tests files that we should run, and the differences between t/TEST and t/harness is fairly high. In the past Nicholas put some effort into unifying the logic, but it seems since then we have drifted apart. Even though t/harness uses t/TEST and the _tests_from_manifest() function, for some time now it has only used it to find which extensions to test, not which test files to run. I have *NOT* dug into whether t/TEST is also missing test files that are in manifest. That can happen in a follow up patch. Long term we should unify all of this logic so that t/TEST and t/harness run the same test files always, and that we will always detect discrepancies between the MANIFEST and the tests we are running. We do not for instance test that they test the same things. :-) :-(
1 parent ec0607f commit 8f426c4

File tree

2 files changed

+36
-6
lines changed

2 files changed

+36
-6
lines changed

t/TEST

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -125,6 +125,9 @@ $| = 1;
125125
# (Windows only uses this script for miniperl.)
126126
@ARGV = eval 'map glob, @ARGV' if $^O eq 'MSWin32';
127127

128+
my $is_win32 = $^O eq "MSWin32";
129+
my $is_os2 = $^O eq "os2";
130+
128131
our $show_elapsed_time = $ENV{HARNESS_TIMER} || 0;
129132

130133
# Cheesy version of Getopt::Std. We can't replace it with that, because we
@@ -391,7 +394,7 @@ sub _populate_hash {
391394
}
392395
393396
sub _tests_from_manifest {
394-
my ($extensions, $known_extensions) = @_;
397+
my ($extensions, $known_extensions, $all) = @_;
395398
my %skip;
396399
my %extensions = _populate_hash($extensions);
397400
my %known_extensions = _populate_hash($known_extensions);
@@ -402,10 +405,13 @@ sub _tests_from_manifest {
402405
}
403406
404407
my @results;
408+
my %non_ext;
409+
push @results, \%non_ext if $all;
405410
my $mani = '../MANIFEST';
406411
if (open(MANI, $mani)) {
407412
while (<MANI>) {
408-
if (m!^((?:cpan|dist|ext)/(\S+)/+(?:[^/\s]+\.t|test\.pl)|lib/\S+?(?:\.t|test\.pl))\s!) {
413+
my ($file)= split /\t/, $_;
414+
if ($file=~m!^((?:cpan|dist|ext)/(\S+)/+(?:[^/\s]+\.t|test\.pl)|lib/\S+?(?:\.t|test\.pl))\z!) {
409415
my $t = $1;
410416
my $extension = $2;
411417
@@ -456,6 +462,17 @@ sub _tests_from_manifest {
456462
$::path_to_name{$path} = $t;
457463
}
458464
}
465+
elsif ($file=~m!/(?:test\.pl|[^/\s]+\.t)\z! and $file ne "t/test.pl") {
466+
my $munged = $file;
467+
next if $munged=~m!^(?:t/)?os2/! and !$is_os2;
468+
next if $munged=~m!^(?:t/)?win32/! and !$is_win32;
469+
next if $munged=~m!^(?:t/)?japh/! and !$::torture;
470+
next if $munged=~m!^(?:t/)?benchmark/! and !($::benchmark or $ENV{PERL_BENCHMARK});
471+
next if $munged=~m!^(?:t/)?bigmem/! and !$ENV{PERL_TEST_MEMORY};
472+
$munged =~ s!t/!! or $munged = "../$munged";
473+
474+
$non_ext{$munged}++;
475+
}
459476
}
460477
close MANI;
461478
} else {

t/harness

Lines changed: 17 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -337,7 +337,7 @@ if (@ARGV) {
337337
my $which = $ENV{PERL_TEST_HARNESS_ASAP} ? \@last : \@next;
338338

339339
push @$which, qw(comp run cmd);
340-
push @$which, qw(io re opbasic op uni mro lib porting perf);
340+
push @$which, qw(io re opbasic op uni mro lib class porting perf test_pl);
341341
push @$which, 'japh' if $torture;
342342
push @$which, 'win32' if $^O eq 'MSWin32';
343343
push @$which, 'benchmark' if $ENV{PERL_BENCHMARK};
@@ -352,13 +352,26 @@ if (@ARGV) {
352352

353353
my $last = { par => '{' . join (',', @last) . '}/*.t' };
354354
@last = _extract_tests ($last);
355-
push @last,
356-
_tests_from_manifest($Config{extensions}, $Config{known_extensions});
357355

358-
push @tests, @last;
356+
my ($non_ext, @ext_from_manifest)=
357+
_tests_from_manifest($Config{extensions}, $Config{known_extensions}, "all");
358+
push @last, @ext_from_manifest;
359+
359360
push @seq, _compute_tests_and_ordering(\@last)->@*;
361+
push @tests, @last;
360362

361363
$rules = { seq => \@seq };
364+
365+
foreach my $test (@tests) {
366+
delete $non_ext->{$test};
367+
}
368+
369+
my @in_manifest_but_not_found = sort keys %$non_ext;
370+
if (@in_manifest_but_not_found) {
371+
die "There are test files which are in MANIFEST but are not found by the t/harness\n",
372+
"directory scanning rules. You should update t/harness line 339 or so.\n",
373+
"Files:\n", map { " $_\n" } @in_manifest_but_not_found;
374+
}
362375
}
363376
}
364377
if ($^O eq 'MSWin32') {

0 commit comments

Comments
 (0)