That looks pretty solid to me, and I can confirm that it passes
on wrasse's host. The only nit I can find to pick is that this:
+ usleep(10_000) until -s "$tempdir/psql.pid" or ($count++ > 180 * 100
and die 'pid file did not appear');
basically assumes that psql.pid will be written atomically.
It'd be marginally safer to wait till psql.pid can be seen to
contain a newline. I don't think that would be too hard to do,
if you put the slurp_file call inside the wait loop and inspect
its result.
I finally came around to have a look at the patch.
For the issue raised above, I can see that the file could be created but
not yet written, but as the patch waits for the file to be non zero, ISTM
that the probability of a torn write of a single integer by echo is so
remote that we could let it as that?
Attached a version with the slurping in the loop anyway.
--
Fabien.
diff --git a/src/bin/psql/t/020_cancel.pl b/src/bin/psql/t/020_cancel.pl
index 7713fff8e4..545ce57ead 100644
--- a/src/bin/psql/t/020_cancel.pl
+++ b/src/bin/psql/t/020_cancel.pl
@@ -7,6 +7,7 @@ use warnings;
use PostgresNode;
use TestLib;
use Test::More tests => 2;
+use Time::HiRes qw(usleep);
my $tempdir = TestLib::tempdir;
@@ -28,19 +29,35 @@ SKIP: {
my ($stdin, $stdout, $stderr);
# Test whether shell supports $PPID. It's part of POSIX, but some
- # pre-/non-POSIX shells don't support it (e.g., NetBSD, Solaris).
+ # pre-/non-POSIX shells don't support it (e.g., NetBSD).
$stdin = "\\! echo \$PPID";
IPC::Run::run(['psql', '-X', '-v', 'ON_ERROR_STOP=1'], '<', \$stdin, '>', \$stdout, '2>', \$stderr);
$stdout =~ /^\d+$/ or skip "shell apparently does not support \$PPID", 2;
- local $SIG{ALRM} = sub {
- my $psql_pid = TestLib::slurp_file("$tempdir/psql.pid");
- kill 'INT', $psql_pid;
- };
- alarm 1;
-
- $stdin = "\\! echo \$PPID >$tempdir/psql.pid\nselect pg_sleep(3);";
- my $result = IPC::Run::run(['psql', '-X', '-v', 'ON_ERROR_STOP=1'], '<', \$stdin, '>', \$stdout, '2>', \$stderr);
+ # Now start the real test
+ my $h = IPC::Run::start(['psql', '-X', '-v', 'ON_ERROR_STOP=1'], \$stdin, \$stdout, \$stderr);
+
+ # Get the PID
+ $stdout = '';
+ $stderr = '';
+ $stdin = "\\! echo \$PPID >$tempdir/psql.pid\n";
+ pump $h while length $stdin;
+ my $count;
+ my $psql_pid;
+ usleep(10_000)
+ until (-s "$tempdir/psql.pid" and ($psql_pid = TestLib::slurp_file("$tempdir/psql.pid")) =~ /^\d+\n/s)
+ or ($count++ > 180 * 100 and die 'pid file did not appear');
+
+ # Send sleep command and wait until the server has registered it
+ $stdin = "select pg_sleep(180);\n";
+ pump $h while length $stdin;
+ $node->poll_query_until('postgres', q{SELECT (SELECT count(*) FROM pg_stat_activity WHERE query ~ 'pg_sleep') > 0;})
+ or die "timed out";
+
+ # Send cancel request
+ kill 'INT', $psql_pid;
+
+ my $result = finish $h;
ok(!$result, 'query failed as expected');
like($stderr, qr/canceling statement due to user request/, 'query was canceled');