9.6 TAP tests and extensions
Hi all
While updating an extension for 9.6 I noticed that while the
$(prove_check) definition is exposed for use by PGXS in
Makefile.global, extensions can't actually use the TAP tests because
we don't install the required Perl modules like PostgresNode.pm.
I don't see any reason not to make this available to extension authors
and doing so is harmless, so here's a small patch to install it. I
think it's reasonable to add this to 9.6 even at this late stage; IMO
it should've been installed from the beginning. They're only installed
if --enable-tap-tests is set, since otherwise $(prove_check) will just
error out with "TAP tests not enabled" anyway.
Not having this in 9.6 will make it way harder for extension authors
to benefit from the new TAP tooling.
Another patch allows the isolation tester to be installed too, again
so that extensions can use it.
The final patch just adds a comment to src/test/Makefile to warn that
src/Makefile doesn't call it directly, because I was confused as to
why 'make -C src/test install' installed everything, but 'make
install' did not.
Sorry this is so late in the piece. It only came up when I switched
from 10.0 dev/review to updating extensions for 9.6. But it's just
adding installed files and I think it's worth doing.
Another small patch (pending) will be needed because we look for
pg_regress in the wrong place when running out-of-tree with
$(prove_installcheck).
Can't exec "/home/craig/projects/2Q/postgres-bdr-extension//home/craig/pg/96/lib/postgresql/pgxs/src/makefiles/../../src/test/regress/pg_regress":
No such file or directory at
/home/craig/pg/96/lib/postgresql/pgxs/src/makefiles/../../src/test/perl/TestLib.pm
line 151.
$(prove_check) won't be usable because it assumes a temp install in
./tmp_install but that's to be expected.
--
Craig Ringer http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
Attachments:
0001-Install-the-Perl-TAP-tests.patchtext/x-patch; charset=US-ASCII; name=0001-Install-the-Perl-TAP-tests.patchDownload
From 1974ef1e771e28c39d5f6acb29c648e864b0f057 Mon Sep 17 00:00:00 2001
From: Craig Ringer <craig@2ndquadrant.com>
Date: Tue, 13 Sep 2016 11:06:58 +0800
Subject: [PATCH 1/3] Install the Perl TAP tests
---
src/Makefile | 3 ++-
src/test/Makefile | 2 +-
src/test/perl/GNUmakefile | 39 +++++++++++++++++++++++++++++++++++++++
3 files changed, 42 insertions(+), 2 deletions(-)
create mode 100644 src/test/perl/GNUmakefile
diff --git a/src/Makefile b/src/Makefile
index b526be7..977f80b 100644
--- a/src/Makefile
+++ b/src/Makefile
@@ -26,7 +26,8 @@ SUBDIRS = \
bin \
pl \
makefiles \
- test/regress
+ test/regress \
+ test/perl
# There are too many interdependencies between the subdirectories, so
# don't attempt parallel make here.
diff --git a/src/test/Makefile b/src/test/Makefile
index 7f7754f..6b40cf5 100644
--- a/src/test/Makefile
+++ b/src/test/Makefile
@@ -12,7 +12,7 @@ subdir = src/test
top_builddir = ../..
include $(top_builddir)/src/Makefile.global
-SUBDIRS = regress isolation modules recovery
+SUBDIRS = perl regress isolation modules recovery
# We don't build or execute examples/, locale/, or thread/ by default,
# but we do want "make clean" etc to recurse into them. Likewise for ssl/,
diff --git a/src/test/perl/GNUmakefile b/src/test/perl/GNUmakefile
new file mode 100644
index 0000000..810a712
--- /dev/null
+++ b/src/test/perl/GNUmakefile
@@ -0,0 +1,39 @@
+#-------------------------------------------------------------------------
+#
+# Makefile for src/test/perl
+#
+# Portions Copyright (c) 1996-2016, PostgreSQL Global Development Group
+# Portions Copyright (c) 1994, Regents of the University of California
+#
+# src/test/perl/Makefile
+#
+#-------------------------------------------------------------------------
+
+subdir = src/test/perl
+top_builddir = ../../..
+include $(top_builddir)/src/Makefile.global
+
+installdirs:
+ $(MKDIR_P) '$(DESTDIR)$(pgxsdir)/$(subdir)'
+
+ifeq ($(enable_tap_tests),yes)
+
+install: all installdirs
+ $(INSTALL_PROGRAM) TestLib.pm '$(DESTDIR)$(pgxsdir)/$(subdir)/TestLib.pm'
+ $(INSTALL_PROGRAM) SimpleTee.pm '$(DESTDIR)$(pgxsdir)/$(subdir)/SimpleTee.pm'
+ $(INSTALL_PROGRAM) RecursiveCopy.pm '$(DESTDIR)$(pgxsdir)/$(subdir)/RecursiveCopy.pm'
+ $(INSTALL_PROGRAM) PostgresNode.pm '$(DESTDIR)$(pgxsdir)/$(subdir)/PostgresNode.pm'
+
+uninstall:
+ rm -f '$(DESTDIR)$(pgxsdir)/$(subdir)/TestLib.pm'
+ rm -f '$(DESTDIR)$(pgxsdir)/$(subdir)/SimpleTee.pm'
+ rm -f '$(DESTDIR)$(pgxsdir)/$(subdir)/RecursiveCopy.pm'
+ rm -f '$(DESTDIR)$(pgxsdir)/$(subdir)/PostgresNode.pm'
+
+else
+
+install: ;
+
+uninstall: ;
+
+endif
--
2.5.5
0002-Add-install-rules-for-isolation-tester.patchtext/x-patch; charset=US-ASCII; name=0002-Add-install-rules-for-isolation-tester.patchDownload
From bc023c43e44a615aaf310e13edeea97ca8928ce6 Mon Sep 17 00:00:00 2001
From: Craig Ringer <craig@2ndquadrant.com>
Date: Tue, 13 Sep 2016 11:00:41 +0800
Subject: [PATCH 2/3] Add install rules for isolation tester
Allow 'make install' for the isolation tester to work so it can be
used from PGXS extensions.
---
src/Makefile | 1 +
src/test/isolation/Makefile | 11 +++++++++++
2 files changed, 12 insertions(+)
diff --git a/src/Makefile b/src/Makefile
index 977f80b..d4aa06b 100644
--- a/src/Makefile
+++ b/src/Makefile
@@ -27,6 +27,7 @@ SUBDIRS = \
pl \
makefiles \
test/regress \
+ test/isolation \
test/perl
# There are too many interdependencies between the subdirectories, so
diff --git a/src/test/isolation/Makefile b/src/test/isolation/Makefile
index 3d272d5..e111bf0 100644
--- a/src/test/isolation/Makefile
+++ b/src/test/isolation/Makefile
@@ -66,3 +66,14 @@ installcheck-prepared-txns: all temp-install
check-prepared-txns: all temp-install
./pg_isolation_regress --temp-instance=./tmp_check $(TEMP_CONF) $(EXTRA_REGRESS_OPTS) --inputdir=$(srcdir) --schedule=$(srcdir)/isolation_schedule prepared-transactions
+
+install: all installdirs
+ $(INSTALL_PROGRAM) isolationtester$(X) '$(DESTDIR)$(pgxsdir)/$(subdir)/isolationtester$(X)'
+ $(INSTALL_PROGRAM) pg_isolation_regress$(X) '$(DESTDIR)$(pgxsdir)/$(subdir)/pg_isolation_regress$(X)'
+
+installdirs:
+ $(MKDIR_P) '$(DESTDIR)$(pgxsdir)/$(subdir)'
+
+uninstall:
+ rm -f '$(DESTDIR)$(pgxsdir)/$(subdir)/isolationtester$(X)'
+ rm -f '$(DESTDIR)$(pgxsdir)/$(subdir)/pg_isolation_regress$(X)'
--
2.5.5
0003-Note-that-src-test-Makefile-is-not-called-from-src-M.patchtext/x-patch; charset=US-ASCII; name=0003-Note-that-src-test-Makefile-is-not-called-from-src-M.patchDownload
From 58daf37d03c2f01533737fffdcdd15949b4cbbad Mon Sep 17 00:00:00 2001
From: Craig Ringer <craig@2ndquadrant.com>
Date: Tue, 13 Sep 2016 11:48:43 +0800
Subject: [PATCH 3/3] Note that src/test/Makefile is not called from
src/Makefile
Add a comment to help developers who're editing src/test/Makefile.
---
src/test/Makefile | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/src/test/Makefile b/src/test/Makefile
index 6b40cf5..a24071e 100644
--- a/src/test/Makefile
+++ b/src/test/Makefile
@@ -11,6 +11,10 @@
subdir = src/test
top_builddir = ../..
include $(top_builddir)/src/Makefile.global
+
+# Note that src/test/Makefile is not normally called recursively by
+# src/Makefile; instead, individual subdirectories are called directly
+# from its SUBDIRS.
SUBDIRS = perl regress isolation modules recovery
--
2.5.5
This was wrong for out-of-tree builds, updated.
Still pending fix for PG_REGRESS path when invoked using
$(prove_check) from PGXS
--
Craig Ringer http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
Attachments:
0001-Install-the-Perl-TAP-tests.patchtext/x-patch; charset=US-ASCII; name=0001-Install-the-Perl-TAP-tests.patchDownload
From 462a0ab51935b45d17820b83b8e9f6abd4ad2904 Mon Sep 17 00:00:00 2001
From: Craig Ringer <craig@2ndquadrant.com>
Date: Tue, 13 Sep 2016 11:06:58 +0800
Subject: [PATCH 1/3] Install the Perl TAP tests
---
src/Makefile | 3 ++-
src/test/Makefile | 2 +-
src/test/perl/GNUmakefile | 39 +++++++++++++++++++++++++++++++++++++++
3 files changed, 42 insertions(+), 2 deletions(-)
create mode 100644 src/test/perl/GNUmakefile
diff --git a/src/Makefile b/src/Makefile
index b526be7..977f80b 100644
--- a/src/Makefile
+++ b/src/Makefile
@@ -26,7 +26,8 @@ SUBDIRS = \
bin \
pl \
makefiles \
- test/regress
+ test/regress \
+ test/perl
# There are too many interdependencies between the subdirectories, so
# don't attempt parallel make here.
diff --git a/src/test/Makefile b/src/test/Makefile
index 7f7754f..6b40cf5 100644
--- a/src/test/Makefile
+++ b/src/test/Makefile
@@ -12,7 +12,7 @@ subdir = src/test
top_builddir = ../..
include $(top_builddir)/src/Makefile.global
-SUBDIRS = regress isolation modules recovery
+SUBDIRS = perl regress isolation modules recovery
# We don't build or execute examples/, locale/, or thread/ by default,
# but we do want "make clean" etc to recurse into them. Likewise for ssl/,
diff --git a/src/test/perl/GNUmakefile b/src/test/perl/GNUmakefile
new file mode 100644
index 0000000..3c5dc70
--- /dev/null
+++ b/src/test/perl/GNUmakefile
@@ -0,0 +1,39 @@
+#-------------------------------------------------------------------------
+#
+# Makefile for src/test/perl
+#
+# Portions Copyright (c) 1996-2016, PostgreSQL Global Development Group
+# Portions Copyright (c) 1994, Regents of the University of California
+#
+# src/test/perl/Makefile
+#
+#-------------------------------------------------------------------------
+
+subdir = src/test/perl
+top_builddir = ../../..
+include $(top_builddir)/src/Makefile.global
+
+installdirs:
+ $(MKDIR_P) '$(DESTDIR)$(pgxsdir)/$(subdir)'
+
+ifeq ($(enable_tap_tests),yes)
+
+install: all installdirs
+ $(INSTALL_DATA) $(srcdir)/TestLib.pm '$(DESTDIR)$(pgxsdir)/$(subdir)/TestLib.pm'
+ $(INSTALL_DATA) $(srcdir)/SimpleTee.pm '$(DESTDIR)$(pgxsdir)/$(subdir)/SimpleTee.pm'
+ $(INSTALL_DATA) $(srcdir)/RecursiveCopy.pm '$(DESTDIR)$(pgxsdir)/$(subdir)/RecursiveCopy.pm'
+ $(INSTALL_DATA) $(srcdir)/PostgresNode.pm '$(DESTDIR)$(pgxsdir)/$(subdir)/PostgresNode.pm'
+
+uninstall:
+ rm -f '$(DESTDIR)$(pgxsdir)/$(subdir)/TestLib.pm'
+ rm -f '$(DESTDIR)$(pgxsdir)/$(subdir)/SimpleTee.pm'
+ rm -f '$(DESTDIR)$(pgxsdir)/$(subdir)/RecursiveCopy.pm'
+ rm -f '$(DESTDIR)$(pgxsdir)/$(subdir)/PostgresNode.pm'
+
+else
+
+install: ;
+
+uninstall: ;
+
+endif
--
2.5.5
0002-Add-install-rules-for-isolation-tester.patchtext/x-patch; charset=US-ASCII; name=0002-Add-install-rules-for-isolation-tester.patchDownload
From 86aff40374e05fec0160cbab0d1879bd01c6f411 Mon Sep 17 00:00:00 2001
From: Craig Ringer <craig@2ndquadrant.com>
Date: Tue, 13 Sep 2016 11:00:41 +0800
Subject: [PATCH 2/3] Add install rules for isolation tester
Allow 'make install' for the isolation tester to work so it can be
used from PGXS extensions.
---
src/Makefile | 1 +
src/test/isolation/Makefile | 11 +++++++++++
2 files changed, 12 insertions(+)
diff --git a/src/Makefile b/src/Makefile
index 977f80b..d4aa06b 100644
--- a/src/Makefile
+++ b/src/Makefile
@@ -27,6 +27,7 @@ SUBDIRS = \
pl \
makefiles \
test/regress \
+ test/isolation \
test/perl
# There are too many interdependencies between the subdirectories, so
diff --git a/src/test/isolation/Makefile b/src/test/isolation/Makefile
index 3d272d5..e111bf0 100644
--- a/src/test/isolation/Makefile
+++ b/src/test/isolation/Makefile
@@ -66,3 +66,14 @@ installcheck-prepared-txns: all temp-install
check-prepared-txns: all temp-install
./pg_isolation_regress --temp-instance=./tmp_check $(TEMP_CONF) $(EXTRA_REGRESS_OPTS) --inputdir=$(srcdir) --schedule=$(srcdir)/isolation_schedule prepared-transactions
+
+install: all installdirs
+ $(INSTALL_PROGRAM) isolationtester$(X) '$(DESTDIR)$(pgxsdir)/$(subdir)/isolationtester$(X)'
+ $(INSTALL_PROGRAM) pg_isolation_regress$(X) '$(DESTDIR)$(pgxsdir)/$(subdir)/pg_isolation_regress$(X)'
+
+installdirs:
+ $(MKDIR_P) '$(DESTDIR)$(pgxsdir)/$(subdir)'
+
+uninstall:
+ rm -f '$(DESTDIR)$(pgxsdir)/$(subdir)/isolationtester$(X)'
+ rm -f '$(DESTDIR)$(pgxsdir)/$(subdir)/pg_isolation_regress$(X)'
--
2.5.5
0003-Note-that-src-test-Makefile-is-not-called-from-src-M.patchtext/x-patch; charset=US-ASCII; name=0003-Note-that-src-test-Makefile-is-not-called-from-src-M.patchDownload
From a651bc09c067bdcc8d0c165a91ae1c26ddb0befc Mon Sep 17 00:00:00 2001
From: Craig Ringer <craig@2ndquadrant.com>
Date: Tue, 13 Sep 2016 11:48:43 +0800
Subject: [PATCH 3/3] Note that src/test/Makefile is not called from
src/Makefile
Add a comment to help developers who're editing src/test/Makefile.
---
src/test/Makefile | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/src/test/Makefile b/src/test/Makefile
index 6b40cf5..a24071e 100644
--- a/src/test/Makefile
+++ b/src/test/Makefile
@@ -11,6 +11,10 @@
subdir = src/test
top_builddir = ../..
include $(top_builddir)/src/Makefile.global
+
+# Note that src/test/Makefile is not normally called recursively by
+# src/Makefile; instead, individual subdirectories are called directly
+# from its SUBDIRS.
SUBDIRS = perl regress isolation modules recovery
--
2.5.5
On 13 September 2016 at 13:27, Craig Ringer <craig@2ndquadrant.com> wrote:
This was wrong for out-of-tree builds, updated.
Still pending fix for PG_REGRESS path when invoked using
$(prove_check) from PGXS
Looking further at this, I think a pgxs-specific patch to add support
for prove tests and isolation tests would be best, and can be done
separately. Possibly even snuck into a point release, but if not, at
least extension authors can invoke prove in their own Makefile if the
required modules get installed. It just needs an adaptation of the
command used in the $(prove_check) definition.
Extension makefiles run tests by listing the tests in REGRESS .
Something similar would make sense for isolation checks. For prove,
probably just a macro that can be invoked to enable prove tests in
pgxs makefiles.
I suggest that the above patches be applied to 9.6 and v10. Then for
v10 I'll look at enhancing PGXS to make it easier to use isolation
tests and prove tests; extensions that want to use them in 9.6 can
just add something like:
prove_check:
rm -rf $(CURDIR)/tmp_check/log
cd $(srcdir) && TESTDIR='$(CURDIR)' PATH="$(shell pg_config
--bindir):$$PATH" PGPORT='6$(DEF_PGPORT)'
top_builddir='$(CURDIR)/$(top_builddir)' PG_REGRESS='pg_regress'
$(PROVE) $(PG_PROVE_FLAGS) $(PROVE_FLAGS) t/*.pl
.PHONY: prove_check
to their Makefile , so it's not necessary to have PGXS support for
this for it to be useful in 9.6.
--
Craig Ringer http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 13 September 2016 at 14:36, Craig Ringer <craig@2ndquadrant.com> wrote:
prove_check:
rm -rf $(CURDIR)/tmp_check/log
cd $(srcdir) && TESTDIR='$(CURDIR)' PATH="$(shell pg_config
--bindir):$$PATH" PGPORT='6$(DEF_PGPORT)'
top_builddir='$(CURDIR)/$(top_builddir)' PG_REGRESS='pg_regress'
$(PROVE) $(PG_PROVE_FLAGS) $(PROVE_FLAGS) t/*.pl.PHONY: prove_check
Actually, that should be
prove_check:
rm -rf $(CURDIR)/tmp_check/log
cd $(srcdir) && TESTDIR='$(CURDIR)' PATH="$(shell $(PG_CONFIG)
--bindir):$$PATH" PGPORT='6$(DEF_PGPORT)'
top_builddir='$(CURDIR)/$(top_builddir)'
PG_REGRESS='$(pgxsdir)/src/test/regress/pg_regress' $(PROVE)
$(PG_PROVE_FLAGS) $(PROVE_FLAGS) t/*.pl
for a typical install.
If PGXS defined that as a var that can be invoked with $(PGXS_PROVE)
or something that'd be handy, but can be done later. It's trivial to
do in an extension Makefile so long as the required files actually get
installed.
--
Craig Ringer http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Craig Ringer wrote:
I suggest that the above patches be applied to 9.6 and v10. Then for
v10
I don't object to patching 9.6 in this way, but kindly do not pollute
this thread with future ideas on what to do on pg10, at least until the
current release is sorted out. You'll only distract people from your
present objective.
--
�lvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Craig Ringer <craig@2ndquadrant.com> writes:
While updating an extension for 9.6 I noticed that while the
$(prove_check) definition is exposed for use by PGXS in
Makefile.global, extensions can't actually use the TAP tests because
we don't install the required Perl modules like PostgresNode.pm.
I don't see any reason not to make this available to extension authors
and doing so is harmless, so here's a small patch to install it. I
think it's reasonable to add this to 9.6 even at this late stage; IMO
it should've been installed from the beginning.
Without taking a position on the merits of this patch per se, I'd like
to say that I find the argument for back-patching into 9.6 and not
further than that to be pretty dubious. $(prove_check) has been there
since 9.4, and in the past we've often regretted it when we failed
to back-patch TAP infrastructure fixes all the way back to 9.4.
Or to be concrete: how is an extension author, or more to the point an
extension Makefile, supposed to know whether it can use $(prove_check)?
How would this patch change that, and how would extension authors cope
with building against both patched and unpatched trees?
regards, tom lane
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 13 September 2016 at 22:02, Tom Lane <tgl@sss.pgh.pa.us> wrote:
Craig Ringer <craig@2ndquadrant.com> writes:
While updating an extension for 9.6 I noticed that while the
$(prove_check) definition is exposed for use by PGXS in
Makefile.global, extensions can't actually use the TAP tests because
we don't install the required Perl modules like PostgresNode.pm.
Whoops, I managed to misplace this thread.
I don't see any reason not to make this available to extension authors
and doing so is harmless, so here's a small patch to install it. I
think it's reasonable to add this to 9.6 even at this late stage; IMO
it should've been installed from the beginning.Without taking a position on the merits of this patch per se, I'd like
to say that I find the argument for back-patching into 9.6 and not
further than that to be pretty dubious. $(prove_check) has been there
since 9.4, and in the past we've often regretted it when we failed
to back-patch TAP infrastructure fixes all the way back to 9.4.
No objection to backpatching, I just thought I'd be more intrusive to
do that than just 9.6.
Since 9.5 and older have more limited versions of PostgresNode which
lack safe_psql, etc, I'm not sure it's very practical for extensions
to bother running TAP tests on 9.4 and 9.5 anyway.
I'd love to be able to, but unless we backport the new src/test/perl
stuff and the changes to the rest of the TAP tests to make them work
with it I don't see it being very useful. Since that's really not
going to fly, I say this should just go in 9.6.
Extension authors can just use:
ifeq ($(MAJORVERSION),9.6)
endif
when defining their prove rules.
Not beautiful, but when it wasn't really designed from the start to
work with PGXS I don't see much alternative.
--
Craig Ringer http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Craig Ringer <craig@2ndquadrant.com> writes:
On 13 September 2016 at 22:02, Tom Lane <tgl@sss.pgh.pa.us> wrote:
Without taking a position on the merits of this patch per se, I'd like
to say that I find the argument for back-patching into 9.6 and not
further than that to be pretty dubious. $(prove_check) has been there
since 9.4, and in the past we've often regretted it when we failed
to back-patch TAP infrastructure fixes all the way back to 9.4.
No objection to backpatching, I just thought I'd be more intrusive to
do that than just 9.6.
Since 9.5 and older have more limited versions of PostgresNode which
lack safe_psql, etc, I'm not sure it's very practical for extensions
to bother running TAP tests on 9.4 and 9.5 anyway.
Certainly there are restrictions, but I'd imagine that every new release
will be adding features to the TAP test infrastructure for some time to
come. I think it's silly to claim that 9.6 is the first branch where
TAP testing is usable at all.
Extension authors can just use:
ifeq ($(MAJORVERSION),9.6)
endif
when defining their prove rules.
That will break as soon as 10 comes out. And numerical >= tests aren't
all that convenient in Make. It'd be much better if a test on whether
$(prove_check) is defined would be sufficient.
regards, tom lane
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 23 September 2016 at 00:32, Tom Lane <tgl@sss.pgh.pa.us> wrote:
Craig Ringer <craig@2ndquadrant.com> writes:
On 13 September 2016 at 22:02, Tom Lane <tgl@sss.pgh.pa.us> wrote:
Without taking a position on the merits of this patch per se, I'd like
to say that I find the argument for back-patching into 9.6 and not
further than that to be pretty dubious. $(prove_check) has been there
since 9.4, and in the past we've often regretted it when we failed
to back-patch TAP infrastructure fixes all the way back to 9.4.No objection to backpatching, I just thought I'd be more intrusive to
do that than just 9.6.Since 9.5 and older have more limited versions of PostgresNode which
lack safe_psql, etc, I'm not sure it's very practical for extensions
to bother running TAP tests on 9.4 and 9.5 anyway.Certainly there are restrictions, but I'd imagine that every new release
will be adding features to the TAP test infrastructure for some time to
come. I think it's silly to claim that 9.6 is the first branch where
TAP testing is usable at all.
OK.
I didn't intend to claim 9.6 is the first branch where it's usable,
just that the differences between 9.4/9.5 and 9.6 mean that supporting
both in extensions will likely be more pain than it is worth. Mainly
due to the safe_psql change in 2c83f435. It might've been good to
backport that, but it was pretty wide reaching and at the time I
wasn't thinking in terms of using TAP tests in extensions so it didn't
occur to me.
Extension Perl code can always use some adapter/glue code to handle
9.4 and 9.5 if they want, or error if they don't, so it's not a fatal
barrier.
Also, despite what I said upthread, there's no need for normal
PGXS-using extensions to define their $(prove_installcheck)
replacement. It works as-is. The problems I was having stemmed from
the fact that I was working with a pretty nonstandard Makefile without
realising that the changes were going to affect prove. $(prove_check)
isn't useful from extensions of course since it expects a temp install
and PGXS doesn't know how to make one, but $(prove_installcheck) does
the job fine.
It's thus sufficient to apply the patch to install the perl modules to
9.4, 9.5 and 9.6. Nothing else is needed. I've attached backports for
9.4 and 9.5.
If you were really keen, we could actually backport the new TAP code
to 9.4 and 9.5 pretty much wholesale. 9.4 and 9.5 don't have the psql
method, PostgresNode, etc, so there's nothing to break. But that's for
separate consideration.
Extension authors can just use:
ifeq ($(MAJORVERSION),9.6)
endif
when defining their prove rules.That will break as soon as 10 comes out. And numerical >= tests aren't
all that convenient in Make. It'd be much better if a test on whether
$(prove_check) is defined would be sufficient.
Fair enough. I forgot how much numeric range tests sucked in Make.
In that case we should backpatch the installation of the Perl modules
right back to 9.4. There's not even a need to test if
$(prove_installcheck) is defined then, just need a semicolon so it
evaluates to empty e.g.
prove_installcheck:
$(prove_installcheck) ;
check: prove_check
So ... patches attached. The 9.6 patch applies to head too.
--
Craig Ringer http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
Attachments:
install-tap-modules-94.patchtext/x-patch; charset=US-ASCII; name=install-tap-modules-94.patchDownload
From 75c5c0fe98618a6e59af82c4314832df47e6268e Mon Sep 17 00:00:00 2001
From: Craig Ringer <craig@2ndquadrant.com>
Date: Fri, 23 Sep 2016 10:15:00 +0800
Subject: [PATCH] Install Perl modules for TAP tests for use from PGXS
---
src/Makefile | 1 +
src/test/Makefile | 2 +-
src/test/perl/GNUmakefile | 35 +++++++++++++++++++++++++++++++++++
3 files changed, 37 insertions(+), 1 deletion(-)
create mode 100644 src/test/perl/GNUmakefile
diff --git a/src/Makefile b/src/Makefile
index e859826..750c3a9 100644
--- a/src/Makefile
+++ b/src/Makefile
@@ -25,6 +25,7 @@ SUBDIRS = \
bin \
pl \
makefiles \
+ test/perl \
test/regress
# There are too many interdependencies between the subdirectories, so
diff --git a/src/test/Makefile b/src/test/Makefile
index 0fd7eab..ef1dcb5 100644
--- a/src/test/Makefile
+++ b/src/test/Makefile
@@ -12,6 +12,6 @@ subdir = src/test
top_builddir = ../..
include $(top_builddir)/src/Makefile.global
-SUBDIRS = regress isolation
+SUBDIRS = regress isolation perl
$(recurse)
diff --git a/src/test/perl/GNUmakefile b/src/test/perl/GNUmakefile
new file mode 100644
index 0000000..09945f7
--- /dev/null
+++ b/src/test/perl/GNUmakefile
@@ -0,0 +1,35 @@
+#-------------------------------------------------------------------------
+#
+# Makefile for src/test/perl
+#
+# Portions Copyright (c) 1996-2016, PostgreSQL Global Development Group
+# Portions Copyright (c) 1994, Regents of the University of California
+#
+# src/test/perl/GNUmakefile
+#
+#-------------------------------------------------------------------------
+
+subdir = src/test/perl
+top_builddir = ../../..
+include $(top_builddir)/src/Makefile.global
+
+installdirs:
+ $(MKDIR_P) '$(DESTDIR)$(pgxsdir)/$(subdir)'
+
+ifeq ($(enable_tap_tests),yes)
+
+install: all installdirs
+ $(INSTALL_DATA) $(srcdir)/TestLib.pm '$(DESTDIR)$(pgxsdir)/$(subdir)/TestLib.pm'
+ $(INSTALL_DATA) $(srcdir)/SimpleTee.pm '$(DESTDIR)$(pgxsdir)/$(subdir)/SimpleTee.pm'
+
+uninstall:
+ rm -f '$(DESTDIR)$(pgxsdir)/$(subdir)/TestLib.pm'
+ rm -f '$(DESTDIR)$(pgxsdir)/$(subdir)/SimpleTee.pm'
+
+else
+
+install: ;
+
+uninstall: ;
+
+endif
--
2.5.5
install-tap-modules-95.patchtext/x-patch; charset=US-ASCII; name=install-tap-modules-95.patchDownload
From 89db9d4a0f5896c53a38e5705dd7ff731aa34bfd Mon Sep 17 00:00:00 2001
From: Craig Ringer <craig@2ndquadrant.com>
Date: Fri, 23 Sep 2016 10:24:33 +0800
Subject: [PATCH] Install Perl modules for TAP tests for use from PGXS
---
src/Makefile | 1 +
src/test/Makefile | 2 +-
src/test/perl/GNUmakefile | 35 +++++++++++++++++++++++++++++++++++
3 files changed, 37 insertions(+), 1 deletion(-)
create mode 100644 src/test/perl/GNUmakefile
diff --git a/src/Makefile b/src/Makefile
index e859826..750c3a9 100644
--- a/src/Makefile
+++ b/src/Makefile
@@ -25,6 +25,7 @@ SUBDIRS = \
bin \
pl \
makefiles \
+ test/perl \
test/regress
# There are too many interdependencies between the subdirectories, so
diff --git a/src/test/Makefile b/src/test/Makefile
index b713c2c..1fef0e9 100644
--- a/src/test/Makefile
+++ b/src/test/Makefile
@@ -12,7 +12,7 @@ subdir = src/test
top_builddir = ../..
include $(top_builddir)/src/Makefile.global
-SUBDIRS = regress isolation modules
+SUBDIRS = regress isolation modules perl
# We don't build or execute examples/, locale/, or thread/ by default,
# but we do want "make clean" etc to recurse into them. Likewise for ssl/,
diff --git a/src/test/perl/GNUmakefile b/src/test/perl/GNUmakefile
new file mode 100644
index 0000000..98f4f02
--- /dev/null
+++ b/src/test/perl/GNUmakefile
@@ -0,0 +1,35 @@
+#-------------------------------------------------------------------------
+#
+# Makefile for src/test/perl
+#
+# Portions Copyright (c) 1996-2016, PostgreSQL Global Development Group
+# Portions Copyright (c) 1994, Regents of the University of California
+#
+# src/test/perl/Makefile
+#
+#-------------------------------------------------------------------------
+
+subdir = src/test/perl
+top_builddir = ../../..
+include $(top_builddir)/src/Makefile.global
+
+installdirs:
+ $(MKDIR_P) '$(DESTDIR)$(pgxsdir)/$(subdir)'
+
+ifeq ($(enable_tap_tests),yes)
+
+install: all installdirs
+ $(INSTALL_DATA) $(srcdir)/TestLib.pm '$(DESTDIR)$(pgxsdir)/$(subdir)/TestLib.pm'
+ $(INSTALL_DATA) $(srcdir)/SimpleTee.pm '$(DESTDIR)$(pgxsdir)/$(subdir)/SimpleTee.pm'
+
+uninstall:
+ rm -f '$(DESTDIR)$(pgxsdir)/$(subdir)/TestLib.pm'
+ rm -f '$(DESTDIR)$(pgxsdir)/$(subdir)/SimpleTee.pm'
+
+else
+
+install: ;
+
+uninstall: ;
+
+endif
--
2.5.5
install-tap-modules-96.patchtext/x-patch; charset=US-ASCII; name=install-tap-modules-96.patchDownload
From e0a023cc76fff4218e0163aee1b7a867128d37dc Mon Sep 17 00:00:00 2001
From: Craig Ringer <craig@2ndquadrant.com>
Date: Tue, 13 Sep 2016 11:06:58 +0800
Subject: [PATCH 1/3] Install Perl modules for TAP tests for use from PGXS
---
src/Makefile | 3 ++-
src/test/Makefile | 2 +-
src/test/perl/GNUmakefile | 39 +++++++++++++++++++++++++++++++++++++++
3 files changed, 42 insertions(+), 2 deletions(-)
create mode 100644 src/test/perl/GNUmakefile
diff --git a/src/Makefile b/src/Makefile
index b526be7..977f80b 100644
--- a/src/Makefile
+++ b/src/Makefile
@@ -26,7 +26,8 @@ SUBDIRS = \
bin \
pl \
makefiles \
- test/regress
+ test/regress \
+ test/perl
# There are too many interdependencies between the subdirectories, so
# don't attempt parallel make here.
diff --git a/src/test/Makefile b/src/test/Makefile
index 7f7754f..6b40cf5 100644
--- a/src/test/Makefile
+++ b/src/test/Makefile
@@ -12,7 +12,7 @@ subdir = src/test
top_builddir = ../..
include $(top_builddir)/src/Makefile.global
-SUBDIRS = regress isolation modules recovery
+SUBDIRS = perl regress isolation modules recovery
# We don't build or execute examples/, locale/, or thread/ by default,
# but we do want "make clean" etc to recurse into them. Likewise for ssl/,
diff --git a/src/test/perl/GNUmakefile b/src/test/perl/GNUmakefile
new file mode 100644
index 0000000..3c5dc70
--- /dev/null
+++ b/src/test/perl/GNUmakefile
@@ -0,0 +1,39 @@
+#-------------------------------------------------------------------------
+#
+# Makefile for src/test/perl
+#
+# Portions Copyright (c) 1996-2016, PostgreSQL Global Development Group
+# Portions Copyright (c) 1994, Regents of the University of California
+#
+# src/test/perl/Makefile
+#
+#-------------------------------------------------------------------------
+
+subdir = src/test/perl
+top_builddir = ../../..
+include $(top_builddir)/src/Makefile.global
+
+installdirs:
+ $(MKDIR_P) '$(DESTDIR)$(pgxsdir)/$(subdir)'
+
+ifeq ($(enable_tap_tests),yes)
+
+install: all installdirs
+ $(INSTALL_DATA) $(srcdir)/TestLib.pm '$(DESTDIR)$(pgxsdir)/$(subdir)/TestLib.pm'
+ $(INSTALL_DATA) $(srcdir)/SimpleTee.pm '$(DESTDIR)$(pgxsdir)/$(subdir)/SimpleTee.pm'
+ $(INSTALL_DATA) $(srcdir)/RecursiveCopy.pm '$(DESTDIR)$(pgxsdir)/$(subdir)/RecursiveCopy.pm'
+ $(INSTALL_DATA) $(srcdir)/PostgresNode.pm '$(DESTDIR)$(pgxsdir)/$(subdir)/PostgresNode.pm'
+
+uninstall:
+ rm -f '$(DESTDIR)$(pgxsdir)/$(subdir)/TestLib.pm'
+ rm -f '$(DESTDIR)$(pgxsdir)/$(subdir)/SimpleTee.pm'
+ rm -f '$(DESTDIR)$(pgxsdir)/$(subdir)/RecursiveCopy.pm'
+ rm -f '$(DESTDIR)$(pgxsdir)/$(subdir)/PostgresNode.pm'
+
+else
+
+install: ;
+
+uninstall: ;
+
+endif
--
2.5.5
Craig Ringer <craig@2ndquadrant.com> writes:
On 23 September 2016 at 00:32, Tom Lane <tgl@sss.pgh.pa.us> wrote:
Certainly there are restrictions, but I'd imagine that every new release
will be adding features to the TAP test infrastructure for some time to
come. I think it's silly to claim that 9.6 is the first branch where
TAP testing is usable at all.
Also, despite what I said upthread, there's no need for normal
PGXS-using extensions to define their $(prove_installcheck)
replacement. It works as-is. The problems I was having stemmed from
the fact that I was working with a pretty nonstandard Makefile without
realising that the changes were going to affect prove. $(prove_check)
isn't useful from extensions of course since it expects a temp install
and PGXS doesn't know how to make one, but $(prove_installcheck) does
the job fine.
It's thus sufficient to apply the patch to install the perl modules to
9.4, 9.5 and 9.6. Nothing else is needed. I've attached backports for
9.4 and 9.5.
Pushed with cosmetic adjustments --- mostly, I followed the project
standard that makefiles are named Makefile. (We use a GNUmakefile
only in directories where it seems likely that non-developers would
invoke make by hand, and there's supposed to be a Makefile beside them
that throws an error whining about how you're not using GNU make.
I see no need for that in src/test/perl.)
Looking back over the thread, I see that you also proposed installing
isolationtester and pg_isolation_regress for the benefit of extensions.
I'm very much less excited about that idea. It'd be substantially more
dead weight in typical installations, and I'm not sure that it'd be useful
to common extensions, and I'm not eager to treat isolationtester's API
and behavior as something we need to hold stable for extension use.
regards, tom lane
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 24 Sep. 2016 04:04, "Tom Lane" <tgl@sss.pgh.pa.us> wrote:.
It's thus sufficient to apply the patch to install the perl modules to
9.4, 9.5 and 9.6. Nothing else is needed. I've attached backports for
9.4 and 9.5.Pushed with cosmetic adjustments ---
Thanks.
Looking back over the thread, I see that you also proposed installing
isolationtester and pg_isolation_regress for the benefit of extensions.
I'm very much less excited about that idea. It'd be substantially more
dead weight in typical installations, and I'm not sure that it'd be useful
to common extensions, and I'm not eager to treat isolationtester's API
and behavior as something we need to hold stable for extension use.
Fine by me. I have no particular problem expecting those to be installed
explicitly by ext devs who want to use them.
On 2016-09-23 16:04:32 -0400, Tom Lane wrote:
Looking back over the thread, I see that you also proposed installing
isolationtester and pg_isolation_regress for the benefit of extensions.
I'm very much less excited about that idea. It'd be substantially more
dead weight in typical installations, and I'm not sure that it'd be useful
to common extensions, and I'm not eager to treat isolationtester's API
and behavior as something we need to hold stable for extension use.
FWIW, I'd be quite happy if it were installed. Running isolationtester
when compiling extensions against distribution postgres packages would
be quite useful.
Andres
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Wed, Oct 26, 2016 at 7:17 AM, Andres Freund <andres@anarazel.de> wrote:
On 2016-09-23 16:04:32 -0400, Tom Lane wrote:
Looking back over the thread, I see that you also proposed installing
isolationtester and pg_isolation_regress for the benefit of extensions.
I'm very much less excited about that idea. It'd be substantially more
dead weight in typical installations, and I'm not sure that it'd be useful
to common extensions, and I'm not eager to treat isolationtester's API
and behavior as something we need to hold stable for extension use.FWIW, I'd be quite happy if it were installed. Running isolationtester
when compiling extensions against distribution postgres packages would
be quite useful.
+1.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 27 October 2016 at 00:42, Robert Haas <robertmhaas@gmail.com> wrote:
On Wed, Oct 26, 2016 at 7:17 AM, Andres Freund <andres@anarazel.de> wrote:
On 2016-09-23 16:04:32 -0400, Tom Lane wrote:
Looking back over the thread, I see that you also proposed installing
isolationtester and pg_isolation_regress for the benefit of extensions.
I'm very much less excited about that idea. It'd be substantially more
dead weight in typical installations, and I'm not sure that it'd be useful
to common extensions, and I'm not eager to treat isolationtester's API
and behavior as something we need to hold stable for extension use.FWIW, I'd be quite happy if it were installed. Running isolationtester
when compiling extensions against distribution postgres packages would
be quite useful.+1.
Patch attached.
--
Craig Ringer http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
Attachments:
0001-Install-isolationtester.patchtext/x-patch; charset=US-ASCII; name=0001-Install-isolationtester.patchDownload
From 40735041753c48d282f3af58482992b8da00c96d Mon Sep 17 00:00:00 2001
From: Craig Ringer <craig@2ndquadrant.com>
Date: Mon, 14 Nov 2016 14:51:09 +0800
Subject: [PATCH] Install isolationtester
Per discussion in <CA+TgmoZ2ShT38LwKXj0YJ0T3yqEBHw6JhtwoYPqH2rTc0VjLBg@mail.gmail.com>
---
src/Makefile | 1 +
src/test/isolation/Makefile | 9 +++++++++
2 files changed, 10 insertions(+)
diff --git a/src/Makefile b/src/Makefile
index 977f80b..d4aa06b 100644
--- a/src/Makefile
+++ b/src/Makefile
@@ -27,6 +27,7 @@ SUBDIRS = \
pl \
makefiles \
test/regress \
+ test/isolation \
test/perl
# There are too many interdependencies between the subdirectories, so
diff --git a/src/test/isolation/Makefile b/src/test/isolation/Makefile
index 3d272d5..613309e 100644
--- a/src/test/isolation/Makefile
+++ b/src/test/isolation/Makefile
@@ -66,3 +66,12 @@ installcheck-prepared-txns: all temp-install
check-prepared-txns: all temp-install
./pg_isolation_regress --temp-instance=./tmp_check $(TEMP_CONF) $(EXTRA_REGRESS_OPTS) --inputdir=$(srcdir) --schedule=$(srcdir)/isolation_schedule prepared-transactions
+
+install: all installdirs
+ $(INSTALL_PROGRAM) isolationtester$(X) '$(DESTDIR)$(pgxsdir)/$(subdir)/isolationtester$(X)'
+
+installdirs:
+ $(MKDIR_P) '$(DESTDIR)$(pgxsdir)/$(subdir)'
+
+uninstall:
+ rm -f '$(DESTDIR)$(pgxsdir)/$(subdir)/isolationtester$(X)'
--
2.5.5
On 14 November 2016 at 14:55, Craig Ringer <craig@2ndquadrant.com> wrote:
On 27 October 2016 at 00:42, Robert Haas <robertmhaas@gmail.com> wrote:
On Wed, Oct 26, 2016 at 7:17 AM, Andres Freund <andres@anarazel.de> wrote:
On 2016-09-23 16:04:32 -0400, Tom Lane wrote:
Looking back over the thread, I see that you also proposed installing
isolationtester and pg_isolation_regress for the benefit of extensions.
I'm very much less excited about that idea. It'd be substantially more
dead weight in typical installations, and I'm not sure that it'd be useful
to common extensions, and I'm not eager to treat isolationtester's API
and behavior as something we need to hold stable for extension use.FWIW, I'd be quite happy if it were installed. Running isolationtester
when compiling extensions against distribution postgres packages would
be quite useful.+1.
Patch attached.
As Andres pointed out elsewhere this is only half the picture. It
should really add PGXS support for ISOLATION_TESTS and bringing up the
test harness too.
--
Craig Ringer http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Craig Ringer wrote:
On 27 October 2016 at 00:42, Robert Haas <robertmhaas@gmail.com> wrote:
On Wed, Oct 26, 2016 at 7:17 AM, Andres Freund <andres@anarazel.de> wrote:
On 2016-09-23 16:04:32 -0400, Tom Lane wrote:
Looking back over the thread, I see that you also proposed installing
isolationtester and pg_isolation_regress for the benefit of extensions.
I'm very much less excited about that idea. It'd be substantially more
dead weight in typical installations, and I'm not sure that it'd be useful
to common extensions, and I'm not eager to treat isolationtester's API
and behavior as something we need to hold stable for extension use.FWIW, I'd be quite happy if it were installed. Running isolationtester
when compiling extensions against distribution postgres packages would
be quite useful.+1.
Patch attached.
Hmm but this only installs isolationtester itself ... don't you need
pg_isolation_regress too?
--
�lvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 25 November 2016 at 02:47, Alvaro Herrera <alvherre@2ndquadrant.com> wrote:
Craig Ringer wrote:
On 27 October 2016 at 00:42, Robert Haas <robertmhaas@gmail.com> wrote:
On Wed, Oct 26, 2016 at 7:17 AM, Andres Freund <andres@anarazel.de> wrote:
On 2016-09-23 16:04:32 -0400, Tom Lane wrote:
Looking back over the thread, I see that you also proposed installing
isolationtester and pg_isolation_regress for the benefit of extensions.
I'm very much less excited about that idea. It'd be substantially more
dead weight in typical installations, and I'm not sure that it'd be useful
to common extensions, and I'm not eager to treat isolationtester's API
and behavior as something we need to hold stable for extension use.FWIW, I'd be quite happy if it were installed. Running isolationtester
when compiling extensions against distribution postgres packages would
be quite useful.+1.
Patch attached.
Hmm but this only installs isolationtester itself ... don't you need
pg_isolation_regress too?
Yeah, as Andres pointed out offlist after I posted this. Meant to
follow up but got side-tracked.
It needs PGXS support to be properly useful.
--
Craig Ringer http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers