From a63c61ca828d186a374d05db264f95b5b07187f9 Mon Sep 17 00:00:00 2001
From: Andres Freund <andres@anarazel.de>
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/20170311194831.vm5ikpczq52c2drg@alap3.anarazel.de
---
 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

