Is pg_regress --use-existing used by anyone or is it broken?
When looking at pg_regress I noticed that the --use-existing support didn't
seem to work. ISTM that the removal/creation of test databases and roles
doesn't run since the conditional is reversed. There is also no support for
using a non-default socket directory with PG_REGRESS_SOCK_DIR. The attached
hack fixes these and allows the tests to execute for me, but even with that the
test_setup suite fails due to the tablespace not being dropped and recreated
like databases and roles.
Is it me who is too thick to get it working, or is it indeed broken? If it's
the latter, it's been like that for a long time which seems to indicate that it
isn't really used and should probably be removed rather than fixed?
Does anyone here use it?
--
Daniel Gustafsson
Attachments:
pg_regress_use_existing.diffapplication/octet-stream; name=pg_regress_use_existing.diff; x-unix-mode=0644Download
diff --git a/src/test/regress/pg_regress.c b/src/test/regress/pg_regress.c
index ec67588cf5..6ba5b13be9 100644
--- a/src/test/regress/pg_regress.c
+++ b/src/test/regress/pg_regress.c
@@ -880,11 +880,17 @@ initialize_environment(void)
pgport = getenv("PGPORT");
if (!pghost)
{
- /* Keep this bit in sync with libpq's default host location: */
- if (DEFAULT_PGSOCKET_DIR[0])
- /* do nothing, we'll print "Unix socket" below */ ;
+ sockdir = getenv("PG_REGRESS_SOCK_DIR");
+ if (sockdir)
+ setenv("PGHOST", sockdir, 1);
else
- pghost = "localhost"; /* DefaultHost in fe-connect.c */
+ {
+ /* Keep this bit in sync with libpq's default host location: */
+ if (DEFAULT_PGSOCKET_DIR[0])
+ /* do nothing, we'll print "Unix socket" below */ ;
+ else
+ pghost = "localhost"; /* DefaultHost in fe-connect.c */
+ }
}
if (pghost && pgport)
@@ -2562,7 +2568,7 @@ regression_main(int argc, char *argv[],
* Using an existing installation, so may need to get rid of
* pre-existing database(s) and role(s)
*/
- if (!use_existing)
+ if (use_existing)
{
for (sl = dblist; sl; sl = sl->next)
drop_database_if_exists(sl->str);
@@ -2574,13 +2580,10 @@ regression_main(int argc, char *argv[],
/*
* Create the test database(s) and role(s)
*/
- if (!use_existing)
- {
- for (sl = dblist; sl; sl = sl->next)
- create_database(sl->str);
- for (sl = extraroles; sl; sl = sl->next)
- create_role(sl->str, dblist);
- }
+ for (sl = dblist; sl; sl = sl->next)
+ create_database(sl->str);
+ for (sl = extraroles; sl; sl = sl->next)
+ create_role(sl->str, dblist);
/*
* Ready to run the tests
On Mon, Aug 28, 2023 at 03:11:15PM +0200, Daniel Gustafsson wrote:
When looking at pg_regress I noticed that the --use-existing support didn't
seem to work. ISTM that the removal/creation of test databases and roles
doesn't run since the conditional is reversed. There is also no support for
using a non-default socket directory with PG_REGRESS_SOCK_DIR. The attached
hack fixes these and allows the tests to execute for me, but even with that the
test_setup suite fails due to the tablespace not being dropped and recreated
like databases and roles.Is it me who is too thick to get it working, or is it indeed broken? If it's
the latter, it's been like that for a long time which seems to indicate that it
isn't really used and should probably be removed rather than fixed?Does anyone here use it?
I don't think I've ever used it. AFAICT it was added with hot standby mode
(efc16ea) to support 'make standbycheck', which was removed last year
(4483b2cf). Unless someone claims to be using it, it's probably fine to
just remove it.
--
Nathan Bossart
Amazon Web Services: https://aws.amazon.com
On 29 Aug 2023, at 23:38, Nathan Bossart <nathandbossart@gmail.com> wrote:
On Mon, Aug 28, 2023 at 03:11:15PM +0200, Daniel Gustafsson wrote:
Does anyone here use it?
I don't think I've ever used it. AFAICT it was added with hot standby mode
(efc16ea) to support 'make standbycheck', which was removed last year
(4483b2cf). Unless someone claims to be using it, it's probably fine to
just remove it.
Having looked a bit more on it I have a feeling that plain removing it would be
the best option. Unless someone chimes in as a user of it I'll propose a patch
to remove it.
--
Daniel Gustafsson
On Tue, Aug 29, 2023 at 2:53 PM Daniel Gustafsson <daniel@yesql.se> wrote:
Having looked a bit more on it I have a feeling that plain removing it would be
the best option. Unless someone chimes in as a user of it I'll propose a patch
to remove it.
-1. I use it.
It's handy when using pg_regress with a custom test suite, where I
don't want to be nagged about disconnecting from the database every
time.
--
Peter Geoghegan
On 30 Aug 2023, at 00:33, Peter Geoghegan <pg@bowt.ie> wrote:
On Tue, Aug 29, 2023 at 2:53 PM Daniel Gustafsson <daniel@yesql.se> wrote:
Having looked a bit more on it I have a feeling that plain removing it would be
the best option. Unless someone chimes in as a user of it I'll propose a patch
to remove it.-1. I use it.
Thanks for confirming!
It's handy when using pg_regress with a custom test suite, where I
don't want to be nagged about disconnecting from the database every
time.
I'm curious about your workflow around it, it seems to me that it's kind of
broken so I wonder if we instead then should make it an equal citizen with temp
instance?
--
Daniel Gustafsson
On Tue, Aug 29, 2023 at 3:37 PM Daniel Gustafsson <daniel@yesql.se> wrote:
It's handy when using pg_regress with a custom test suite, where I
don't want to be nagged about disconnecting from the database every
time.I'm curious about your workflow around it, it seems to me that it's kind of
broken so I wonder if we instead then should make it an equal citizen with temp
instance?
I'm confused. You seem to think that it's a problem that
--use-existing doesn't create databases and roles. But that's the
whole point, at least for me.
I don't use --use-existing to run the standard regression tests, or
anything like that. I use it to run my own custom test suite, often
while relying upon the database having certain data already. Sometimes
it's a nontrivial amount of data. I don't want to have to set up and
tear down the data every time, since it isn't usually necessary.
I usually have a relatively small and fast running read-only test
suite, and a larger test suite that does indeed need to do various
setup and teardown steps. It isn't possible to run the smaller test
suite without having first run the larger one at least once. But this
is just for me, during development. Right now, with my SAOP nbtree
project, the smaller test suite takes me about 50ms to run, while the
larger one takes almost 10 seconds.
--
Peter Geoghegan
On 30 Aug 2023, at 00:55, Peter Geoghegan <pg@bowt.ie> wrote:
On Tue, Aug 29, 2023 at 3:37 PM Daniel Gustafsson <daniel@yesql.se> wrote:
It's handy when using pg_regress with a custom test suite, where I
don't want to be nagged about disconnecting from the database every
time.I'm curious about your workflow around it, it seems to me that it's kind of
broken so I wonder if we instead then should make it an equal citizen with temp
instance?I'm confused. You seem to think that it's a problem that
--use-existing doesn't create databases and roles. But that's the
whole point, at least for me.
Well, I think it's problematic that it doesn't handle database and role
creation due to it being buggy. I'll have another look at fixing the issues to
see if there is more than what I posted upthread, while at the same time making
sure it will still support your use-case.
--
Daniel Gustafsson