MSVC buildfarm critters are not running modules' TAP tests - Mailing list pgsql-hackers

I noticed $subject while checking to see if commit db4383189's
new test script was behaving properly in the buildfarm.  dory,
for one, should be running it but it just isn't.

It looks to me like the reason is that src/tools/msvc/vcregress.pl's
subroutine subdircheck isn't considering the possibility that
subdirectories of src/test/modules contain TAP tests.  The
same code is used for contrib, so several existing TAP tests
are being missed there too.

I took a stab at fixing this, but lacking a Windows environment
to test in, I can't be sure if it works.  The attached does kinda
sorta work if I run it in a Linux environment --- but I found that
system() doesn't automatically expand "t/*.pl" on Linux.  Is that
an expected difference between Linux and Windows perl?  I hacked
around that by adding a glob() call in sub tap_check, as seen in
the first hunk below, but I'm not very sure if that hunk should
get committed or not.

For ease of review, I did not re-indent the main part of sub
subdircheck, though that needs to be done before committing.

Anybody with suitable tools care to test/commit this?

            regards, tom lane

diff --git a/src/tools/msvc/vcregress.pl b/src/tools/msvc/vcregress.pl
index 33d8fb5..c2f2695 100644
--- a/src/tools/msvc/vcregress.pl
+++ b/src/tools/msvc/vcregress.pl
@@ -207,7 +207,7 @@ sub tap_check
     my $dir = shift;
     chdir $dir;
 
-    my @args = ("prove", @flags, "t/*.pl");
+    my @args = ("prove", @flags, glob("t/*.pl"));
 
     # adjust the environment for just this test
     local %ENV = %ENV;
@@ -391,27 +391,24 @@ sub plcheck
     return;
 }
 
+# Run tests in a specified subdirectory of current directory.
+# Returns 0 if OK, else exit code
 sub subdircheck
 {
     my $module = shift;
 
-    if (   !-d "$module/sql"
-        || !-d "$module/expected"
-        || (!-f "$module/GNUmakefile" && !-f "$module/Makefile"))
-    {
-        return;
-    }
+    chdir($module) || return 0;
 
-    chdir $module;
-    my @tests = fetchTests();
+    my $mstat = 0;
 
-    # Leave if no tests are listed in the module.
-    if (scalar @tests == 0)
+    # Look for traditional-style regression tests.
+    if (-d "sql" && -d "expected"
+        && (-f "GNUmakefile" || -f "Makefile"))
     {
-        chdir "..";
-        return;
-    }
+    my @tests = fetchTests();
 
+    if (scalar @tests > 0)
+    {
     my @opts = fetchRegressOpts();
 
     # Special processing for python transform modules, see their respective
@@ -437,15 +434,29 @@ sub subdircheck
     }
 
     print "============================================================\n";
-    print "Checking $module\n";
+    print "Running $module regression tests\n";
     my @args = (
         "$topdir/$Config/pg_regress/pg_regress",
         "--bindir=${topdir}/${Config}/psql",
         "--dbname=contrib_regression", @opts, @tests);
     print join(' ', @args), "\n";
     system(@args);
+    my $status = $? >> 8;
+    $mstat ||= $status;
+    }
+    }
+
+    # Look for TAP tests.
+    if ($config->{tap_tests} && -d "t")
+    {
+        print "============================================================\n";
+        print "Running $module TAP tests\n";
+        my $status = tap_check(getcwd());
+        $mstat ||= $status;
+    }
+
     chdir "..";
-    return;
+    return $mstat;
 }
 
 sub contribcheck
@@ -462,8 +473,7 @@ sub contribcheck
         next if ($module =~ /_plpython$/ && !defined($config->{python}));
         next if ($module eq "sepgsql");
 
-        subdircheck($module);
-        my $status = $? >> 8;
+        my $status = subdircheck($module);
         $mstat ||= $status;
     }
     exit $mstat if $mstat;
@@ -476,8 +486,7 @@ sub modulescheck
     my $mstat = 0;
     foreach my $module (glob("*"))
     {
-        subdircheck($module);
-        my $status = $? >> 8;
+        my $status = subdircheck($module);
         $mstat ||= $status;
     }
     exit $mstat if $mstat;