From d135d15650a9eaee318949cfd5f4681c08b59b44 Mon Sep 17 00:00:00 2001 From: Daniel Gustafsson Date: Thu, 20 Feb 2025 00:55:14 +0100 Subject: [PATCH v4 2/2] Report test failure rather than aborting in case of timeout When an querying an interactive, or background, psql process the call would die() in case of a timeout since it used the IPC::Run timeout construct and not a timer. This aborts tests prematurely which makes debugging harder, and stops getting results for the remaining tests in the suite. This converts to using a timer object instead which will transfer control back to the caller regardless, returning undef in case of a timeout along with logging a test failure. Affected callsites are also updated to reflect this. Reviewed-by: Heikki Linnakangas Reviewed-by: Andrew Dunstan Discussion: https://postgr.es/m/1100715.1712265845@sss.pgh.pa.us --- src/test/modules/test_misc/t/005_timeouts.pl | 6 +----- .../perl/PostgreSQL/Test/BackgroundPsql.pm | 18 ++++++++++++------ src/test/recovery/t/031_recovery_conflict.pl | 2 +- 3 files changed, 14 insertions(+), 12 deletions(-) diff --git a/src/test/modules/test_misc/t/005_timeouts.pl b/src/test/modules/test_misc/t/005_timeouts.pl index cdce2afd935..5478d3394d1 100644 --- a/src/test/modules/test_misc/t/005_timeouts.pl +++ b/src/test/modules/test_misc/t/005_timeouts.pl @@ -118,11 +118,7 @@ $node->safe_psql('postgres', # We just initialize the GUC and wait. No transaction is required. $psql_session = $node->background_psql('postgres'); -$psql_session->query_until( - qr/starting_bg_psql/, q( - \echo starting_bg_psql - SET idle_session_timeout to '10ms'; -)); +$psql_session->query(q(SET idle_session_timeout to '10ms';)); # Wait until the backend enters the timeout injection point. $node->wait_for_event('client backend', 'idle-session-timeout'); diff --git a/src/test/perl/PostgreSQL/Test/BackgroundPsql.pm b/src/test/perl/PostgreSQL/Test/BackgroundPsql.pm index c611a61cf4e..3cdd0852bd7 100644 --- a/src/test/perl/PostgreSQL/Test/BackgroundPsql.pm +++ b/src/test/perl/PostgreSQL/Test/BackgroundPsql.pm @@ -100,7 +100,7 @@ sub new "Forbidden caller of constructor: package: $package, file: $file:$line" unless $package->isa('PostgreSQL::Test::Cluster'); - $psql->{timeout} = IPC::Run::timeout( + $psql->{timeout} = IPC::Run::timer( defined($timeout) ? $timeout : $PostgreSQL::Test::Utils::timeout_default); @@ -176,8 +176,10 @@ sub wait_connect =item $session->quit -Close the session and clean up resources. Each test run must be closed with -C. +Close the psql session and clean up resources. Each psql session must be +closed with C 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. =cut @@ -187,7 +189,9 @@ sub quit $self->{stdin} .= "\\q\n"; - return $self->{run}->finish; + my $ret = $self->{run}->finish; + ok($ret, 'Terminating interactive psql session'); + return $ret; } =pod @@ -271,7 +275,8 @@ sub query $self->{run}, $self->{timeout}, \$self->{stderr}, qr/$banner_match/); - die "psql query timed out" if $self->{timeout}->is_expired; + ok(!$self->{timeout}->is_expired, 'psql query timed out') || return; + $output = $self->{stdout}; note "results query $query_cnt:\n", explain { @@ -339,7 +344,8 @@ sub query_until pump_until($self->{run}, $self->{timeout}, \$self->{stdout}, $until); - die "psql query timed out" if $self->{timeout}->is_expired; + ok(!$self->{timeout}->is_expired, 'psql query_until did not time out') + || return; $ret = $self->{stdout}; diff --git a/src/test/recovery/t/031_recovery_conflict.pl b/src/test/recovery/t/031_recovery_conflict.pl index 028b0b5f0e1..965537d1f97 100644 --- a/src/test/recovery/t/031_recovery_conflict.pl +++ b/src/test/recovery/t/031_recovery_conflict.pl @@ -253,7 +253,7 @@ $res = $psql_standby->query_until( -- wait for lock held by prepared transaction SELECT * FROM $table2; ]); -ok(1, +isnt($res, undef, "$sect: cursor holding conflicting pin, also waiting for lock, established" ); -- 2.39.3 (Apple Git-146)