Horton

Re: Making background psql nicer to use in tap tests

Andrew Dunstan <andrew@dunslane.net>

View thread View on postgr.es
From: Andrew Dunstan <andrew@dunslane.net>
To: Andres Freund <andres@anarazel.de>, Tom Lane <tgl@sss.pgh.pa.us>
Cc: pgsql-hackers@postgresql.org, Melanie Plageman <melanieplageman@gmail.com>
Date: 2023-03-15T14:10:20Z
Lists: pgsql-hackers 
On 2023-01-30 Mo 19:00, Andres Freund wrote:
> Hi,
>
> On 2023-01-30 15:06:46 -0500, Tom Lane wrote:
>> Andres Freund<andres@anarazel.de>  writes:
>>> It's annoyingly hard to wait for the result of a query in a generic way with
>>> background_psql(), and more generally for psql. background_psql() uses -XAtq,
>>> which means that we'll not get "status" output (like "BEGIN" or "(1 row)"),
>>> and that queries not returning anything are completely invisible.
>> Yeah, the empty-query-result problem was giving me fits recently.
>> +1 for wrapping this into something more convenient to use.
> I've hacked some on this. I first tried to just introduce a few helper
> functions in Cluster.pm, but that ended up being awkward. So I bit the bullet
> and introduced a new class (in BackgroundPsql.pm), and made background_psql()
> and interactive_psql() return an instance of it.
>
> This is just a rough prototype. Several function names don't seem great, it
> need POD documentation, etc.


Since this class is only intended to have instances created from 
Cluster, I would be inclined just to put it at the end of Cluster.pm 
instead of creating a new file. That makes it clearer that the new 
package is not standalone. We already have instances of that.

The first param of the constructor is a bit opaque. If it were going to 
be called from elsewhere I'd want something a bit more obvious, but I 
guess we can live with it here. An alternative might be 
multiple_constructors (e.g. new_background, new_interactive) which use a 
common private routine.

Don't have comments yet on the other things, will continue looking.


cheers


andrew

--
Andrew Dunstan
EDB:https://www.enterprisedb.com

Commits

  1. Fix missing installation/uninstallation rules for BackgroundPsql.pm

  2. Backport BackgroundPsql perl test module

  3. Add missing uninstallation rule for BackgroundPsql.pm

  4. Fix missing installation rules for BackgroundPsql.pm

  5. Skip \password TAP test on old IPC::Run versions

  6. Test SCRAM iteration changes with psql \password

  7. Refactor background psql TAP functions