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