v1-0002-Report-test-failure-rather-than-aborting-in-case-.patch

From 4e7187b824b1a1df3144775eceb40bd996a2f3df Mon Sep 17 00:00:00 2001
From: Daniel Gustafsson <dgustafsson@postgresql.org>
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<quit>.
+C<quit>.  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)