Re: Making background psql nicer to use in tap tests

Daniel Gustafsson <daniel@yesql.se>

From: Daniel Gustafsson <daniel@yesql.se>
To: Andres Freund <andres@anarazel.de>
Cc: Andrew Dunstan <andrew@dunslane.net>, Tom Lane <tgl@sss.pgh.pa.us>, PostgreSQL Hackers <pgsql-hackers@postgresql.org>, Melanie Plageman <melanieplageman@gmail.com>, Amit Kapila <amit.kapila16@gmail.com>
Date: 2023-04-03T08:39:41Z
Lists: pgsql-hackers
> On 2 Apr 2023, at 23:37, Andres Freund <andres@anarazel.de> wrote:

> There's this XXX that I added:
> 
>> @@ -57,11 +51,10 @@ sub test_streaming
>> 	COMMIT;
>> 	});
>> 
>> -	$in .= q{
>> -	COMMIT;
>> -	\q
>> -	};
>> -	$h->finish;    # errors make the next test fail, so ignore them here
>> +	$h->query_safe('COMMIT');
>> +	$h->quit;
>> +	# XXX: Not sure what this means
>> +    # errors make the next test fail, so ignore them here
>> 
>> 	$node_publisher->wait_for_catchup($appname);
> 
> I still don't know what that comment is supposed to mean, unfortunately.

My reading of it is that it's ignoring any croak errors which IPC::Run might
throw if ->finish() isn't able to reap the psql process which had the \q.

I've added Amit who committed it in 216a784829c on cc: to see if he remembers
the comment in question and can shed some light.  Skimming the linked thread
yields no immediate clues.

--
Daniel Gustafsson




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