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
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)
signature.asc
Description: PGP signature
