Re: pgsql: Close stdin where it's not needed in TestLib.pm procedures

[Andrew Dunstan]

On 11/26/19 3:47 AM, Michael Paquier wrote:
> On Tue, Nov 26, 2019 at 02:13:50AM -0500, Tom Lane wrote:
>> Perhaps the "where possible" caveat needs to include a test whether
>> IO::Pty is installed?  It's evidently not there by default everywhere.
>>
>> It's possible that we should just move the goalposts and say IO::Pty
>> is required for TAP testing.  I can foresee that we'll have to do
>> that if we ever want automated tests of psql's tab completion,
>> for example.
> Hmm.  I am not sure that we are at that point yet (in which case we'd
> need to check after it in configure).  Any other modules we require to
> be installed are directly imported to our code, but here we visibly
> have a dependency forced by the loaded code.  My point is that it does
> not impact platforms that do not need it, so it seems like a waste to
> require for everybody something which is actually necessary only for a
> portion of users running the TAP tests.



Yeah, I don't want to go that far at this stage at least.


here's my proposed fix.


I will adjust the code that wants to use it to have a block containing
the tests that need the pty code something like this:


SKIP:
{
  skip 3, "no Pty support" unless $have_io_pty;
  ...
}


cheers


andrew


-- 
Andrew Dunstan                https://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services


diff --git a/src/test/perl/TestLib.pm b/src/test/perl/TestLib.pm
index 18b4ce35c2..7d62914218 100644
--- a/src/test/perl/TestLib.pm
+++ b/src/test/perl/TestLib.pm
@@ -21,6 +21,7 @@ TestLib - helper module for writing PostgreSQL's C<prove> tests.
 
   # Miscellanea
   print "on Windows" if $TestLib::windows_os;
+  print "IO::Pty is available" If $TestLib::have_io_pty;
   my $path = TestLib::perl2host($backup_dir);
   ok(check_mode_recursive($stream_dir, 0700, 0600),
     "check stream dir permissions");
@@ -83,9 +84,10 @@ our @EXPORT = qw(
   command_checks_all
 
   $windows_os
+  $have_io_pty
 );
 
-our ($windows_os, $tmp_check, $log_path, $test_logfile);
+our ($windows_os, $tmp_check, $log_path, $test_logfile, $have_io_pty);
 
 my @no_stdin;
 
@@ -119,6 +121,9 @@ BEGIN
 		require Win32API::File;
 		Win32API::File->import(qw(createFile OsFHandleOpen CloseHandle));
 	}
+
+	eval { require IO::Pty; };
+	$have_io_pty = 1 unless $@;
 }
 
 =pod
@@ -133,6 +138,11 @@ Set to true when running under Windows, except on Cygwin.
 
 =back
 
+=item C<$have_io_pty>
+
+Set to true when IO::Pty is available.
+
+=back
 =cut
 
 INIT
@@ -182,18 +192,17 @@ INIT
 	autoflush $testlog 1;
 
 	# Settings to close stdin for certain commands.
-	# On non-Windows, use a pseudo-terminal, so that libraries like openssl
-	# which open the tty if they think stdin isn't one for a password
-	# don't block. Windows doesn't have ptys, so just provide an empty
-	# string for stdin.
-	if ($windows_os)
+	# If IO::Pty is present, use a pseudo-terminal, so that libraries like
+	# openssl which open the tty if they think stdin isn't one for a password
+	# don't block. Elsewhere just provide an empty string for stdin.
+	if ($have_io_pty)
 	{
-		@no_stdin = ('<', \"");
+		use charnames ':full';
+		@no_stdin = ('<pty<', \"\N{END OF TRANSMISSION}");
 	}
 	else
 	{
-		use charnames ':full';
-		@no_stdin = ('<pty<', \"\N{END OF TRANSMISSION}");
+		@no_stdin = ('<', \"");
 	}
 }