pgsql: Respect TEMP_CONFIG when running contrib regression tests.

Started by Robert Haasalmost 10 years ago13 messages
#1Robert Haas
rhaas@postgresql.org

Respect TEMP_CONFIG when running contrib regression tests.

Thomas Munro

Branch
------
master

Details
-------
http://git.postgresql.org/pg/commitdiff/9117985b6ba9beda4f280f596035649fc23b6233

Modified Files
--------------
contrib/contrib-global.mk | 5 +++++
1 file changed, 5 insertions(+)

--
Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-committers

#2Andrew Dunstan
andrew@dunslane.net
In reply to: Robert Haas (#1)
Re: pgsql: Respect TEMP_CONFIG when running contrib regression tests.

On 02/26/2016 02:10 AM, Robert Haas wrote:

Respect TEMP_CONFIG when running contrib regression tests.

Wouldn't it be better to move this stuff from here and the regress
Makefile into Makefile.global.in?

That's what I was planning to do when I had time.

cheers

andrew

--
Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-committers

#3Robert Haas
robertmhaas@gmail.com
In reply to: Andrew Dunstan (#2)
Re: pgsql: Respect TEMP_CONFIG when running contrib regression tests.

On Sat, Feb 27, 2016 at 6:19 AM, Andrew Dunstan <andrew@dunslane.net> wrote:

On 02/26/2016 02:10 AM, Robert Haas wrote:

Respect TEMP_CONFIG when running contrib regression tests.

Wouldn't it be better to move this stuff from here and the regress Makefile
into Makefile.global.in?

Possibly. What difference does it make?

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

--
Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-committers

#4Andres Freund
andres@anarazel.de
In reply to: Robert Haas (#3)
Re: pgsql: Respect TEMP_CONFIG when running contrib regression tests.

On 2016-02-27 07:43:15 +0530, Robert Haas wrote:

On Sat, Feb 27, 2016 at 6:19 AM, Andrew Dunstan <andrew@dunslane.net> wrote:

On 02/26/2016 02:10 AM, Robert Haas wrote:

Respect TEMP_CONFIG when running contrib regression tests.

Wouldn't it be better to move this stuff from here and the regress Makefile
into Makefile.global.in?

Possibly. What difference does it make?

Less duplication?

--
Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-committers

#5Robert Haas
robertmhaas@gmail.com
In reply to: Andres Freund (#4)
Re: pgsql: Respect TEMP_CONFIG when running contrib regression tests.

On Sat, Feb 27, 2016 at 7:47 AM, Andres Freund <andres@anarazel.de> wrote:

On 2016-02-27 07:43:15 +0530, Robert Haas wrote:

On Sat, Feb 27, 2016 at 6:19 AM, Andrew Dunstan <andrew@dunslane.net> wrote:

On 02/26/2016 02:10 AM, Robert Haas wrote:

Respect TEMP_CONFIG when running contrib regression tests.

Wouldn't it be better to move this stuff from here and the regress Makefile
into Makefile.global.in?

Possibly. What difference does it make?

Less duplication?

Sure. Saving three lines of Makefile duplication is hardly a
world-shattering event, so I thought there might be some other
purpose. But I'm not against saving three lines of duplication
either, if it won't break anything.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

--
Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-committers

#6Andrew Dunstan
andrew@dunslane.net
In reply to: Robert Haas (#5)
Re: pgsql: Respect TEMP_CONFIG when running contrib regression tests.

On 02/26/2016 09:21 PM, Robert Haas wrote:

On Sat, Feb 27, 2016 at 7:47 AM, Andres Freund <andres@anarazel.de> wrote:

On 2016-02-27 07:43:15 +0530, Robert Haas wrote:

On Sat, Feb 27, 2016 at 6:19 AM, Andrew Dunstan <andrew@dunslane.net> wrote:

On 02/26/2016 02:10 AM, Robert Haas wrote:

Respect TEMP_CONFIG when running contrib regression tests.

Wouldn't it be better to move this stuff from here and the regress Makefile
into Makefile.global.in?

Possibly. What difference does it make?

Less duplication?

Sure. Saving three lines of Makefile duplication is hardly a
world-shattering event, so I thought there might be some other
purpose. But I'm not against saving three lines of duplication
either, if it won't break anything.

The point is that we should do this for several other test sets as well
as contrib - isolation tests, PL tests and ecpg tests.

cheers

andrew

--
Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-committers

#7Robert Haas
robertmhaas@gmail.com
In reply to: Andrew Dunstan (#6)
Re: [COMMITTERS] pgsql: Respect TEMP_CONFIG when running contrib regression tests.

On Sat, Feb 27, 2016 at 9:00 AM, Andrew Dunstan <andrew@dunslane.net> wrote:

Sure. Saving three lines of Makefile duplication is hardly a
world-shattering event, so I thought there might be some other
purpose. But I'm not against saving three lines of duplication
either, if it won't break anything.

The point is that we should do this for several other test sets as well as
contrib - isolation tests, PL tests and ecpg tests.

OK, I was wondering about that. I can try to write a patch, or
someone else can, but if you already understand what needs to be done,
perhaps you should just go ahead.

--
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

#8Andrew Dunstan
andrew@dunslane.net
In reply to: Robert Haas (#7)
1 attachment(s)
Re: [COMMITTERS] pgsql: Respect TEMP_CONFIG when running contrib regression tests.

On 02/26/2016 10:59 PM, Robert Haas wrote:

On Sat, Feb 27, 2016 at 9:00 AM, Andrew Dunstan <andrew@dunslane.net> wrote:

Sure. Saving three lines of Makefile duplication is hardly a
world-shattering event, so I thought there might be some other
purpose. But I'm not against saving three lines of duplication
either, if it won't break anything.

The point is that we should do this for several other test sets as well as
contrib - isolation tests, PL tests and ecpg tests.

OK, I was wondering about that. I can try to write a patch, or
someone else can, but if you already understand what needs to be done,
perhaps you should just go ahead.

What I had in mind was something like the attached.

In testing this seems to do the right thing, and the nice part is that
it will be picked up by the buildfarm in the one case that's relevant,
namely the ecpg tests.

The only fly in the ointment is that there are a few places that set
--temp-config explicitly:

./contrib/test_decoding/Makefile: --temp-config
$(top_srcdir)/contrib/test_decoding/logical.conf \
./contrib/test_decoding/Makefile: --temp-config
$(top_srcdir)/contrib/test_decoding/logical.conf \
./src/test/modules/commit_ts/Makefile:REGRESS_OPTS =
--temp-config=$(top_srcdir)/src/test/modules/commit_ts/commit_ts.conf
./src/test/modules/test_rls_hooks/Makefile:REGRESS_OPTS =
--temp-config=$(top_srcdir)/src/test/modules/test_rls_hooks/rls_hooks.conf

Perhaps what we need to do is modify pg_regress.c slightly to allow more
than one --temp-config argument. But that could be done later.

cheers

andrew

Attachments:

regress-temp-config.patchtext/x-patch; name=regress-temp-config.patchDownload
diff --git a/contrib/contrib-global.mk b/contrib/contrib-global.mk
index ba49610..6ac8e9b 100644
--- a/contrib/contrib-global.mk
+++ b/contrib/contrib-global.mk
@@ -1,9 +1,4 @@
 # contrib/contrib-global.mk
 
-# file with extra config for temp build
-ifdef TEMP_CONFIG
-REGRESS_OPTS += --temp-config=$(TEMP_CONFIG)
-endif
-
 NO_PGXS = 1
 include $(top_srcdir)/src/makefiles/pgxs.mk
diff --git a/src/Makefile.global.in b/src/Makefile.global.in
index e94d6a5..47b265e 100644
--- a/src/Makefile.global.in
+++ b/src/Makefile.global.in
@@ -524,14 +524,20 @@ ifdef NO_LOCALE
 NOLOCALE += --no-locale
 endif
 
+# file with extra config for temp build
+TEMP_CONF =
+ifdef TEMP_CONFIG
+TEMP_CONF += --temp-config=$(TEMP_CONFIG)
+endif
+
 pg_regress_locale_flags = $(if $(ENCODING),--encoding=$(ENCODING)) $(NOLOCALE)
 
-pg_regress_check = $(with_temp_install) $(top_builddir)/src/test/regress/pg_regress --inputdir=$(srcdir) --temp-instance=./tmp_check --bindir= $(pg_regress_locale_flags) $(EXTRA_REGRESS_OPTS)
+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_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 --bindir= $(pg_regress_locale_flags) $(EXTRA_REGRESS_OPTS)
+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)
 
 ##########################################################################
diff --git a/src/interfaces/ecpg/test/Makefile b/src/interfaces/ecpg/test/Makefile
index a4ac021..4ed785b 100644
--- a/src/interfaces/ecpg/test/Makefile
+++ b/src/interfaces/ecpg/test/Makefile
@@ -78,11 +78,11 @@ endif
 REGRESS_OPTS = --dbname=regress1,connectdb --create-role=connectuser,connectdb $(EXTRA_REGRESS_OPTS)
 
 check: all
-	$(with_temp_install) ./pg_regress $(REGRESS_OPTS) --temp-instance=./tmp_check --bindir= $(pg_regress_locale_flags) $(THREAD) --schedule=$(srcdir)/ecpg_schedule
+	$(with_temp_install) ./pg_regress $(REGRESS_OPTS) --temp-instance=./tmp_check $(TEMP_CONF) --bindir= $(pg_regress_locale_flags) $(THREAD) --schedule=$(srcdir)/ecpg_schedule
 
 # the same options, but with --listen-on-tcp
 checktcp: all
-	$(with_temp_install) ./pg_regress $(REGRESS_OPTS) --temp-instance=./tmp_check --bindir= $(pg_regress_locale_flags) $(THREAD) --schedule=$(srcdir)/ecpg_schedule_tcp --host=localhost
+	$(with_temp_install) ./pg_regress $(REGRESS_OPTS) --temp-instance=./tmp_check $(TEMP_CONF) --bindir= $(pg_regress_locale_flags) $(THREAD) --schedule=$(srcdir)/ecpg_schedule_tcp --host=localhost
 
 installcheck: all
 	./pg_regress $(REGRESS_OPTS) --bindir='$(bindir)' $(pg_regress_locale_flags) $(THREAD) --schedule=$(srcdir)/ecpg_schedule
diff --git a/src/test/isolation/Makefile b/src/test/isolation/Makefile
index 4577509..3d272d5 100644
--- a/src/test/isolation/Makefile
+++ b/src/test/isolation/Makefile
@@ -55,7 +55,7 @@ installcheck: all
 	./pg_isolation_regress --bindir='$(bindir)' $(EXTRA_REGRESS_OPTS) --inputdir=$(srcdir) --schedule=$(srcdir)/isolation_schedule
 
 check: all
-	$(with_temp_install) ./pg_isolation_regress --temp-instance=./tmp_check --inputdir=$(srcdir) --bindir= $(EXTRA_REGRESS_OPTS) --schedule=$(srcdir)/isolation_schedule
+	$(with_temp_install) ./pg_isolation_regress --temp-instance=./tmp_check $(TEMP_CONF) --inputdir=$(srcdir) --bindir= $(EXTRA_REGRESS_OPTS) --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,
@@ -65,4 +65,4 @@ installcheck-prepared-txns: all temp-install
 	./pg_isolation_regress --bindir='$(bindir)' $(EXTRA_REGRESS_OPTS) --inputdir=$(srcdir) --schedule=$(srcdir)/isolation_schedule prepared-transactions
 
 check-prepared-txns: all temp-install
-	./pg_isolation_regress --temp-instance=./tmp_check $(EXTRA_REGRESS_OPTS) --inputdir=$(srcdir) --schedule=$(srcdir)/isolation_schedule prepared-transactions
+	./pg_isolation_regress --temp-instance=./tmp_check $(TEMP_CONF) $(EXTRA_REGRESS_OPTS) --inputdir=$(srcdir) --schedule=$(srcdir)/isolation_schedule prepared-transactions
diff --git a/src/test/regress/GNUmakefile b/src/test/regress/GNUmakefile
index 204099a..4f55f52 100644
--- a/src/test/regress/GNUmakefile
+++ b/src/test/regress/GNUmakefile
@@ -17,12 +17,6 @@ subdir = src/test/regress
 top_builddir = ../../..
 include $(top_builddir)/src/Makefile.global
 
-# file with extra config for temp build
-TEMP_CONF =
-ifdef TEMP_CONFIG
-TEMP_CONF += --temp-config=$(TEMP_CONFIG)
-endif
-
 # maximum simultaneous connections for parallel tests
 MAXCONNOPT =
 ifdef MAX_CONNECTIONS
#9Robert Haas
robertmhaas@gmail.com
In reply to: Andrew Dunstan (#8)
Re: [COMMITTERS] pgsql: Respect TEMP_CONFIG when running contrib regression tests.

On Sat, Feb 27, 2016 at 7:08 PM, Andrew Dunstan <andrew@dunslane.net> wrote:

What I had in mind was something like the attached.

In testing this seems to do the right thing, and the nice part is that it
will be picked up by the buildfarm in the one case that's relevant, namely
the ecpg tests.

The only fly in the ointment is that there are a few places that set
--temp-config explicitly:

./contrib/test_decoding/Makefile: --temp-config
$(top_srcdir)/contrib/test_decoding/logical.conf \
./contrib/test_decoding/Makefile: --temp-config
$(top_srcdir)/contrib/test_decoding/logical.conf \
./src/test/modules/commit_ts/Makefile:REGRESS_OPTS =
--temp-config=$(top_srcdir)/src/test/modules/commit_ts/commit_ts.conf
./src/test/modules/test_rls_hooks/Makefile:REGRESS_OPTS =

--temp-config=$(top_srcdir)/src/test/modules/test_rls_hooks/rls_hooks.conf

Perhaps what we need to do is modify pg_regress.c slightly to allow more
than one --temp-config argument. But that could be done later.

Well, I'm pretty interested in using --temp-config for parallelism
testing; I want to be able to run the whole regression test suite with
a given --temp-config. I'm in agreement with this change but if it
doesn't play well with that need, I suppose I'll be writing that
pg_regress.c patch sooner rather than later.

--
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

#10Andrew Dunstan
andrew@dunslane.net
In reply to: Robert Haas (#9)
Re: [COMMITTERS] pgsql: Respect TEMP_CONFIG when running contrib regression tests.

On 02/27/2016 09:25 AM, Robert Haas wrote:

On Sat, Feb 27, 2016 at 7:08 PM, Andrew Dunstan <andrew@dunslane.net> wrote:

What I had in mind was something like the attached.

In testing this seems to do the right thing, and the nice part is that it
will be picked up by the buildfarm in the one case that's relevant, namely
the ecpg tests.

The only fly in the ointment is that there are a few places that set
--temp-config explicitly:

./contrib/test_decoding/Makefile: --temp-config
$(top_srcdir)/contrib/test_decoding/logical.conf \
./contrib/test_decoding/Makefile: --temp-config
$(top_srcdir)/contrib/test_decoding/logical.conf \
./src/test/modules/commit_ts/Makefile:REGRESS_OPTS =
--temp-config=$(top_srcdir)/src/test/modules/commit_ts/commit_ts.conf
./src/test/modules/test_rls_hooks/Makefile:REGRESS_OPTS =

--temp-config=$(top_srcdir)/src/test/modules/test_rls_hooks/rls_hooks.conf

Perhaps what we need to do is modify pg_regress.c slightly to allow more
than one --temp-config argument. But that could be done later.

Well, I'm pretty interested in using --temp-config for parallelism
testing; I want to be able to run the whole regression test suite with
a given --temp-config. I'm in agreement with this change but if it
doesn't play well with that need, I suppose I'll be writing that
pg_regress.c patch sooner rather than later.

"doesn't meet your need" is probably a better way of putting it. The
facility's use has grown beyond what I originally envisaged, so I think
we will need that patch.

Would you like me to apply what I have?

cheers

andrew

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#11Robert Haas
robertmhaas@gmail.com
In reply to: Andrew Dunstan (#10)
Re: [COMMITTERS] pgsql: Respect TEMP_CONFIG when running contrib regression tests.

On Sat, Feb 27, 2016 at 8:36 PM, Andrew Dunstan <andrew@dunslane.net> wrote:

"doesn't meet your need" is probably a better way of putting it. The
facility's use has grown beyond what I originally envisaged, so I think we
will need that patch.

Would you like me to apply what I have?

Go for it.

--
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

#12John Gorman
johngorman2@gmail.com
In reply to: Robert Haas (#9)
1 attachment(s)
Re: Re: [COMMITTERS] pgsql: Respect TEMP_CONFIG when running contrib regression tests.

On Sat, Feb 27, 2016 at 9:25 AM, Robert Haas <robertmhaas@gmail.com> wrote:

On Sat, Feb 27, 2016 at 7:08 PM, Andrew Dunstan <andrew@dunslane.net>
wrote:

Perhaps what we need to do is modify pg_regress.c slightly to allow more
than one --temp-config argument. But that could be done later.

Well, I'm pretty interested in using --temp-config for parallelism
testing; I want to be able to run the whole regression test suite with
a given --temp-config. I'm in agreement with this change but if it
doesn't play well with that need, I suppose I'll be writing that
pg_regress.c patch sooner rather than later.

Here is a patch to allow pg_regress to include several --temp-config files.

Attachments:

pg_regress-temp-configs-v1.patchapplication/octet-stream; name=pg_regress-temp-configs-v1.patchDownload
diff --git a/src/test/regress/pg_regress.c b/src/test/regress/pg_regress.c
index a1902fe..416829d 100644
--- a/src/test/regress/pg_regress.c
+++ b/src/test/regress/pg_regress.c
@@ -80,7 +80,7 @@ static char *encoding = NULL;
 static _stringlist *schedulelist = NULL;
 static _stringlist *extra_tests = NULL;
 static char *temp_instance = NULL;
-static char *temp_config = NULL;
+static _stringlist *temp_configs = NULL;
 static bool nolocale = false;
 static bool use_existing = false;
 static char *hostname = NULL;
@@ -2117,7 +2117,7 @@ regression_main(int argc, char *argv[], init_function ifunc, test_function tfunc
 				split_to_stringlist(strdup(optarg), ", ", &extraroles);
 				break;
 			case 19:
-				temp_config = strdup(optarg);
+				add_stringlist_item(&temp_configs, optarg);
 				break;
 			case 20:
 				use_existing = true;
@@ -2249,8 +2249,9 @@ regression_main(int argc, char *argv[], init_function ifunc, test_function tfunc
 		fputs("log_temp_files = 128kB\n", pg_conf);
 		fputs("max_prepared_transactions = 2\n", pg_conf);
 
-		if (temp_config != NULL)
+		for (sl = temp_configs; sl != NULL; sl = sl->next)
 		{
+			char	   *temp_config = sl->str;
 			FILE	   *extra_conf;
 			char		line_buf[1024];
 
#13Andrew Dunstan
andrew@dunslane.net
In reply to: John Gorman (#12)
Re: Re: [COMMITTERS] pgsql: Respect TEMP_CONFIG when running contrib regression tests.

On 02/27/2016 01:24 PM, John Gorman wrote:

On Sat, Feb 27, 2016 at 9:25 AM, Robert Haas <robertmhaas@gmail.com
<mailto:robertmhaas@gmail.com>> wrote:

On Sat, Feb 27, 2016 at 7:08 PM, Andrew Dunstan
<andrew@dunslane.net <mailto:andrew@dunslane.net>> wrote:

Perhaps what we need to do is modify pg_regress.c slightly to

allow more

than one --temp-config argument. But that could be done later.

Well, I'm pretty interested in using --temp-config for parallelism
testing; I want to be able to run the whole regression test suite with
a given --temp-config. I'm in agreement with this change but if it
doesn't play well with that need, I suppose I'll be writing that
pg_regress.c patch sooner rather than later.

Here is a patch to allow pg_regress to include several --temp-config
files.

Thanks, wonderfully small patch. Applied.

cheers

andrew

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers