On Sun, Jan 09, 2022 at 11:57:44AM -0800, Andres Freund wrote: > On 2022-01-09 13:16:50 -0600, Justin Pryzby wrote: > > diff --git a/contrib/test_decoding/Makefile b/contrib/test_decoding/Makefile > > index 9a31e0b8795..14fd847ba7f 100644 > > --- a/contrib/test_decoding/Makefile > > +++ b/contrib/test_decoding/Makefile > > @@ -10,7 +10,7 @@ ISOLATION = mxact delayed_startup ondisk_startup > > concurrent_ddl_dml \ > > oldest_xmin snapshot_transfer subxact_without_top concurrent_stream \ > > twophase_snapshot > > > > -REGRESS_OPTS = --temp-config > > $(top_srcdir)/contrib/test_decoding/logical.conf > > +REGRESS_OPTS = > > --temp-config=$(top_srcdir)/contrib/test_decoding/logical.conf > > ISOLATION_OPTS = --temp-config > > $(top_srcdir)/contrib/test_decoding/logical.conf > > Not sure why these are part of the diff?
Because otherwise vcregress runs pg_regress --temp-config test1 test2 [...] ..which means test1 gets eaten as the argument to --temp-config > > diff --git a/src/tools/ci/pg_ci_base.conf b/src/tools/ci/pg_ci_base.conf > > index d8faa9c26c1..52cdb697a57 100644 > > --- a/src/tools/ci/pg_ci_base.conf > > +++ b/src/tools/ci/pg_ci_base.conf > > @@ -12,3 +12,24 @@ log_connections = true > > log_disconnections = true > > log_line_prefix = '%m [%p][%b] %q[%a][%v:%x] ' > > log_lock_waits = true > > + > > +# test_decoding > > +wal_level = logical > > +max_replication_slots = 4 > > +logical_decoding_work_mem = 64kB > > [ more ] > > This doesn't really seem like a scalable path forward - duplicating > configuration in more places doesn't seem sane. It seems it'd make more sense > to teach vcregress.pl to run NO_INSTALLCHECK targets properly? ISTM that > changing the options passed to pg_regress based on fetchTests() return value > wouldn't be too hard? It needs to run the tests with separate instance. Maybe you're suggesting to use --temp-instance. It needs to avoid running on the buildfarm, right ? -- Justin
>From 0818f79b27de42182e26dd9dad991de8258c8238 Mon Sep 17 00:00:00 2001 From: Justin Pryzby <pryz...@telsasoft.com> Date: Sun, 9 Jan 2022 18:25:02 -0600 Subject: [PATCH 1/3] vcregress/ci: test modules/contrib with NO_INSTALLCHECK=1 --- .cirrus.yml | 4 +- contrib/pg_stat_statements/Makefile | 2 +- contrib/test_decoding/Makefile | 2 +- src/test/modules/snapshot_too_old/Makefile | 2 +- src/test/modules/worker_spi/Makefile | 2 +- src/tools/msvc/vcregress.pl | 46 +++++++++++++++++++--- 6 files changed, 47 insertions(+), 11 deletions(-) diff --git a/.cirrus.yml b/.cirrus.yml index 19b3737fa11..02ea7e67189 100644 --- a/.cirrus.yml +++ b/.cirrus.yml @@ -398,9 +398,9 @@ task: test_isolation_script: - perl src/tools/msvc/vcregress.pl isolationcheck test_modules_script: - - perl src/tools/msvc/vcregress.pl modulescheck + - perl src/tools/msvc/vcregress.pl modulescheck install test_contrib_script: - - perl src/tools/msvc/vcregress.pl contribcheck + - perl src/tools/msvc/vcregress.pl contribcheck install stop_script: - tmp_install\bin\pg_ctl.exe stop -D tmp_check/db -l tmp_check/postmaster.log test_ssl_script: diff --git a/contrib/pg_stat_statements/Makefile b/contrib/pg_stat_statements/Makefile index 7fabd96f38d..d732e1ade73 100644 --- a/contrib/pg_stat_statements/Makefile +++ b/contrib/pg_stat_statements/Makefile @@ -15,7 +15,7 @@ PGFILEDESC = "pg_stat_statements - execution statistics of SQL statements" LDFLAGS_SL += $(filter -lm, $(LIBS)) -REGRESS_OPTS = --temp-config $(top_srcdir)/contrib/pg_stat_statements/pg_stat_statements.conf +REGRESS_OPTS = --temp-config=$(top_srcdir)/contrib/pg_stat_statements/pg_stat_statements.conf REGRESS = pg_stat_statements oldextversions # Disabled because these tests require "shared_preload_libraries=pg_stat_statements", # which typical installcheck users do not have (e.g. buildfarm clients). diff --git a/contrib/test_decoding/Makefile b/contrib/test_decoding/Makefile index 9a31e0b8795..14fd847ba7f 100644 --- a/contrib/test_decoding/Makefile +++ b/contrib/test_decoding/Makefile @@ -10,7 +10,7 @@ ISOLATION = mxact delayed_startup ondisk_startup concurrent_ddl_dml \ oldest_xmin snapshot_transfer subxact_without_top concurrent_stream \ twophase_snapshot -REGRESS_OPTS = --temp-config $(top_srcdir)/contrib/test_decoding/logical.conf +REGRESS_OPTS = --temp-config=$(top_srcdir)/contrib/test_decoding/logical.conf ISOLATION_OPTS = --temp-config $(top_srcdir)/contrib/test_decoding/logical.conf # Disabled because these tests require "wal_level=logical", which diff --git a/src/test/modules/snapshot_too_old/Makefile b/src/test/modules/snapshot_too_old/Makefile index dfb4537f63c..752a0039fdc 100644 --- a/src/test/modules/snapshot_too_old/Makefile +++ b/src/test/modules/snapshot_too_old/Makefile @@ -5,7 +5,7 @@ EXTRA_CLEAN = $(pg_regress_clean_files) ISOLATION = sto_using_cursor sto_using_select sto_using_hash_index -ISOLATION_OPTS = --temp-config $(top_srcdir)/src/test/modules/snapshot_too_old/sto.conf +ISOLATION_OPTS = --temp-config=$(top_srcdir)/src/test/modules/snapshot_too_old/sto.conf # Disabled because these tests require "old_snapshot_threshold" >= 0, which # typical installcheck users do not have (e.g. buildfarm clients). diff --git a/src/test/modules/worker_spi/Makefile b/src/test/modules/worker_spi/Makefile index cbf9b2e37fd..d9f7d9bab6d 100644 --- a/src/test/modules/worker_spi/Makefile +++ b/src/test/modules/worker_spi/Makefile @@ -9,7 +9,7 @@ PGFILEDESC = "worker_spi - background worker example" REGRESS = worker_spi # enable our module in shared_preload_libraries for dynamic bgworkers -REGRESS_OPTS = --temp-config $(top_srcdir)/src/test/modules/worker_spi/dynamic.conf +REGRESS_OPTS = --temp-config=$(top_srcdir)/src/test/modules/worker_spi/dynamic.conf # Disable installcheck to ensure we cover dynamic bgworkers. NO_INSTALLCHECK = 1 diff --git a/src/tools/msvc/vcregress.pl b/src/tools/msvc/vcregress.pl index 8f3e3fa937b..c751a53e5d4 100644 --- a/src/tools/msvc/vcregress.pl +++ b/src/tools/msvc/vcregress.pl @@ -443,6 +443,7 @@ sub plcheck sub subdircheck { my $module = shift; + my $installcheck = shift || 1; if ( !-d "$module/sql" || !-d "$module/expected" @@ -452,7 +453,7 @@ sub subdircheck } chdir $module; - my @tests = fetchTests(); + my @tests = fetchTests($installcheck); # Leave if no tests are listed in the module. if (scalar @tests == 0) @@ -462,6 +463,7 @@ sub subdircheck } my @opts = fetchRegressOpts(); + push @opts, "--temp-instance=tmp_check" if $installcheck == -1; # Special processing for python transform modules, see their respective # Makefiles for more details regarding Python-version specific @@ -489,7 +491,7 @@ sub subdircheck print "Checking $module\n"; my @args = ( "$topdir/$Config/pg_regress/pg_regress", - "--bindir=${topdir}/${Config}/psql", + "--bindir=$tmp_installdir/bin", "--dbname=contrib_regression", @opts, @tests); print join(' ', @args), "\n"; system(@args); @@ -499,6 +501,8 @@ sub subdircheck sub contribcheck { + my $mode = shift || ''; + chdir "../../../contrib"; my $mstat = 0; foreach my $module (glob("*")) @@ -516,12 +520,25 @@ sub contribcheck my $status = $? >> 8; $mstat ||= $status; } + + # As above, but creates new DB instance for each module. For CI. + if ($mode eq "install") + { + foreach my $module (glob("*")) + { + subdircheck("$module", -1); + $mstat ||= $? >> 8; + } + } + exit $mstat if $mstat; return; } sub modulescheck { + my $mode = shift || ''; + chdir "../../../src/test/modules"; my $mstat = 0; foreach my $module (glob("*")) @@ -530,6 +547,17 @@ sub modulescheck my $status = $? >> 8; $mstat ||= $status; } + + # As above, but creates new DB instance for each module. For CI. + if ($mode eq "install") + { + foreach my $module (glob("*")) + { + subdircheck("$module", -1); + $mstat ||= $? >> 8; + } + } + exit $mstat if $mstat; return; } @@ -700,6 +728,7 @@ sub fetchRegressOpts # option starting with "--". @opts = grep { !/\$\(/ && /^--/ } map { (my $x = $_) =~ s/\Q$(top_builddir)\E/\"$topdir\"/; $x; } + map { (my $x = $_) =~ s/\Q$(top_srcdir)\E/\"$topdir\"/; $x; } split(/\s+/, $1); } if ($m =~ /^\s*ENCODING\s*=\s*(\S+)/m) @@ -726,14 +755,19 @@ sub fetchTests my $m = <$handle>; close($handle); my $t = ""; + my $installcheck = shift || 1; $m =~ s{\\\r?\n}{}g; - # A module specifying NO_INSTALLCHECK does not support installcheck, - # so bypass its run by returning an empty set of tests. if ($m =~ /^\s*NO_INSTALLCHECK\s*=\s*\S+/m) { - return (); + # Skip modules marked installcheck unless running installcheck tests. + return () if $installcheck == 1; + } + else + { + # Skip modules not marked installcheck if running installcheck tests. + return () if $installcheck == -1; } if ($m =~ /^REGRESS\s*=\s*(.*)$/gm) @@ -799,6 +833,8 @@ sub usage "\nOptions for <arg>: (used by check and installcheck)\n", " serial serial mode\n", " parallel parallel mode\n", + "\nOptions for <arg>: (used by contribcheck and modulescheck)\n", + " install also run tests which require a new instance\n", "\nOption for <arg>: for taptest\n", " TEST_DIR (required) directory where tests reside\n"; exit(1); -- 2.17.1
>From 2a3bf17c13eefd6516f744e2e314669e988f9422 Mon Sep 17 00:00:00 2001 From: Justin Pryzby <pryz...@telsasoft.com> Date: Sun, 9 Jan 2022 22:54:32 -0600 Subject: [PATCH 2/3] CI: run initdb with --no-sync for windows --- .cirrus.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.cirrus.yml b/.cirrus.yml index 02ea7e67189..a0d733bb594 100644 --- a/.cirrus.yml +++ b/.cirrus.yml @@ -390,7 +390,7 @@ task: - perl src/tools/msvc/vcregress.pl check parallel startcreate_script: # paths to binaries need backslashes - - tmp_install\bin\pg_ctl.exe initdb -D tmp_check/db -l tmp_check/initdb.log + - tmp_install\bin\pg_ctl.exe initdb -D tmp_check/db -l tmp_check/initdb.log --options=--no-sync - echo include '%TEMP_CONFIG%' >> tmp_check/db/postgresql.conf - tmp_install\bin\pg_ctl.exe start -D tmp_check/db -l tmp_check/postmaster.log test_pl_script: -- 2.17.1
>From 91e134926ef60f63f191b2664f09fef8b4a9ffeb Mon Sep 17 00:00:00 2001 From: Justin Pryzby <pryz...@telsasoft.com> Date: Sun, 9 Jan 2022 23:05:18 -0600 Subject: [PATCH 3/3] vcregress: style --- src/tools/msvc/vcregress.pl | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/tools/msvc/vcregress.pl b/src/tools/msvc/vcregress.pl index c751a53e5d4..62a2667cb4f 100644 --- a/src/tools/msvc/vcregress.pl +++ b/src/tools/msvc/vcregress.pl @@ -566,7 +566,6 @@ sub recoverycheck { InstallTemp(); - my $mstat = 0; my $dir = "$topdir/src/test/recovery"; my $status = tap_check($dir); exit $status if $status; @@ -746,7 +745,6 @@ sub fetchRegressOpts # list is returned if the module does not need to run anything. sub fetchTests { - my $handle; open($handle, '<', "GNUmakefile") || open($handle, '<', "Makefile") -- 2.17.1