On Mon, 14 Aug 2023 23:37:25 +0900 Yugo NAGATA <nag...@sraoss.co.jp> wrote:
> On Mon, 14 Aug 2023 08:29:25 +0900 > Michael Paquier <mich...@paquier.xyz> 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 <nag...@sraoss.co.jp> > > -- Yugo NAGATA <nag...@sraoss.co.jp>
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();