Re: IPC::Run::time[r|out] vs our TAP tests
Andrew Dunstan <andrew@dunslane.net>
From: Andrew Dunstan <andrew@dunslane.net>
To: Daniel Gustafsson <daniel@yesql.se>
Cc: Heikki Linnakangas <hlinnaka@iki.fi>,
PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>,
Tom Lane <tgl@sss.pgh.pa.us>
Date: 2025-02-20T13:06:12Z
Lists: pgsql-hackers
On 2025-02-19 We 6:56 PM, Daniel Gustafsson wrote:
>> On 19 Feb 2025, at 23:08, Andrew Dunstan<andrew@dunslane.net> wrote:
>> On 2024-10-31 Th 6:18 PM, Heikki Linnakangas wrote:
> Thanks for review!
>
>>> Perhaps sommething like this:
>>>
>>> "Close the psql session and clean up resources. Each psql session must be closed with C<quit> before the end of the test.
>>> Returns TRUE if psql exited successfully (i.e. with zero exit code), otherwise returns FALSE and reports a test failure."
>>>
>>> Would that be accurate?
>> I would be OK with Heikki's version.
> Fixed.
>
>> The patches have bitrotted slightly.
> The attached rebases over current HEAD and passes check-world locally for me.
>
>> Also this is wrong, I think:
>>
>> isnt($self->{timeout}->is_expired, 'psql query_until timed out');
>>
>> I think it should be
>>
>> ok(! $self->{timeout}->is_expired, 'psql query_until did not time out');
> Fixed.
Actually, since ok() and friends return true iff the test succeeds,
instead of
+ ok(! $self->{timeout}->is_expired, 'psql query_until did not time
out');
+ return undef if $self->{timeout}->is_expired;
you can avoid doing the same test twice and say:
ok(! $self->{timeout}->is_expired, 'psql query_until did not time
out') || return undef;
although for slightly technical reasons perlcritic disapproves of
"return undef" and prefers that you just write a bare "return" so we
should also fix that.
Sorry for taking a second bite at the cherry.
cheers
andrew
--
Andrew Dunstan
EDB:https://www.enterprisedb.com
Commits
-
Restart BackgroundPsql's timer more nicely.
- 53c6bd0aa3de 19 (unreleased) landed
-
Implement pg_wal_replay_wait() stored procedure
- 3c5db1d6b016 18.0 cited
-
psql: fix variable existence tab completion
- 927332b95e77 17.0 cited