On 2017-03-13 00:35:06 -0400, Tom Lane wrote:
> Andres Freund <[email protected]> writes:
> > On 2017-03-11 22:14:07 -0500, Tom Lane wrote:
> >> This looks generally sane to me, although I'm not very happy about folding
> >> the "$(MKDIR_P) output_iso" call into pg_isolation_regress_check --- that
> >> seems weird and unlike the way it's done for the regular regression test
> >> case.
>
> > Yea, not super happy about that either - alternatively we could fold it
> > into pg_regress.
>
> Yeah, teaching pg_regress to auto-create the --temp-instance directory
> seems perfectly sane from here.
I was thinking about outputdir, not temp-instance. The latter is
already created:
/* make the temp instance top directory */
make_directory(temp_instance);
Attached is an updated patch that creates outputdir if necessary. This
is possibly going to trigger a time-to-check-time-to-use coverity
warning, but the rest of pg_regress does if(!exists) mkdir() type logic,
so I did the same.
Besides the pg_regress change, the only thing I've changed is to remove
the in-line "$(MKDIR_P) output_iso && \".
- Andres
>From a63c61ca828d186a374d05db264f95b5b07187f9 Mon Sep 17 00:00:00 2001
From: Andres Freund <[email protected]>
Date: Sat, 11 Mar 2017 15:03:18 -0800
Subject: [PATCH] Improve isolation regression tests infrastructure.
Previously if a directory had both isolationtester and plain
regression tests, they couldn't be run in parallel, because they'd
access the same files/directories. That, so far, only affected
contrib/test_decoding.
Rather than fix that locally in contrib/test_decoding improve
pg_regress_isolation_[install]check to use separate resources from
plain regression tests.
That requires a minor change in pg_regress, namely that the
--outputdir is created if not already existing, that seems like good
idea anyway.
Use the improved helpers even where previously not used.
Author: Tom Lane and Andres Freund
Discussion: https://postgr.es/m/[email protected]
---
contrib/test_decoding/.gitignore | 5 +++--
contrib/test_decoding/Makefile | 7 +------
src/Makefile.global.in | 29 ++++++++++++++++++++++------
src/test/isolation/.gitignore | 5 ++---
src/test/isolation/Makefile | 8 ++++----
src/test/modules/snapshot_too_old/.gitignore | 2 +-
src/test/modules/snapshot_too_old/Makefile | 6 +++---
src/test/regress/pg_regress.c | 6 +++++-
8 files changed, 42 insertions(+), 26 deletions(-)
diff --git a/contrib/test_decoding/.gitignore b/contrib/test_decoding/.gitignore
index 1f95503494..b4903eba65 100644
--- a/contrib/test_decoding/.gitignore
+++ b/contrib/test_decoding/.gitignore
@@ -1,5 +1,6 @@
# Generated subdirectories
/log/
-/isolation_output/
-/regression_output/
+/results/
+/output_iso/
/tmp_check/
+/tmp_check_iso/
diff --git a/contrib/test_decoding/Makefile b/contrib/test_decoding/Makefile
index d2bc8b8350..6c18189d9d 100644
--- a/contrib/test_decoding/Makefile
+++ b/contrib/test_decoding/Makefile
@@ -5,7 +5,7 @@ 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) ./regression_output ./isolation_output
+EXTRA_CLEAN = $(pg_regress_clean_files)
ifdef USE_PGXS
PG_CONFIG = pg_config
@@ -42,11 +42,8 @@ REGRESSCHECKS=ddl xact rewrite toast permissions decoding_in_xact \
spill slot
regresscheck: | submake-regress submake-test_decoding temp-install
- $(MKDIR_P) regression_output
$(pg_regress_check) \
--temp-config $(top_srcdir)/contrib/test_decoding/logical.conf \
- --temp-instance=./tmp_check \
- --outputdir=./regression_output \
$(REGRESSCHECKS)
regresscheck-install-force: | submake-regress submake-test_decoding temp-install
@@ -56,10 +53,8 @@ regresscheck-install-force: | submake-regress submake-test_decoding temp-install
ISOLATIONCHECKS=mxact delayed_startup ondisk_startup concurrent_ddl_dml
isolationcheck: | submake-isolation submake-test_decoding temp-install
- $(MKDIR_P) isolation_output
$(pg_isolation_regress_check) \
--temp-config $(top_srcdir)/contrib/test_decoding/logical.conf \
- --outputdir=./isolation_output \
$(ISOLATIONCHECKS)
isolationcheck-install-force: all | submake-isolation submake-test_decoding temp-install
diff --git a/src/Makefile.global.in b/src/Makefile.global.in
index 831d39a9d1..e7862016aa 100644
--- a/src/Makefile.global.in
+++ b/src/Makefile.global.in
@@ -545,14 +545,31 @@ TEMP_CONF += --temp-config=$(TEMP_CONFIG)
endif
pg_regress_locale_flags = $(if $(ENCODING),--encoding=$(ENCODING)) $(NOLOCALE)
+pg_regress_clean_files = results/ regression.diffs regression.out tmp_check/ tmp_check_iso/ log/ output_iso/
-pg_regress_check = $(with_temp_install) $(top_builddir)/src/test/regress/pg_regress --inputdir=$(srcdir) --temp-instance=./tmp_check $(TEMP_CONF) --bindir= $(pg_regress_locale_flags) $(EXTRA_REGRESS_OPTS)
-pg_regress_installcheck = $(top_builddir)/src/test/regress/pg_regress --inputdir=$(srcdir) --bindir='$(bindir)' $(pg_regress_locale_flags) $(EXTRA_REGRESS_OPTS)
+pg_regress_check = \
+ $(with_temp_install) \
+ $(top_builddir)/src/test/regress/pg_regress \
+ --temp-instance=./tmp_check \
+ --inputdir=$(srcdir) \
+ $(TEMP_CONF) \
+ --bindir= $(pg_regress_locale_flags) $(EXTRA_REGRESS_OPTS)
+pg_regress_installcheck = \
+ $(top_builddir)/src/test/regress/pg_regress \
+ --inputdir=$(srcdir) \
+ --bindir='$(bindir)' $(pg_regress_locale_flags) $(EXTRA_REGRESS_OPTS)
-pg_regress_clean_files = results/ regression.diffs regression.out tmp_check/ log/
-
-pg_isolation_regress_check = $(with_temp_install) $(top_builddir)/src/test/isolation/pg_isolation_regress --inputdir=$(srcdir) --temp-instance=./tmp_check $(TEMP_CONF) --bindir= $(pg_regress_locale_flags) $(EXTRA_REGRESS_OPTS)
-pg_isolation_regress_installcheck = $(top_builddir)/src/test/isolation/pg_isolation_regress --inputdir=$(srcdir) $(pg_regress_locale_flags) $(EXTRA_REGRESS_OPTS)
+pg_isolation_regress_check = \
+ $(with_temp_install) \
+ $(top_builddir)/src/test/isolation/pg_isolation_regress \
+ --temp-instance=./tmp_check_iso \
+ --inputdir=$(srcdir) --outputdir=output_iso \
+ $(TEMP_CONF) \
+ --bindir= $(pg_regress_locale_flags) $(EXTRA_REGRESS_OPTS)
+pg_isolation_regress_installcheck = \
+ $(top_builddir)/src/test/isolation/pg_isolation_regress \
+ --inputdir=$(srcdir) \
+ $(pg_regress_locale_flags) $(EXTRA_REGRESS_OPTS)
##########################################################################
#
diff --git a/src/test/isolation/.gitignore b/src/test/isolation/.gitignore
index 42ee945744..44bcf95854 100644
--- a/src/test/isolation/.gitignore
+++ b/src/test/isolation/.gitignore
@@ -7,6 +7,5 @@
/specscanner.c
# Generated subdirectories
-/results/
-/log/
-/tmp_check/
+/output_iso/
+/tmp_check_iso/
diff --git a/src/test/isolation/Makefile b/src/test/isolation/Makefile
index 3d272d5b59..8eb4969e9b 100644
--- a/src/test/isolation/Makefile
+++ b/src/test/isolation/Makefile
@@ -52,17 +52,17 @@ maintainer-clean: distclean
rm -f specparse.c specscanner.c
installcheck: all
- ./pg_isolation_regress --bindir='$(bindir)' $(EXTRA_REGRESS_OPTS) --inputdir=$(srcdir) --schedule=$(srcdir)/isolation_schedule
+ $(pg_isolation_regress_installcheck) --schedule=$(srcdir)/isolation_schedule
check: all
- $(with_temp_install) ./pg_isolation_regress --temp-instance=./tmp_check $(TEMP_CONF) --inputdir=$(srcdir) --bindir= $(EXTRA_REGRESS_OPTS) --schedule=$(srcdir)/isolation_schedule
+ $(pg_isolation_regress_check) --schedule=$(srcdir)/isolation_schedule
# Versions of the check tests that include the prepared_transactions test
# It only makes sense to run these if set up to use prepared transactions,
# via TEMP_CONFIG for the check case, or via the postgresql.conf for the
# installcheck case.
installcheck-prepared-txns: all temp-install
- ./pg_isolation_regress --bindir='$(bindir)' $(EXTRA_REGRESS_OPTS) --inputdir=$(srcdir) --schedule=$(srcdir)/isolation_schedule prepared-transactions
+ $(pg_isolation_regress_installcheck) --schedule=$(srcdir)/isolation_schedule prepared-transactions
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
+ $(pg_isolation_regress_check) --schedule=$(srcdir)/isolation_schedule prepared-transactions
diff --git a/src/test/modules/snapshot_too_old/.gitignore b/src/test/modules/snapshot_too_old/.gitignore
index ef3609b7da..5cf29ed6f8 100644
--- a/src/test/modules/snapshot_too_old/.gitignore
+++ b/src/test/modules/snapshot_too_old/.gitignore
@@ -1 +1 @@
-/isolation_output/
+/output_iso/
diff --git a/src/test/modules/snapshot_too_old/Makefile b/src/test/modules/snapshot_too_old/Makefile
index 16339f0366..a72bfad43a 100644
--- a/src/test/modules/snapshot_too_old/Makefile
+++ b/src/test/modules/snapshot_too_old/Makefile
@@ -1,6 +1,8 @@
# src/test/modules/snapshot_too_old/Makefile
-EXTRA_CLEAN = ./isolation_output
+# 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)
ISOLATIONCHECKS=sto_using_cursor sto_using_select
@@ -32,10 +34,8 @@ 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
- $(MKDIR_P) isolation_output
$(pg_isolation_regress_check) \
--temp-config $(top_srcdir)/src/test/modules/snapshot_too_old/sto.conf \
- --outputdir=./isolation_output \
$(ISOLATIONCHECKS)
isolationcheck-install-force: all | submake-isolation submake-test_snapshot_too_old temp-install
diff --git a/src/test/regress/pg_regress.c b/src/test/regress/pg_regress.c
index c393ae1f51..2e58895066 100644
--- a/src/test/regress/pg_regress.c
+++ b/src/test/regress/pg_regress.c
@@ -1871,6 +1871,10 @@ open_result_files(void)
char file[MAXPGPATH];
FILE *difffile;
+ /* create outputdir directory if not present */
+ if (!directory_exists(outputdir))
+ make_directory(outputdir);
+
/* create the log file (copy of running status output) */
snprintf(file, sizeof(file), "%s/regression.out", outputdir);
logfilename = pg_strdup(file);
@@ -1895,7 +1899,7 @@ open_result_files(void)
/* we don't keep the diffs file open continuously */
fclose(difffile);
- /* also create the output directory if not present */
+ /* also create the results directory if not present */
snprintf(file, sizeof(file), "%s/results", outputdir);
if (!directory_exists(file))
make_directory(file);
--
2.12.0.264.gd6db3f2165.dirty
--
Sent via pgsql-hackers mailing list ([email protected])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
