From 4e7187b824b1a1df3144775eceb40bd996a2f3df Mon Sep 17 00:00:00 2001 From: Daniel Gustafsson Date: Tue, 9 Apr 2024 18:00:17 +0200 Subject: [PATCH v1 2/3] 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. --- src/test/modules/test_misc/t/005_timeouts.pl | 6 +----- src/test/perl/PostgreSQL/Test/BackgroundPsql.pm | 15 ++++++++++----- src/test/recovery/t/031_recovery_conflict.pl | 4 +--- src/test/recovery/t/037_invalid_database.pl | 1 + 4 files changed, 13 insertions(+), 13 deletions(-) diff --git a/src/test/modules/test_misc/t/005_timeouts.pl b/src/test/modules/test_misc/t/005_timeouts.pl index a792610231..8e7086402c 100644 --- a/src/test/modules/test_misc/t/005_timeouts.pl +++ b/src/test/modules/test_misc/t/005_timeouts.pl @@ -109,11 +109,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 4091c311b8..5df972a20d 100644 --- a/src/test/perl/PostgreSQL/Test/BackgroundPsql.pm +++ b/src/test/perl/PostgreSQL/Test/BackgroundPsql.pm @@ -96,7 +96,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); @@ -148,7 +148,8 @@ sub _wait_connect =item $session->quit Close the session and clean up resources. Each test run must be closed with -C. +C. Returns TRUE when the session was cleanly terminated, otherwise +FALSE. A testfailure will be issued in case the session failed to finish. =cut @@ -158,7 +159,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 @@ -219,7 +222,8 @@ sub query pump_until($self->{run}, $self->{timeout}, \$self->{stdout}, qr/$banner/); - die "psql query timed out" if $self->{timeout}->is_expired; + isnt($self->{timeout}->is_expired, 'psql query timed out'); + return undef if $self->{timeout}->is_expired; $output = $self->{stdout}; # remove banner again, our caller doesn't care @@ -278,7 +282,8 @@ sub query_until pump_until($self->{run}, $self->{timeout}, \$self->{stdout}, $until); - die "psql query timed out" if $self->{timeout}->is_expired; + isnt($self->{timeout}->is_expired, 'psql query_until timed out'); + return undef if $self->{timeout}->is_expired; $ret = $self->{stdout}; diff --git a/src/test/recovery/t/031_recovery_conflict.pl b/src/test/recovery/t/031_recovery_conflict.pl index d87efa823f..62936f52d2 100644 --- a/src/test/recovery/t/031_recovery_conflict.pl +++ b/src/test/recovery/t/031_recovery_conflict.pl @@ -253,9 +253,7 @@ $res = $psql_standby->query_until( -- wait for lock held by prepared transaction SELECT * FROM $table2; ]); -ok(1, - "$sect: cursor holding conflicting pin, also waiting for lock, established" -); +isnt($res, undef, "$sect: cursor holding conflicting pin, also waiting for lock, established"); # just to make sure we're waiting for lock already ok( $node_standby->poll_query_until( diff --git a/src/test/recovery/t/037_invalid_database.pl b/src/test/recovery/t/037_invalid_database.pl index 32b7d8af57..256314d165 100644 --- a/src/test/recovery/t/037_invalid_database.pl +++ b/src/test/recovery/t/037_invalid_database.pl @@ -87,6 +87,7 @@ is($node->psql('postgres', 'DROP DATABASE regression_invalid'), # dropping the database, making it a suitable point to wait. my $bgpsql = $node->background_psql('postgres', on_error_stop => 0); my $pid = $bgpsql->query('SELECT pg_backend_pid()'); +isnt($pid, undef, 'Get backend PID'); # create the database, prevent drop database via lock held by a 2PC transaction ok( $bgpsql->query_safe( -- 2.39.3 (Apple Git-146)