On 2024-04-04 Th 17:24, Tom Lane wrote:
TIL that IPC::Run::timer is not the same as IPC::Run::timeout.
With a timer object you have to check $timer->is_expired to see
if the timeout has elapsed, but with a timeout object you don't
because it will throw a Perl exception upon timing out, probably
killing your test program.
It appears that a good chunk of our TAP codebase has not read this
memo, because I see plenty of places that are checking is_expired
in the naive belief that they'll still have control after a timeout
has fired.
I started having a look at these.
Here are the cases I found:
./src/bin/psql/t/010_tab_completion.pl: my $okay = ($out =~ $pattern
&& !$h->{timeout}->is_expired);
./src/test/perl/PostgreSQL/Test/BackgroundPsql.pm: until
$self->{stdout} =~ /$banner/ || $self->{timeout}->is_expired;
./src/test/perl/PostgreSQL/Test/BackgroundPsql.pm: die "psql startup
timed out" if $self->{timeout}->is_expired;
./src/test/perl/PostgreSQL/Test/BackgroundPsql.pm: die "psql query
timed out" if $self->{timeout}->is_expired;
./src/test/perl/PostgreSQL/Test/BackgroundPsql.pm: die "psql query
timed out" if $self->{timeout}->is_expired;
./src/test/perl/PostgreSQL/Test/Cluster.pm: # timeout, which
we'll handle by testing is_expired
./src/test/perl/PostgreSQL/Test/Cluster.pm: unless
$timeout->is_expired;
./src/test/perl/PostgreSQL/Test/Cluster.pm:timeout is the
IPC::Run::Timeout object whose is_expired method can be tested
./src/test/perl/PostgreSQL/Test/Cluster.pm: # timeout, which
we'll handle by testing is_expired
./src/test/perl/PostgreSQL/Test/Cluster.pm: unless
$timeout->is_expired;
./src/test/perl/PostgreSQL/Test/Utils.pm: if ($timeout->is_expired)
./src/test/recovery/t/021_row_visibility.pl: if
($psql_timeout->is_expired)
./src/test/recovery/t/032_relfilenode_reuse.pl: if
($psql_timeout->is_expired)
Those in Cluster.pm look correct - they are doing the run() in an eval
block and testing for the is_expired setting in an exception block. The
other cases look more suspect. I'll take a closer look.
cheers
andrew
--
Andrew Dunstan
EDB:https://www.enterprisedb.com
