From 323fc9c9d07c6a87e6453ff8ea0b083ec319d804 Mon Sep 17 00:00:00 2001 From: Soumendra Ganguly <soumendraganguly@gmail.com> Date: Sun, 25 Oct 2020 01:30:15 -0500 Subject: [PATCH 01/10] Updated tests for the pty standard library Signed-off-by: Soumendra Ganguly <soumendraganguly@gmail.com> --- Lib/test/test_pty.py | 104 ++++++++++++++++++++++++++++++++++++++----- 1 file changed, 93 insertions(+), 11 deletions(-) diff --git a/Lib/test/test_pty.py b/Lib/test/test_pty.py index 7ca0557800b6d8..22c46c51b4948a 100644 --- a/Lib/test/test_pty.py +++ b/Lib/test/test_pty.py @@ -5,6 +5,10 @@ import_module('termios') import errno +# try: +# import pty2 as pty +# except ModuleNotFoundError: +# import pty import pty import os import sys @@ -14,6 +18,11 @@ import io # readline import unittest +import struct +import tty +import fcntl +import platform + TEST_STRING_1 = b"I wish to buy a fish license.\n" TEST_STRING_2 = b"For my pet fish, Eric.\n" @@ -60,11 +69,16 @@ def _readline(fd): reader = io.FileIO(fd, mode='rb', closefd=False) return reader.readline() +def expectedFailureOnBSD(fun): + if platform.system().endswith("BSD"): + return unittest.expectedFailure(fun) + return fun # Marginal testing of pty suite. Cannot do extensive 'do or fail' testing # because pty code is not too portable. # XXX(nnorwitz): these tests leak fds when there is an error. +# Soumendra: test_openpty may leave tty in abnormal state upon failure. class PtyTest(unittest.TestCase): def setUp(self): old_alarm = signal.signal(signal.SIGALRM, self.handle_sig) @@ -81,6 +95,7 @@ def setUp(self): def handle_sig(self, sig, frame): self.fail("isatty hung") + # RELEVANT ANYMORE? @staticmethod def handle_sighup(signum, frame): # bpo-38547: if the process is the session leader, os.close(master_fd) @@ -88,21 +103,66 @@ def handle_sighup(signum, frame): # signal: just ignore the signal. pass - def test_basic(self): + @unittest.expectedFailure + def test_openpty(self): + try: + mode = tty.tcgetattr(pty.STDIN_FILENO) + except tty.error: + # pty.STDIN_FILENO not a tty? + debug("tty.tcgetattr(pty.STDIN_FILENO) failed") + mode = None + try: - debug("Calling master_open()") - master_fd, slave_name = pty.master_open() - debug("Got master_fd '%d', slave_name '%s'" % - (master_fd, slave_name)) - debug("Calling slave_open(%r)" % (slave_name,)) - slave_fd = pty.slave_open(slave_name) - debug("Got slave_fd '%d'" % slave_fd) + TIOCGWINSZ = tty.TIOCGWINSZ + TIOCSWINSZ = tty.TIOCSWINSZ + except AttributeError: + debug("TIOCSWINSZ/TIOCGWINSZ not available") + winsz = None + else: + try: + debug("Setting pty.STDIN_FILENO window size") + current_stdin_winsz = os.get_terminal_size(pty.STDIN_FILENO) + + # Set number of columns and rows to be the + # floors of 1/5 of respective original values + winsz = struct.pack("HHHH", current_stdin_winsz.lines//5, + current_stdin_winsz.columns//5, 0, 0) + fcntl.ioctl(pty.STDIN_FILENO, TIOCSWINSZ, winsz) + + # Were we able to set the window size + # of pty.STDIN_FILENO successfully? + s = struct.pack("HHHH", 0, 0, 0, 0) + new_stdin_winsz = fcntl.ioctl(pty.STDIN_FILENO, TIOCGWINSZ, s) + self.assertEqual(new_stdin_winsz, winsz, + "pty.STDIN_FILENO window size unchanged") + except OSError: + # pty.STDIN_FILENO not a tty? + debug("Failed to set pty.STDIN_FILENO window size") + winsz = None + + try: + debug("Calling pty.openpty()") + try: + master_fd, slave_fd = pty.openpty(mode, winsz) + except TypeError: + master_fd, slave_fd = pty.openpty() + debug("Got master_fd '%d', slave_fd '%d'" % + (master_fd, slave_fd)) except OSError: # " An optional feature could not be imported " ... ? raise unittest.SkipTest("Pseudo-terminals (seemingly) not functional.") - self.assertTrue(os.isatty(slave_fd), 'slave_fd is not a tty') + self.assertTrue(os.isatty(slave_fd), "slave_fd is not a tty") + if mode: + self.assertEqual(tty.tcgetattr(slave_fd), mode, + "openpty() failed to set slave termios") + if winsz: + s = struct.pack("HHHH", 0, 0, 0, 0) + self.assertEqual(fcntl.ioctl(slave_fd, TIOCGWINSZ, s), winsz, + "openpty() failed to set slave window size") + + # RELEVANT ANYMORE? # Solaris requires reading the fd before anything is returned. # My guess is that since we open and close the slave fd # in master_open(), we need to read the EOF. @@ -139,6 +199,13 @@ def test_basic(self): # to ignore this signal. os.close(master_fd) + if winsz: + winsz = struct.pack("HHHH", current_stdin_winsz.lines, + current_stdin_winsz.columns, 0, 0) + fcntl.ioctl(pty.STDIN_FILENO, TIOCSWINSZ, winsz) + + # pty.openpty() passed. + def test_fork(self): debug("calling pty.fork()") pid, master_fd = pty.fork() @@ -224,6 +291,21 @@ def test_fork(self): # pty.fork() passed. + @expectedFailureOnBSD + def test_master_read(self): + debug("Calling pty.openpty()") + master_fd, slave_fd = pty.openpty() + debug("Got master_fd '%d', slave_fd '%d'" % + (master_fd, slave_fd)) + + debug("Closing slave_fd") + os.close(slave_fd) + + debug("Reading from master_fd") + with self.assertRaises(OSError): + os.read(master_fd, 1) + + os.close(master_fd) class SmallPtyTests(unittest.TestCase): """These tests don't spawn children or hang.""" @@ -262,8 +344,9 @@ def _socketpair(self): self.files.extend(socketpair) return socketpair - def _mock_select(self, rfds, wfds, xfds): + def _mock_select(self, rfds, wfds, xfds, timeout=0): # This will raise IndexError when no more expected calls exist. + # This ignores the timeout self.assertEqual(self.select_rfds_lengths.pop(0), len(rfds)) return self.select_rfds_results.pop(0), [], [] @@ -318,7 +401,6 @@ def test__copy_eof_on_all(self): with self.assertRaises(IndexError): pty._copy(masters[0]) - def tearDownModule(): reap_children() From 4bb22ca70e8ca10a11d3c403ba57bd50d0687d2b Mon Sep 17 00:00:00 2001 From: Soumendra Ganguly <soumendraganguly@gmail.com> Date: Sun, 25 Oct 2020 02:33:27 -0500 Subject: [PATCH 02/10] In Lib/test/test_pty.py, wrap @expectedFailure to check if pty.STDIN_FILENO is a tty Signed-off-by: Soumendra Ganguly <soumendraganguly@gmail.com> --- Lib/test/test_pty.py | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/Lib/test/test_pty.py b/Lib/test/test_pty.py index 22c46c51b4948a..937ba9fd761a06 100644 --- a/Lib/test/test_pty.py +++ b/Lib/test/test_pty.py @@ -69,6 +69,15 @@ def _readline(fd): reader = io.FileIO(fd, mode='rb', closefd=False) return reader.readline() +def expectedFailureIfStdinIsTTY(fun): + # avoid isatty() for now + try: + tty.tcgetattr(pty.STDIN_FILENO) + return unittest.expectedFailure(fun) + except tty.error: + pass + return fun + def expectedFailureOnBSD(fun): if platform.system().endswith("BSD"): return unittest.expectedFailure(fun) @@ -103,7 +112,7 @@ def handle_sighup(signum, frame): # signal: just ignore the signal. pass - @unittest.expectedFailure + @expectedFailureIfStdinIsTTY def test_openpty(self): try: mode = tty.tcgetattr(pty.STDIN_FILENO) From a8723e7f03ec12b8b57de65eb94ed176be4059f2 Mon Sep 17 00:00:00 2001 From: Soumendra Ganguly <soumendraganguly@gmail.com> Date: Sun, 25 Oct 2020 03:36:19 -0500 Subject: [PATCH 03/10] In Lib/test/test_pty.py, add a check for "Darwin" in expectedFailureOnBSD Signed-off-by: Soumendra Ganguly <soumendraganguly@gmail.com> --- Lib/test/test_pty.py | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/Lib/test/test_pty.py b/Lib/test/test_pty.py index 937ba9fd761a06..59ef017c3aaa58 100644 --- a/Lib/test/test_pty.py +++ b/Lib/test/test_pty.py @@ -79,7 +79,8 @@ def expectedFailureIfStdinIsTTY(fun): return fun def expectedFailureOnBSD(fun): - if platform.system().endswith("BSD"): + PLATFORM = platform.system() + if PLATFORM.endswith("BSD") or PLATFORM == "Darwin": return unittest.expectedFailure(fun) return fun @@ -209,9 +210,9 @@ def test_openpty(self): os.close(master_fd) if winsz: - winsz = struct.pack("HHHH", current_stdin_winsz.lines, - current_stdin_winsz.columns, 0, 0) - fcntl.ioctl(pty.STDIN_FILENO, TIOCSWINSZ, winsz) + winsz = struct.pack("HHHH", current_stdin_winsz.lines, + current_stdin_winsz.columns, 0, 0) + fcntl.ioctl(pty.STDIN_FILENO, TIOCSWINSZ, winsz) # pty.openpty() passed. From 26c092820366ba417027109ca98c0ce1cf44f165 Mon Sep 17 00:00:00 2001 From: Soumendra Ganguly <soumendraganguly@gmail.com> Date: Mon, 26 Oct 2020 09:15:34 -0500 Subject: [PATCH 04/10] Remove some comments. Signed-off-by: Soumendra Ganguly <soumendraganguly@gmail.com> --- Lib/test/test_pty.py | 4 ---- 1 file changed, 4 deletions(-) diff --git a/Lib/test/test_pty.py b/Lib/test/test_pty.py index 59ef017c3aaa58..8a5744571ed0fe 100644 --- a/Lib/test/test_pty.py +++ b/Lib/test/test_pty.py @@ -5,10 +5,6 @@ import_module('termios') import errno -# try: -# import pty2 as pty -# except ModuleNotFoundError: -# import pty import pty import os import sys From 9a6e70fc9c9d6108357583344a416d7c84159ed9 Mon Sep 17 00:00:00 2001 From: Soumendra Ganguly <soumendraganguly@gmail.com> Date: Mon, 2 Nov 2020 15:45:05 -0600 Subject: [PATCH 05/10] Restructure the tests, perform cleanup. Signed-off-by: Soumendra Ganguly <soumendraganguly@gmail.com> --- Lib/test/test_pty.py | 88 +++++++++++++++++++++++++------------------- 1 file changed, 51 insertions(+), 37 deletions(-) diff --git a/Lib/test/test_pty.py b/Lib/test/test_pty.py index 8a5744571ed0fe..2338ee9c668d67 100644 --- a/Lib/test/test_pty.py +++ b/Lib/test/test_pty.py @@ -22,6 +22,13 @@ TEST_STRING_1 = b"I wish to buy a fish license.\n" TEST_STRING_2 = b"For my pet fish, Eric.\n" +try: + _TIOCGWINSZ = tty.TIOCGWINSZ + _TIOCSWINSZ = tty.TIOCSWINSZ + _HAVE_WINSZ = True +except AttributeError: + _HAVE_WINSZ = False + if verbose: def debug(msg): print(msg) @@ -80,11 +87,17 @@ def expectedFailureOnBSD(fun): return unittest.expectedFailure(fun) return fun +def _get_term_winsz(fd): + s = struct.pack("HHHH", 0, 0, 0, 0) + return fcntl.ioctl(fd, _TIOCGWINSZ, s) + +def _set_term_winsz(fd, winsz): + fcntl.ioctl(fd, _TIOCSWINSZ, winsz) + # Marginal testing of pty suite. Cannot do extensive 'do or fail' testing # because pty code is not too portable. # XXX(nnorwitz): these tests leak fds when there is an error. -# Soumendra: test_openpty may leave tty in abnormal state upon failure. class PtyTest(unittest.TestCase): def setUp(self): old_alarm = signal.signal(signal.SIGALRM, self.handle_sig) @@ -98,15 +111,32 @@ def setUp(self): self.addCleanup(signal.alarm, 0) signal.alarm(10) + # Save original stdin window size + self.stdin_rows = None + self.stdin_cols = None + if _HAVE_WINSZ: + try: + stdin_dim = os.get_terminal_size(pty.STDIN_FILENO) + self.stdin_rows = stdin_dim.lines + self.stdin_cols = stdin_dim.columns + old_stdin_winsz = struct.pack("HHHH", self.stdin_rows, + self.stdin_cols, 0, 0) + self.addCleanup(_set_term_winsz, pty.STDIN_FILENO, old_stdin_winsz) + except OSError: + # possible reason: current stdin is not a tty + pass + def handle_sig(self, sig, frame): self.fail("isatty hung") - # RELEVANT ANYMORE? @staticmethod def handle_sighup(signum, frame): # bpo-38547: if the process is the session leader, os.close(master_fd) # of "master_fd, slave_name = pty.master_open()" raises SIGHUP # signal: just ignore the signal. + # + # NOTE: the above comment is from an older version of the test; + # master_open() is not being used anymore. pass @expectedFailureIfStdinIsTTY @@ -114,46 +144,37 @@ def test_openpty(self): try: mode = tty.tcgetattr(pty.STDIN_FILENO) except tty.error: - # pty.STDIN_FILENO not a tty? + # possible reason: current stdin is not a tty debug("tty.tcgetattr(pty.STDIN_FILENO) failed") mode = None - try: - TIOCGWINSZ = tty.TIOCGWINSZ - TIOCSWINSZ = tty.TIOCSWINSZ - except AttributeError: - debug("TIOCSWINSZ/TIOCGWINSZ not available") - winsz = None - else: + new_stdin_winsz = None + if _HAVE_WINSZ: try: debug("Setting pty.STDIN_FILENO window size") - current_stdin_winsz = os.get_terminal_size(pty.STDIN_FILENO) - # Set number of columns and rows to be the # floors of 1/5 of respective original values - winsz = struct.pack("HHHH", current_stdin_winsz.lines//5, - current_stdin_winsz.columns//5, 0, 0) - fcntl.ioctl(pty.STDIN_FILENO, TIOCSWINSZ, winsz) + target_stdin_winsz = struct.pack("HHHH", self.stdin_rows//5, + self.stdin_cols//5, 0, 0) + _set_term_winsz(pty.STDIN_FILENO, target_stdin_winsz) # Were we able to set the window size # of pty.STDIN_FILENO successfully? - s = struct.pack("HHHH", 0, 0, 0, 0) - new_stdin_winsz = fcntl.ioctl(pty.STDIN_FILENO, TIOCGWINSZ, s) - self.assertEqual(new_stdin_winsz, winsz, + new_stdin_winsz = _get_term_winsz(pty.STDIN_FILENO) + self.assertEqual(new_stdin_winsz, target_stdin_winsz, "pty.STDIN_FILENO window size unchanged") except OSError: - # pty.STDIN_FILENO not a tty? - debug("Failed to set pty.STDIN_FILENO window size") - winsz = None + # possible reason: current stdin is not a tty + warnings.warn("Failed to set pty.STDIN_FILENO window size") + pass try: debug("Calling pty.openpty()") try: - master_fd, slave_fd = pty.openpty(mode, winsz) + master_fd, slave_fd = pty.openpty(mode, new_stdin_winsz) except TypeError: master_fd, slave_fd = pty.openpty() - debug("Got master_fd '%d', slave_fd '%d'" % - (master_fd, slave_fd)) + debug(f"Got master_fd '{master_fd}', slave_fd '{slave_fd}'") except OSError: # " An optional feature could not be imported " ... ? raise unittest.SkipTest("Pseudo-terminals (seemingly) not functional.") @@ -163,15 +184,16 @@ def test_openpty(self): if mode: self.assertEqual(tty.tcgetattr(slave_fd), mode, "openpty() failed to set slave termios") - if winsz: - s = struct.pack("HHHH", 0, 0, 0, 0) - self.assertEqual(fcntl.ioctl(slave_fd, TIOCGWINSZ, s), winsz, + if new_stdin_winsz: + self.assertEqual(_get_term_winsz(slave_fd), new_stdin_winsz, "openpty() failed to set slave window size") - # RELEVANT ANYMORE? # Solaris requires reading the fd before anything is returned. # My guess is that since we open and close the slave fd # in master_open(), we need to read the EOF. + # + # NOTE: the above comment is from an older version of the test; + # master_open() is not being used anymore. # Ensure the fd is non-blocking in case there's nothing to read. blocking = os.get_blocking(master_fd) @@ -205,13 +227,6 @@ def test_openpty(self): # to ignore this signal. os.close(master_fd) - if winsz: - winsz = struct.pack("HHHH", current_stdin_winsz.lines, - current_stdin_winsz.columns, 0, 0) - fcntl.ioctl(pty.STDIN_FILENO, TIOCSWINSZ, winsz) - - # pty.openpty() passed. - def test_fork(self): debug("calling pty.fork()") pid, master_fd = pty.fork() @@ -295,8 +310,6 @@ def test_fork(self): os.close(master_fd) - # pty.fork() passed. - @expectedFailureOnBSD def test_master_read(self): debug("Calling pty.openpty()") @@ -407,6 +420,7 @@ def test__copy_eof_on_all(self): with self.assertRaises(IndexError): pty._copy(masters[0]) + def tearDownModule(): reap_children() From f6d01f880083221d6abb2e6f8ead1077f57d52e3 Mon Sep 17 00:00:00 2001 From: Soumendra Ganguly <soumendraganguly@gmail.com> Date: Mon, 2 Nov 2020 16:16:16 -0600 Subject: [PATCH 06/10] Restructure test_openpty(). Signed-off-by: Soumendra Ganguly <soumendraganguly@gmail.com> --- Lib/test/test_pty.py | 1 + 1 file changed, 1 insertion(+) diff --git a/Lib/test/test_pty.py b/Lib/test/test_pty.py index 2338ee9c668d67..d5231f64b35a1f 100644 --- a/Lib/test/test_pty.py +++ b/Lib/test/test_pty.py @@ -18,6 +18,7 @@ import tty import fcntl import platform +import warnings TEST_STRING_1 = b"I wish to buy a fish license.\n" TEST_STRING_2 = b"For my pet fish, Eric.\n" From 85f173b5ec29bf01ba23d314f8cd5683e0a613e1 Mon Sep 17 00:00:00 2001 From: Soumendra Ganguly <soumendraganguly@gmail.com> Date: Mon, 2 Nov 2020 17:12:48 -0600 Subject: [PATCH 07/10] Restructure test_openpty(). Signed-off-by: Soumendra Ganguly <soumendraganguly@gmail.com> --- Lib/test/test_pty.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Lib/test/test_pty.py b/Lib/test/test_pty.py index d5231f64b35a1f..0cfd0f1dd7ec95 100644 --- a/Lib/test/test_pty.py +++ b/Lib/test/test_pty.py @@ -150,7 +150,7 @@ def test_openpty(self): mode = None new_stdin_winsz = None - if _HAVE_WINSZ: + if self.stdin_rows != None and self.stdin_cols != None: try: debug("Setting pty.STDIN_FILENO window size") # Set number of columns and rows to be the From 654693818afd24d3d614440ac953c44d6e54de22 Mon Sep 17 00:00:00 2001 From: Soumendra Ganguly <soumendraganguly@gmail.com> Date: Mon, 2 Nov 2020 17:37:51 -0600 Subject: [PATCH 08/10] Restructure test_openpty(). Signed-off-by: Soumendra Ganguly <soumendraganguly@gmail.com> --- Lib/test/test_pty.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/Lib/test/test_pty.py b/Lib/test/test_pty.py index 0cfd0f1dd7ec95..c44298b4799021 100644 --- a/Lib/test/test_pty.py +++ b/Lib/test/test_pty.py @@ -315,8 +315,7 @@ def test_fork(self): def test_master_read(self): debug("Calling pty.openpty()") master_fd, slave_fd = pty.openpty() - debug("Got master_fd '%d', slave_fd '%d'" % - (master_fd, slave_fd)) + debug(f"Got master_fd '{master_fd}', slave_fd '{slave_fd}'") debug("Closing slave_fd") os.close(slave_fd) From 0cde3f28ff866af19da331c1aace5439846dd21a Mon Sep 17 00:00:00 2001 From: Soumendra Ganguly <soumendraganguly@gmail.com> Date: Fri, 20 Nov 2020 08:37:39 -0600 Subject: [PATCH 09/10] Remove/change some comments. Signed-off-by: Soumendra Ganguly <soumendraganguly@gmail.com> --- Lib/test/test_pty.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/Lib/test/test_pty.py b/Lib/test/test_pty.py index c44298b4799021..7de568806ed7d8 100644 --- a/Lib/test/test_pty.py +++ b/Lib/test/test_pty.py @@ -124,7 +124,6 @@ def setUp(self): self.stdin_cols, 0, 0) self.addCleanup(_set_term_winsz, pty.STDIN_FILENO, old_stdin_winsz) except OSError: - # possible reason: current stdin is not a tty pass def handle_sig(self, sig, frame): @@ -145,7 +144,7 @@ def test_openpty(self): try: mode = tty.tcgetattr(pty.STDIN_FILENO) except tty.error: - # possible reason: current stdin is not a tty + # not a tty or bad/closed fd debug("tty.tcgetattr(pty.STDIN_FILENO) failed") mode = None @@ -165,7 +164,6 @@ def test_openpty(self): self.assertEqual(new_stdin_winsz, target_stdin_winsz, "pty.STDIN_FILENO window size unchanged") except OSError: - # possible reason: current stdin is not a tty warnings.warn("Failed to set pty.STDIN_FILENO window size") pass From e990b7980043f06789777f970111c11c41fe2588 Mon Sep 17 00:00:00 2001 From: "blurb-it[bot]" <43283697+blurb-it[bot]@users.noreply.github.com> Date: Fri, 20 Nov 2020 14:44:08 +0000 Subject: [PATCH 10/10] =?UTF-8?q?=F0=9F=93=9C=F0=9F=A4=96=20Added=20by=20b?= =?UTF-8?q?lurb=5Fit.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../NEWS.d/next/Library/2020-11-20-14-44-07.bpo-41818.33soAw.rst | 1 + 1 file changed, 1 insertion(+) create mode 100644 Misc/NEWS.d/next/Library/2020-11-20-14-44-07.bpo-41818.33soAw.rst diff --git a/Misc/NEWS.d/next/Library/2020-11-20-14-44-07.bpo-41818.33soAw.rst b/Misc/NEWS.d/next/Library/2020-11-20-14-44-07.bpo-41818.33soAw.rst new file mode 100644 index 00000000000000..005bf7e2af910e --- /dev/null +++ b/Misc/NEWS.d/next/Library/2020-11-20-14-44-07.bpo-41818.33soAw.rst @@ -0,0 +1 @@ +Updated tests for the pty library. test_basic() has been changed to test_openpty(); this additionally checks if slave termios and slave winsize are being set properly by pty.openpty(). In order to add support for FreeBSD, NetBSD, OpenBSD, and Darwin, this also adds test_master_read(), which demonstrates that pty.spawn() should not depend on an OSError to exit from its copy loop. \ No newline at end of file