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 https://github.com/Perl/perl5/pull/20637#discussion_r1137878155 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. :-) :-(

2 files changed, 36 insertions, 6 deletions

diff --git a/t/TEST b/t/TEST
index 72c865d4ce..1dee698272 100755
--- a/t/TEST
+++ b/t/TEST
@@ -125,6 +125,9 @@ $| = 1;
 # (Windows only uses this script for miniperl.)
 @ARGV = eval 'map glob, @ARGV' if $^O eq 'MSWin32';
 
+my $is_win32 = $^O eq "MSWin32";
+my $is_os2 = $^O eq "os2";
+
 our $show_elapsed_time = $ENV{HARNESS_TIMER} || 0;
 
 # Cheesy version of Getopt::Std.  We can't replace it with that, because we
@@ -391,7 +394,7 @@ sub _populate_hash {
 }
 
 sub _tests_from_manifest {
-    my ($extensions, $known_extensions) = @_;
+    my ($extensions, $known_extensions, $all) = @_;
     my %skip;
     my %extensions = _populate_hash($extensions);
     my %known_extensions = _populate_hash($known_extensions);
@@ -402,10 +405,13 @@ sub _tests_from_manifest {
     }
 
     my @results;
+    my %non_ext;
+    push @results, \%non_ext if $all;
     my $mani = '../MANIFEST';
     if (open(MANI, $mani)) {
 	while (<MANI>) {
-	    if (m!^((?:cpan|dist|ext)/(\S+)/+(?:[^/\s]+\.t|test\.pl)|lib/\S+?(?:\.t|test\.pl))\s!) {
+	    my ($file)= split /\t/, $_;
+	    if ($file=~m!^((?:cpan|dist|ext)/(\S+)/+(?:[^/\s]+\.t|test\.pl)|lib/\S+?(?:\.t|test\.pl))\z!) {
 		my $t = $1;
 		my $extension = $2;
 
@@ -456,6 +462,17 @@ sub _tests_from_manifest {
 		    $::path_to_name{$path} = $t;
 		}
 	    }
+	    elsif ($file=~m!/(?:test\.pl|[^/\s]+\.t)\z! and $file ne "t/test.pl") {
+		my $munged = $file;
+		next if $munged=~m!^(?:t/)?os2/!	and !$is_os2;
+		next if $munged=~m!^(?:t/)?win32/!	and !$is_win32;
+		next if $munged=~m!^(?:t/)?japh/!	and !$::torture;
+		next if $munged=~m!^(?:t/)?benchmark/!	and !($::benchmark or $ENV{PERL_BENCHMARK});
+		next if $munged=~m!^(?:t/)?bigmem/!	and !$ENV{PERL_TEST_MEMORY};
+		$munged =~ s!t/!! or $munged = "../$munged";
+
+		$non_ext{$munged}++;
+	    }
 	}
 	close MANI;
     } else {
diff --git a/t/harness b/t/harness
index d61c2708cf..f2c8b39cd7 100644
--- a/t/harness
+++ b/t/harness
@@ -337,7 +337,7 @@ if (@ARGV) {
         my $which = $ENV{PERL_TEST_HARNESS_ASAP} ? \@last : \@next;
 
         push @$which, qw(comp run cmd);
-        push @$which, qw(io re opbasic op uni mro lib porting perf);
+        push @$which, qw(io re opbasic op uni mro lib class porting perf test_pl);
         push @$which, 'japh' if $torture;
         push @$which, 'win32' if $^O eq 'MSWin32';
         push @$which, 'benchmark' if $ENV{PERL_BENCHMARK};
@@ -352,13 +352,26 @@ if (@ARGV) {
 
         my $last = { par => '{' . join (',', @last) . '}/*.t' };
         @last = _extract_tests ($last);
-        push @last,
-          _tests_from_manifest($Config{extensions}, $Config{known_extensions});
 
-        push @tests, @last;
+        my ($non_ext, @ext_from_manifest)=
+            _tests_from_manifest($Config{extensions}, $Config{known_extensions}, "all");
+        push @last, @ext_from_manifest;
+
         push @seq, _compute_tests_and_ordering(\@last)->@*;
+        push @tests, @last;
 
         $rules = { seq => \@seq };
+
+        foreach my $test (@tests) {
+            delete $non_ext->{$test};
+        }
+
+        my @in_manifest_but_not_found = sort keys %$non_ext;
+        if (@in_manifest_but_not_found) {
+            die "There are test files which are in MANIFEST but are not found by the t/harness\n",
+                 "directory scanning rules. You should update t/harness line 339 or so.\n",
+                 "Files:\n", map { "    $_\n" } @in_manifest_but_not_found;
+        }
     }
 }
 if ($^O eq 'MSWin32') {