On Mon, 14 Aug 2023 23:37:25 +0900
Yugo NAGATA <[email protected]> wrote:
> On Mon, 14 Aug 2023 08:29:25 +0900
> Michael Paquier <[email protected]> wrote:
>
> > On Sun, Aug 13, 2023 at 11:22:33AM +0200, Fabien COELHO wrote:
> > > Test run is ok on my Ubuntu laptop.
> >
> > I have a few comments about this patch.
> >
> > On HEAD and even after this patch, we still have the following:
> > SKIP:
> >
> > {
> >
> > skip "cancel test requires a Unix
> > shell", 2 if $windows_os;
> >
> > Could the SKIP be removed for $windows_os? If not, this had better be
> > documented because the reason for the skip becomes incorrect.
> >
> > The comment at the top of the SKIP block still states the following:
> > # There is, as of this writing, no documented way to get the PID of
> > # the process from IPC::Run. As a workaround, we have psql print its
> > # own PID (which is the parent of the shell launched by psql) to a
> > # file.
> >
> > This is also incorrect.
>
> Thank you for your comments
>
> I will check whether the test works in Windows and remove SKIP if possible.
> Also, I'll fix the comment in either case.
I checked if the test using IPC::Run::signal can work on Windows, and
confirmed that this didn't work because sending SIGINT caused to
terminate the test itself. Here is the results;
t/001_basic.pl ........... ok
t/010_tab_completion.pl .. skipped: readline is not supported by this build
t/020_cancel.pl .......... Terminating on signal SIGINT(2)
Therefore, this test should be skipped on Windows.
I attached the update patch. I removed the incorrect comments and
unnecessary lines. Also, I rewrote the test to use "skip_all" instead
of SKIP because we skip the whole test rather than a part of it.
Regards,
Yugo Nagata
>
> Regards,
> Yugo Nagata
>
> --
> Yugo NAGATA <[email protected]>
>
>
--
Yugo NAGATA <[email protected]>
diff --git a/src/bin/psql/t/020_cancel.pl b/src/bin/psql/t/020_cancel.pl
index 0765d82b92..cabbf0210a 100644
--- a/src/bin/psql/t/020_cancel.pl
+++ b/src/bin/psql/t/020_cancel.pl
@@ -9,72 +9,39 @@ use PostgreSQL::Test::Utils;
use Test::More;
use Time::HiRes qw(usleep);
-my $tempdir = PostgreSQL::Test::Utils::tempdir;
+# Test query canceling by sending SIGINT to a running psql
+
+if ($windows_os)
+{
+ plan skip_all => 'sending SIGINT on Windows terminates the test itself';
+}
my $node = PostgreSQL::Test::Cluster->new('main');
$node->init;
$node->start;
-# Test query canceling by sending SIGINT to a running psql
-#
-# There is, as of this writing, no documented way to get the PID of
-# the process from IPC::Run. As a workaround, we have psql print its
-# own PID (which is the parent of the shell launched by psql) to a
-# file.
-SKIP:
-{
- skip "cancel test requires a Unix shell", 2 if $windows_os;
+local %ENV = $node->_get_env();
- local %ENV = $node->_get_env();
+my ($stdin, $stdout, $stderr);
+my $h = IPC::Run::start([ 'psql', '-X', '-v', 'ON_ERROR_STOP=1' ],
+ \$stdin, \$stdout, \$stderr);
- my ($stdin, $stdout, $stderr);
+# Send sleep command and wait until the server has registered it
+$stdin = "select pg_sleep($PostgreSQL::Test::Utils::timeout_default);\n";
+pump $h while length $stdin;
+$node->poll_query_until('postgres',
+ q{SELECT (SELECT count(*) FROM pg_stat_activity WHERE query ~ '^select pg_sleep') > 0;}
+) or die "timed out";
- # Test whether shell supports $PPID. It's part of POSIX, but some
- # 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;
+# Send cancel request
+$h->signal('INT');
- # Now start the real test
- my $h = IPC::Run::start([ 'psql', '-X', '-v', 'ON_ERROR_STOP=1' ],
- \$stdin, \$stdout, \$stderr);
+my $result = finish $h;
- # Get the PID
- $stdout = '';
- $stderr = '';
- $stdin = "\\! echo \$PPID >$tempdir/psql.pid\n";
- pump $h while length $stdin;
- my $count;
- my $psql_pid;
- until (
- -s "$tempdir/psql.pid"
- and ($psql_pid =
- PostgreSQL::Test::Utils::slurp_file("$tempdir/psql.pid")) =~
- /^\d+\n/s)
- {
- ($count++ < 100 * $PostgreSQL::Test::Utils::timeout_default)
- or die "pid file did not appear";
- usleep(10_000);
- }
-
- # Send sleep command and wait until the server has registered it
- $stdin = "select pg_sleep($PostgreSQL::Test::Utils::timeout_default);\n";
- pump $h while length $stdin;
- $node->poll_query_until('postgres',
- q{SELECT (SELECT count(*) FROM pg_stat_activity WHERE query ~ '^select 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');
-}
+ok(!$result, 'query failed as expected');
+like(
+ $stderr,
+ qr/canceling statement due to user request/,
+ 'query was canceled');
done_testing();
