Add extension options to control TAP and isolation tests

Started by Michael Paquierover 7 years ago18 messages
#1Michael Paquier
michael@paquier.xyz
1 attachment(s)

Hi all,

On a recent thread of pgsql-committers has been discussed the fact that
we lacked a bit of infrastructure to allow extensions to control
isolation and TAP tests:
/messages/by-id/20180905174527.GA2726@paquier.xyz

Attached is a patch which is the result of the previous thread, where a
couple of variables are added for extension authors:
- ISOLATION, similar to REGRESS for pg_regress, which lists isolation
tests.
- ISOLATION_OPTS, which can be used to pass an option set to
pg_isolation_regress.
- TAP_TESTS, a switch to enable running TAP tests.

This results in a bit of cleanup in the Makefile of modules we ship with
upstream:
- modules/brin
- modules/commit_ts
- modules/test_pg_dump
- contrib/bloom, where the TAP tests were actually not running.
- contrib/oid2name
- contrib/test_decoding, which keeps the same feature set, and is
heavily cleaned up (it missed for example the use of the existing
NO_INSTALLCHECK).
- contrib/vacuumlo

Thoughts?
--
Michael

Attachments:

pgxs-isolation-tap.patchtext/x-diff; charset=us-asciiDownload
diff --git a/contrib/bloom/Makefile b/contrib/bloom/Makefile
index 13bd397b70..da9553a2d0 100644
--- a/contrib/bloom/Makefile
+++ b/contrib/bloom/Makefile
@@ -8,6 +8,7 @@ DATA = bloom--1.0.sql
 PGFILEDESC = "bloom access method - signature file based index"
 
 REGRESS = bloom
+TAP_TESTS = 1
 
 ifdef USE_PGXS
 PG_CONFIG = pg_config
@@ -19,6 +20,3 @@ top_builddir = ../..
 include $(top_builddir)/src/Makefile.global
 include $(top_srcdir)/contrib/contrib-global.mk
 endif
-
-wal-check: temp-install
-	$(prove_check)
diff --git a/contrib/oid2name/Makefile b/contrib/oid2name/Makefile
index 908e078714..361a80a7a1 100644
--- a/contrib/oid2name/Makefile
+++ b/contrib/oid2name/Makefile
@@ -6,11 +6,11 @@ PGAPPICON = win32
 PROGRAM = oid2name
 OBJS	= oid2name.o $(WIN32RES)
 
+TAP_TESTS = 1
+
 PG_CPPFLAGS = -I$(libpq_srcdir)
 PG_LIBS_INTERNAL = $(libpq_pgport)
 
-EXTRA_CLEAN = tmp_check
-
 ifdef USE_PGXS
 PG_CONFIG = pg_config
 PGXS := $(shell $(PG_CONFIG) --pgxs)
@@ -21,9 +21,3 @@ top_builddir = ../..
 include $(top_builddir)/src/Makefile.global
 include $(top_srcdir)/contrib/contrib-global.mk
 endif
-
-check:
-	$(prove_check)
-
-installcheck:
-	$(prove_installcheck)
diff --git a/contrib/test_decoding/Makefile b/contrib/test_decoding/Makefile
index afcab930f7..8cd83a763f 100644
--- a/contrib/test_decoding/Makefile
+++ b/contrib/test_decoding/Makefile
@@ -3,9 +3,20 @@
 MODULES = test_decoding
 PGFILEDESC = "test_decoding - example of a logical decoding output plugin"
 
-# Note: because we don't tell the Makefile there are any regression tests,
-# we have to clean those result files explicitly
-EXTRA_CLEAN = $(pg_regress_clean_files)
+EXTRA_INSTALL=contrib/test_decoding
+
+REGRESS = ddl xact rewrite toast permissions decoding_in_xact \
+	decoding_into_rel binary prepared replorigin time messages \
+	spill slot truncate
+ISOLATION = mxact delayed_startup ondisk_startup concurrent_ddl_dml \
+	oldest_xmin snapshot_transfer
+
+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
+# typical installcheck users do not have (e.g. buildfarm clients).
+NO_INSTALLCHECK = 1
 
 ifdef USE_PGXS
 PG_CONFIG = pg_config
@@ -18,52 +29,8 @@ include $(top_builddir)/src/Makefile.global
 include $(top_srcdir)/contrib/contrib-global.mk
 endif
 
-# Disabled because these tests require "wal_level=logical", which
-# typical installcheck users do not have (e.g. buildfarm clients).
-installcheck:;
-
 # But it can nonetheless be very helpful to run tests on preexisting
 # installation, allow to do so, but only if requested explicitly.
-installcheck-force: regresscheck-install-force isolationcheck-install-force
-
-check: regresscheck isolationcheck
-
-submake-regress:
-	$(MAKE) -C $(top_builddir)/src/test/regress all
-
-submake-isolation:
-	$(MAKE) -C $(top_builddir)/src/test/isolation all
-
-submake-test_decoding:
-	$(MAKE) -C $(top_builddir)/contrib/test_decoding
-
-REGRESSCHECKS=ddl xact rewrite toast permissions decoding_in_xact \
-	decoding_into_rel binary prepared replorigin time messages \
-	spill slot truncate
-
-regresscheck: | submake-regress submake-test_decoding temp-install
-	$(pg_regress_check) \
-	    --temp-config $(top_srcdir)/contrib/test_decoding/logical.conf \
-	    $(REGRESSCHECKS)
-
-regresscheck-install-force: | submake-regress submake-test_decoding temp-install
-	$(pg_regress_installcheck) \
-	    $(REGRESSCHECKS)
-
-ISOLATIONCHECKS=mxact delayed_startup ondisk_startup concurrent_ddl_dml \
-	oldest_xmin snapshot_transfer
-
-isolationcheck: | submake-isolation submake-test_decoding temp-install
-	$(pg_isolation_regress_check) \
-	    --temp-config $(top_srcdir)/contrib/test_decoding/logical.conf \
-	    $(ISOLATIONCHECKS)
-
-isolationcheck-install-force: all | submake-isolation submake-test_decoding temp-install
-	$(pg_isolation_regress_installcheck) \
-	    $(ISOLATIONCHECKS)
-
-.PHONY: submake-test_decoding submake-regress check \
-	regresscheck regresscheck-install-force \
-	isolationcheck isolationcheck-install-force
-
-temp-install: EXTRA_INSTALL=contrib/test_decoding
+installcheck-force:
+	$(pg_regress_installcheck) $(REGRESS)
+	$(pg_isolation_regress_installcheck) $(ISOLATION)
diff --git a/contrib/vacuumlo/Makefile b/contrib/vacuumlo/Makefile
index 5de506151e..3efcb46735 100644
--- a/contrib/vacuumlo/Makefile
+++ b/contrib/vacuumlo/Makefile
@@ -6,11 +6,11 @@ PGAPPICON = win32
 PROGRAM = vacuumlo
 OBJS	= vacuumlo.o $(WIN32RES)
 
+TAP_TESTS = 1
+
 PG_CPPFLAGS = -I$(libpq_srcdir)
 PG_LIBS_INTERNAL = $(libpq_pgport)
 
-EXTRA_CLEAN = tmp_check
-
 ifdef USE_PGXS
 PG_CONFIG = pg_config
 PGXS := $(shell $(PG_CONFIG) --pgxs)
@@ -21,9 +21,3 @@ top_builddir = ../..
 include $(top_builddir)/src/Makefile.global
 include $(top_srcdir)/contrib/contrib-global.mk
 endif
-
-check:
-	$(prove_check)
-
-installcheck:
-	$(prove_installcheck)
diff --git a/doc/src/sgml/extend.sgml b/doc/src/sgml/extend.sgml
index 1b1adae1a6..701e8593a9 100644
--- a/doc/src/sgml/extend.sgml
+++ b/doc/src/sgml/extend.sgml
@@ -1293,6 +1293,34 @@ include $(PGXS)
       </listitem>
      </varlistentry>
 
+     <varlistentry>
+      <term><varname>ISOLATION</varname></term>
+      <listitem>
+       <para>
+        list of isolation test cases
+       </para>
+      </listitem>
+     </varlistentry>
+
+     <varlistentry>
+      <term><varname>ISOLATION_OPTS</varname></term>
+      <listitem>
+       <para>
+        additional switches to pass to
+        <application>pg_isolation_regress</application>
+       </para>
+      </listitem>
+     </varlistentry>
+
+     <varlistentry>
+      <term><varname>TAP_TESTS</varname></term>
+      <listitem>
+       <para>
+        switch defining if TAP tests need to be run
+       </para>
+      </listitem>
+     </varlistentry>
+
      <varlistentry>
       <term><varname>NO_INSTALLCHECK</varname></term>
       <listitem>
diff --git a/src/makefiles/pgxs.mk b/src/makefiles/pgxs.mk
index fe7d899c5c..06cda28829 100644
--- a/src/makefiles/pgxs.mk
+++ b/src/makefiles/pgxs.mk
@@ -46,6 +46,9 @@
 #   HEADERS_built_$(MODULE) -- as above but built first
 #   REGRESS -- list of regression test cases (without suffix)
 #   REGRESS_OPTS -- additional switches to pass to pg_regress
+#   TAP_TESTS -- switch to enable TAP tests
+#   ISOLATION -- list of isolation test cases
+#   ISOLATION_OPTS -- additional switches to pass to pg_isolation_regress
 #   NO_INSTALLCHECK -- don't define an installcheck target, useful e.g. if
 #     tests require special configuration, or don't use pg_regress
 #   EXTRA_CLEAN -- extra files to remove in 'make clean'
@@ -336,6 +339,12 @@ ifeq ($(PORTNAME), win)
 	rm -f regress.def
 endif
 endif # REGRESS
+ifdef TAP_TESTS
+	rm -rf tmp_check/
+endif
+ifdef ISOLATION
+	rm -rf output_iso/ tmp_check_iso/
+endif
 
 ifdef MODULE_big
 clean: clean-lib
@@ -370,28 +379,51 @@ $(test_files_build): $(abs_builddir)/%: $(srcdir)/%
 	$(MKDIR_P) $(dir $@)
 	ln -s $< $@
 endif # VPATH
+endif # REGRESS
 
 .PHONY: submake
-submake:
 ifndef PGXS
+submake:
+ifdef REGRESS
 	$(MAKE) -C $(top_builddir)/src/test/regress pg_regress$(X)
 endif
+ifdef ISOLATION
+	$(MAKE) -C $(top_builddir)/src/test/isolation all
+endif
+endif # PGXS
 
-# against installed postmaster
+# Standard rules to run regression tests including multiple test suites.
+# Runs against an installed postmaster
 ifndef NO_INSTALLCHECK
 installcheck: submake $(REGRESS_PREP)
+ifdef REGRESS
 	$(pg_regress_installcheck) $(REGRESS_OPTS) $(REGRESS)
 endif
+ifdef ISOLATION
+	$(pg_isolation_regress_installcheck) $(ISOLATION_OPTS) $(ISOLATION)
+endif
+ifdef TAP_TESTS
+	$(prove_installcheck)
+endif
+endif # NO_INSTALLCHECK
 
+# Runs independently of any installation
 ifdef PGXS
 check:
 	@echo '"$(MAKE) check" is not supported.'
 	@echo 'Do "$(MAKE) install", then "$(MAKE) installcheck" instead.'
 else
 check: submake $(REGRESS_PREP)
+ifdef REGRESS
 	$(pg_regress_check) $(REGRESS_OPTS) $(REGRESS)
 endif
-endif # REGRESS
+ifdef ISOLATION
+	$(pg_isolation_regress_check) $(ISOLATION_OPTS) $(ISOLATION)
+endif
+ifdef TAP_TESTS
+	$(prove_check)
+endif
+endif # PGXS
 
 ifndef NO_TEMP_INSTALL
 temp-install: EXTRA_INSTALL+=$(subdir)
diff --git a/src/test/modules/brin/.gitignore b/src/test/modules/brin/.gitignore
index 62bbe8f6b1..44f600cb6c 100644
--- a/src/test/modules/brin/.gitignore
+++ b/src/test/modules/brin/.gitignore
@@ -1,3 +1,3 @@
 # Generated subdirectories
-/isolation_output/
+/output_iso/
 /tmp_check/
diff --git a/src/test/modules/brin/Makefile b/src/test/modules/brin/Makefile
index 566655cd61..c871593906 100644
--- a/src/test/modules/brin/Makefile
+++ b/src/test/modules/brin/Makefile
@@ -1,12 +1,9 @@
 # src/test/modules/brin/Makefile
 
-# Note: because we don't tell the Makefile there are any regression tests,
-# we have to clean those result files explicitly
-EXTRA_CLEAN = $(pg_regress_clean_files) ./isolation_output
+EXTRA_INSTALL = contrib/pageinspect
 
-EXTRA_INSTALL=contrib/pageinspect
-
-ISOLATIONCHECKS=summarization-and-inprogress-insertion
+ISOLATION = summarization-and-inprogress-insertion
+TAP_TESTS = 1
 
 ifdef USE_PGXS
 PG_CONFIG = pg_config
@@ -18,19 +15,3 @@ top_builddir = ../../../..
 include $(top_builddir)/src/Makefile.global
 include $(top_srcdir)/contrib/contrib-global.mk
 endif
-
-check: isolation-check prove-check
-
-isolation-check: | submake-isolation temp-install
-	$(MKDIR_P) isolation_output
-	$(pg_isolation_regress_check) \
-	    --outputdir=./isolation_output \
-	    $(ISOLATIONCHECKS)
-
-prove-check: | temp-install
-	$(prove_check)
-
-.PHONY: check isolation-check prove-check
-
-submake-isolation:
-	$(MAKE) -C $(top_builddir)/src/test/isolation all
diff --git a/src/test/modules/commit_ts/Makefile b/src/test/modules/commit_ts/Makefile
index 6d4f3be358..7a24bb3c6d 100644
--- a/src/test/modules/commit_ts/Makefile
+++ b/src/test/modules/commit_ts/Makefile
@@ -2,6 +2,7 @@
 
 REGRESS = commit_timestamp
 REGRESS_OPTS = --temp-config=$(top_srcdir)/src/test/modules/commit_ts/commit_ts.conf
+TAP_TESTS = 1
 
 ifdef USE_PGXS
 PG_CONFIG = pg_config
@@ -13,8 +14,3 @@ top_builddir = ../../../..
 include $(top_builddir)/src/Makefile.global
 include $(top_srcdir)/contrib/contrib-global.mk
 endif
-
-check: prove-check
-
-prove-check: | temp-install
-	$(prove_check)
diff --git a/src/test/modules/test_pg_dump/Makefile b/src/test/modules/test_pg_dump/Makefile
index c64b353707..6123b994f6 100644
--- a/src/test/modules/test_pg_dump/Makefile
+++ b/src/test/modules/test_pg_dump/Makefile
@@ -7,6 +7,7 @@ EXTENSION = test_pg_dump
 DATA = test_pg_dump--1.0.sql
 
 REGRESS = test_pg_dump
+TAP_TESTS = 1
 
 ifdef USE_PGXS
 PG_CONFIG = pg_config
@@ -18,8 +19,3 @@ top_builddir = ../../../..
 include $(top_builddir)/src/Makefile.global
 include $(top_srcdir)/contrib/contrib-global.mk
 endif
-
-check: prove-check
-
-prove-check: | temp-install
-	$(prove_check)
#2Michael Paquier
michael@paquier.xyz
In reply to: Michael Paquier (#1)
Re: Add extension options to control TAP and isolation tests

On Wed, Sep 05, 2018 at 06:48:49PM -0700, Michael Paquier wrote:

On a recent thread of pgsql-committers has been discussed the fact that
we lacked a bit of infrastructure to allow extensions to control
isolation and TAP tests:
/messages/by-id/20180905174527.GA2726@paquier.xyz

Attached is a patch which is the result of the previous thread, where a
couple of variables are added for extension authors:
- ISOLATION, similar to REGRESS for pg_regress, which lists isolation
tests.
- ISOLATION_OPTS, which can be used to pass an option set to
pg_isolation_regress.
- TAP_TESTS, a switch to enable running TAP tests.

Tom, Alvaro, any thoughts on the proposed patch? Please note that one
thing which is missing, and that I left on purpose so as the buildfarm
client does not need any extra tweaks, is support for those options in
src/tools/msvc. It is already possible to run easily any TAP test suite
using vcregress taptest $dir, and test_decoding has some special
handling in the buildfarm code to run isolation tests. It seems to me
that the amount of cleanup done by the initial patch in all the
Makefiles justifies its existence, and I could always follow-up with a
second patch for MSVC if needed.
--
Michael

#3Adam Berlin
berlin.ab@gmail.com
In reply to: Michael Paquier (#2)
Re: Add extension options to control TAP and isolation tests

Nice cleanup. Also, I like the ability to enable/control more types of tests easily from the Makefile. What are the next steps for this patch?

#4Michael Paquier
michael@paquier.xyz
In reply to: Adam Berlin (#3)
Re: Add extension options to control TAP and isolation tests

On Fri, Nov 09, 2018 at 03:14:00PM +0000, Adam Berlin wrote:

Nice cleanup. Also, I like the ability to enable/control more types of
tests easily from the Makefile. What are the next steps for this
patch?

Thanks. It seems to me that a complete review is still in order,
particularly regarding the new makefile option names. And then, if
everybody caring about the topic is happy with the way the patch is
shaped, it can be carried over to being committed, which would be most
likely something I'll do.
--
Michael

#5Nikolay Shaplov
dhyan@nataraj.su
In reply to: Michael Paquier (#4)
Re: Add extension options to control TAP and isolation tests

В письме от 10 ноября 2018 09:14:19 пользователь Michael Paquier написал:

Nice cleanup. Also, I like the ability to enable/control more types of
tests easily from the Makefile. What are the next steps for this
patch?

Thanks. It seems to me that a complete review is still in order,
particularly regarding the new makefile option names. And then, if
everybody caring about the topic is happy with the way the patch is
shaped, it can be carried over to being committed, which would be most
likely something I'll do.

Is it ok, if I join the reviewing? I like test, especially TAP one, you know
;-)
Since you are much more experienced in postgres then me, I'd try to understand
how does the patch work, try to use if for writing more TAP test, and will
report problems and thoughts I came across while doing that.

So far while first reading the patch I came to following two

1. 
--- a/src/test/modules/brin/.gitignore
+++ b/src/test/modules/brin/.gitignore
@@ -1,3 +1,3 @@
 # Generated subdirectories
-/isolation_output/
+/output_iso/
 /tmp_check/

For me name "output_iso" means nothing. iso is something about CD/DVD or about
standards. I would not guess that iso stands for isolation if I did not know
it already. isolation_output is more sensible: I have heard that there are
some isolation tests, this must be something about it. May be it would be
better to change it to isolation_output everywhere instead of changing to
output_iso

2.

--- a/doc/src/sgml/extend.sgml
+++ b/doc/src/sgml/extend.sgml
@@ -1293,6 +1293,34 @@ include $(PGXS)
       </listitem>
      </varlistentry>
.
+     <varlistentry>
+      <term><varname>ISOLATION</varname></term>
+      <listitem>
+       <para>
+        list of isolation test cases
+       </para>
+      </listitem>
+     </varlistentry>
+
+     <varlistentry>
+      <term><varname>ISOLATION_OPTS</varname></term>
+      <listitem>
+       <para>
+        additional switches to pass to
+        <application>pg_isolation_regress</application>
+       </para>
+      </listitem>
+     </varlistentry>
+
+     <varlistentry>
+      <term><varname>TAP_TESTS</varname></term>
+      <listitem>
+       <para>
+        switch defining if TAP tests need to be run
+       </para>
+      </listitem>
+     </varlistentry>
+
      <varlistentry>
       <term><varname>NO_INSTALLCHECK</varname></term>
       <listitem>

I tried to find definition in documentation what does "isolation test" exactly
means, but did not find. There is some general words about TAP tests in main
postgres documentation
https://www.postgresql.org/docs/11/regress-tap.html,
but I would not understand anything from it if I did not already know how it
works.

In current extend-pgxs documentation there is some explanation about
regression test, it sensible enough. Since TAP and isolation tests are
introduced now, there should be same short explanation for both of them.

And also it would be good to add links from extend-pgxs to regress-tap and
regress saying that for more info about these tests one can look at postgres
doc, because they work in a similar way.

That's all so far. I'll look more into it later...

--
Do code for fun.

#6Michael Paquier
michael@paquier.xyz
In reply to: Nikolay Shaplov (#5)
1 attachment(s)
Re: Add extension options to control TAP and isolation tests

On Tue, Nov 20, 2018 at 07:30:39PM +0300, Nikolay Shaplov wrote:

Is it ok, if I join the reviewing? I like test, especially TAP one, you know
;-)

Since you are much more experienced in postgres then me, I'd try to
understand how does the patch work, try to use if for writing more TAP
test, and will report problems and thoughts I came across while doing that.

Thanks for bumping in the field.

For me name "output_iso" means nothing. iso is something about CD/DVD or about
standards. I would not guess that iso stands for isolation if I did not know
it already. isolation_output is more sensible: I have heard that there are
some isolation tests, this must be something about it. May be it would be
better to change it to isolation_output everywhere instead of changing to
output_iso

That's just a default configuration used by the isolation tester.
That's not much bothering with in my opinion for this patch, as the
patch is here to make sure that the default configuration gets used
where it had better be used. Changing this default value would be of
course doable technically, but that's around for years to changing it
does not seem like good idea.

I tried to find definition in documentation what does "isolation test" exactly
means, but did not find. There is some general words about TAP tests in main
postgres documentation
https://www.postgresql.org/docs/11/regress-tap.html,
but I would not understand anything from it if I did not already know how it
works.

Those are mentioned here as part of the additional test suites:
https://www.postgresql.org/docs/11/regress-run.html#id-1.6.20.5.5

In current extend-pgxs documentation there is some explanation about
regression test, it sensible enough. Since TAP and isolation tests are
introduced now, there should be same short explanation for both of
them.

I see your point here, and it is true that documentation ought to be
better. So I have written out a new set of paragraphs which explain the
whereabouts of the new variables a bit more in depth in the section of
extend-pgxs.

And also it would be good to add links from extend-pgxs to regress-tap and
regress saying that for more info about these tests one can look at postgres
doc, because they work in a similar way.

I have added a reference to regress-tap in one of the new paragraphs.
Linking the existing stuff to point to "regress" is a separate issue
though, and while pointing to the TAP section is adapted as its
guidelines are rather general, I am not sure which one would make the
most sense though.
--
Michael

Attachments:

pgxs-isolation-tap-v2.patchtext/x-diff; charset=us-asciiDownload
diff --git a/contrib/bloom/Makefile b/contrib/bloom/Makefile
index 13bd397b70..da9553a2d0 100644
--- a/contrib/bloom/Makefile
+++ b/contrib/bloom/Makefile
@@ -8,6 +8,7 @@ DATA = bloom--1.0.sql
 PGFILEDESC = "bloom access method - signature file based index"
 
 REGRESS = bloom
+TAP_TESTS = 1
 
 ifdef USE_PGXS
 PG_CONFIG = pg_config
@@ -19,6 +20,3 @@ top_builddir = ../..
 include $(top_builddir)/src/Makefile.global
 include $(top_srcdir)/contrib/contrib-global.mk
 endif
-
-wal-check: temp-install
-	$(prove_check)
diff --git a/contrib/oid2name/Makefile b/contrib/oid2name/Makefile
index 908e078714..361a80a7a1 100644
--- a/contrib/oid2name/Makefile
+++ b/contrib/oid2name/Makefile
@@ -6,11 +6,11 @@ PGAPPICON = win32
 PROGRAM = oid2name
 OBJS	= oid2name.o $(WIN32RES)
 
+TAP_TESTS = 1
+
 PG_CPPFLAGS = -I$(libpq_srcdir)
 PG_LIBS_INTERNAL = $(libpq_pgport)
 
-EXTRA_CLEAN = tmp_check
-
 ifdef USE_PGXS
 PG_CONFIG = pg_config
 PGXS := $(shell $(PG_CONFIG) --pgxs)
@@ -21,9 +21,3 @@ top_builddir = ../..
 include $(top_builddir)/src/Makefile.global
 include $(top_srcdir)/contrib/contrib-global.mk
 endif
-
-check:
-	$(prove_check)
-
-installcheck:
-	$(prove_installcheck)
diff --git a/contrib/test_decoding/Makefile b/contrib/test_decoding/Makefile
index afcab930f7..8cd83a763f 100644
--- a/contrib/test_decoding/Makefile
+++ b/contrib/test_decoding/Makefile
@@ -3,9 +3,20 @@
 MODULES = test_decoding
 PGFILEDESC = "test_decoding - example of a logical decoding output plugin"
 
-# Note: because we don't tell the Makefile there are any regression tests,
-# we have to clean those result files explicitly
-EXTRA_CLEAN = $(pg_regress_clean_files)
+EXTRA_INSTALL=contrib/test_decoding
+
+REGRESS = ddl xact rewrite toast permissions decoding_in_xact \
+	decoding_into_rel binary prepared replorigin time messages \
+	spill slot truncate
+ISOLATION = mxact delayed_startup ondisk_startup concurrent_ddl_dml \
+	oldest_xmin snapshot_transfer
+
+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
+# typical installcheck users do not have (e.g. buildfarm clients).
+NO_INSTALLCHECK = 1
 
 ifdef USE_PGXS
 PG_CONFIG = pg_config
@@ -18,52 +29,8 @@ include $(top_builddir)/src/Makefile.global
 include $(top_srcdir)/contrib/contrib-global.mk
 endif
 
-# Disabled because these tests require "wal_level=logical", which
-# typical installcheck users do not have (e.g. buildfarm clients).
-installcheck:;
-
 # But it can nonetheless be very helpful to run tests on preexisting
 # installation, allow to do so, but only if requested explicitly.
-installcheck-force: regresscheck-install-force isolationcheck-install-force
-
-check: regresscheck isolationcheck
-
-submake-regress:
-	$(MAKE) -C $(top_builddir)/src/test/regress all
-
-submake-isolation:
-	$(MAKE) -C $(top_builddir)/src/test/isolation all
-
-submake-test_decoding:
-	$(MAKE) -C $(top_builddir)/contrib/test_decoding
-
-REGRESSCHECKS=ddl xact rewrite toast permissions decoding_in_xact \
-	decoding_into_rel binary prepared replorigin time messages \
-	spill slot truncate
-
-regresscheck: | submake-regress submake-test_decoding temp-install
-	$(pg_regress_check) \
-	    --temp-config $(top_srcdir)/contrib/test_decoding/logical.conf \
-	    $(REGRESSCHECKS)
-
-regresscheck-install-force: | submake-regress submake-test_decoding temp-install
-	$(pg_regress_installcheck) \
-	    $(REGRESSCHECKS)
-
-ISOLATIONCHECKS=mxact delayed_startup ondisk_startup concurrent_ddl_dml \
-	oldest_xmin snapshot_transfer
-
-isolationcheck: | submake-isolation submake-test_decoding temp-install
-	$(pg_isolation_regress_check) \
-	    --temp-config $(top_srcdir)/contrib/test_decoding/logical.conf \
-	    $(ISOLATIONCHECKS)
-
-isolationcheck-install-force: all | submake-isolation submake-test_decoding temp-install
-	$(pg_isolation_regress_installcheck) \
-	    $(ISOLATIONCHECKS)
-
-.PHONY: submake-test_decoding submake-regress check \
-	regresscheck regresscheck-install-force \
-	isolationcheck isolationcheck-install-force
-
-temp-install: EXTRA_INSTALL=contrib/test_decoding
+installcheck-force:
+	$(pg_regress_installcheck) $(REGRESS)
+	$(pg_isolation_regress_installcheck) $(ISOLATION)
diff --git a/contrib/vacuumlo/Makefile b/contrib/vacuumlo/Makefile
index 5de506151e..3efcb46735 100644
--- a/contrib/vacuumlo/Makefile
+++ b/contrib/vacuumlo/Makefile
@@ -6,11 +6,11 @@ PGAPPICON = win32
 PROGRAM = vacuumlo
 OBJS	= vacuumlo.o $(WIN32RES)
 
+TAP_TESTS = 1
+
 PG_CPPFLAGS = -I$(libpq_srcdir)
 PG_LIBS_INTERNAL = $(libpq_pgport)
 
-EXTRA_CLEAN = tmp_check
-
 ifdef USE_PGXS
 PG_CONFIG = pg_config
 PGXS := $(shell $(PG_CONFIG) --pgxs)
@@ -21,9 +21,3 @@ top_builddir = ../..
 include $(top_builddir)/src/Makefile.global
 include $(top_srcdir)/contrib/contrib-global.mk
 endif
-
-check:
-	$(prove_check)
-
-installcheck:
-	$(prove_installcheck)
diff --git a/doc/src/sgml/extend.sgml b/doc/src/sgml/extend.sgml
index 695e07fb38..7885b0aa74 100644
--- a/doc/src/sgml/extend.sgml
+++ b/doc/src/sgml/extend.sgml
@@ -1303,6 +1303,34 @@ include $(PGXS)
       </listitem>
      </varlistentry>
 
+     <varlistentry>
+      <term><varname>ISOLATION</varname></term>
+      <listitem>
+       <para>
+        list of isolation test cases, see below for more details
+       </para>
+      </listitem>
+     </varlistentry>
+
+     <varlistentry>
+      <term><varname>ISOLATION_OPTS</varname></term>
+      <listitem>
+       <para>
+        additional switches to pass to
+        <application>pg_isolation_regress</application>
+       </para>
+      </listitem>
+     </varlistentry>
+
+     <varlistentry>
+      <term><varname>TAP_TESTS</varname></term>
+      <listitem>
+       <para>
+        switch defining if TAP tests need to be run, see below
+       </para>
+      </listitem>
+     </varlistentry>
+
      <varlistentry>
       <term><varname>NO_INSTALLCHECK</varname></term>
       <listitem>
@@ -1423,13 +1451,42 @@ make VPATH=/path/to/extension/source/tree install
     have all expected files.
    </para>
 
+   <para>
+    The scripts listed in the <varname>ISOLATION</varname> variable are used
+    for tests stressing behavior of concurrent session with your module, which
+    can be invoked by <literal>make installcheck</literal> after doing
+    <literal>make install</literal>.  For this to work you must have a
+    running <productname>PostgreSQL</productname> server.  The script files
+    listed in <varname>ISOLATION</varname> must appear in a subdirectory
+    name <literal>specs/</literal> in your extension's directory.  These files
+    must have extension <literal>.spec</literal>, which must not be included
+    in the <varname>ISOLATION</varname> list in the makefile.  For each test
+    there should also be a file containing the expected output in a
+    subdirectory name <literal>expected/</literal>, with the same stem and
+    extension <literal>.out</literal>.  <literal>make installcheck</literal>
+    executes each test script, and compares the resulting output to the
+    matching expected file.  Any differences will be written to the file
+    <literal>output_iso/regression.diffs</literal> in
+    <command>diff -c</command> format.  Note that trying to run a test that is
+    missing its expected file will be reported as <quote>trouble</quote>, so
+    make sure you have all expected files.
+   </para>
+
+   <para>
+    <literal>TAP_TESTS</literal> enables the use of TAP tests.  Data from each
+    run is present in a subdirectory named <literal>tmp_check/</literal>.
+    See also <xref linkend="regress-tap"/> for more details.
+   </para>
+
    <tip>
     <para>
      The easiest way to create the expected files is to create empty files,
      then do a test run (which will of course report differences).  Inspect
      the actual result files found in the <literal>results/</literal>
-     directory, then copy them to <literal>expected/</literal> if they match
-     what you expect from the test.
+     directory (for tests in <literal>REGRESS</literal>), or
+     <literal>output_iso/results/</literal> directory (for tests in
+     <literal>ISOLATION</literal>), then copy them to
+     <literal>expected/</literal> if they match what you expect from the test.
     </para>
 
    </tip>
diff --git a/src/makefiles/pgxs.mk b/src/makefiles/pgxs.mk
index 070d151018..249061541e 100644
--- a/src/makefiles/pgxs.mk
+++ b/src/makefiles/pgxs.mk
@@ -46,6 +46,9 @@
 #   HEADERS_built_$(MODULE) -- as above but built first (also NOT cleaned)
 #   REGRESS -- list of regression test cases (without suffix)
 #   REGRESS_OPTS -- additional switches to pass to pg_regress
+#   TAP_TESTS -- switch to enable TAP tests
+#   ISOLATION -- list of isolation test cases
+#   ISOLATION_OPTS -- additional switches to pass to pg_isolation_regress
 #   NO_INSTALLCHECK -- don't define an installcheck target, useful e.g. if
 #     tests require special configuration, or don't use pg_regress
 #   EXTRA_CLEAN -- extra files to remove in 'make clean'
@@ -349,6 +352,12 @@ ifeq ($(PORTNAME), win)
 	rm -f regress.def
 endif
 endif # REGRESS
+ifdef TAP_TESTS
+	rm -rf tmp_check/
+endif
+ifdef ISOLATION
+	rm -rf output_iso/ tmp_check_iso/
+endif
 
 ifdef MODULE_big
 clean: clean-lib
@@ -383,28 +392,51 @@ $(test_files_build): $(abs_builddir)/%: $(srcdir)/%
 	$(MKDIR_P) $(dir $@)
 	ln -s $< $@
 endif # VPATH
+endif # REGRESS
 
 .PHONY: submake
-submake:
 ifndef PGXS
+submake:
+ifdef REGRESS
 	$(MAKE) -C $(top_builddir)/src/test/regress pg_regress$(X)
 endif
+ifdef ISOLATION
+	$(MAKE) -C $(top_builddir)/src/test/isolation all
+endif
+endif # PGXS
 
-# against installed postmaster
+# Standard rules to run regression tests including multiple test suites.
+# Runs against an installed postmaster
 ifndef NO_INSTALLCHECK
 installcheck: submake $(REGRESS_PREP)
+ifdef REGRESS
 	$(pg_regress_installcheck) $(REGRESS_OPTS) $(REGRESS)
 endif
+ifdef ISOLATION
+	$(pg_isolation_regress_installcheck) $(ISOLATION_OPTS) $(ISOLATION)
+endif
+ifdef TAP_TESTS
+	$(prove_installcheck)
+endif
+endif # NO_INSTALLCHECK
 
+# Runs independently of any installation
 ifdef PGXS
 check:
 	@echo '"$(MAKE) check" is not supported.'
 	@echo 'Do "$(MAKE) install", then "$(MAKE) installcheck" instead.'
 else
 check: submake $(REGRESS_PREP)
+ifdef REGRESS
 	$(pg_regress_check) $(REGRESS_OPTS) $(REGRESS)
 endif
-endif # REGRESS
+ifdef ISOLATION
+	$(pg_isolation_regress_check) $(ISOLATION_OPTS) $(ISOLATION)
+endif
+ifdef TAP_TESTS
+	$(prove_check)
+endif
+endif # PGXS
 
 ifndef NO_TEMP_INSTALL
 temp-install: EXTRA_INSTALL+=$(subdir)
diff --git a/src/test/modules/brin/.gitignore b/src/test/modules/brin/.gitignore
index 62bbe8f6b1..44f600cb6c 100644
--- a/src/test/modules/brin/.gitignore
+++ b/src/test/modules/brin/.gitignore
@@ -1,3 +1,3 @@
 # Generated subdirectories
-/isolation_output/
+/output_iso/
 /tmp_check/
diff --git a/src/test/modules/brin/Makefile b/src/test/modules/brin/Makefile
index 566655cd61..c871593906 100644
--- a/src/test/modules/brin/Makefile
+++ b/src/test/modules/brin/Makefile
@@ -1,12 +1,9 @@
 # src/test/modules/brin/Makefile
 
-# Note: because we don't tell the Makefile there are any regression tests,
-# we have to clean those result files explicitly
-EXTRA_CLEAN = $(pg_regress_clean_files) ./isolation_output
+EXTRA_INSTALL = contrib/pageinspect
 
-EXTRA_INSTALL=contrib/pageinspect
-
-ISOLATIONCHECKS=summarization-and-inprogress-insertion
+ISOLATION = summarization-and-inprogress-insertion
+TAP_TESTS = 1
 
 ifdef USE_PGXS
 PG_CONFIG = pg_config
@@ -18,19 +15,3 @@ top_builddir = ../../../..
 include $(top_builddir)/src/Makefile.global
 include $(top_srcdir)/contrib/contrib-global.mk
 endif
-
-check: isolation-check prove-check
-
-isolation-check: | submake-isolation temp-install
-	$(MKDIR_P) isolation_output
-	$(pg_isolation_regress_check) \
-	    --outputdir=./isolation_output \
-	    $(ISOLATIONCHECKS)
-
-prove-check: | temp-install
-	$(prove_check)
-
-.PHONY: check isolation-check prove-check
-
-submake-isolation:
-	$(MAKE) -C $(top_builddir)/src/test/isolation all
diff --git a/src/test/modules/commit_ts/Makefile b/src/test/modules/commit_ts/Makefile
index 6d4f3be358..7a24bb3c6d 100644
--- a/src/test/modules/commit_ts/Makefile
+++ b/src/test/modules/commit_ts/Makefile
@@ -2,6 +2,7 @@
 
 REGRESS = commit_timestamp
 REGRESS_OPTS = --temp-config=$(top_srcdir)/src/test/modules/commit_ts/commit_ts.conf
+TAP_TESTS = 1
 
 ifdef USE_PGXS
 PG_CONFIG = pg_config
@@ -13,8 +14,3 @@ top_builddir = ../../../..
 include $(top_builddir)/src/Makefile.global
 include $(top_srcdir)/contrib/contrib-global.mk
 endif
-
-check: prove-check
-
-prove-check: | temp-install
-	$(prove_check)
diff --git a/src/test/modules/test_pg_dump/Makefile b/src/test/modules/test_pg_dump/Makefile
index c64b353707..6123b994f6 100644
--- a/src/test/modules/test_pg_dump/Makefile
+++ b/src/test/modules/test_pg_dump/Makefile
@@ -7,6 +7,7 @@ EXTENSION = test_pg_dump
 DATA = test_pg_dump--1.0.sql
 
 REGRESS = test_pg_dump
+TAP_TESTS = 1
 
 ifdef USE_PGXS
 PG_CONFIG = pg_config
@@ -18,8 +19,3 @@ top_builddir = ../../../..
 include $(top_builddir)/src/Makefile.global
 include $(top_srcdir)/contrib/contrib-global.mk
 endif
-
-check: prove-check
-
-prove-check: | temp-install
-	$(prove_check)
#7Nikolay Shaplov
dhyan@nataraj.su
In reply to: Michael Paquier (#6)
Re: Add extension options to control TAP and isolation tests

В письме от 21 ноября 2018 09:39:53 пользователь Michael Paquier написал:

For me name "output_iso" means nothing. iso is something about CD/DVD or
about standards. I would not guess that iso stands for isolation if I did
not know it already. isolation_output is more sensible: I have heard that
there are some isolation tests, this must be something about it. May be
it would be better to change it to isolation_output everywhere instead of
changing to output_iso

That's just a default configuration used by the isolation tester.
That's not much bothering with in my opinion for this patch, as the
patch is here to make sure that the default configuration gets used
where it had better be used. Changing this default value would be of
course doable technically, but that's around for years to changing it
does not seem like good idea.

Ok. If it is long time tradition, let it be :-)

I tried to find definition in documentation what does "isolation test"
exactly means, but did not find. There is some general words about TAP
tests in main postgres documentation
https://www.postgresql.org/docs/11/regress-tap.html,
but I would not understand anything from it if I did not already know how
it works.

Those are mentioned here as part of the additional test suites:
https://www.postgresql.org/docs/11/regress-run.html#id-1.6.20.5.5

Oh thanks (I feel urge to start editing documentation right now. I will
surpress it, and do it, I hope, later.)
May I also ask a question, I came across. It is not part of the patch, but
nevertheless...
If I look in the code, regression test are sql files with expected output that
are in src/test/regress. If I look in the documentation, all the tests are
regression tests including TAP tests.
https://www.postgresql.org/docs/11/regress.html

what is the right way to look at it?

In current extend-pgxs documentation there is some explanation about
regression test, it sensible enough. Since TAP and isolation tests are
introduced now, there should be same short explanation for both of
them.

I see your point here, and it is true that documentation ought to be
better. So I have written out a new set of paragraphs which explain the
whereabouts of the new variables a bit more in depth in the section of
extend-pgxs.

Oh thanks! Now it is much more clear.

So, I continued exploring your patch. First I carefully read changes in
pgxs.mk. The only part of it I did not understand is

 .PHONY: submake
-submake:
 ifndef PGXS
+submake:
+ifdef REGRESS
    $(MAKE) -C $(top_builddir)/src/test/regress pg_regress$(X)
 endif
+ifdef ISOLATION
+   $(MAKE) -C $(top_builddir)/src/test/isolation all
+endif
+endif # PGXS

I suppose it is because the purpose of PGXS is never explained, not in the
documentation, not in the code comments, and PGXS := $(shell $(PG_CONFIG) --
pgxs) sounds like some strange magic spell, that explains nothing. In what
cases it is defined? In what cases it is not defined? What exactly does it
store? Can we make things more easy to understand here?

2. As far as you said that TAP tests for bloom were never executed,
I guess I should just ignore

-
-wal-check: temp-install
- (prove_check)

as a part of wrong way to execute TAP tests. (I fond no traces of "wal-check"
string in the code, so I guess this build target is an invention of bloom
authors)

3. The rest are trivial, except changes for contrib/test_decoding/ and
src/test/modules/brin I will explore them in my next postgres-dev time slot
and then my review work will be done :-)

--
Do code for fun.

#8Michael Paquier
michael@paquier.xyz
In reply to: Nikolay Shaplov (#7)
Re: Add extension options to control TAP and isolation tests

On Thu, Nov 22, 2018 at 10:01:26PM +0300, Nikolay Shaplov wrote:

В письме от 21 ноября 2018 09:39:53 пользователь Michael Paquier написал:

Those are mentioned here as part of the additional test suites:
https://www.postgresql.org/docs/11/regress-run.html#id-1.6.20.5.5

Oh thanks (I feel urge to start editing documentation right now. I will
surpress it, and do it, I hope, later.)

If you have a patch to improve the existing docs, please feel free to
submit those on a different thread. If you have suggestions about
improving this patch's docs, of course please feel free to suggest them
here!

May I also ask a question, I came across. It is not part of the patch, but
nevertheless...
If I look in the code, regression test are sql files with expected output that
are in src/test/regress. If I look in the documentation, all the tests are
regression tests including TAP tests.
https://www.postgresql.org/docs/11/regress.html

what is the right way to look at it?

That's for the main regression test suite within src/test/regress, TAP
is also a regression test suite, the page of the link reflects the
general set of test suites available.

So, I continued exploring your patch. First I carefully read changes in
pgxs.mk. The only part of it I did not understand is

.PHONY: submake
-submake:
ifndef PGXS
+submake:
+ifdef REGRESS
$(MAKE) -C $(top_builddir)/src/test/regress pg_regress$(X)
endif
+ifdef ISOLATION
+   $(MAKE) -C $(top_builddir)/src/test/isolation all
+endif
+endif # PGXS

This is to make sure that the necessary tools are compiled before
running the related tests. "submake:" needs to be moved out actually.
Thinking about it a bit more, we should also remove the "ifdef REGRESS"
and "ifdef ISOLATION" because there are some dependencies here. For
example TAP tests call pg_regress to initialize connection policy. TAP
tests may also use isolation_tester or such.

I suppose it is because the purpose of PGXS is never explained, not in the
documentation, not in the code comments, and PGXS := $(shell $(PG_CONFIG) --
pgxs) sounds like some strange magic spell, that explains nothing. In what
cases it is defined? In what cases it is not defined? What exactly does it
store? Can we make things more easy to understand here?

That's part of a larger scope which is here:
https://www.postgresql.org/docs/11/extend-pgxs.html

2. As far as you said that TAP tests for bloom were never executed,
I guess I should just ignore

-
-wal-check: temp-install
- (prove_check)

as a part of wrong way to execute TAP tests. (I fond no traces of "wal-check"
string in the code, so I guess this build target is an invention of bloom
authors)

Yes. It seems that it was useful for debugging at this time, but this
will rot if let as-is... I am pretty sure that most people don't use
that wrapper.

3. The rest are trivial, except changes for contrib/test_decoding/ and
src/test/modules/brin I will explore them in my next postgres-dev time slot
and then my review work will be done :-)

Thanks for the input, Nikolay!
--
Michael

#9Arthur Zakirov
a.zakirov@postgrespro.ru
In reply to: Michael Paquier (#6)
1 attachment(s)
Re: Add extension options to control TAP and isolation tests

Hello,

On 21.11.2018 03:39, Michael Paquier wrote:

I have added a reference to regress-tap in one of the new paragraphs.
Linking the existing stuff to point to "regress" is a separate issue
though, and while pointing to the TAP section is adapted as its
guidelines are rather general, I am not sure which one would make the
most sense though.
--
Michael

The patch is very useful. Using TAP_TESTS is more convenient and clearer
than adding wal-check target. Every time I was adding TAP tests for a
extension I had to remember that I should add wal-check.

After applying the patch all tests pass, there wasn't any error.

Also I tested it in one of our extension which has TAP tests.
installcheck and check work as expected.

I think the patch can be marked as "Ready for Committer".

But there is a problem that you need to copy your extension to the
contrib directory if you want to run TAP tests. I tried to run TAP test
of the extension outside of PostgreSQL source directory. And it failed
to run the test. It is because `prove_installcheck` redefines
`top_builddir` and `PG_REGRESS`:

cd ./ && TESTDIR='/home/artur/source/pg/rum'
PATH="/home/artur/progs/pgsql/bin:$PATH" PGPORT='65432'
top_builddir='/home/artur/source/pg/rum//home/artur/progs/pgsql/lib/pgxs/src/makefiles/../..'
PG_REGRESS='/home/artur/source/pg/rum//home/artur/progs/pgsql/lib/pgxs/src/makefiles/../../src/test/regress/pg_regress'
/usr/sbin/prove -I
/home/artur/progs/pgsql/lib/pgxs/src/makefiles/../../src/test/perl/ -I
./ t/*.pl
t/001_wal.pl .. Bailout called. Further testing stopped: system
/home/artur/source/pg/rum//home/artur/progs/pgsql/lib/pgxs/src/makefiles/../../src/test/regress/pg_regress
failed

Unfortunately I didn't find the way to run it, maybe I miss something.
It can be fixed by an additional patch I attached. I think I can create
an entry in the future commitfest or it can be joined into your patch.

--
Arthur Zakirov
Postgres Professional: http://www.postgrespro.com
Russian Postgres Company

Attachments:

use_pgxs_tap_tests.patchtext/x-patch; name=use_pgxs_tap_tests.patchDownload
diff --git a/src/makefiles/pgxs.mk b/src/makefiles/pgxs.mk
index 249061541e..c096f82d60 100644
--- a/src/makefiles/pgxs.mk
+++ b/src/makefiles/pgxs.mk
@@ -90,6 +90,9 @@ endif
 ifeq ($(FLEX),)
 FLEX = flex
 endif
+ifeq ($(PROVE),)
+PROVE = prove
+endif
 
 endif # PGXS
 
@@ -405,6 +408,12 @@ ifdef ISOLATION
 endif
 endif # PGXS
 
+# There are common routines in src/test/perl, and some test suites have
+# extra perl modules in their own directory.
+PG_PROVE_FLAGS = -I $(top_srcdir)/src/test/perl/ -I $(srcdir)
+# User-supplied prove flags such as --verbose can be provided in PROVE_FLAGS.
+PROVE_FLAGS =
+
 # Standard rules to run regression tests including multiple test suites.
 # Runs against an installed postmaster
 ifndef NO_INSTALLCHECK
@@ -416,7 +425,9 @@ ifdef ISOLATION
 	$(pg_isolation_regress_installcheck) $(ISOLATION_OPTS) $(ISOLATION)
 endif
 ifdef TAP_TESTS
-	$(prove_installcheck)
+	rm -rf '$(CURDIR)'/tmp_check
+	$(MKDIR_P) '$(CURDIR)'/tmp_check
+	cd $(srcdir) && TESTDIR='$(CURDIR)' PATH="$(bindir):$$PATH" PGPORT='6$(DEF_PGPORT)' top_builddir='$(top_builddir)' PG_REGRESS='$(top_builddir)/src/test/regress/pg_regress' $(PROVE) $(PG_PROVE_FLAGS) $(PROVE_FLAGS) $(if $(PROVE_TESTS),$(PROVE_TESTS),t/*.pl)
 endif
 endif # NO_INSTALLCHECK
 
@@ -434,7 +445,9 @@ ifdef ISOLATION
 	$(pg_isolation_regress_check) $(ISOLATION_OPTS) $(ISOLATION)
 endif
 ifdef TAP_TESTS
-	$(prove_check)
+	rm -rf '$(CURDIR)'/tmp_check
+	$(MKDIR_P) '$(CURDIR)'/tmp_check
+	cd $(srcdir) && TESTDIR='$(CURDIR)' $(with_temp_install) PGPORT='6$(DEF_PGPORT)' PG_REGRESS='$(top_builddir)/src/test/regress/pg_regress' $(PROVE) $(PG_PROVE_FLAGS) $(PROVE_FLAGS) $(if $(PROVE_TESTS),$(PROVE_TESTS),t/*.pl)
 endif
 endif # PGXS
 
#10Michael Paquier
michael@paquier.xyz
In reply to: Arthur Zakirov (#9)
1 attachment(s)
Re: Add extension options to control TAP and isolation tests

On Fri, Nov 23, 2018 at 05:29:00PM +0300, Arthur Zakirov wrote:

The patch is very useful. Using TAP_TESTS is more convenient and clearer
than adding wal-check target. Every time I was adding TAP tests for a
extension I had to remember that I should add wal-check.

wal-check is a custom option part of contrib/bloom/ which is not aimed
at spreading around.

After applying the patch all tests pass, there wasn't any error.

Also I tested it in one of our extension which has TAP tests. installcheck
and check work as expected.

Thanks.

But there is a problem that you need to copy your extension to the contrib
directory if you want to run TAP tests. I tried to run TAP test of the
extension outside of PostgreSQL source directory. And it failed to run the
test. It is because `prove_installcheck` redefines `top_builddir` and
`PG_REGRESS`:

I have tested that as well with one of my custom extensions, which has
some TAP tests, and the following Makefile additions:
TAP_TESTS = 1
ifdef USE_PGXS
PG_CONFIG = pg_config
PGXS := $(shell $(PG_CONFIG) --pgxs)
include $(PGXS)
else
subdir = contrib/EXTENSION_NAME_HERE
top_builddir = ../..
include $(top_builddir)/src/Makefile.global
include $(top_srcdir)/contrib/contrib-global.mk
endif

Running make clean at the root of the tree, then running make check from
contrib/EXTENSION_NAME_HERE works for me.

Unfortunately I didn't find the way to run it, maybe I miss something. It
can be fixed by an additional patch I attached. I think I can create an
entry in the future commitfest or it can be joined into your patch.

The previous version of the patch I sent make the build of
src/test/regress dependent on if REGRESS is set, but I missed the fact
that TAP tests also call pg_regress, which is the error you are seeing.
The attached patch will be able to work.

Thanks all for the reviews, I'll do a last lookup on Monday my time and
I'll try to get that committed by then. That's a nice cleanup of the
tree.
--
Michael

Attachments:

pgxs-isolation-tap-v3.patchtext/x-diff; charset=us-asciiDownload
diff --git a/contrib/bloom/Makefile b/contrib/bloom/Makefile
index 13bd397b70..da9553a2d0 100644
--- a/contrib/bloom/Makefile
+++ b/contrib/bloom/Makefile
@@ -8,6 +8,7 @@ DATA = bloom--1.0.sql
 PGFILEDESC = "bloom access method - signature file based index"
 
 REGRESS = bloom
+TAP_TESTS = 1
 
 ifdef USE_PGXS
 PG_CONFIG = pg_config
@@ -19,6 +20,3 @@ top_builddir = ../..
 include $(top_builddir)/src/Makefile.global
 include $(top_srcdir)/contrib/contrib-global.mk
 endif
-
-wal-check: temp-install
-	$(prove_check)
diff --git a/contrib/oid2name/Makefile b/contrib/oid2name/Makefile
index 908e078714..361a80a7a1 100644
--- a/contrib/oid2name/Makefile
+++ b/contrib/oid2name/Makefile
@@ -6,11 +6,11 @@ PGAPPICON = win32
 PROGRAM = oid2name
 OBJS	= oid2name.o $(WIN32RES)
 
+TAP_TESTS = 1
+
 PG_CPPFLAGS = -I$(libpq_srcdir)
 PG_LIBS_INTERNAL = $(libpq_pgport)
 
-EXTRA_CLEAN = tmp_check
-
 ifdef USE_PGXS
 PG_CONFIG = pg_config
 PGXS := $(shell $(PG_CONFIG) --pgxs)
@@ -21,9 +21,3 @@ top_builddir = ../..
 include $(top_builddir)/src/Makefile.global
 include $(top_srcdir)/contrib/contrib-global.mk
 endif
-
-check:
-	$(prove_check)
-
-installcheck:
-	$(prove_installcheck)
diff --git a/contrib/test_decoding/Makefile b/contrib/test_decoding/Makefile
index afcab930f7..8cd83a763f 100644
--- a/contrib/test_decoding/Makefile
+++ b/contrib/test_decoding/Makefile
@@ -3,9 +3,20 @@
 MODULES = test_decoding
 PGFILEDESC = "test_decoding - example of a logical decoding output plugin"
 
-# Note: because we don't tell the Makefile there are any regression tests,
-# we have to clean those result files explicitly
-EXTRA_CLEAN = $(pg_regress_clean_files)
+EXTRA_INSTALL=contrib/test_decoding
+
+REGRESS = ddl xact rewrite toast permissions decoding_in_xact \
+	decoding_into_rel binary prepared replorigin time messages \
+	spill slot truncate
+ISOLATION = mxact delayed_startup ondisk_startup concurrent_ddl_dml \
+	oldest_xmin snapshot_transfer
+
+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
+# typical installcheck users do not have (e.g. buildfarm clients).
+NO_INSTALLCHECK = 1
 
 ifdef USE_PGXS
 PG_CONFIG = pg_config
@@ -18,52 +29,8 @@ include $(top_builddir)/src/Makefile.global
 include $(top_srcdir)/contrib/contrib-global.mk
 endif
 
-# Disabled because these tests require "wal_level=logical", which
-# typical installcheck users do not have (e.g. buildfarm clients).
-installcheck:;
-
 # But it can nonetheless be very helpful to run tests on preexisting
 # installation, allow to do so, but only if requested explicitly.
-installcheck-force: regresscheck-install-force isolationcheck-install-force
-
-check: regresscheck isolationcheck
-
-submake-regress:
-	$(MAKE) -C $(top_builddir)/src/test/regress all
-
-submake-isolation:
-	$(MAKE) -C $(top_builddir)/src/test/isolation all
-
-submake-test_decoding:
-	$(MAKE) -C $(top_builddir)/contrib/test_decoding
-
-REGRESSCHECKS=ddl xact rewrite toast permissions decoding_in_xact \
-	decoding_into_rel binary prepared replorigin time messages \
-	spill slot truncate
-
-regresscheck: | submake-regress submake-test_decoding temp-install
-	$(pg_regress_check) \
-	    --temp-config $(top_srcdir)/contrib/test_decoding/logical.conf \
-	    $(REGRESSCHECKS)
-
-regresscheck-install-force: | submake-regress submake-test_decoding temp-install
-	$(pg_regress_installcheck) \
-	    $(REGRESSCHECKS)
-
-ISOLATIONCHECKS=mxact delayed_startup ondisk_startup concurrent_ddl_dml \
-	oldest_xmin snapshot_transfer
-
-isolationcheck: | submake-isolation submake-test_decoding temp-install
-	$(pg_isolation_regress_check) \
-	    --temp-config $(top_srcdir)/contrib/test_decoding/logical.conf \
-	    $(ISOLATIONCHECKS)
-
-isolationcheck-install-force: all | submake-isolation submake-test_decoding temp-install
-	$(pg_isolation_regress_installcheck) \
-	    $(ISOLATIONCHECKS)
-
-.PHONY: submake-test_decoding submake-regress check \
-	regresscheck regresscheck-install-force \
-	isolationcheck isolationcheck-install-force
-
-temp-install: EXTRA_INSTALL=contrib/test_decoding
+installcheck-force:
+	$(pg_regress_installcheck) $(REGRESS)
+	$(pg_isolation_regress_installcheck) $(ISOLATION)
diff --git a/contrib/vacuumlo/Makefile b/contrib/vacuumlo/Makefile
index 5de506151e..3efcb46735 100644
--- a/contrib/vacuumlo/Makefile
+++ b/contrib/vacuumlo/Makefile
@@ -6,11 +6,11 @@ PGAPPICON = win32
 PROGRAM = vacuumlo
 OBJS	= vacuumlo.o $(WIN32RES)
 
+TAP_TESTS = 1
+
 PG_CPPFLAGS = -I$(libpq_srcdir)
 PG_LIBS_INTERNAL = $(libpq_pgport)
 
-EXTRA_CLEAN = tmp_check
-
 ifdef USE_PGXS
 PG_CONFIG = pg_config
 PGXS := $(shell $(PG_CONFIG) --pgxs)
@@ -21,9 +21,3 @@ top_builddir = ../..
 include $(top_builddir)/src/Makefile.global
 include $(top_srcdir)/contrib/contrib-global.mk
 endif
-
-check:
-	$(prove_check)
-
-installcheck:
-	$(prove_installcheck)
diff --git a/doc/src/sgml/extend.sgml b/doc/src/sgml/extend.sgml
index 695e07fb38..7885b0aa74 100644
--- a/doc/src/sgml/extend.sgml
+++ b/doc/src/sgml/extend.sgml
@@ -1303,6 +1303,34 @@ include $(PGXS)
       </listitem>
      </varlistentry>
 
+     <varlistentry>
+      <term><varname>ISOLATION</varname></term>
+      <listitem>
+       <para>
+        list of isolation test cases, see below for more details
+       </para>
+      </listitem>
+     </varlistentry>
+
+     <varlistentry>
+      <term><varname>ISOLATION_OPTS</varname></term>
+      <listitem>
+       <para>
+        additional switches to pass to
+        <application>pg_isolation_regress</application>
+       </para>
+      </listitem>
+     </varlistentry>
+
+     <varlistentry>
+      <term><varname>TAP_TESTS</varname></term>
+      <listitem>
+       <para>
+        switch defining if TAP tests need to be run, see below
+       </para>
+      </listitem>
+     </varlistentry>
+
      <varlistentry>
       <term><varname>NO_INSTALLCHECK</varname></term>
       <listitem>
@@ -1423,13 +1451,42 @@ make VPATH=/path/to/extension/source/tree install
     have all expected files.
    </para>
 
+   <para>
+    The scripts listed in the <varname>ISOLATION</varname> variable are used
+    for tests stressing behavior of concurrent session with your module, which
+    can be invoked by <literal>make installcheck</literal> after doing
+    <literal>make install</literal>.  For this to work you must have a
+    running <productname>PostgreSQL</productname> server.  The script files
+    listed in <varname>ISOLATION</varname> must appear in a subdirectory
+    name <literal>specs/</literal> in your extension's directory.  These files
+    must have extension <literal>.spec</literal>, which must not be included
+    in the <varname>ISOLATION</varname> list in the makefile.  For each test
+    there should also be a file containing the expected output in a
+    subdirectory name <literal>expected/</literal>, with the same stem and
+    extension <literal>.out</literal>.  <literal>make installcheck</literal>
+    executes each test script, and compares the resulting output to the
+    matching expected file.  Any differences will be written to the file
+    <literal>output_iso/regression.diffs</literal> in
+    <command>diff -c</command> format.  Note that trying to run a test that is
+    missing its expected file will be reported as <quote>trouble</quote>, so
+    make sure you have all expected files.
+   </para>
+
+   <para>
+    <literal>TAP_TESTS</literal> enables the use of TAP tests.  Data from each
+    run is present in a subdirectory named <literal>tmp_check/</literal>.
+    See also <xref linkend="regress-tap"/> for more details.
+   </para>
+
    <tip>
     <para>
      The easiest way to create the expected files is to create empty files,
      then do a test run (which will of course report differences).  Inspect
      the actual result files found in the <literal>results/</literal>
-     directory, then copy them to <literal>expected/</literal> if they match
-     what you expect from the test.
+     directory (for tests in <literal>REGRESS</literal>), or
+     <literal>output_iso/results/</literal> directory (for tests in
+     <literal>ISOLATION</literal>), then copy them to
+     <literal>expected/</literal> if they match what you expect from the test.
     </para>
 
    </tip>
diff --git a/src/makefiles/pgxs.mk b/src/makefiles/pgxs.mk
index 070d151018..e8fecaec0f 100644
--- a/src/makefiles/pgxs.mk
+++ b/src/makefiles/pgxs.mk
@@ -46,6 +46,9 @@
 #   HEADERS_built_$(MODULE) -- as above but built first (also NOT cleaned)
 #   REGRESS -- list of regression test cases (without suffix)
 #   REGRESS_OPTS -- additional switches to pass to pg_regress
+#   TAP_TESTS -- switch to enable TAP tests
+#   ISOLATION -- list of isolation test cases
+#   ISOLATION_OPTS -- additional switches to pass to pg_isolation_regress
 #   NO_INSTALLCHECK -- don't define an installcheck target, useful e.g. if
 #     tests require special configuration, or don't use pg_regress
 #   EXTRA_CLEAN -- extra files to remove in 'make clean'
@@ -349,6 +352,12 @@ ifeq ($(PORTNAME), win)
 	rm -f regress.def
 endif
 endif # REGRESS
+ifdef TAP_TESTS
+	rm -rf tmp_check/
+endif
+ifdef ISOLATION
+	rm -rf output_iso/ tmp_check_iso/
+endif
 
 ifdef MODULE_big
 clean: clean-lib
@@ -383,28 +392,47 @@ $(test_files_build): $(abs_builddir)/%: $(srcdir)/%
 	$(MKDIR_P) $(dir $@)
 	ln -s $< $@
 endif # VPATH
+endif # REGRESS
 
 .PHONY: submake
 submake:
 ifndef PGXS
 	$(MAKE) -C $(top_builddir)/src/test/regress pg_regress$(X)
+	$(MAKE) -C $(top_builddir)/src/test/isolation all
 endif
 
-# against installed postmaster
+# Standard rules to run regression tests including multiple test suites.
+# Runs against an installed postmaster
 ifndef NO_INSTALLCHECK
 installcheck: submake $(REGRESS_PREP)
+ifdef REGRESS
 	$(pg_regress_installcheck) $(REGRESS_OPTS) $(REGRESS)
 endif
+ifdef ISOLATION
+	$(pg_isolation_regress_installcheck) $(ISOLATION_OPTS) $(ISOLATION)
+endif
+ifdef TAP_TESTS
+	$(prove_installcheck)
+endif
+endif # NO_INSTALLCHECK
 
+# Runs independently of any installation
 ifdef PGXS
 check:
 	@echo '"$(MAKE) check" is not supported.'
 	@echo 'Do "$(MAKE) install", then "$(MAKE) installcheck" instead.'
 else
 check: submake $(REGRESS_PREP)
+ifdef REGRESS
 	$(pg_regress_check) $(REGRESS_OPTS) $(REGRESS)
 endif
-endif # REGRESS
+ifdef ISOLATION
+	$(pg_isolation_regress_check) $(ISOLATION_OPTS) $(ISOLATION)
+endif
+ifdef TAP_TESTS
+	$(prove_check)
+endif
+endif # PGXS
 
 ifndef NO_TEMP_INSTALL
 temp-install: EXTRA_INSTALL+=$(subdir)
diff --git a/src/test/modules/brin/.gitignore b/src/test/modules/brin/.gitignore
index 62bbe8f6b1..44f600cb6c 100644
--- a/src/test/modules/brin/.gitignore
+++ b/src/test/modules/brin/.gitignore
@@ -1,3 +1,3 @@
 # Generated subdirectories
-/isolation_output/
+/output_iso/
 /tmp_check/
diff --git a/src/test/modules/brin/Makefile b/src/test/modules/brin/Makefile
index 566655cd61..c871593906 100644
--- a/src/test/modules/brin/Makefile
+++ b/src/test/modules/brin/Makefile
@@ -1,12 +1,9 @@
 # src/test/modules/brin/Makefile
 
-# Note: because we don't tell the Makefile there are any regression tests,
-# we have to clean those result files explicitly
-EXTRA_CLEAN = $(pg_regress_clean_files) ./isolation_output
+EXTRA_INSTALL = contrib/pageinspect
 
-EXTRA_INSTALL=contrib/pageinspect
-
-ISOLATIONCHECKS=summarization-and-inprogress-insertion
+ISOLATION = summarization-and-inprogress-insertion
+TAP_TESTS = 1
 
 ifdef USE_PGXS
 PG_CONFIG = pg_config
@@ -18,19 +15,3 @@ top_builddir = ../../../..
 include $(top_builddir)/src/Makefile.global
 include $(top_srcdir)/contrib/contrib-global.mk
 endif
-
-check: isolation-check prove-check
-
-isolation-check: | submake-isolation temp-install
-	$(MKDIR_P) isolation_output
-	$(pg_isolation_regress_check) \
-	    --outputdir=./isolation_output \
-	    $(ISOLATIONCHECKS)
-
-prove-check: | temp-install
-	$(prove_check)
-
-.PHONY: check isolation-check prove-check
-
-submake-isolation:
-	$(MAKE) -C $(top_builddir)/src/test/isolation all
diff --git a/src/test/modules/commit_ts/Makefile b/src/test/modules/commit_ts/Makefile
index 6d4f3be358..7a24bb3c6d 100644
--- a/src/test/modules/commit_ts/Makefile
+++ b/src/test/modules/commit_ts/Makefile
@@ -2,6 +2,7 @@
 
 REGRESS = commit_timestamp
 REGRESS_OPTS = --temp-config=$(top_srcdir)/src/test/modules/commit_ts/commit_ts.conf
+TAP_TESTS = 1
 
 ifdef USE_PGXS
 PG_CONFIG = pg_config
@@ -13,8 +14,3 @@ top_builddir = ../../../..
 include $(top_builddir)/src/Makefile.global
 include $(top_srcdir)/contrib/contrib-global.mk
 endif
-
-check: prove-check
-
-prove-check: | temp-install
-	$(prove_check)
diff --git a/src/test/modules/test_pg_dump/Makefile b/src/test/modules/test_pg_dump/Makefile
index c64b353707..6123b994f6 100644
--- a/src/test/modules/test_pg_dump/Makefile
+++ b/src/test/modules/test_pg_dump/Makefile
@@ -7,6 +7,7 @@ EXTENSION = test_pg_dump
 DATA = test_pg_dump--1.0.sql
 
 REGRESS = test_pg_dump
+TAP_TESTS = 1
 
 ifdef USE_PGXS
 PG_CONFIG = pg_config
@@ -18,8 +19,3 @@ top_builddir = ../../../..
 include $(top_builddir)/src/Makefile.global
 include $(top_srcdir)/contrib/contrib-global.mk
 endif
-
-check: prove-check
-
-prove-check: | temp-install
-	$(prove_check)
#11Nikolay Shaplov
dhyan@nataraj.su
In reply to: Michael Paquier (#8)
Re: Add extension options to control TAP and isolation tests

В письме от 23 ноября 2018 09:43:45 пользователь Michael Paquier написал:

So, I continued exploring your patch. First I carefully read changes in
pgxs.mk. The only part of it I did not understand is

.PHONY: submake

-submake:
ifndef PGXS

+submake:
+ifdef REGRESS

$(MAKE) -C $(top_builddir)/src/test/regress pg_regress$(X)

endif

+ifdef ISOLATION
+   $(MAKE) -C $(top_builddir)/src/test/isolation all
+endif
+endif # PGXS

This is to make sure that the necessary tools are compiled before
running the related tests. "submake:" needs to be moved out actually.
Thinking about it a bit more, we should also remove the "ifdef REGRESS"
and "ifdef ISOLATION" because there are some dependencies here. For
example TAP tests call pg_regress to initialize connection policy. TAP
tests may also use isolation_tester or such.

Can you add some comments in this part of pgxs.mk that explains this thing
about pre-building needed tools? This will make understanding it more easy...

I suppose it is because the purpose of PGXS is never explained, not in the
documentation, not in the code comments, and PGXS := $(shell $(PG_CONFIG)
-- pgxs) sounds like some strange magic spell, that explains nothing. In
what cases it is defined? In what cases it is not defined? What exactly
does it store? Can we make things more easy to understand here?

That's part of a larger scope which is here:
https://www.postgresql.org/docs/11/extend-pgxs.html

I've carefully read this documentation. And did not get clear explanation of
what is the true purpose of PGXS environment variable. Only

"The last three lines should always be the same. Earlier in the file, you
assign variables or add custom make rules."

May be it should bot be discussed in the doc, but it should be mentioned in
pgxs.mk . I guess both PGXS nad NO_PGXS should be explicitly described, in
order to make the rest of the code more readable. (As for me I now have some
intuitive understanding of it's nature, but it would be better to have strict
explanation)

So about test_decoding

contrib/test_decoding/Makefile:

EXTRA_INSTALL=contrib/test_decoding

This sounds a bit strange to me. Why in make file for <some_extantions> we
should explicitly specify that this <some_extantions> is needed for tests. It
is obviously needed! Man, we are testing it!! ;-)

I would suspect that is should be added somewhere in pgxs.mk code, when it is
needed. Or this is not as obvious and trivial as I see it?

I guess it came from
-submake-test_decoding:
- $(MAKE) -C $(top_builddir)/contrib/test_decoding

but still there is no logic in it.

+REGRESS_OPTS = --temp-config $(top_srcdir)/contrib/test_decoding/logical.conf
+ISOLATION_OPTS = --temp-config 
$(top_srcdir)/contrib/test_decoding/logical.conf

When I read these lines an idea about ISOLATION_CONF and REGRESS_CONF
variables came into my mind. That are transformed into proper _OPTS in pgxs.mk
? Since there is only one use case, may be it do not worth it. So I just speak
this thought aloud without any intention to make it reality.

- $(MAKE) -C $(top_builddir)/src/test/regress all
is replaced with
$(MAKE) -C $(top_builddir)/src/test/regress pg_regress$(X)
in pgxs.mk. I hope changing "all" to "pg_regress$(X)" is ok, because I do not
understand it.

I've also greped for "pg_isolation_regress_check" and found that it is also
used in src/test/modules/snapshot_too_old that also uses PGXS (at least as an
option) should not we include it in your patch too?

So I think that is it. Since Artur said, he successfully used your TAP patch
in other extensions, I will not do it, considering it is already checked. If
you think it is good to recheck, I can do it too :-)

#12Michael Paquier
michael@paquier.xyz
In reply to: Nikolay Shaplov (#11)
Re: Add extension options to control TAP and isolation tests

On Sun, Nov 25, 2018 at 05:49:42PM +0300, Nikolay Shaplov wrote:

I've carefully read this documentation. And did not get clear explanation of
what is the true purpose of PGXS environment variable. Only

"The last three lines should always be the same. Earlier in the file, you
assign variables or add custom make rules."

The definition of PGXS is here:
https://www.postgresql.org/docs/11/extend-pgxs.html

May be it should bot be discussed in the doc, but it should be mentioned in
pgxs.mk . I guess both PGXS nad NO_PGXS should be explicitly described, in
order to make the rest of the code more readable. (As for me I now have some
intuitive understanding of it's nature, but it would be better to have strict
explanation)

I am not sure that documenting NO_PGXS makes much sense for extensions,
which have normally no need of the surrounding code. If you have
suggestions of improvements for the existing docs, please feel free to
think about a patch and then post it. Docs improvements are a
never-ending task.

When I read these lines an idea about ISOLATION_CONF and REGRESS_CONF
variables came into my mind. That are transformed into proper _OPTS in pgxs.mk
? Since there is only one use case, may be it do not worth it. So I just speak
this thought aloud without any intention to make it reality.

ISOLATION_OPTS and REGRESS_OPTS already allow to pass down custom
options, and are more extensible than the _CONF variants proposed here.
Keeping the number of options low is not a bad idea either.

I've also greped for "pg_isolation_regress_check" and found that it is also
used in src/test/modules/snapshot_too_old that also uses PGXS (at least as an
option) should not we include it in your patch too?

Good point. This can be cleaned up too, so done.

So I think that is it. Since Artur said, he successfully used your TAP patch
in other extensions, I will not do it, considering it is already checked. If
you think it is good to recheck, I can do it too :-)

Thanks, I have re-checked, and the thing is working as I would expect.
So committed.
--
Michael

#13Nikolay Shaplov
dhyan@nataraj.su
In reply to: Nikolay Shaplov (#11)
Re: Add extension options to control TAP and isolation tests

В письме от 26 ноября 2018 08:53:20 Вы написали:

I've carefully read this documentation. And did not get clear explanation
of what is the true purpose of PGXS environment variable. Only

"The last three lines should always be the same. Earlier in the file, you
assign variables or add custom make rules."

The definition of PGXS is here:
https://www.postgresql.org/docs/11/extend-pgxs.html

Can you please provide the quotation? I see there PGXS mentioned several times
as "a build infrastructure for extensions" and as an environment variable it
is mentioned only in code sample

PGXS := $(shell $(PG_CONFIG) --pgxs)
include $(PGXS)

So for me there is no explanation. Or it is hard to find (that is also bad)

If we are done with your patch, can we still finish this line of discussion in
order to create another small patch concerning PGXS-env-var comment?

#14Michael Paquier
michael@paquier.xyz
In reply to: Nikolay Shaplov (#13)
Re: Add extension options to control TAP and isolation tests

On Mon, Nov 26, 2018 at 10:50:35AM +0300, Nikolay Shaplov wrote:

I see there PGXS mentioned several times as "a build infrastructure
for extensions" and as an environment variable it is mentioned only in
code sample

As a variable PGXS stands for the location of the makefile which holds
all the basic rules an extension can use, as pointed for example by
pg_config --pgxs when it comes to extensions compiled out of the tree.
--
Michael

#15Michael Paquier
michael@paquier.xyz
In reply to: Michael Paquier (#8)
1 attachment(s)
Re: Add extension options to control TAP and isolation tests

On Fri, Nov 23, 2018 at 09:43:45AM +0900, Michael Paquier wrote:

That's for the main regression test suite within src/test/regress, TAP
is also a regression test suite, the page of the link reflects the
general set of test suites available.

The main patch had to be reverted a couple of days ago via 1d7dd18, and
I have been digging and fixing the set of issues the buildfarm has been
complaining about since. There were two problems:
- MSVC scripts were not able to handle modules with NO_INSTALLCHECK
defined, and vcregress.pl runs with the assumption that installcheck is
used. Support for that has been added in 431f159. Some modules have
been also lacking NO_INSTALLCHECK.
- contrib/bloom/ are unstable. I have not spent time investigating the
actual root issue, but spawned a thread with the authors of the tests:
/messages/by-id/20181126025125.GH1776@paquier.xyz

I am attaching the patch I would like to commit to close this thread,
which has as single change TAP_TESTS commented out in contrib/bloom/ to
not explode the buildfarm. Any objections to that?
--
Michael

Attachments:

0001-Add-PGXS-options-to-control-TAP-and-isolation-tests-.patchtext/x-diff; charset=iso-8859-1Download
From 7fd3112342b1d7ea94ac7f8aa96f07579b35ce9a Mon Sep 17 00:00:00 2001
From: Michael Paquier <michael@paquier.xyz>
Date: Mon, 26 Nov 2018 08:39:19 +0900
Subject: [PATCH] Add PGXS options to control TAP and isolation tests, take two
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The following options are added for extensions:
- TAP_TESTS, to allow an extention to run TAP tests which are the ones
present in t/*.pl.  A subset of tests can always be run with the
existing PROVE_TESTS for developers.
- ISOLATION, to define a list of isolation tests.
- ISOLATION_OPTS, to pass custom options to isolation_tester.

A couple of custom Makefile rules have been accumulated across the tree
to cover the lack of facility in PGXS for a couple of releases when
using those test suites, which are all now replaced with the new flags,
without reducing the test coverage.  Note that tests of contrib/bloom/
are not enabled yet, as those are proving unstable in the buildfarm.

Author: Michael Paquier
Reviewed-by: Adam Berlin, Álvaro Herrera, Tom Lane, Nikolay Shaplov,
Arthur Zakirov
Discussion: https://postgr.es/m/20180906014849.GG2726@paquier.xyz
---
 contrib/bloom/Makefile                     |  7 ++-
 contrib/oid2name/Makefile                  | 10 +---
 contrib/test_decoding/Makefile             | 67 ++++++----------------
 contrib/vacuumlo/Makefile                  | 10 +---
 doc/src/sgml/extend.sgml                   | 61 +++++++++++++++++++-
 src/makefiles/pgxs.mk                      | 32 ++++++++++-
 src/test/modules/brin/.gitignore           |  2 +-
 src/test/modules/brin/Makefile             | 25 +-------
 src/test/modules/commit_ts/Makefile        |  7 +--
 src/test/modules/snapshot_too_old/Makefile | 35 +++--------
 src/test/modules/test_pg_dump/Makefile     |  6 +-
 11 files changed, 129 insertions(+), 133 deletions(-)

diff --git a/contrib/bloom/Makefile b/contrib/bloom/Makefile
index 13bd397b70..146878870e 100644
--- a/contrib/bloom/Makefile
+++ b/contrib/bloom/Makefile
@@ -9,6 +9,10 @@ PGFILEDESC = "bloom access method - signature file based index"
 
 REGRESS = bloom
 
+# Disable TAP tests for this module for now, as these are unstable on several
+# buildfarm environments.
+# TAP_TESTS = 1
+
 ifdef USE_PGXS
 PG_CONFIG = pg_config
 PGXS := $(shell $(PG_CONFIG) --pgxs)
@@ -19,6 +23,3 @@ top_builddir = ../..
 include $(top_builddir)/src/Makefile.global
 include $(top_srcdir)/contrib/contrib-global.mk
 endif
-
-wal-check: temp-install
-	$(prove_check)
diff --git a/contrib/oid2name/Makefile b/contrib/oid2name/Makefile
index 908e078714..361a80a7a1 100644
--- a/contrib/oid2name/Makefile
+++ b/contrib/oid2name/Makefile
@@ -6,11 +6,11 @@ PGAPPICON = win32
 PROGRAM = oid2name
 OBJS	= oid2name.o $(WIN32RES)
 
+TAP_TESTS = 1
+
 PG_CPPFLAGS = -I$(libpq_srcdir)
 PG_LIBS_INTERNAL = $(libpq_pgport)
 
-EXTRA_CLEAN = tmp_check
-
 ifdef USE_PGXS
 PG_CONFIG = pg_config
 PGXS := $(shell $(PG_CONFIG) --pgxs)
@@ -21,9 +21,3 @@ top_builddir = ../..
 include $(top_builddir)/src/Makefile.global
 include $(top_srcdir)/contrib/contrib-global.mk
 endif
-
-check:
-	$(prove_check)
-
-installcheck:
-	$(prove_installcheck)
diff --git a/contrib/test_decoding/Makefile b/contrib/test_decoding/Makefile
index afcab930f7..8cd83a763f 100644
--- a/contrib/test_decoding/Makefile
+++ b/contrib/test_decoding/Makefile
@@ -3,9 +3,20 @@
 MODULES = test_decoding
 PGFILEDESC = "test_decoding - example of a logical decoding output plugin"
 
-# Note: because we don't tell the Makefile there are any regression tests,
-# we have to clean those result files explicitly
-EXTRA_CLEAN = $(pg_regress_clean_files)
+EXTRA_INSTALL=contrib/test_decoding
+
+REGRESS = ddl xact rewrite toast permissions decoding_in_xact \
+	decoding_into_rel binary prepared replorigin time messages \
+	spill slot truncate
+ISOLATION = mxact delayed_startup ondisk_startup concurrent_ddl_dml \
+	oldest_xmin snapshot_transfer
+
+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
+# typical installcheck users do not have (e.g. buildfarm clients).
+NO_INSTALLCHECK = 1
 
 ifdef USE_PGXS
 PG_CONFIG = pg_config
@@ -18,52 +29,8 @@ include $(top_builddir)/src/Makefile.global
 include $(top_srcdir)/contrib/contrib-global.mk
 endif
 
-# Disabled because these tests require "wal_level=logical", which
-# typical installcheck users do not have (e.g. buildfarm clients).
-installcheck:;
-
 # But it can nonetheless be very helpful to run tests on preexisting
 # installation, allow to do so, but only if requested explicitly.
-installcheck-force: regresscheck-install-force isolationcheck-install-force
-
-check: regresscheck isolationcheck
-
-submake-regress:
-	$(MAKE) -C $(top_builddir)/src/test/regress all
-
-submake-isolation:
-	$(MAKE) -C $(top_builddir)/src/test/isolation all
-
-submake-test_decoding:
-	$(MAKE) -C $(top_builddir)/contrib/test_decoding
-
-REGRESSCHECKS=ddl xact rewrite toast permissions decoding_in_xact \
-	decoding_into_rel binary prepared replorigin time messages \
-	spill slot truncate
-
-regresscheck: | submake-regress submake-test_decoding temp-install
-	$(pg_regress_check) \
-	    --temp-config $(top_srcdir)/contrib/test_decoding/logical.conf \
-	    $(REGRESSCHECKS)
-
-regresscheck-install-force: | submake-regress submake-test_decoding temp-install
-	$(pg_regress_installcheck) \
-	    $(REGRESSCHECKS)
-
-ISOLATIONCHECKS=mxact delayed_startup ondisk_startup concurrent_ddl_dml \
-	oldest_xmin snapshot_transfer
-
-isolationcheck: | submake-isolation submake-test_decoding temp-install
-	$(pg_isolation_regress_check) \
-	    --temp-config $(top_srcdir)/contrib/test_decoding/logical.conf \
-	    $(ISOLATIONCHECKS)
-
-isolationcheck-install-force: all | submake-isolation submake-test_decoding temp-install
-	$(pg_isolation_regress_installcheck) \
-	    $(ISOLATIONCHECKS)
-
-.PHONY: submake-test_decoding submake-regress check \
-	regresscheck regresscheck-install-force \
-	isolationcheck isolationcheck-install-force
-
-temp-install: EXTRA_INSTALL=contrib/test_decoding
+installcheck-force:
+	$(pg_regress_installcheck) $(REGRESS)
+	$(pg_isolation_regress_installcheck) $(ISOLATION)
diff --git a/contrib/vacuumlo/Makefile b/contrib/vacuumlo/Makefile
index 5de506151e..3efcb46735 100644
--- a/contrib/vacuumlo/Makefile
+++ b/contrib/vacuumlo/Makefile
@@ -6,11 +6,11 @@ PGAPPICON = win32
 PROGRAM = vacuumlo
 OBJS	= vacuumlo.o $(WIN32RES)
 
+TAP_TESTS = 1
+
 PG_CPPFLAGS = -I$(libpq_srcdir)
 PG_LIBS_INTERNAL = $(libpq_pgport)
 
-EXTRA_CLEAN = tmp_check
-
 ifdef USE_PGXS
 PG_CONFIG = pg_config
 PGXS := $(shell $(PG_CONFIG) --pgxs)
@@ -21,9 +21,3 @@ top_builddir = ../..
 include $(top_builddir)/src/Makefile.global
 include $(top_srcdir)/contrib/contrib-global.mk
 endif
-
-check:
-	$(prove_check)
-
-installcheck:
-	$(prove_installcheck)
diff --git a/doc/src/sgml/extend.sgml b/doc/src/sgml/extend.sgml
index 695e07fb38..a6b77c1cfe 100644
--- a/doc/src/sgml/extend.sgml
+++ b/doc/src/sgml/extend.sgml
@@ -1303,6 +1303,34 @@ include $(PGXS)
       </listitem>
      </varlistentry>
 
+     <varlistentry>
+      <term><varname>ISOLATION</varname></term>
+      <listitem>
+       <para>
+        list of isolation test cases, see below for more details
+       </para>
+      </listitem>
+     </varlistentry>
+
+     <varlistentry>
+      <term><varname>ISOLATION_OPTS</varname></term>
+      <listitem>
+       <para>
+        additional switches to pass to
+        <application>pg_isolation_regress</application>
+       </para>
+      </listitem>
+     </varlistentry>
+
+     <varlistentry>
+      <term><varname>TAP_TESTS</varname></term>
+      <listitem>
+       <para>
+        switch defining if TAP tests need to be run, see below
+       </para>
+      </listitem>
+     </varlistentry>
+
      <varlistentry>
       <term><varname>NO_INSTALLCHECK</varname></term>
       <listitem>
@@ -1423,13 +1451,42 @@ make VPATH=/path/to/extension/source/tree install
     have all expected files.
    </para>
 
+   <para>
+    The scripts listed in the <varname>ISOLATION</varname> variable are used
+    for tests stressing behavior of concurrent session with your module, which
+    can be invoked by <literal>make installcheck</literal> after doing
+    <literal>make install</literal>.  For this to work you must have a
+    running <productname>PostgreSQL</productname> server.  The script files
+    listed in <varname>ISOLATION</varname> must appear in a subdirectory
+    named <literal>specs/</literal> in your extension's directory.  These files
+    must have extension <literal>.spec</literal>, which must not be included
+    in the <varname>ISOLATION</varname> list in the makefile.  For each test
+    there should also be a file containing the expected output in a
+    subdirectory named <literal>expected/</literal>, with the same stem and
+    extension <literal>.out</literal>.  <literal>make installcheck</literal>
+    executes each test script, and compares the resulting output to the
+    matching expected file.  Any differences will be written to the file
+    <literal>output_iso/regression.diffs</literal> in
+    <command>diff -c</command> format.  Note that trying to run a test that is
+    missing its expected file will be reported as <quote>trouble</quote>, so
+    make sure you have all expected files.
+   </para>
+
+   <para>
+    <literal>TAP_TESTS</literal> enables the use of TAP tests.  Data from each
+    run is present in a subdirectory named <literal>tmp_check/</literal>.
+    See also <xref linkend="regress-tap"/> for more details.
+   </para>
+
    <tip>
     <para>
      The easiest way to create the expected files is to create empty files,
      then do a test run (which will of course report differences).  Inspect
      the actual result files found in the <literal>results/</literal>
-     directory, then copy them to <literal>expected/</literal> if they match
-     what you expect from the test.
+     directory (for tests in <literal>REGRESS</literal>), or
+     <literal>output_iso/results/</literal> directory (for tests in
+     <literal>ISOLATION</literal>), then copy them to
+     <literal>expected/</literal> if they match what you expect from the test.
     </para>
 
    </tip>
diff --git a/src/makefiles/pgxs.mk b/src/makefiles/pgxs.mk
index 070d151018..e8fecaec0f 100644
--- a/src/makefiles/pgxs.mk
+++ b/src/makefiles/pgxs.mk
@@ -46,6 +46,9 @@
 #   HEADERS_built_$(MODULE) -- as above but built first (also NOT cleaned)
 #   REGRESS -- list of regression test cases (without suffix)
 #   REGRESS_OPTS -- additional switches to pass to pg_regress
+#   TAP_TESTS -- switch to enable TAP tests
+#   ISOLATION -- list of isolation test cases
+#   ISOLATION_OPTS -- additional switches to pass to pg_isolation_regress
 #   NO_INSTALLCHECK -- don't define an installcheck target, useful e.g. if
 #     tests require special configuration, or don't use pg_regress
 #   EXTRA_CLEAN -- extra files to remove in 'make clean'
@@ -349,6 +352,12 @@ ifeq ($(PORTNAME), win)
 	rm -f regress.def
 endif
 endif # REGRESS
+ifdef TAP_TESTS
+	rm -rf tmp_check/
+endif
+ifdef ISOLATION
+	rm -rf output_iso/ tmp_check_iso/
+endif
 
 ifdef MODULE_big
 clean: clean-lib
@@ -383,28 +392,47 @@ $(test_files_build): $(abs_builddir)/%: $(srcdir)/%
 	$(MKDIR_P) $(dir $@)
 	ln -s $< $@
 endif # VPATH
+endif # REGRESS
 
 .PHONY: submake
 submake:
 ifndef PGXS
 	$(MAKE) -C $(top_builddir)/src/test/regress pg_regress$(X)
+	$(MAKE) -C $(top_builddir)/src/test/isolation all
 endif
 
-# against installed postmaster
+# Standard rules to run regression tests including multiple test suites.
+# Runs against an installed postmaster
 ifndef NO_INSTALLCHECK
 installcheck: submake $(REGRESS_PREP)
+ifdef REGRESS
 	$(pg_regress_installcheck) $(REGRESS_OPTS) $(REGRESS)
 endif
+ifdef ISOLATION
+	$(pg_isolation_regress_installcheck) $(ISOLATION_OPTS) $(ISOLATION)
+endif
+ifdef TAP_TESTS
+	$(prove_installcheck)
+endif
+endif # NO_INSTALLCHECK
 
+# Runs independently of any installation
 ifdef PGXS
 check:
 	@echo '"$(MAKE) check" is not supported.'
 	@echo 'Do "$(MAKE) install", then "$(MAKE) installcheck" instead.'
 else
 check: submake $(REGRESS_PREP)
+ifdef REGRESS
 	$(pg_regress_check) $(REGRESS_OPTS) $(REGRESS)
 endif
-endif # REGRESS
+ifdef ISOLATION
+	$(pg_isolation_regress_check) $(ISOLATION_OPTS) $(ISOLATION)
+endif
+ifdef TAP_TESTS
+	$(prove_check)
+endif
+endif # PGXS
 
 ifndef NO_TEMP_INSTALL
 temp-install: EXTRA_INSTALL+=$(subdir)
diff --git a/src/test/modules/brin/.gitignore b/src/test/modules/brin/.gitignore
index 62bbe8f6b1..44f600cb6c 100644
--- a/src/test/modules/brin/.gitignore
+++ b/src/test/modules/brin/.gitignore
@@ -1,3 +1,3 @@
 # Generated subdirectories
-/isolation_output/
+/output_iso/
 /tmp_check/
diff --git a/src/test/modules/brin/Makefile b/src/test/modules/brin/Makefile
index 566655cd61..c871593906 100644
--- a/src/test/modules/brin/Makefile
+++ b/src/test/modules/brin/Makefile
@@ -1,12 +1,9 @@
 # src/test/modules/brin/Makefile
 
-# Note: because we don't tell the Makefile there are any regression tests,
-# we have to clean those result files explicitly
-EXTRA_CLEAN = $(pg_regress_clean_files) ./isolation_output
+EXTRA_INSTALL = contrib/pageinspect
 
-EXTRA_INSTALL=contrib/pageinspect
-
-ISOLATIONCHECKS=summarization-and-inprogress-insertion
+ISOLATION = summarization-and-inprogress-insertion
+TAP_TESTS = 1
 
 ifdef USE_PGXS
 PG_CONFIG = pg_config
@@ -18,19 +15,3 @@ top_builddir = ../../../..
 include $(top_builddir)/src/Makefile.global
 include $(top_srcdir)/contrib/contrib-global.mk
 endif
-
-check: isolation-check prove-check
-
-isolation-check: | submake-isolation temp-install
-	$(MKDIR_P) isolation_output
-	$(pg_isolation_regress_check) \
-	    --outputdir=./isolation_output \
-	    $(ISOLATIONCHECKS)
-
-prove-check: | temp-install
-	$(prove_check)
-
-.PHONY: check isolation-check prove-check
-
-submake-isolation:
-	$(MAKE) -C $(top_builddir)/src/test/isolation all
diff --git a/src/test/modules/commit_ts/Makefile b/src/test/modules/commit_ts/Makefile
index 6a9c3971fb..113bcfa210 100644
--- a/src/test/modules/commit_ts/Makefile
+++ b/src/test/modules/commit_ts/Makefile
@@ -6,6 +6,8 @@ REGRESS_OPTS = --temp-config=$(top_srcdir)/src/test/modules/commit_ts/commit_ts.
 # which typical installcheck users do not have (e.g. buildfarm clients).
 NO_INSTALLCHECK = 1
 
+TAP_TESTS = 1
+
 ifdef USE_PGXS
 PG_CONFIG = pg_config
 PGXS := $(shell $(PG_CONFIG) --pgxs)
@@ -16,8 +18,3 @@ top_builddir = ../../../..
 include $(top_builddir)/src/Makefile.global
 include $(top_srcdir)/contrib/contrib-global.mk
 endif
-
-check: prove-check
-
-prove-check: | temp-install
-	$(prove_check)
diff --git a/src/test/modules/snapshot_too_old/Makefile b/src/test/modules/snapshot_too_old/Makefile
index b6d998f320..dfb4537f63 100644
--- a/src/test/modules/snapshot_too_old/Makefile
+++ b/src/test/modules/snapshot_too_old/Makefile
@@ -4,7 +4,12 @@
 # we have to clean those result files explicitly
 EXTRA_CLEAN = $(pg_regress_clean_files)
 
-ISOLATIONCHECKS=sto_using_cursor sto_using_select sto_using_hash_index
+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
+
+# Disabled because these tests require "old_snapshot_threshold" >= 0, which
+# typical installcheck users do not have (e.g. buildfarm clients).
+NO_INSTALLCHECK = 1
 
 ifdef USE_PGXS
 PG_CONFIG = pg_config
@@ -17,31 +22,7 @@ include $(top_builddir)/src/Makefile.global
 include $(top_srcdir)/contrib/contrib-global.mk
 endif
 
-# Disabled because these tests require "old_snapshot_threshold" >= 0, which
-# typical installcheck users do not have (e.g. buildfarm clients).
-installcheck:;
-
 # But it can nonetheless be very helpful to run tests on preexisting
 # installation, allow to do so, but only if requested explicitly.
-installcheck-force: isolationcheck-install-force
-
-check: isolationcheck
-
-submake-isolation:
-	$(MAKE) -C $(top_builddir)/src/test/isolation all
-
-submake-test_snapshot_too_old:
-	$(MAKE) -C $(top_builddir)/src/test/modules/snapshot_too_old
-
-isolationcheck: | submake-isolation submake-test_snapshot_too_old temp-install
-	$(pg_isolation_regress_check) \
-	    --temp-config $(top_srcdir)/src/test/modules/snapshot_too_old/sto.conf \
-	    $(ISOLATIONCHECKS)
-
-isolationcheck-install-force: all | submake-isolation submake-test_snapshot_too_old temp-install
-	$(pg_isolation_regress_installcheck) \
-	    $(ISOLATIONCHECKS)
-
-.PHONY: check submake-test_snapshot_too_old isolationcheck isolationcheck-install-force
-
-temp-install: EXTRA_INSTALL=src/test/modules/snapshot_too_old
+installcheck-force:
+	$(pg_isolation_regress_installcheck) $(ISOLATION)
diff --git a/src/test/modules/test_pg_dump/Makefile b/src/test/modules/test_pg_dump/Makefile
index c64b353707..6123b994f6 100644
--- a/src/test/modules/test_pg_dump/Makefile
+++ b/src/test/modules/test_pg_dump/Makefile
@@ -7,6 +7,7 @@ EXTENSION = test_pg_dump
 DATA = test_pg_dump--1.0.sql
 
 REGRESS = test_pg_dump
+TAP_TESTS = 1
 
 ifdef USE_PGXS
 PG_CONFIG = pg_config
@@ -18,8 +19,3 @@ top_builddir = ../../../..
 include $(top_builddir)/src/Makefile.global
 include $(top_srcdir)/contrib/contrib-global.mk
 endif
-
-check: prove-check
-
-prove-check: | temp-install
-	$(prove_check)
-- 
2.19.1

#16Arthur Zakirov
a.zakirov@postgrespro.ru
In reply to: Michael Paquier (#15)
Re: Add extension options to control TAP and isolation tests

On 29.11.2018 05:00, Michael Paquier wrote:

The main patch had to be reverted a couple of days ago via 1d7dd18, and
I have been digging and fixing the set of issues the buildfarm has been
complaining about since. There were two problems:

Thank you for working on the patch.

I run world-check. It works perfectly. I think the patch is in good shape.

I am attaching the patch I would like to commit to close this thread,
which has as single change TAP_TESTS commented out in contrib/bloom/ to
not explode the buildfarm. Any objections to that?

As far as I understand bloom TAP tests are stable on some platforms,
such as linux. Can be TAP test disabled only for some specific
platforms? For example (as far as I know 'darwin' refers to MacOS):

ifeq (,$(filter win32 darwin,$(PORTNAME)))
TAP_TESTS = 1
endif

PS. But after rereading the Makefile I see that the problem here is to
get $(PORTNAME). It is defined in Makefile.global, which is included
below defining TAP_TESTS variable.

--
Arthur Zakirov
Postgres Professional: http://www.postgrespro.com
Russian Postgres Company

#17Michael Paquier
michael@paquier.xyz
In reply to: Arthur Zakirov (#16)
Re: Add extension options to control TAP and isolation tests

On Fri, Nov 30, 2018 at 02:18:04PM +0300, Arthur Zakirov wrote:

As far as I understand bloom TAP tests are stable on some platforms, such as
linux. Can be TAP test disabled only for some specific platforms?

That can be done with PORTNAME. However I am doubting that those tests
are even reliable on Linux.. So it would be much better to understand
the root issue before enabling them.
--
Michael

#18Michael Paquier
michael@paquier.xyz
In reply to: Michael Paquier (#15)
Re: Add extension options to control TAP and isolation tests

On Thu, Nov 29, 2018 at 11:00:11AM +0900, Michael Paquier wrote:

I am attaching the patch I would like to commit to close this thread,
which has as single change TAP_TESTS commented out in contrib/bloom/ to
not explode the buildfarm. Any objections to that?

And I gave this stuff another shot with d3c09b9. Fingers crossed.
--
Michael