From 703c1d13eeacb4c6d52fe96d27cb7569aef26079 Mon Sep 17 00:00:00 2001 From: diekmann Date: Mon, 10 Apr 2017 14:31:49 +0200 Subject: [PATCH 01/26] migrating issue to github this is pty.patch, "test suite v4 including patch for issue26228", 2017-02-09 18:31 http://bugs.python.org/file46613/pty.patch http://bugs.python.org/issue29070 --- Doc/library/pty.rst | 31 +- Lib/pty.py | 25 +- Lib/test/test_pty.py | 872 +++++++++++++++++++++++++++++++++++++++++-- 3 files changed, 885 insertions(+), 43 deletions(-) diff --git a/Doc/library/pty.rst b/Doc/library/pty.rst index 0ab766065d6e81..0c6ef4d1052936 100644 --- a/Doc/library/pty.rst +++ b/Doc/library/pty.rst @@ -2,8 +2,8 @@ ======================================== .. module:: pty - :platform: Linux - :synopsis: Pseudo-Terminal Handling for Linux. + :platform: Unix + :synopsis: Pseudo-Terminal Handling for Unix. .. moduleauthor:: Steen Lumholt .. sectionauthor:: Moshe Zadka @@ -16,9 +16,9 @@ The :mod:`pty` module defines operations for handling the pseudo-terminal concept: starting another process and being able to write to and read from its controlling terminal programmatically. -Because pseudo-terminal handling is highly platform dependent, there is code to -do it only for Linux. (The Linux code is supposed to work on other platforms, -but hasn't been tested yet.) +Pseudo-terminal handling is highly platform dependent. This code is mainly +tested on Linux, FreeBSD, and OS X (it is supposed to work on other POSIX +platforms). The :mod:`pty` module defines the following functions: @@ -41,9 +41,13 @@ The :mod:`pty` module defines the following functions: .. function:: spawn(argv[, master_read[, stdin_read]]) - Spawn a process, and connect its controlling terminal with the current - process's standard io. This is often used to baffle programs which insist on - reading from the controlling terminal. + Spawn a child process, and connect its controlling terminal with the + current process's standard io. This is often used to baffle programs which + insist on reading from the controlling terminal. + + A loop copies STDIN of the current process to the child and data received + from the child to STDOUT of the current process. It is not signaled to the + child if STDIN of the current process closes down. The functions *master_read* and *stdin_read* should be functions which read from a file descriptor. The defaults try to read 1024 bytes each time they are @@ -91,3 +95,14 @@ pseudo-terminal to record all input and output of a terminal session in a script.write(('Script done on %s\n' % time.asctime()).encode()) print('Script done, file is', filename) + +Caveats +------- + +.. sectionauthor:: Cornelius Diekmann + +Be aware that processes started by :func:`spawn` do not receive any information +about STDIN of their parent shutting down. For example, if run on a terminal on +a Linux system, ``/bin/sh < /dev/null`` closes immediately. However, +``./python -c 'import pty; pty.spawn("/bin/sh")' < /dev/null`` does not close +because the spawned child shell is not notified that STDIN is closed. diff --git a/Lib/pty.py b/Lib/pty.py index e841f12f3edb9b..18ef640160cc69 100644 --- a/Lib/pty.py +++ b/Lib/pty.py @@ -1,13 +1,14 @@ """Pseudo terminal utilities.""" # Bugs: No signal handling. Doesn't set slave termios and window size. -# Only tested on Linux. +# Only tested on Linux, FreeBSD, and OS X. # See: W. Richard Stevens. 1992. Advanced Programming in the # UNIX Environment. Chapter 19. # Author: Steen Lumholt -- with additions by Guido. from select import select import os +import sys import tty __all__ = ["openpty","fork","spawn"] @@ -133,11 +134,16 @@ def _copy(master_fd, master_read=_read, stdin_read=_read): standard input -> pty master (stdin_read)""" fds = [master_fd, STDIN_FILENO] while True: + # The expected path to leave this infinite loop is that the + # child exits and its slave_fd is destroyed. In this case, + # master_fd will become ready in select() and reading from + # master_fd either raises an OSError (Input/output error) on + # Linux or returns EOF on BSD. rfds, wfds, xfds = select(fds, [], []) if master_fd in rfds: data = master_read(master_fd) if not data: # Reached EOF. - fds.remove(master_fd) + return else: os.write(STDOUT_FILENO, data) if STDIN_FILENO in rfds: @@ -153,7 +159,16 @@ def spawn(argv, master_read=_read, stdin_read=_read): argv = (argv,) pid, master_fd = fork() if pid == CHILD: - os.execlp(argv[0], *argv) + try: + #XXX issue17824 still open + os.execlp(argv[0], *argv) + except: + # If we wanted to be really clever, we would use + # the same method as subprocess() to pass the error + # back to the parent. For now just dump stack trace. + sys.excepthook(*sys.exc_info()) + finally: + os._exit(1) try: mode = tty.tcgetattr(STDIN_FILENO) tty.setraw(STDIN_FILENO) @@ -163,6 +178,10 @@ def spawn(argv, master_read=_read, stdin_read=_read): try: _copy(master_fd, master_read, stdin_read) except OSError: + # Some OSes never return an EOF on pty, just raise + # an error instead. + pass + finally: if restore: tty.tcsetattr(STDIN_FILENO, tty.TCSAFLUSH, mode) diff --git a/Lib/test/test_pty.py b/Lib/test/test_pty.py index 15f88e4fcd7984..d709dbcddb2d50 100644 --- a/Lib/test/test_pty.py +++ b/Lib/test/test_pty.py @@ -1,7 +1,7 @@ from test.support import verbose, import_module, reap_children # Skip these tests if termios is not available -import_module('termios') +termios = import_module('termios') import errno import pty @@ -10,6 +10,7 @@ import select import signal import socket +import textwrap import unittest TEST_STRING_1 = b"I wish to buy a fish license.\n" @@ -17,7 +18,10 @@ if verbose: def debug(msg): - print(msg) + # Print debug information in a way we can call it from a forked + # child which uses the same STDOUT as the parent. Flush, so + # that we can debug deadlocks and blocking of the test suite. + print(msg, flush=True) else: def debug(msg): pass @@ -44,11 +48,86 @@ def normalize_output(data): return data +def _os_timeout_read(fd, n): + """Raw wrapper around os.read which raises a TimeoutError if no data + arrived within 10 seconds.""" + rd, _, _ = select.select([fd], [], [], 10) + if not rd: + raise TimeoutError + return os.read(fd, n) + +# Note that os.read() is nondeterministic so we need to be very careful +# to make the test suite deterministic. A normal call to os.read() may +# give us less than expected. Three wrappers with different focus +# around os.read() follow. +# +# Beware, on my Linux system, if I put 'foo\n' into a terminal fd, I get +# back 'foo\r\n' at the other end. The behavior depends on the termios +# setting. The newline translation may be OS-specific. To make the +# test suite deterministic and OS-independent, _os_readline and +# normalize_output can be used. +# +# In order to avoid newline translation and normalize_output completely, +# some test cases never emit newline characters and flush the fd +# manually. For example, using print('foo', end='', flush=True) in a +# forked child allows to read exactly len('foo') in the parent. For +# this, _os_read_exactly and _os_read_exhaust_exactly can be used. + +def _os_readline(fd): + """Use os.read() to read byte by byte until a newline is + encountered. May block forever if no newline is read.""" + buf = [] + while True: + r = os.read(fd, 1) + if not r: + raise EOFError + buf.append(r) + if r == b'\n': + break + return b''.join(buf) + +def _os_read_exactly(fd, numbytes): + """Read exactly numbytes out of fd. Blocks until we have enough or + raises TimeoutError. Does not touch the channel beyond numbytes.""" + ret = [] + numread = 0 + + while numread < numbytes: + if numread > 0: + # Possible non-determinism caught and prevented + debug("[_os_read_exactly] More than one os.read() call") + r = _os_timeout_read(fd, numbytes - numread) + if not r: + raise EOFError + ret.append(r) + numread += len(r) + assert numread == numbytes + return b''.join(ret) + +def _os_read_exhaust_exactly(fd, numbytes): + """Read exactly numbytes out of fd. Blocks until we have enough or + raises TimeoutError. Raises ValueError if more data is in fd.""" + assert numbytes > 0 + first = _os_read_exactly(fd, numbytes - 1) + final = _os_timeout_read(fd, 1024) #expect to read exactly 1 byte + ret = first + final + + # The protocol used for the test suite expects exactly the specified + # amount of data in fd. If there is more data, there is an error. + if len(ret) != numbytes: + raise ValueError("Read more data than expected. Fix your protocol. " + "Read: {:s} ({:d} bytes), expected to read only " + "{:d} bytes".format(repr(ret), len(ret), numbytes)) + return ret + + +# We will access internal functions for mocking. +#pylint: disable=protected-access # 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. -class PtyTest(unittest.TestCase): +class PtyBasicTest(unittest.TestCase): def setUp(self): # isatty() and close() can hang on some platforms. Set an alarm # before running the test to make sure we don't hang forever. @@ -61,11 +140,13 @@ def tearDown(self): signal.signal(signal.SIGALRM, self.old_alarm) def handle_sig(self, sig, frame): + #pylint: disable=unused-argument self.fail("isatty hung") def test_basic(self): try: debug("Calling master_open()") + # XXX deprecated function master_fd, slave_name = pty.master_open() debug("Got master_fd '%d', slave_name '%s'" % (master_fd, slave_name)) @@ -98,14 +179,14 @@ def test_basic(self): debug("Writing to slave_fd") os.write(slave_fd, TEST_STRING_1) - s1 = os.read(master_fd, 1024) + s1 = _os_readline(master_fd) self.assertEqual(b'I wish to buy a fish license.\n', normalize_output(s1)) debug("Writing chunked output") os.write(slave_fd, TEST_STRING_2[:5]) os.write(slave_fd, TEST_STRING_2[5:]) - s2 = os.read(master_fd, 1024) + s2 = _os_readline(master_fd) self.assertEqual(b'For my pet fish, Eric.\n', normalize_output(s2)) os.close(slave_fd) @@ -197,55 +278,711 @@ def test_fork(self): # pty.fork() passed. +class PtyPosixIntegrationTest(unittest.TestCase): + """Test black-box functionality. May actually fork() and exec() a + fresh python interpreter. Should not be intrusive for your local + machine. + """ + # Tests go amok if you pipe data to STDIN. This is expected and + # should never happen, unless the way the test suite is called is + # completely broken. + + def _spawn_py_get_retcode(self, python_src): + """Helper function to do pty.spawn() on the supplied python code + and return the return code, assuming successful termination.""" + cmd = [sys.executable, "-c", python_src] + debug("executing: {:s}".format(' '.join(cmd))) + ret = pty.spawn(cmd) + + # behavior of waitpid in module posix + self.assertLess(ret, 2**16) + killsig = ret & 0xff + self.assertEqual(killsig, 0) -class SmallPtyTests(unittest.TestCase): - """These tests don't spawn children or hang.""" + retcode = (ret & 0xff00) >> 8 + return retcode + + def test_spawn_exitsuccess(self): + """Spawn the python-equivalent of /bin/true.""" + retcode = self._spawn_py_get_retcode('import sys; sys.exit()') + self.assertEqual(retcode, 0) + + def test_spawn_exitfailure(self): + """Spawn the python-equivalent of /bin/false.""" + retcode = self._spawn_py_get_retcode('import sys; sys.exit(1)') + self.assertEqual(retcode, 1) + + def test_spawn_uncommon_exit_code(self): + """Test an uncommon exit code, which is less likely to be caused + by a Python exception or other failure.""" + retcode = self._spawn_py_get_retcode('import sys; sys.exit(81)') + self.assertEqual(retcode, 81) + + +class PtyMockingTestBase(unittest.TestCase): + """Base class for tests which replace STDIN and STDOUT of the pty + module with their own pipes.""" def setUp(self): - self.orig_stdin_fileno = pty.STDIN_FILENO - self.orig_stdout_fileno = pty.STDOUT_FILENO - self.orig_pty_select = pty.select + save_and_restore = ['pty.STDIN_FILENO', + 'pty.STDOUT_FILENO', + 'pty.select', + 'pty.fork', + 'os.forkpty'] + self.saved = dict() + for k in save_and_restore: + module, attr = k.split('.') + module = globals()[module] + self.saved[k] = getattr(module, attr) + self.fds = [] # A list of file descriptors to close. self.files = [] self.select_rfds_lengths = [] self.select_rfds_results = [] def tearDown(self): - pty.STDIN_FILENO = self.orig_stdin_fileno - pty.STDOUT_FILENO = self.orig_stdout_fileno - pty.select = self.orig_pty_select + for k, v in self.saved.items(): + module, attr = k.split('.') + module = globals()[module] + setattr(module, attr, v) + for file in self.files: try: file.close() except OSError: - pass + debug("close error {}".format(file)) for fd in self.fds: try: os.close(fd) except OSError: - pass + debug("os.close error {}".format(fd)) def _pipe(self): pipe_fds = os.pipe() self.fds.extend(pipe_fds) return pipe_fds + def _mock_stdin_stdout(self): + """Mock STDIN and STDOUT with two fresh pipes. Replaces + pty.STDIN_FILENO/pty.STDOUT_FILENO by one end of the pipe. + Returns the other end of the pipe.""" + read_from_stdout_fd, mock_stdout_fd = self._pipe() + pty.STDOUT_FILENO = mock_stdout_fd + mock_stdin_fd, write_to_stdin_fd = self._pipe() + pty.STDIN_FILENO = mock_stdin_fd + + # STDIN and STDOUT fileno of the pty module are replaced by our + # mocks. This is required for the pty._copy loop. In contrast, + # when pty.fork is called, the child's input/output must be set + # up properly, i.e. STDIN=0, STDOUT=1; not our mock fds. If + # pty.fork can delegate its work to os.forkpty, STDIN and STDOUT + # are correctly set. If os.forkpty is not available, the backup + # path of pty.fork would break in the presence of our mocks. We + # wrap pty.fork to temporarily restore the STDIN/STDOUT file + # descriptors for forking and reintroduce our mocks immediately + # afterwards. + def forkwrap(): + # debug("Calling wrapped pty.fork ({}), " + # "delegating to {}".format(pty.fork, + # self.saved['pty.fork'])) + pty.STDIN_FILENO = 0 + pty.STDOUT_FILENO = 1 + self.assertEqual(pty.STDERR_FILENO, 2, "pty.fork correct STDERR") + ret = self.saved['pty.fork']() + pty.STDIN_FILENO = mock_stdin_fd + pty.STDOUT_FILENO = mock_stdout_fd + return ret + pty.fork = forkwrap + + return (write_to_stdin_fd, read_from_stdout_fd) + + def _disable_os_forkpty(self): + """os.forkpty is only available on some flavours of UNIX. + Replace it by a function which always fails. Used to trigger + both code paths in pty.fork.""" + os.forkpty = self._mock_disabled_osforkpty + + @staticmethod + def _mock_disabled_osforkpty(): + """Simulate function failure or unavailability.""" + debug("os.forkpty disabled by mock function.") + raise OSError + +class PtySpawnTestBase(PtyMockingTestBase): + """A base class for the following integration test setup: A child + process is spawned with pty.spawn(). The child runs a fresh python + interpreter; its python code is passed via command line argument as + string. A background process is forked, reusing the current + instance of the python interpreter. These processes are connected + over STDIN/STDOUT pipes. These tests run fork(), select(), + execlp(), and other calls on your system. + + Starting from the parent (the main thread of this test suite), two + additional processes are forked in this test setup. We call the + spawn()-ed child the 'slave' because it is connected to the slave + side of the pty. The background process is connected to the master + side of the pty. + + parent + | + create mock + STDIN/STDOUT + pipes + | + | + ..os.fork().> background + | | + | + .......................pty.spawn(*)..>slave + pty._copy + and wait | <-- STDIN/STDOUT --> | + wait slave | pipes | + | | + | | + exit | + | + exit + wait for + background + | + + + *) python -c "slave child python code here" + + + The code for the spawned slave is python code in a string. This + makes the test suite very portable. + """ + # We introduce this generic base class for the test setup to + # encapsulate multiple different types of tests in individual + # classes. + + # Helper building blocks for the spawned (slave) python shell + _EXEC_IMPORTS = textwrap.dedent("""\ + import sys + import time + import signal + import tty, termios + """) + + @staticmethod + def _fork_background_process(master_fun, io_fds): + pid = os.fork() + assert pid >= 0, "fork failure must raise OSError" + if pid > 0: + debug("forked child ({:d}) from parent ({:d})".format(pid, os.getpid())) + return pid + + # Forked. Run master_fun and pass return code back to parent, + # wrapped to catch all exceptions. + try: + debug("[background] started ({:d})".format(os.getpid())) + rc = master_fun(*io_fds) + if not isinstance(rc, int): + raise Exception("master_fun must return an int") + except: + debug("[background] Abort due to exception") + sys.excepthook(*sys.exc_info()) + rc = 1 + finally: + if rc != 0: + debug("[background] Abnormal termination ({:d})".format(os.getpid())) + # Destroy forked background process. + # Do not use sys.exit(), it is hooked by the test suite. + sys.stdout.flush() + sys.stderr.flush() + os._exit(rc) + + def _spawn_master_and_slave(self, master_fun, slave_src, close_stdin=False): + """Spawn a slave and fork a master background process. + master_fun must be a python function. slave_src must be python + code as string. This function forks them and connects their + STDIN/STDOUT over a pipe and checks that they cleanly exit. + Control never returns from master_fun.""" + io_fds = self._mock_stdin_stdout() + # io_fds[0]: write to slave's STDIN + # io_fds[1]: read from slave's STDOUT + + if close_stdin: + debug("Closing stdin") + os.close(io_fds[0]) + self.fds.remove(io_fds[0]) + + sys.stdout.flush() + sys.stderr.flush() + # Without this flush, we interfere with the debug output from + # the child that will be spawned and execute master_fun. Don't + # confuse it with the child spawned by spawn(). It will work + # fine without the flush, unless you enable debug and have your + # STDOUT piped! The forked child will print to the same fd as + # we (the parent) print our debug information to. + + background_pid = self._fork_background_process(master_fun, io_fds) + + # spawn the slave in a new python interpreter, passing the code + # with the -c option + retcode_slave = pty.spawn([sys.executable, '-c', slave_src]) + if retcode_slave != 0: + debug("Slave failed.") + errmsg = ["Spawned slave returned but failed.", + "Exit code {:d}".format(retcode_slave)] + debug("killing background process ({:d})".format(background_pid)) + os.kill(background_pid, 9) + if verbose: + read_from_stdout_fd = io_fds[1] + errmsg.extend(["The failed child code was:", + "--- BEGIN child code ---", + slave_src, + "--- END child code ---"]) + rd, _, _ = select.select([read_from_stdout_fd], [], [], 0) + if rd: + errmsg.append("Dumping what the slave wrote last:") + rawoutput = os.read(read_from_stdout_fd, 1024*1024) + errmsg.append(repr(rawoutput)) + else: + errmsg.append("No output from child.") + self.fail('\n'.join(errmsg)) + self.assertEqual(retcode_slave, 0) + + retcode_background = os.waitpid(background_pid, 0)[1] + self.assertEqual(retcode_background, 0) + debug("background and slave are done") + + # We require that b'slave exits now' is left in the slave's + # STDOUT to confirm clean exit. + expecting = b'slave exits now' + slave_wrote = _os_read_exhaust_exactly(io_fds[1], len(expecting)) + self.assertEqual(slave_wrote, expecting) + + +class PtyPingTest(PtySpawnTestBase): + """Master and Slave count to 1000 by turns.""" + + _EXEC_CHILD = textwrap.dedent(""" + # Set terminal to a well-defined state. Disable echoing by + # setting it to raw mode. + tty.setraw(sys.stdin.fileno()) + + # Ping-Pong count to 1000 with master. We start. + for i in range(1000): + print("Ping {:d}".format(i), end='', flush=True) + pong, num = input().split() + if pong != "Pong" or int(num) != i: + sys.exit("Did not get Pong") + + # Send final confirmation that all went well to master. + print("slave exits now", end='', flush=True) + sys.exit() #success + """) + + @staticmethod + def _background_process(to_stdin, from_stdout): + debug("Staring Ping Pong") + # Ping-Pong count to 1000 with slave. + # Read Ping from slave, reply with pong. + # If something is wrong, this may block. + for i in range(1000): + expected = "Ping {:d}".format(i) + received = _os_read_exhaust_exactly(from_stdout, + len(expected)).decode('ascii') + if expected != received: + raise RuntimeError("Expected {:s}, received " + "{:s}".format(expected, received)) + answer = "Pong {:d}\n".format(i).encode('ascii') + pty._writen(to_stdin, answer) + + return 0 # success + + def test_alternate_ping(self): + """Spawn a slave and fork a master background process. Let them + Ping-Pong count to 1000 by turns.""" + child_code = self._EXEC_IMPORTS + self._EXEC_CHILD + self._spawn_master_and_slave(self._background_process, child_code) + + def test_alternate_ping_disable_osforkpty(self): + """Spawn a slave and fork a master background process. Let them + Ping-Pong count to 1000 by turns. Disable os.forkpty(), trigger + pty.fork() backup code.""" + self._disable_os_forkpty() + child_code = self._EXEC_IMPORTS + self._EXEC_CHILD + self._spawn_master_and_slave(self._background_process, child_code) + +class PtyReadAllTest(PtySpawnTestBase): + """Read from (slow) pty.spawn()ed child, make sure we get + everything. Slow tests.""" + + @staticmethod + def _background_process(to_stdin, from_stdout): + debug("[background] starting to read") + + bytes_transferred = 0 + for i in range(500): + expected = "long cat is long "*10 + "ID {:d}".format(i) + expected = expected.encode('ascii') + received = _os_read_exactly(from_stdout, len(expected)) + if expected != received: + raise RuntimeError("Expected {!r} but got {!r}".format(expected, received)) + bytes_transferred += len(received) + + debug("[background] received {} bytes from the slave.".format(bytes_transferred)) + return 0 # success + + # a dynamic sleep time needs to be formatted. + _EXEC_CHILD_FMT = textwrap.dedent(""" + tty.setraw(sys.stdin.fileno()) + + for i in range(500): + print("long cat is long "*10 + "ID {{:d}}".format(i), end='', flush=True) + if {sleeptime:f} and i % 400 == 0: + time.sleep({sleeptime:f}) # make slow, once. + + # Send final confirmation that all went well to master. + print("slave exits now", end='', flush=True) + sys.exit() + """) + + def test_read(self): + """Spawn a slave and fork a master background process. Receive + several kBytes from the slave.""" + child_code = self._EXEC_IMPORTS + \ + self._EXEC_CHILD_FMT.format(sleeptime=0.05) + debug("Test may take up to 1 second ...") + self._spawn_master_and_slave(self._background_process, child_code) + + def test_read_close_stdin(self): + """Spawn a slave and fork a master background process. Close + STDIN and receive several kBytes from the slave.""" + # only sleep in one test to speed this up + child_code = self._EXEC_IMPORTS + \ + self._EXEC_CHILD_FMT.format(sleeptime=0) + self._spawn_master_and_slave(self._background_process, child_code, + close_stdin=True) + +class PtyTermiosIntegrationTest(PtySpawnTestBase): + """Terminals are not just pipes. This integration testsuite asserts + that specific terminal functionality is operational. It tests ISIG, + which transforms sending 0x03 at the master side (usually triggered + by humans by pressing ctrl+c) to sending an INTR signal to the + child. In addition, on Linux, it tests pretty printing of control + characters, for example ^G, which is not defined in the POSIX.1-2008 + Standard but implemented. + + This class contains larger integration tests which verify the subtle + interplay of several modules. It depends on termios, pty, signal, + and os. It uses the Linux-only control character pretty printing + feature because this is one of the simple features of terminals + which are easy to test without digging into full-fledged os-level + integration tests. + """ + + @staticmethod + def _wait_for_slave(to_stdin, from_stdout): + # Expected to be called by _background_process. Wait for the + # slave to become ready and initialized. + debug("[background] waiting for slave process.") + read = _os_read_exhaust_exactly(from_stdout, len(b'slave ready!')) + if read != b'slave ready!': + raise ValueError('handshake with slave failed') + debug("[background] slave ready.") + + def _enable_echoctl(self): + if sys.platform == 'linux': + self.echoctl = True + else: + raise unittest.SkipTest('Test only available on Linux.') + + _EXEC_BASE_TERMINAL_SETUP_FMT = textwrap.dedent(r""" + def _base_terminal_setup(additional_lflag=0): + "Set up terminal to sane defaults (with regard to my Linux" + "system). See POSIX.1-2008, Chapter 11, General Terminal" + "Interface." + + # Warning: ECHOCTL is not defined in POSIX. Works on + # Linux 4.4 with Ubuntu GLIBC 2.23. Did not work on Mac. + if {echoctl}: + echoctl = termios.ECHOCTL + else: + echoctl = 0 + + terminal_fd = sys.stdin.fileno() + old = termios.tcgetattr(terminal_fd) + # don't need iflag + old[0] = 0 + + # oflag: output processing: replace \n by \r\n + old[1] = termios.ONLCR | termios.OPOST + + # don't need cflag + old[2] = 0 + + # lflag: canonical mode (line-buffer), + # normal echoing, + # echoing of control chars in caret notation (for example ^C) + old[3] = termios.ICANON | termios.ECHO | echoctl | additional_lflag + + termios.tcsetattr(terminal_fd, termios.TCSADRAIN, old) + """) + + @staticmethod + def _background_process_echo(to_stdin, from_stdout): + PtyTermiosIntegrationTest._wait_for_slave(to_stdin, from_stdout) + + answer = b"Hello, I'm background process!\n" + pty._writen(to_stdin, answer) + + # Slave terminal echoes back everything, rewriting line endings. + answer = answer[:-1] + b'\r\n' + read = _os_read_exactly(from_stdout, len(answer)) + if read != answer: + debug("Unexpected answer: {!r}".format(read)) + raise ValueError('Getting echoed data failed') + return 0 + + _EXEC_CHILD_ECHO = textwrap.dedent(r""" + _base_terminal_setup() + print("slave ready!", end='', flush=True) + + inp = input() + if inp != "Hello, I'm background process!": + sys.exit("failure getting answer, got `{}'".format(inp)) + + # Send final confirmation that all went well to master. + print("slave exits now", end='', flush=True) + sys.exit() + """) + + def test_echo(self): + """Terminals: Echoing of all characters written to the master + side and newline output translation.""" + child_code = self._EXEC_IMPORTS + \ + self._EXEC_BASE_TERMINAL_SETUP_FMT.format(echoctl=False) + \ + self._EXEC_CHILD_ECHO + self._spawn_master_and_slave(self._background_process_echo, child_code) + + @staticmethod + def _background_process_bell(to_stdin, from_stdout): + PtyTermiosIntegrationTest._wait_for_slave(to_stdin, from_stdout) + + debug("[background] sending bell escape sequence to slave") + BELL = b'\a' + to_slave = b'Bell here -> '+BELL+b' <-Hello slave!\n' + pty._writen(to_stdin, to_slave) + + # Bell character gets `pretty-printed' when echoed by terminal + expected = b'Bell here -> ^G <-Hello slave!\r\n' + received = _os_read_exactly(from_stdout, len(expected)) + if received != expected: + raise RuntimeError("Expecting {!r} but got {!r}".format(expected, received)) + + debug("[background] got it back") + return 0 + + _EXEC_CHILD_BELL = textwrap.dedent(r""" + _base_terminal_setup() + print("slave ready!", end='', flush=True) + + command = input() + # note how background process gets ^G and slave gets \a + if command != 'Bell here -> \a <-Hello slave!': + sys.exit("failure getting bell") + # terminal has automatically echoed the command, we can ignore it + + # Send final confirmation that all went well to master. + print("slave exits now", end='', flush=True) + sys.exit() + """) + + def test_bell_echoctl(self): + """Terminals: Pretty printing of the bell character in caret + notation.""" + self._enable_echoctl() + child_code = self._EXEC_IMPORTS + \ + self._EXEC_BASE_TERMINAL_SETUP_FMT.format(echoctl=self.echoctl) + \ + self._EXEC_CHILD_BELL + self._spawn_master_and_slave(self._background_process_bell, child_code) + + @staticmethod + def _background_process_eof(to_stdin, from_stdout): + PtyTermiosIntegrationTest._wait_for_slave(to_stdin, from_stdout) + + debug("[background] sending slave an EOF") + EOF = b'\x04' + pty._writen(to_stdin, EOF) + + # On OS X, we found that this test leaves an EOF character in + # STDOUT. Tested on OS X 10.6.8 and 10.11.2. Wipe EOF + # character which may remain here. + c = os.read(from_stdout, 1) + if c == b'\x04': + c = os.read(from_stdout, 1) + if c != b'!': + raise RuntimeError("Did not receive marker.") + + return 0 + + _EXEC_CHILD_EOF = textwrap.dedent(""" + _base_terminal_setup() + print("slave ready!", end='', flush=True) + + try: + input() + # unreachable if we got our EOF: + sys.exit("failure, no EOF received") + except EOFError: + # we expect an EOF here, this is good + pass + + # OS X leaves an EOF character in the channel which we want to + # remove. We set an exclamation mark as marker and in the + # background process, we read everything until we reach this + # marker. + print("!", end='', flush=True) + + # Send final confirmation that all went well to master. + print("slave exits now", end='', flush=True) + sys.exit() + """) + + def test_eof(self): + """Terminals: Processing of the special EOF character.""" + self.echoctl = False + child_code = self._EXEC_IMPORTS + \ + self._EXEC_BASE_TERMINAL_SETUP_FMT.format(echoctl=self.echoctl) + \ + self._EXEC_CHILD_EOF + self._spawn_master_and_slave(self._background_process_eof, child_code) + + def test_eof_echoctl(self): + """Terminals: Processing of the special EOF character with + ECHOCTL enabled.""" + # ^D is usually not pretty printed + self._enable_echoctl() + child_code = self._EXEC_IMPORTS + \ + self._EXEC_BASE_TERMINAL_SETUP_FMT.format(echoctl=self.echoctl) + \ + self._EXEC_CHILD_EOF + self._spawn_master_and_slave(self._background_process_eof, child_code) + + @staticmethod + def _background_process_intr(to_stdin, from_stdout): + """Try to send SIGINT to child. Careful: Testsuite also watches + for SIGINT. We only set our signal handler in the forked + slave.""" + PtyTermiosIntegrationTest._wait_for_slave(to_stdin, from_stdout) + + debug("[background] sending interrupt escape sequence to slave.") + INTR = b'\x03' + to_slave = b'This buffered stuff will be ignored'+INTR+b' Ohai slave!\n' + pty._writen(to_stdin, to_slave) + + expected = INTR+b' Ohai slave!\r\n' + received = _os_read_exactly(from_stdout, len(expected)) + if received != expected: + raise RuntimeError("Expecting {!r} but got {!r}".format(expected, received)) + + debug("[background] got it back") + return 0 + + _EXEC_CHILD_INTR = textwrap.dedent(""" + _sigint_received = False + + def _SIGINT_handler(a, b): + global _sigint_received + _sigint_received = True + + signal.signal(signal.SIGINT, _SIGINT_handler) + + # tell our controlling terminal to send signals on special characters + _base_terminal_setup(termios.ISIG) + print("slave ready!", end='', flush=True) + + command = input() + # Yes, only this arrives at STDIN here! + if command != ' Ohai slave!': + print(command) + sys.exit("failure getting interrupted input") + # terminal has automatically echoed the command and ^C, we can ignore it + + # Send final confirmation that all went well to master. + print("slave exits now", end='', flush=True) + + if _sigint_received: + sys.exit() + else: + sys.exit("failure, did not receive SIGINT") + """) + + def test_intr(self): + """Terminals: Writing a x03 char to the master side is + translated to sending an INTR signal to the slave. Simulates + pressing ctrl+c in master.""" + self.echoctl = False + child_code = self._EXEC_IMPORTS + \ + self._EXEC_BASE_TERMINAL_SETUP_FMT.format(echoctl=self.echoctl) + \ + self._EXEC_CHILD_INTR + self._spawn_master_and_slave(self._background_process_intr, child_code) + + +class _MockSelectEternalWait(Exception): + """Used both as exception and placeholder value. Models that no + more select activity is expected and that a test can be + terminated.""" + pass + +class PtyCopyTests(PtyMockingTestBase): + """Whitebox mocking tests which don't spawn children or hang. Test + the _copy loop to transfer data between parent and child.""" + def _socketpair(self): socketpair = socket.socketpair() self.files.extend(socketpair) return socketpair def _mock_select(self, rfds, wfds, xfds): + """Simulates the behavior of select.select. Only implemented + for reader waiting list (first parameter).""" + assert wfds == [] and xfds == [] # This will raise IndexError when no more expected calls exist. self.assertEqual(self.select_rfds_lengths.pop(0), len(rfds)) - return self.select_rfds_results.pop(0), [], [] + if len(rfds) == 0: + # called with three empty lists as file descriptors to wait + # on. Behavior of real select is platform-dependent and + # likely infinite blocking on Linux. + raise self.fail("mock select on no waitables") + rfds_result = self.select_rfds_results.pop(0) + + if rfds_result is _MockSelectEternalWait: + raise _MockSelectEternalWait + return rfds_result, [], [] + + def test__mock_select(self): + """Test the select proxy of the test class. Such meta testing. + """ + self.select_rfds_lengths.append(0) + with self.assertRaises(AssertionError): + self._mock_select([], [], []) + + # Prepare two select calls. Second one will block forever. + self.select_rfds_lengths.append(3) + self.select_rfds_results.append("foo") + self.select_rfds_lengths.append(3) + self.select_rfds_results.append(_MockSelectEternalWait) + + # Call one + self.assertEqual(self._mock_select([1, 2, 3], [], []), + ("foo", [], [])) + + # Call two + with self.assertRaises(_MockSelectEternalWait): + self._mock_select([1, 2, 3], [], []) + + # lists are cleaned + self.assertEqual(self.select_rfds_lengths, []) + self.assertEqual(self.select_rfds_results, []) def test__copy_to_each(self): """Test the normal data case on both master_fd and stdin.""" - read_from_stdout_fd, mock_stdout_fd = self._pipe() - pty.STDOUT_FILENO = mock_stdout_fd - mock_stdin_fd, write_to_stdin_fd = self._pipe() - pty.STDIN_FILENO = mock_stdin_fd + write_to_stdin_fd, read_from_stdout_fd = self._mock_stdin_stdout() + mock_stdin_fd = pty.STDIN_FILENO + self.assertGreater(mock_stdin_fd, 2, "replaced by our mock") socketpair = self._socketpair() masters = [s.fileno() for s in socketpair] @@ -253,13 +990,16 @@ def test__copy_to_each(self): os.write(masters[1], b'from master') os.write(write_to_stdin_fd, b'from stdin') - # Expect two select calls, the last one will cause IndexError + # monkey-patch pty.select with our mock pty.select = self._mock_select + + # Expect two select calls, the last one will simulate eternal waiting self.select_rfds_lengths.append(2) self.select_rfds_results.append([mock_stdin_fd, masters[0]]) self.select_rfds_lengths.append(2) + self.select_rfds_results.append(_MockSelectEternalWait) - with self.assertRaises(IndexError): + with self.assertRaises(_MockSelectEternalWait): pty._copy(masters[0]) # Test that the right data went to the right places. @@ -268,29 +1008,97 @@ def test__copy_to_each(self): self.assertEqual(os.read(read_from_stdout_fd, 20), b'from master') self.assertEqual(os.read(masters[1], 20), b'from stdin') + def _copy_eof_close_slave_helper(self, close_stdin): + """Helper to test the empty read EOF case on master_fd and/or + stdin.""" + write_to_stdin_fd, read_from_stdout_fd = self._mock_stdin_stdout() + mock_stdin_fd = pty.STDIN_FILENO + self.assertGreater(mock_stdin_fd, 2, "replaced by our mock") + socketpair = self._socketpair() + masters = [s.fileno() for s in socketpair] + + # This side of the channel would usually be the slave_fd of the + # child. We simulate that the child has exited and its side of + # the channel is destroyed. + socketpair[1].close() + self.files.remove(socketpair[1]) + + # optionally close fd or fill with dummy data in order to + # prevent blocking on one read call + if close_stdin: + os.close(write_to_stdin_fd) + self.fds.remove(write_to_stdin_fd) + else: + os.write(write_to_stdin_fd, b'from stdin') + + # monkey-patch pty.select with our mock + pty.select = self._mock_select + + # Expect exactly one select() call. This call returns master_fd + # and STDIN. Since the slave side of masters is closed, we + # expect the _copy loop to exit immediately. + self.select_rfds_lengths.append(2) + self.select_rfds_results.append([mock_stdin_fd, masters[0]]) + + # run the _copy test, which returns nothing and cleanly exits + self.assertIsNone(pty._copy(masters[0])) + + # We expect that everything is consumed + self.assertEqual(self.select_rfds_results, []) + self.assertEqual(self.select_rfds_lengths, []) + + # Test that STDIN was not touched. This test simulated the + # scenario where the child process immediately closed its end of + # the pipe. This means, nothing should be copied. + rfds = select.select([read_from_stdout_fd, mock_stdin_fd], [], [], 0)[0] + # data or EOF is still sitting unconsumed in mock_stdin_fd + self.assertEqual(rfds, [mock_stdin_fd]) + unconsumed = os.read(mock_stdin_fd, 20) + if close_stdin: + self.assertFalse(unconsumed) #EOF + else: + self.assertEqual(unconsumed, b'from stdin') + def test__copy_eof_on_all(self): """Test the empty read EOF case on both master_fd and stdin.""" - read_from_stdout_fd, mock_stdout_fd = self._pipe() - pty.STDOUT_FILENO = mock_stdout_fd - mock_stdin_fd, write_to_stdin_fd = self._pipe() - pty.STDIN_FILENO = mock_stdin_fd + self._copy_eof_close_slave_helper(close_stdin=True) + + def test__copy_eof_on_master(self): + """Test the empty read EOF case on only master_fd.""" + self._copy_eof_close_slave_helper(close_stdin=False) + + def test__copy_eof_on_stdin(self): + """Test the empty read EOF case on stdin.""" + write_to_stdin_fd, read_from_stdout_fd = self._mock_stdin_stdout() + mock_stdin_fd = pty.STDIN_FILENO + self.assertGreater(mock_stdin_fd, 2, "replaced by our mock") socketpair = self._socketpair() masters = [s.fileno() for s in socketpair] - socketpair[1].close() + # Fill with dummy data + os.write(masters[1], b'from master') + os.close(write_to_stdin_fd) + self.fds.remove(write_to_stdin_fd) - # Expect two select calls, the last one will cause IndexError + # monkey-patch pty.select with our mock pty.select = self._mock_select + + # Expect two select() calls. The first call returns master_fd + # and STDIN. self.select_rfds_lengths.append(2) self.select_rfds_results.append([mock_stdin_fd, masters[0]]) - # We expect that both fds were removed from the fds list as they - # both encountered an EOF before the second select call. - self.select_rfds_lengths.append(0) + # The second call causes _MockSelectEternalWait. We expect that + # STDIN is removed from the waiters as it reached EOF. + self.select_rfds_lengths.append(1) + self.select_rfds_results.append(_MockSelectEternalWait) - with self.assertRaises(IndexError): + with self.assertRaises(_MockSelectEternalWait): pty._copy(masters[0]) + # We expect that everything is consumed + self.assertEqual(self.select_rfds_results, []) + self.assertEqual(self.select_rfds_lengths, []) def tearDownModule(): reap_children() From 78c165aad018efb62f1a41dad6d0792923968a79 Mon Sep 17 00:00:00 2001 From: diekmann Date: Fri, 14 Apr 2017 13:24:58 +0200 Subject: [PATCH 02/26] using subprocess.run to run pty.spawn --- Lib/test/test_pty.py | 27 ++++++++++++++++++++++----- 1 file changed, 22 insertions(+), 5 deletions(-) diff --git a/Lib/test/test_pty.py b/Lib/test/test_pty.py index d709dbcddb2d50..4d0e86447a7da3 100644 --- a/Lib/test/test_pty.py +++ b/Lib/test/test_pty.py @@ -11,6 +11,7 @@ import signal import socket import textwrap +import subprocess import unittest TEST_STRING_1 = b"I wish to buy a fish license.\n" @@ -465,6 +466,23 @@ class PtySpawnTestBase(PtyMockingTestBase): import tty, termios """) + @staticmethod + def _greenfield_pty_spawn(args): + """Execute pty.spawn() in a fresh python interpreter, capture + stdin and stdout with a pipe""" + # We cannot use the test.support.captured_output() functions + # because the pty module writes to filedescriptors directly. + # We cannot use test.support.script_helper because we need to + # interact with the spawned child. + # Invoking this functions creates two children: the spawn_runner + # as isolated child to execute pty.spawn and capture stdout and + # its grandchild (running args) created by pty.spawn. + spawn_runner = 'import pty, sys;'\ + 'ret = pty.spawn(sys.argv[1:]);'\ + 'retcode = (ret & 0xff00) >> 8;'\ + 'sys.exit(retcode)' + subprocess.run([sys.executable, "-c", spawn_runner]+args, stdin=pty.STDIN_FILENO, stdout=pty.STDOUT_FILENO, check=True) + @staticmethod def _fork_background_process(master_fun, io_fds): pid = os.fork() @@ -521,11 +539,11 @@ def _spawn_master_and_slave(self, master_fun, slave_src, close_stdin=False): # spawn the slave in a new python interpreter, passing the code # with the -c option - retcode_slave = pty.spawn([sys.executable, '-c', slave_src]) - if retcode_slave != 0: + try: + self._greenfield_pty_spawn([sys.executable, '-c', slave_src]) + except subprocess.CalledProcessError as e: debug("Slave failed.") - errmsg = ["Spawned slave returned but failed.", - "Exit code {:d}".format(retcode_slave)] + errmsg = ["Spawned slave returned but failed."] debug("killing background process ({:d})".format(background_pid)) os.kill(background_pid, 9) if verbose: @@ -542,7 +560,6 @@ def _spawn_master_and_slave(self, master_fun, slave_src, close_stdin=False): else: errmsg.append("No output from child.") self.fail('\n'.join(errmsg)) - self.assertEqual(retcode_slave, 0) retcode_background = os.waitpid(background_pid, 0)[1] self.assertEqual(retcode_background, 0) From 28e918e1323954b04cbdd29dd6b837eb9b5d5a4d Mon Sep 17 00:00:00 2001 From: diekmann Date: Fri, 14 Apr 2017 13:42:33 +0200 Subject: [PATCH 03/26] move _mock_stdin_stdout to the actual tests --- Lib/test/test_pty.py | 67 +++++++++++++++----------------------------- 1 file changed, 22 insertions(+), 45 deletions(-) diff --git a/Lib/test/test_pty.py b/Lib/test/test_pty.py index 4d0e86447a7da3..db6a936f5daf75 100644 --- a/Lib/test/test_pty.py +++ b/Lib/test/test_pty.py @@ -363,40 +363,6 @@ def _pipe(self): self.fds.extend(pipe_fds) return pipe_fds - def _mock_stdin_stdout(self): - """Mock STDIN and STDOUT with two fresh pipes. Replaces - pty.STDIN_FILENO/pty.STDOUT_FILENO by one end of the pipe. - Returns the other end of the pipe.""" - read_from_stdout_fd, mock_stdout_fd = self._pipe() - pty.STDOUT_FILENO = mock_stdout_fd - mock_stdin_fd, write_to_stdin_fd = self._pipe() - pty.STDIN_FILENO = mock_stdin_fd - - # STDIN and STDOUT fileno of the pty module are replaced by our - # mocks. This is required for the pty._copy loop. In contrast, - # when pty.fork is called, the child's input/output must be set - # up properly, i.e. STDIN=0, STDOUT=1; not our mock fds. If - # pty.fork can delegate its work to os.forkpty, STDIN and STDOUT - # are correctly set. If os.forkpty is not available, the backup - # path of pty.fork would break in the presence of our mocks. We - # wrap pty.fork to temporarily restore the STDIN/STDOUT file - # descriptors for forking and reintroduce our mocks immediately - # afterwards. - def forkwrap(): - # debug("Calling wrapped pty.fork ({}), " - # "delegating to {}".format(pty.fork, - # self.saved['pty.fork'])) - pty.STDIN_FILENO = 0 - pty.STDOUT_FILENO = 1 - self.assertEqual(pty.STDERR_FILENO, 2, "pty.fork correct STDERR") - ret = self.saved['pty.fork']() - pty.STDIN_FILENO = mock_stdin_fd - pty.STDOUT_FILENO = mock_stdout_fd - return ret - pty.fork = forkwrap - - return (write_to_stdin_fd, read_from_stdout_fd) - def _disable_os_forkpty(self): """os.forkpty is only available on some flavours of UNIX. Replace it by a function which always fails. Used to trigger @@ -467,13 +433,15 @@ class PtySpawnTestBase(PtyMockingTestBase): """) @staticmethod - def _greenfield_pty_spawn(args): + def _greenfield_pty_spawn(stdin, stdout, args): """Execute pty.spawn() in a fresh python interpreter, capture stdin and stdout with a pipe""" # We cannot use the test.support.captured_output() functions # because the pty module writes to filedescriptors directly. # We cannot use test.support.script_helper because we need to # interact with the spawned child. + # We cannot monkey-patch pty.STDIN_FILENO to a pipe because the + # fallback path of pty.fork() relies on them. # Invoking this functions creates two children: the spawn_runner # as isolated child to execute pty.spawn and capture stdout and # its grandchild (running args) created by pty.spawn. @@ -481,7 +449,7 @@ def _greenfield_pty_spawn(args): 'ret = pty.spawn(sys.argv[1:]);'\ 'retcode = (ret & 0xff00) >> 8;'\ 'sys.exit(retcode)' - subprocess.run([sys.executable, "-c", spawn_runner]+args, stdin=pty.STDIN_FILENO, stdout=pty.STDOUT_FILENO, check=True) + subprocess.run([sys.executable, "-c", spawn_runner]+args, stdin=stdin, stdout=stdout, check=True) @staticmethod def _fork_background_process(master_fun, io_fds): @@ -517,14 +485,13 @@ def _spawn_master_and_slave(self, master_fun, slave_src, close_stdin=False): code as string. This function forks them and connects their STDIN/STDOUT over a pipe and checks that they cleanly exit. Control never returns from master_fun.""" - io_fds = self._mock_stdin_stdout() - # io_fds[0]: write to slave's STDIN - # io_fds[1]: read from slave's STDOUT + mock_stdin_fd, write_to_stdin_fd = self._pipe() + read_from_stdout_fd, mock_stdout_fd = self._pipe() if close_stdin: debug("Closing stdin") - os.close(io_fds[0]) - self.fds.remove(io_fds[0]) + os.close(write_to_stdin_fd) + self.fds.remove(write_to_stdin_fd) sys.stdout.flush() sys.stderr.flush() @@ -535,19 +502,19 @@ def _spawn_master_and_slave(self, master_fun, slave_src, close_stdin=False): # STDOUT piped! The forked child will print to the same fd as # we (the parent) print our debug information to. - background_pid = self._fork_background_process(master_fun, io_fds) + background_pid = self._fork_background_process(master_fun, (write_to_stdin_fd, read_from_stdout_fd)) # spawn the slave in a new python interpreter, passing the code # with the -c option try: - self._greenfield_pty_spawn([sys.executable, '-c', slave_src]) + self._greenfield_pty_spawn(mock_stdin_fd, mock_stdout_fd, [sys.executable, '-c', slave_src]) except subprocess.CalledProcessError as e: debug("Slave failed.") errmsg = ["Spawned slave returned but failed."] debug("killing background process ({:d})".format(background_pid)) os.kill(background_pid, 9) if verbose: - read_from_stdout_fd = io_fds[1] + read_from_stdout_fd = read_from_stdout_fd errmsg.extend(["The failed child code was:", "--- BEGIN child code ---", slave_src, @@ -568,7 +535,7 @@ def _spawn_master_and_slave(self, master_fun, slave_src, close_stdin=False): # We require that b'slave exits now' is left in the slave's # STDOUT to confirm clean exit. expecting = b'slave exits now' - slave_wrote = _os_read_exhaust_exactly(io_fds[1], len(expecting)) + slave_wrote = _os_read_exhaust_exactly(read_from_stdout_fd, len(expecting)) self.assertEqual(slave_wrote, expecting) @@ -948,6 +915,16 @@ class PtyCopyTests(PtyMockingTestBase): """Whitebox mocking tests which don't spawn children or hang. Test the _copy loop to transfer data between parent and child.""" + def _mock_stdin_stdout(self): + """Mock STDIN and STDOUT with two fresh pipes. Replaces + pty.STDIN_FILENO/pty.STDOUT_FILENO by one end of the pipe. + Returns the other end of the pipe.""" + read_from_stdout_fd, mock_stdout_fd = self._pipe() + pty.STDOUT_FILENO = mock_stdout_fd + mock_stdin_fd, write_to_stdin_fd = self._pipe() + pty.STDIN_FILENO = mock_stdin_fd + return (write_to_stdin_fd, read_from_stdout_fd) + def _socketpair(self): socketpair = socket.socketpair() self.files.extend(socketpair) From 6e7f587c31d4ca9290f13bdf4eb075f16e00dcc8 Mon Sep 17 00:00:00 2001 From: diekmann Date: Fri, 14 Apr 2017 14:19:04 +0200 Subject: [PATCH 04/26] Get os.forkpty disabled again. We need to monkey patch the freshly spawned child! Tested that it is actually called by writing to stderr (test code commented out, I don't want to push the verbose flag through all those children). --- Lib/test/test_pty.py | 55 ++++++++++++++++++++++++++------------------ 1 file changed, 33 insertions(+), 22 deletions(-) diff --git a/Lib/test/test_pty.py b/Lib/test/test_pty.py index db6a936f5daf75..4b6eac22975033 100644 --- a/Lib/test/test_pty.py +++ b/Lib/test/test_pty.py @@ -363,18 +363,6 @@ def _pipe(self): self.fds.extend(pipe_fds) return pipe_fds - def _disable_os_forkpty(self): - """os.forkpty is only available on some flavours of UNIX. - Replace it by a function which always fails. Used to trigger - both code paths in pty.fork.""" - os.forkpty = self._mock_disabled_osforkpty - - @staticmethod - def _mock_disabled_osforkpty(): - """Simulate function failure or unavailability.""" - debug("os.forkpty disabled by mock function.") - raise OSError - class PtySpawnTestBase(PtyMockingTestBase): """A base class for the following integration test setup: A child process is spawned with pty.spawn(). The child runs a fresh python @@ -433,9 +421,10 @@ class PtySpawnTestBase(PtyMockingTestBase): """) @staticmethod - def _greenfield_pty_spawn(stdin, stdout, args): + def _greenfield_pty_spawn(stdin, stdout, args, pre_spawn_hook=''): """Execute pty.spawn() in a fresh python interpreter, capture - stdin and stdout with a pipe""" + stdin and stdout with a pipe. Use the pre_spawn_hook to allow + monkey patching of the pty module.""" # We cannot use the test.support.captured_output() functions # because the pty module writes to filedescriptors directly. # We cannot use test.support.script_helper because we need to @@ -445,10 +434,11 @@ def _greenfield_pty_spawn(stdin, stdout, args): # Invoking this functions creates two children: the spawn_runner # as isolated child to execute pty.spawn and capture stdout and # its grandchild (running args) created by pty.spawn. - spawn_runner = 'import pty, sys;'\ - 'ret = pty.spawn(sys.argv[1:]);'\ - 'retcode = (ret & 0xff00) >> 8;'\ - 'sys.exit(retcode)' + spawn_runner = pre_spawn_hook +textwrap.dedent(""" + import pty, sys; + ret = pty.spawn(sys.argv[1:]); + retcode = (ret & 0xff00) >> 8; + sys.exit(retcode)""") subprocess.run([sys.executable, "-c", spawn_runner]+args, stdin=stdin, stdout=stdout, check=True) @staticmethod @@ -479,7 +469,8 @@ def _fork_background_process(master_fun, io_fds): sys.stderr.flush() os._exit(rc) - def _spawn_master_and_slave(self, master_fun, slave_src, close_stdin=False): + def _spawn_master_and_slave(self, master_fun, slave_src, + close_stdin=False, disable_osforkpty=False): """Spawn a slave and fork a master background process. master_fun must be a python function. slave_src must be python code as string. This function forks them and connects their @@ -507,7 +498,12 @@ def _spawn_master_and_slave(self, master_fun, slave_src, close_stdin=False): # spawn the slave in a new python interpreter, passing the code # with the -c option try: - self._greenfield_pty_spawn(mock_stdin_fd, mock_stdout_fd, [sys.executable, '-c', slave_src]) + if disable_osforkpty: + hook = self._EXEC_DISABLE_OS_FORKPTY + else: + hook = '' + self._greenfield_pty_spawn(mock_stdin_fd, mock_stdout_fd, + [sys.executable, '-c', slave_src], pre_spawn_hook=hook) except subprocess.CalledProcessError as e: debug("Slave failed.") errmsg = ["Spawned slave returned but failed."] @@ -538,6 +534,22 @@ def _spawn_master_and_slave(self, master_fun, slave_src, close_stdin=False): slave_wrote = _os_read_exhaust_exactly(read_from_stdout_fd, len(expecting)) self.assertEqual(slave_wrote, expecting) + _EXEC_DISABLE_OS_FORKPTY = textwrap.dedent(""" + import os + + def _mock_disabled_osforkpty(): + "Simulate function failure or unavailability." + #os.write(2, b'os.forkpty successfully disabled.\\n') + raise OSError + + #os.write(2, b' disabling os.forkpty.\\n') + # os.forkpty is only available on some flavours of UNIX. + # Replace it by a function which always fails. Used to guarantee + # that the fallback code path in pty.fork is also tested. + os.forkpty = _mock_disabled_osforkpty + + """) + class PtyPingTest(PtySpawnTestBase): """Master and Slave count to 1000 by turns.""" @@ -587,9 +599,8 @@ def test_alternate_ping_disable_osforkpty(self): """Spawn a slave and fork a master background process. Let them Ping-Pong count to 1000 by turns. Disable os.forkpty(), trigger pty.fork() backup code.""" - self._disable_os_forkpty() child_code = self._EXEC_IMPORTS + self._EXEC_CHILD - self._spawn_master_and_slave(self._background_process, child_code) + self._spawn_master_and_slave(self._background_process, child_code, disable_osforkpty=True) class PtyReadAllTest(PtySpawnTestBase): """Read from (slow) pty.spawn()ed child, make sure we get From fd61cad9000f9c6e903b78525a9d164e9b3b4066 Mon Sep 17 00:00:00 2001 From: diekmann Date: Fri, 14 Apr 2017 14:22:24 +0200 Subject: [PATCH 05/26] Yes, way less monkey patching. Awesome idea by Martin Panter! --- 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 4b6eac22975033..f554ebeb7fac9c 100644 --- a/Lib/test/test_pty.py +++ b/Lib/test/test_pty.py @@ -327,9 +327,7 @@ class PtyMockingTestBase(unittest.TestCase): def setUp(self): save_and_restore = ['pty.STDIN_FILENO', 'pty.STDOUT_FILENO', - 'pty.select', - 'pty.fork', - 'os.forkpty'] + 'pty.select'] self.saved = dict() for k in save_and_restore: module, attr = k.split('.') From e9b0b8b48972fe6e81eba8bb009a0eb831e57e48 Mon Sep 17 00:00:00 2001 From: diekmann Date: Fri, 14 Apr 2017 14:39:38 +0200 Subject: [PATCH 06/26] Decoupling PtySpawnTestBase and PtyMockingTestBase --- Lib/test/test_pty.py | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/Lib/test/test_pty.py b/Lib/test/test_pty.py index f554ebeb7fac9c..8df0f4bbc7a1ea 100644 --- a/Lib/test/test_pty.py +++ b/Lib/test/test_pty.py @@ -361,7 +361,7 @@ def _pipe(self): self.fds.extend(pipe_fds) return pipe_fds -class PtySpawnTestBase(PtyMockingTestBase): +class PtySpawnTestBase: """A base class for the following integration test setup: A child process is spawned with pty.spawn(). The child runs a fresh python interpreter; its python code is passed via command line argument as @@ -474,13 +474,12 @@ def _spawn_master_and_slave(self, master_fun, slave_src, code as string. This function forks them and connects their STDIN/STDOUT over a pipe and checks that they cleanly exit. Control never returns from master_fun.""" - mock_stdin_fd, write_to_stdin_fd = self._pipe() - read_from_stdout_fd, mock_stdout_fd = self._pipe() + mock_stdin_fd, write_to_stdin_fd = os.pipe() + read_from_stdout_fd, mock_stdout_fd = os.pipe() if close_stdin: debug("Closing stdin") os.close(write_to_stdin_fd) - self.fds.remove(write_to_stdin_fd) sys.stdout.flush() sys.stderr.flush() @@ -531,6 +530,10 @@ def _spawn_master_and_slave(self, master_fun, slave_src, expecting = b'slave exits now' slave_wrote = _os_read_exhaust_exactly(read_from_stdout_fd, len(expecting)) self.assertEqual(slave_wrote, expecting) + for fd in [mock_stdin_fd, read_from_stdout_fd, mock_stdout_fd]: + os.close(fd) + if not close_stdin: + os.close(write_to_stdin_fd) _EXEC_DISABLE_OS_FORKPTY = textwrap.dedent(""" import os @@ -549,7 +552,7 @@ def _mock_disabled_osforkpty(): """) -class PtyPingTest(PtySpawnTestBase): +class PtyPingTest(PtySpawnTestBase, unittest.TestCase): """Master and Slave count to 1000 by turns.""" _EXEC_CHILD = textwrap.dedent(""" @@ -600,7 +603,7 @@ def test_alternate_ping_disable_osforkpty(self): child_code = self._EXEC_IMPORTS + self._EXEC_CHILD self._spawn_master_and_slave(self._background_process, child_code, disable_osforkpty=True) -class PtyReadAllTest(PtySpawnTestBase): +class PtyReadAllTest(PtySpawnTestBase, unittest.TestCase): """Read from (slow) pty.spawn()ed child, make sure we get everything. Slow tests.""" @@ -651,7 +654,7 @@ def test_read_close_stdin(self): self._spawn_master_and_slave(self._background_process, child_code, close_stdin=True) -class PtyTermiosIntegrationTest(PtySpawnTestBase): +class PtyTermiosIntegrationTest(PtySpawnTestBase, unittest.TestCase): """Terminals are not just pipes. This integration testsuite asserts that specific terminal functionality is operational. It tests ISIG, which transforms sending 0x03 at the master side (usually triggered From a42755bbbc3a5eac56fce108ac379165885285ca Mon Sep 17 00:00:00 2001 From: diekmann Date: Fri, 14 Apr 2017 14:41:00 +0200 Subject: [PATCH 07/26] Move PtyMockingTestBase closer to PtyCopyTests --- Lib/test/test_pty.py | 82 ++++++++++++++++++++++---------------------- 1 file changed, 41 insertions(+), 41 deletions(-) diff --git a/Lib/test/test_pty.py b/Lib/test/test_pty.py index 8df0f4bbc7a1ea..fdb4a28c14bb31 100644 --- a/Lib/test/test_pty.py +++ b/Lib/test/test_pty.py @@ -320,47 +320,6 @@ def test_spawn_uncommon_exit_code(self): self.assertEqual(retcode, 81) -class PtyMockingTestBase(unittest.TestCase): - """Base class for tests which replace STDIN and STDOUT of the pty - module with their own pipes.""" - - def setUp(self): - save_and_restore = ['pty.STDIN_FILENO', - 'pty.STDOUT_FILENO', - 'pty.select'] - self.saved = dict() - for k in save_and_restore: - module, attr = k.split('.') - module = globals()[module] - self.saved[k] = getattr(module, attr) - - self.fds = [] # A list of file descriptors to close. - self.files = [] - self.select_rfds_lengths = [] - self.select_rfds_results = [] - - def tearDown(self): - for k, v in self.saved.items(): - module, attr = k.split('.') - module = globals()[module] - setattr(module, attr, v) - - for file in self.files: - try: - file.close() - except OSError: - debug("close error {}".format(file)) - for fd in self.fds: - try: - os.close(fd) - except OSError: - debug("os.close error {}".format(fd)) - - def _pipe(self): - pipe_fds = os.pipe() - self.fds.extend(pipe_fds) - return pipe_fds - class PtySpawnTestBase: """A base class for the following integration test setup: A child process is spawned with pty.spawn(). The child runs a fresh python @@ -917,6 +876,47 @@ def test_intr(self): self._spawn_master_and_slave(self._background_process_intr, child_code) +class PtyMockingTestBase(unittest.TestCase): + """Base class for tests which replace STDIN and STDOUT of the pty + module with their own pipes.""" + + def setUp(self): + save_and_restore = ['pty.STDIN_FILENO', + 'pty.STDOUT_FILENO', + 'pty.select'] + self.saved = dict() + for k in save_and_restore: + module, attr = k.split('.') + module = globals()[module] + self.saved[k] = getattr(module, attr) + + self.fds = [] # A list of file descriptors to close. + self.files = [] + self.select_rfds_lengths = [] + self.select_rfds_results = [] + + def tearDown(self): + for k, v in self.saved.items(): + module, attr = k.split('.') + module = globals()[module] + setattr(module, attr, v) + + for file in self.files: + try: + file.close() + except OSError: + debug("close error {}".format(file)) + for fd in self.fds: + try: + os.close(fd) + except OSError: + debug("os.close error {}".format(fd)) + + def _pipe(self): + pipe_fds = os.pipe() + self.fds.extend(pipe_fds) + return pipe_fds + class _MockSelectEternalWait(Exception): """Used both as exception and placeholder value. Models that no more select activity is expected and that a test can be From 7451f9798582685a2c7e0520c538c3c10c96eb3f Mon Sep 17 00:00:00 2001 From: diekmann Date: Fri, 14 Apr 2017 14:42:35 +0200 Subject: [PATCH 08/26] Tuned class hierarchy --- Lib/test/test_pty.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Lib/test/test_pty.py b/Lib/test/test_pty.py index fdb4a28c14bb31..6aefd5446be370 100644 --- a/Lib/test/test_pty.py +++ b/Lib/test/test_pty.py @@ -876,7 +876,7 @@ def test_intr(self): self._spawn_master_and_slave(self._background_process_intr, child_code) -class PtyMockingTestBase(unittest.TestCase): +class PtyMockingTestBase: """Base class for tests which replace STDIN and STDOUT of the pty module with their own pipes.""" @@ -923,7 +923,7 @@ class _MockSelectEternalWait(Exception): terminated.""" pass -class PtyCopyTests(PtyMockingTestBase): +class PtyCopyTests(PtyMockingTestBase, unittest.TestCase): """Whitebox mocking tests which don't spawn children or hang. Test the _copy loop to transfer data between parent and child.""" From b604d83b51f79cc36aaec7d9dd8a9fee8d15b7e1 Mon Sep 17 00:00:00 2001 From: diekmann Date: Fri, 14 Apr 2017 16:17:22 +0200 Subject: [PATCH 09/26] fix line length and error path of base classes PtySpawnTestBase (though it does not define any tests) wants to inherit from unittest to have self.fail() available --- Lib/test/test_pty.py | 34 +++++++++++++++++++--------------- 1 file changed, 19 insertions(+), 15 deletions(-) diff --git a/Lib/test/test_pty.py b/Lib/test/test_pty.py index 6aefd5446be370..8c9c0d38814576 100644 --- a/Lib/test/test_pty.py +++ b/Lib/test/test_pty.py @@ -320,7 +320,7 @@ def test_spawn_uncommon_exit_code(self): self.assertEqual(retcode, 81) -class PtySpawnTestBase: +class PtySpawnTestBase(unittest.TestCase): """A base class for the following integration test setup: A child process is spawned with pty.spawn(). The child runs a fresh python interpreter; its python code is passed via command line argument as @@ -386,8 +386,8 @@ def _greenfield_pty_spawn(stdin, stdout, args, pre_spawn_hook=''): # because the pty module writes to filedescriptors directly. # We cannot use test.support.script_helper because we need to # interact with the spawned child. - # We cannot monkey-patch pty.STDIN_FILENO to a pipe because the - # fallback path of pty.fork() relies on them. + # We cannot monkey-patch pty.STDIN_FILENO to point to a pipe + # because the fallback path of pty.fork() relies on them. # Invoking this functions creates two children: the spawn_runner # as isolated child to execute pty.spawn and capture stdout and # its grandchild (running args) created by pty.spawn. @@ -396,7 +396,8 @@ def _greenfield_pty_spawn(stdin, stdout, args, pre_spawn_hook=''): ret = pty.spawn(sys.argv[1:]); retcode = (ret & 0xff00) >> 8; sys.exit(retcode)""") - subprocess.run([sys.executable, "-c", spawn_runner]+args, stdin=stdin, stdout=stdout, check=True) + subprocess.run([sys.executable, "-c", spawn_runner]+args, + stdin=stdin, stdout=stdout, check=True) @staticmethod def _fork_background_process(master_fun, io_fds): @@ -426,8 +427,8 @@ def _fork_background_process(master_fun, io_fds): sys.stderr.flush() os._exit(rc) - def _spawn_master_and_slave(self, master_fun, slave_src, - close_stdin=False, disable_osforkpty=False): + def _spawn_master_and_slave(self, master_fun, slave_src, close_stdin=False, + disable_osforkpty=False): """Spawn a slave and fork a master background process. master_fun must be a python function. slave_src must be python code as string. This function forks them and connects their @@ -449,7 +450,8 @@ def _spawn_master_and_slave(self, master_fun, slave_src, # STDOUT piped! The forked child will print to the same fd as # we (the parent) print our debug information to. - background_pid = self._fork_background_process(master_fun, (write_to_stdin_fd, read_from_stdout_fd)) + io_fds = (write_to_stdin_fd, read_from_stdout_fd) + background_pid = self._fork_background_process(master_fun, io_fds) # spawn the slave in a new python interpreter, passing the code # with the -c option @@ -459,7 +461,8 @@ def _spawn_master_and_slave(self, master_fun, slave_src, else: hook = '' self._greenfield_pty_spawn(mock_stdin_fd, mock_stdout_fd, - [sys.executable, '-c', slave_src], pre_spawn_hook=hook) + [sys.executable, '-c', slave_src], + pre_spawn_hook=hook) except subprocess.CalledProcessError as e: debug("Slave failed.") errmsg = ["Spawned slave returned but failed."] @@ -494,6 +497,9 @@ def _spawn_master_and_slave(self, master_fun, slave_src, if not close_stdin: os.close(write_to_stdin_fd) + # os.forkpty is only available on some flavours of UNIX. Replace it + # by a function which always fails. Used to guarantee that the + # fallback code path in pty.fork is also tested. _EXEC_DISABLE_OS_FORKPTY = textwrap.dedent(""" import os @@ -503,15 +509,12 @@ def _mock_disabled_osforkpty(): raise OSError #os.write(2, b' disabling os.forkpty.\\n') - # os.forkpty is only available on some flavours of UNIX. - # Replace it by a function which always fails. Used to guarantee - # that the fallback code path in pty.fork is also tested. os.forkpty = _mock_disabled_osforkpty """) -class PtyPingTest(PtySpawnTestBase, unittest.TestCase): +class PtyPingTest(PtySpawnTestBase): """Master and Slave count to 1000 by turns.""" _EXEC_CHILD = textwrap.dedent(""" @@ -560,9 +563,10 @@ def test_alternate_ping_disable_osforkpty(self): Ping-Pong count to 1000 by turns. Disable os.forkpty(), trigger pty.fork() backup code.""" child_code = self._EXEC_IMPORTS + self._EXEC_CHILD - self._spawn_master_and_slave(self._background_process, child_code, disable_osforkpty=True) + self._spawn_master_and_slave(self._background_process, child_code, + disable_osforkpty=True) -class PtyReadAllTest(PtySpawnTestBase, unittest.TestCase): +class PtyReadAllTest(PtySpawnTestBase): """Read from (slow) pty.spawn()ed child, make sure we get everything. Slow tests.""" @@ -613,7 +617,7 @@ def test_read_close_stdin(self): self._spawn_master_and_slave(self._background_process, child_code, close_stdin=True) -class PtyTermiosIntegrationTest(PtySpawnTestBase, unittest.TestCase): +class PtyTermiosIntegrationTest(PtySpawnTestBase): """Terminals are not just pipes. This integration testsuite asserts that specific terminal functionality is operational. It tests ISIG, which transforms sending 0x03 at the master side (usually triggered From cbe7d14ad72b8c4c83c05ccd06cc1d1c66745f0d Mon Sep 17 00:00:00 2001 From: diekmann Date: Fri, 14 Apr 2017 16:29:32 +0200 Subject: [PATCH 10/26] clarify difference to alternatives We don't capture stdin/stdout by running the child and returning what was written once the child is done. We spawn the child and have the pipes available to communicate with the child while it is running. --- Lib/test/test_pty.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Lib/test/test_pty.py b/Lib/test/test_pty.py index 8c9c0d38814576..a1b2b9bdc4a0ec 100644 --- a/Lib/test/test_pty.py +++ b/Lib/test/test_pty.py @@ -379,9 +379,9 @@ class PtySpawnTestBase(unittest.TestCase): @staticmethod def _greenfield_pty_spawn(stdin, stdout, args, pre_spawn_hook=''): - """Execute pty.spawn() in a fresh python interpreter, capture - stdin and stdout with a pipe. Use the pre_spawn_hook to allow - monkey patching of the pty module.""" + """Execute pty.spawn() in a fresh python interpreter, make stdin + and stdout available through pipes. Use the pre_spawn_hook to + allow monkey patching of the pty module.""" # We cannot use the test.support.captured_output() functions # because the pty module writes to filedescriptors directly. # We cannot use test.support.script_helper because we need to From 8afd1e73c357b3be02c4ca989d37c218bd0d0195 Mon Sep 17 00:00:00 2001 From: diekmann Date: Fri, 14 Apr 2017 18:43:44 +0200 Subject: [PATCH 11/26] move disable-os.forkpty-code closer to the test --- Lib/test/test_pty.py | 41 +++++++++++++++++------------------------ 1 file changed, 17 insertions(+), 24 deletions(-) diff --git a/Lib/test/test_pty.py b/Lib/test/test_pty.py index a1b2b9bdc4a0ec..f8d58284e47b9c 100644 --- a/Lib/test/test_pty.py +++ b/Lib/test/test_pty.py @@ -428,12 +428,14 @@ def _fork_background_process(master_fun, io_fds): os._exit(rc) def _spawn_master_and_slave(self, master_fun, slave_src, close_stdin=False, - disable_osforkpty=False): + pre_spawn_hook=''): """Spawn a slave and fork a master background process. master_fun must be a python function. slave_src must be python code as string. This function forks them and connects their STDIN/STDOUT over a pipe and checks that they cleanly exit. - Control never returns from master_fun.""" + Control never returns from master_fun. Optionally, python code + supplied via the pre_spawn_hook will be executed before the + slave is spawned.""" mock_stdin_fd, write_to_stdin_fd = os.pipe() read_from_stdout_fd, mock_stdout_fd = os.pipe() @@ -456,13 +458,9 @@ def _spawn_master_and_slave(self, master_fun, slave_src, close_stdin=False, # spawn the slave in a new python interpreter, passing the code # with the -c option try: - if disable_osforkpty: - hook = self._EXEC_DISABLE_OS_FORKPTY - else: - hook = '' self._greenfield_pty_spawn(mock_stdin_fd, mock_stdout_fd, [sys.executable, '-c', slave_src], - pre_spawn_hook=hook) + pre_spawn_hook=pre_spawn_hook) except subprocess.CalledProcessError as e: debug("Slave failed.") errmsg = ["Spawned slave returned but failed."] @@ -497,22 +495,6 @@ def _spawn_master_and_slave(self, master_fun, slave_src, close_stdin=False, if not close_stdin: os.close(write_to_stdin_fd) - # os.forkpty is only available on some flavours of UNIX. Replace it - # by a function which always fails. Used to guarantee that the - # fallback code path in pty.fork is also tested. - _EXEC_DISABLE_OS_FORKPTY = textwrap.dedent(""" - import os - - def _mock_disabled_osforkpty(): - "Simulate function failure or unavailability." - #os.write(2, b'os.forkpty successfully disabled.\\n') - raise OSError - - #os.write(2, b' disabling os.forkpty.\\n') - os.forkpty = _mock_disabled_osforkpty - - """) - class PtyPingTest(PtySpawnTestBase): """Master and Slave count to 1000 by turns.""" @@ -558,13 +540,24 @@ def test_alternate_ping(self): child_code = self._EXEC_IMPORTS + self._EXEC_CHILD self._spawn_master_and_slave(self._background_process, child_code) + # os.forkpty is only available on some flavours of UNIX. Replace it + # by a function which always fails. Used to guarantee that the + # fallback code path in pty.fork is also tested. + _DISABLE_OS_FORKPTY = textwrap.dedent(""" + import os + def _mock_disabled_osforkpty(): + #os.write(2, b'os.forkpty successfully disabled.\\n') + raise OSError + os.forkpty = _mock_disabled_osforkpty + """) + def test_alternate_ping_disable_osforkpty(self): """Spawn a slave and fork a master background process. Let them Ping-Pong count to 1000 by turns. Disable os.forkpty(), trigger pty.fork() backup code.""" child_code = self._EXEC_IMPORTS + self._EXEC_CHILD self._spawn_master_and_slave(self._background_process, child_code, - disable_osforkpty=True) + pre_spawn_hook=self._DISABLE_OS_FORKPTY) class PtyReadAllTest(PtySpawnTestBase): """Read from (slow) pty.spawn()ed child, make sure we get From 2c3498febcb65023f213d753fb86068ae407cab6 Mon Sep 17 00:00:00 2001 From: diekmann Date: Fri, 14 Apr 2017 19:37:10 +0200 Subject: [PATCH 12/26] cosmetic changes * remove unrealted code comment * tune documentation * rename function * remove if verbose --- Lib/test/test_pty.py | 116 +++++++++++++++++++++++-------------------- 1 file changed, 62 insertions(+), 54 deletions(-) diff --git a/Lib/test/test_pty.py b/Lib/test/test_pty.py index f8d58284e47b9c..b663c3b9eaffcc 100644 --- a/Lib/test/test_pty.py +++ b/Lib/test/test_pty.py @@ -71,7 +71,7 @@ def _os_timeout_read(fd, n): # In order to avoid newline translation and normalize_output completely, # some test cases never emit newline characters and flush the fd # manually. For example, using print('foo', end='', flush=True) in a -# forked child allows to read exactly len('foo') in the parent. For +# forked child allows reading exactly len('foo') in the parent. For # this, _os_read_exactly and _os_read_exhaust_exactly can be used. def _os_readline(fd): @@ -147,7 +147,6 @@ def handle_sig(self, sig, frame): def test_basic(self): try: debug("Calling master_open()") - # XXX deprecated function master_fd, slave_name = pty.master_open() debug("Got master_fd '%d', slave_name '%s'" % (master_fd, slave_name)) @@ -326,40 +325,51 @@ class PtySpawnTestBase(unittest.TestCase): interpreter; its python code is passed via command line argument as string. A background process is forked, reusing the current instance of the python interpreter. These processes are connected - over STDIN/STDOUT pipes. These tests run fork(), select(), - execlp(), and other calls on your system. - - Starting from the parent (the main thread of this test suite), two - additional processes are forked in this test setup. We call the - spawn()-ed child the 'slave' because it is connected to the slave - side of the pty. The background process is connected to the master - side of the pty. - - parent - | - create mock + over STDIN/STDOUT pipes. The tests run fork(), select(), execlp(), + and other calls on your system. + + Starting from the parent (the main thread of this test suite), three + processes are forked for this setup. We call the spawn()-ed child + the 'slave' because it is connected to the slave side of the pty. + The background process is connected to the master side of the pty. + Internal details require a spawn_runner. The actual tests are + executed between slave and background. + + Sequence diagram of the overall test setup: + + ┌──────┐ + │parent│ + └──────┘ + | + create STDIN/STDOUT - pipes - | - | - ..os.fork().> background - | | - | - .......................pty.spawn(*)..>slave - pty._copy - and wait | <-- STDIN/STDOUT --> | - wait slave | pipes | - | | - | | - exit | - | - exit - wait for - background - | - - - *) python -c "slave child python code here" + pipes + | + | ┌──────────┐ + |---------------------os.fork()------------------->│background│ + | └──────────┘ + | ┌──────┐ | + |-_pty_spawn->│spawn │ master_fun + | │runner│ | + wait for └──────┘ | + slave | | + . | ┌─────┐ | + . |--pty.spawn-->│slave│ | + . . └─────┘ | + . . | | + . . slave_src | + . . | <- STDIN/STDOUT -> | + . . | pipes | + . . | | + . . | | + . . | exit + . . | . + . |< . . . . . . . exit . + |< . . . . . . .exit . + wait for . + background . + |< . . . . . . . . . . . . . . . . . . . . . . . . . . . . + The code for the spawned slave is python code in a string. This @@ -378,7 +388,7 @@ class PtySpawnTestBase(unittest.TestCase): """) @staticmethod - def _greenfield_pty_spawn(stdin, stdout, args, pre_spawn_hook=''): + def _pty_spawn(stdin, stdout, args, pre_spawn_hook=''): """Execute pty.spawn() in a fresh python interpreter, make stdin and stdout available through pipes. Use the pre_spawn_hook to allow monkey patching of the pty module.""" @@ -391,7 +401,7 @@ def _greenfield_pty_spawn(stdin, stdout, args, pre_spawn_hook=''): # Invoking this functions creates two children: the spawn_runner # as isolated child to execute pty.spawn and capture stdout and # its grandchild (running args) created by pty.spawn. - spawn_runner = pre_spawn_hook +textwrap.dedent(""" + spawn_runner = pre_spawn_hook + textwrap.dedent(""" import pty, sys; ret = pty.spawn(sys.argv[1:]); retcode = (ret & 0xff00) >> 8; @@ -458,27 +468,25 @@ def _spawn_master_and_slave(self, master_fun, slave_src, close_stdin=False, # spawn the slave in a new python interpreter, passing the code # with the -c option try: - self._greenfield_pty_spawn(mock_stdin_fd, mock_stdout_fd, - [sys.executable, '-c', slave_src], - pre_spawn_hook=pre_spawn_hook) + self._pty_spawn(mock_stdin_fd, mock_stdout_fd, + [sys.executable, '-c', slave_src], + pre_spawn_hook=pre_spawn_hook) except subprocess.CalledProcessError as e: debug("Slave failed.") - errmsg = ["Spawned slave returned but failed."] debug("killing background process ({:d})".format(background_pid)) os.kill(background_pid, 9) - if verbose: - read_from_stdout_fd = read_from_stdout_fd - errmsg.extend(["The failed child code was:", - "--- BEGIN child code ---", - slave_src, - "--- END child code ---"]) - rd, _, _ = select.select([read_from_stdout_fd], [], [], 0) - if rd: - errmsg.append("Dumping what the slave wrote last:") - rawoutput = os.read(read_from_stdout_fd, 1024*1024) - errmsg.append(repr(rawoutput)) - else: - errmsg.append("No output from child.") + errmsg = ["Spawned slave returned but failed.", + "The failed child code was:", + "--- BEGIN child code ---", + slave_src, + "--- END child code ---"] + rd, _, _ = select.select([read_from_stdout_fd], [], [], 0) + if rd: + errmsg.append("Dumping what the slave wrote last:") + rawoutput = os.read(read_from_stdout_fd, 1024*1024) + errmsg.append(repr(rawoutput)) + else: + errmsg.append("No output from child.") self.fail('\n'.join(errmsg)) retcode_background = os.waitpid(background_pid, 0)[1] From ec58a624f8f08005a5b1e94678efe31a30e71556 Mon Sep 17 00:00:00 2001 From: diekmann Date: Fri, 14 Apr 2017 19:45:37 +0200 Subject: [PATCH 13/26] polish --- Lib/test/test_pty.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Lib/test/test_pty.py b/Lib/test/test_pty.py index b663c3b9eaffcc..324e2a0531cc98 100644 --- a/Lib/test/test_pty.py +++ b/Lib/test/test_pty.py @@ -334,7 +334,7 @@ class PtySpawnTestBase(unittest.TestCase): The background process is connected to the master side of the pty. Internal details require a spawn_runner. The actual tests are executed between slave and background. - + Sequence diagram of the overall test setup: ┌──────┐ @@ -554,10 +554,10 @@ def test_alternate_ping(self): _DISABLE_OS_FORKPTY = textwrap.dedent(""" import os def _mock_disabled_osforkpty(): - #os.write(2, b'os.forkpty successfully disabled.\\n') + if {}: os.write(2, b'os.forkpty successfully disabled.\\n') raise OSError os.forkpty = _mock_disabled_osforkpty - """) + """.format(verbose)) def test_alternate_ping_disable_osforkpty(self): """Spawn a slave and fork a master background process. Let them From 0be6c54f725a2559d1c5fdafdda054cdc6e5959c Mon Sep 17 00:00:00 2001 From: diekmann Date: Fri, 14 Apr 2017 19:52:12 +0200 Subject: [PATCH 14/26] let exceptions out and fail the tests --- Lib/test/test_pty.py | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/Lib/test/test_pty.py b/Lib/test/test_pty.py index 324e2a0531cc98..5e083b439d5199 100644 --- a/Lib/test/test_pty.py +++ b/Lib/test/test_pty.py @@ -907,15 +907,9 @@ def tearDown(self): setattr(module, attr, v) for file in self.files: - try: - file.close() - except OSError: - debug("close error {}".format(file)) + file.close() for fd in self.fds: - try: - os.close(fd) - except OSError: - debug("os.close error {}".format(fd)) + os.close(fd) def _pipe(self): pipe_fds = os.pipe() From 250ae50704f959f000644eefbce9fa3110111f45 Mon Sep 17 00:00:00 2001 From: diekmann Date: Fri, 14 Apr 2017 20:57:37 +0200 Subject: [PATCH 15/26] Pull related code for select mocking tests together --- Lib/test/test_pty.py | 69 +++++++++++++++++++------------------------- 1 file changed, 30 insertions(+), 39 deletions(-) diff --git a/Lib/test/test_pty.py b/Lib/test/test_pty.py index 5e083b439d5199..6a5d41af910aba 100644 --- a/Lib/test/test_pty.py +++ b/Lib/test/test_pty.py @@ -881,9 +881,15 @@ def test_intr(self): self._spawn_master_and_slave(self._background_process_intr, child_code) -class PtyMockingTestBase: - """Base class for tests which replace STDIN and STDOUT of the pty - module with their own pipes.""" +class _MockSelectEternalWait(Exception): + """Used both as exception and placeholder value. Models that no + more select activity is expected and that a test can be + terminated.""" + pass + +class PtyCopyTests(unittest.TestCase): + """Whitebox mocking tests which don't spawn children or hang. Test + the _copy loop to transfer data between parent and child.""" def setUp(self): save_and_restore = ['pty.STDIN_FILENO', @@ -900,6 +906,10 @@ def setUp(self): self.select_rfds_lengths = [] self.select_rfds_results = [] + # monkey-patch pty.select with our mock + pty.select = self._mock_select + + def tearDown(self): for k, v in self.saved.items(): module, attr = k.split('.') @@ -916,31 +926,6 @@ def _pipe(self): self.fds.extend(pipe_fds) return pipe_fds -class _MockSelectEternalWait(Exception): - """Used both as exception and placeholder value. Models that no - more select activity is expected and that a test can be - terminated.""" - pass - -class PtyCopyTests(PtyMockingTestBase, unittest.TestCase): - """Whitebox mocking tests which don't spawn children or hang. Test - the _copy loop to transfer data between parent and child.""" - - def _mock_stdin_stdout(self): - """Mock STDIN and STDOUT with two fresh pipes. Replaces - pty.STDIN_FILENO/pty.STDOUT_FILENO by one end of the pipe. - Returns the other end of the pipe.""" - read_from_stdout_fd, mock_stdout_fd = self._pipe() - pty.STDOUT_FILENO = mock_stdout_fd - mock_stdin_fd, write_to_stdin_fd = self._pipe() - pty.STDIN_FILENO = mock_stdin_fd - return (write_to_stdin_fd, read_from_stdout_fd) - - def _socketpair(self): - socketpair = socket.socketpair() - self.files.extend(socketpair) - return socketpair - def _mock_select(self, rfds, wfds, xfds): """Simulates the behavior of select.select. Only implemented for reader waiting list (first parameter).""" @@ -959,8 +944,8 @@ def _mock_select(self, rfds, wfds, xfds): return rfds_result, [], [] def test__mock_select(self): - """Test the select proxy of the test class. Such meta testing. - """ + """Test the select proxy of this test class. Meta testing.""" + self.select_rfds_lengths.append(0) with self.assertRaises(AssertionError): self._mock_select([], [], []) @@ -983,6 +968,21 @@ def test__mock_select(self): self.assertEqual(self.select_rfds_lengths, []) self.assertEqual(self.select_rfds_results, []) + def _mock_stdin_stdout(self): + """Mock STDIN and STDOUT with two fresh pipes. Replaces + pty.STDIN_FILENO/pty.STDOUT_FILENO by one end of the pipe. + Returns the other end of the pipe.""" + read_from_stdout_fd, mock_stdout_fd = self._pipe() + pty.STDOUT_FILENO = mock_stdout_fd + mock_stdin_fd, write_to_stdin_fd = self._pipe() + pty.STDIN_FILENO = mock_stdin_fd + return (write_to_stdin_fd, read_from_stdout_fd) + + def _socketpair(self): + socketpair = socket.socketpair() + self.files.extend(socketpair) + return socketpair + def test__copy_to_each(self): """Test the normal data case on both master_fd and stdin.""" write_to_stdin_fd, read_from_stdout_fd = self._mock_stdin_stdout() @@ -995,9 +995,6 @@ def test__copy_to_each(self): os.write(masters[1], b'from master') os.write(write_to_stdin_fd, b'from stdin') - # monkey-patch pty.select with our mock - pty.select = self._mock_select - # Expect two select calls, the last one will simulate eternal waiting self.select_rfds_lengths.append(2) self.select_rfds_results.append([mock_stdin_fd, masters[0]]) @@ -1036,9 +1033,6 @@ def _copy_eof_close_slave_helper(self, close_stdin): else: os.write(write_to_stdin_fd, b'from stdin') - # monkey-patch pty.select with our mock - pty.select = self._mock_select - # Expect exactly one select() call. This call returns master_fd # and STDIN. Since the slave side of masters is closed, we # expect the _copy loop to exit immediately. @@ -1086,9 +1080,6 @@ def test__copy_eof_on_stdin(self): os.close(write_to_stdin_fd) self.fds.remove(write_to_stdin_fd) - # monkey-patch pty.select with our mock - pty.select = self._mock_select - # Expect two select() calls. The first call returns master_fd # and STDIN. self.select_rfds_lengths.append(2) From 7ced6d8d9d4449b6ed72e573bff331d5e84b8720 Mon Sep 17 00:00:00 2001 From: diekmann Date: Fri, 14 Apr 2017 21:19:46 +0200 Subject: [PATCH 16/26] Tune PtyCopyTests Now that code is closer together, we can move up more into the setUp methods. --- Lib/test/test_pty.py | 73 +++++++++++++++++++------------------------- 1 file changed, 31 insertions(+), 42 deletions(-) diff --git a/Lib/test/test_pty.py b/Lib/test/test_pty.py index 6a5d41af910aba..c931b73aec644e 100644 --- a/Lib/test/test_pty.py +++ b/Lib/test/test_pty.py @@ -906,9 +906,9 @@ def setUp(self): self.select_rfds_lengths = [] self.select_rfds_results = [] - # monkey-patch pty.select with our mock + # monkey-patch and replace with mock pty.select = self._mock_select - + self._mock_stdin_stdout() def tearDown(self): for k, v in self.saved.items(): @@ -926,6 +926,13 @@ def _pipe(self): self.fds.extend(pipe_fds) return pipe_fds + def _mock_stdin_stdout(self): + """Mock STDIN and STDOUT with two fresh pipes. Replaces + pty.STDIN_FILENO/pty.STDOUT_FILENO by one end of the pipe. + Makes the other end of the pipe available via self.""" + self.read_from_stdout_fd, pty.STDOUT_FILENO = self._pipe() + pty.STDIN_FILENO, self.write_to_stdin_fd = self._pipe() + def _mock_select(self, rfds, wfds, xfds): """Simulates the behavior of select.select. Only implemented for reader waiting list (first parameter).""" @@ -943,9 +950,11 @@ def _mock_select(self, rfds, wfds, xfds): raise _MockSelectEternalWait return rfds_result, [], [] + def test__mock_stdin_stdout(self): + self.assertGreater(pty.STDIN_FILENO, 2, "replaced by our mock") + def test__mock_select(self): """Test the select proxy of this test class. Meta testing.""" - self.select_rfds_lengths.append(0) with self.assertRaises(AssertionError): self._mock_select([], [], []) @@ -968,16 +977,6 @@ def test__mock_select(self): self.assertEqual(self.select_rfds_lengths, []) self.assertEqual(self.select_rfds_results, []) - def _mock_stdin_stdout(self): - """Mock STDIN and STDOUT with two fresh pipes. Replaces - pty.STDIN_FILENO/pty.STDOUT_FILENO by one end of the pipe. - Returns the other end of the pipe.""" - read_from_stdout_fd, mock_stdout_fd = self._pipe() - pty.STDOUT_FILENO = mock_stdout_fd - mock_stdin_fd, write_to_stdin_fd = self._pipe() - pty.STDIN_FILENO = mock_stdin_fd - return (write_to_stdin_fd, read_from_stdout_fd) - def _socketpair(self): socketpair = socket.socketpair() self.files.extend(socketpair) @@ -985,19 +984,15 @@ def _socketpair(self): def test__copy_to_each(self): """Test the normal data case on both master_fd and stdin.""" - write_to_stdin_fd, read_from_stdout_fd = self._mock_stdin_stdout() - mock_stdin_fd = pty.STDIN_FILENO - self.assertGreater(mock_stdin_fd, 2, "replaced by our mock") - socketpair = self._socketpair() - masters = [s.fileno() for s in socketpair] + masters = [s.fileno() for s in self._socketpair()] # Feed data. Smaller than PIPEBUF. These writes will not block. os.write(masters[1], b'from master') - os.write(write_to_stdin_fd, b'from stdin') + os.write(self.write_to_stdin_fd, b'from stdin') # Expect two select calls, the last one will simulate eternal waiting self.select_rfds_lengths.append(2) - self.select_rfds_results.append([mock_stdin_fd, masters[0]]) + self.select_rfds_results.append([pty.STDIN_FILENO, masters[0]]) self.select_rfds_lengths.append(2) self.select_rfds_results.append(_MockSelectEternalWait) @@ -1005,17 +1000,14 @@ def test__copy_to_each(self): pty._copy(masters[0]) # Test that the right data went to the right places. - rfds = select.select([read_from_stdout_fd, masters[1]], [], [], 0)[0] - self.assertEqual([read_from_stdout_fd, masters[1]], rfds) - self.assertEqual(os.read(read_from_stdout_fd, 20), b'from master') + rfds = select.select([self.read_from_stdout_fd, masters[1]], [], [], 0)[0] + self.assertEqual([self.read_from_stdout_fd, masters[1]], rfds) + self.assertEqual(os.read(self.read_from_stdout_fd, 20), b'from master') self.assertEqual(os.read(masters[1], 20), b'from stdin') def _copy_eof_close_slave_helper(self, close_stdin): """Helper to test the empty read EOF case on master_fd and/or stdin.""" - write_to_stdin_fd, read_from_stdout_fd = self._mock_stdin_stdout() - mock_stdin_fd = pty.STDIN_FILENO - self.assertGreater(mock_stdin_fd, 2, "replaced by our mock") socketpair = self._socketpair() masters = [s.fileno() for s in socketpair] @@ -1025,21 +1017,21 @@ def _copy_eof_close_slave_helper(self, close_stdin): socketpair[1].close() self.files.remove(socketpair[1]) - # optionally close fd or fill with dummy data in order to + # Optionally close fd or fill with dummy data in order to # prevent blocking on one read call if close_stdin: - os.close(write_to_stdin_fd) - self.fds.remove(write_to_stdin_fd) + os.close(self.write_to_stdin_fd) + self.fds.remove(self.write_to_stdin_fd) else: - os.write(write_to_stdin_fd, b'from stdin') + os.write(self.write_to_stdin_fd, b'from stdin') # Expect exactly one select() call. This call returns master_fd # and STDIN. Since the slave side of masters is closed, we # expect the _copy loop to exit immediately. self.select_rfds_lengths.append(2) - self.select_rfds_results.append([mock_stdin_fd, masters[0]]) + self.select_rfds_results.append([pty.STDIN_FILENO, masters[0]]) - # run the _copy test, which returns nothing and cleanly exits + # Run the _copy test, which returns nothing and cleanly exits self.assertIsNone(pty._copy(masters[0])) # We expect that everything is consumed @@ -1049,10 +1041,10 @@ def _copy_eof_close_slave_helper(self, close_stdin): # Test that STDIN was not touched. This test simulated the # scenario where the child process immediately closed its end of # the pipe. This means, nothing should be copied. - rfds = select.select([read_from_stdout_fd, mock_stdin_fd], [], [], 0)[0] - # data or EOF is still sitting unconsumed in mock_stdin_fd - self.assertEqual(rfds, [mock_stdin_fd]) - unconsumed = os.read(mock_stdin_fd, 20) + rfds = select.select([self.read_from_stdout_fd, pty.STDIN_FILENO], [], [], 0)[0] + # data or EOF is still sitting unconsumed in STDIN + self.assertEqual(rfds, [pty.STDIN_FILENO]) + unconsumed = os.read(pty.STDIN_FILENO, 20) if close_stdin: self.assertFalse(unconsumed) #EOF else: @@ -1068,22 +1060,19 @@ def test__copy_eof_on_master(self): def test__copy_eof_on_stdin(self): """Test the empty read EOF case on stdin.""" - write_to_stdin_fd, read_from_stdout_fd = self._mock_stdin_stdout() - mock_stdin_fd = pty.STDIN_FILENO - self.assertGreater(mock_stdin_fd, 2, "replaced by our mock") socketpair = self._socketpair() masters = [s.fileno() for s in socketpair] # Fill with dummy data os.write(masters[1], b'from master') - os.close(write_to_stdin_fd) - self.fds.remove(write_to_stdin_fd) + os.close(self.write_to_stdin_fd) + self.fds.remove(self.write_to_stdin_fd) # Expect two select() calls. The first call returns master_fd # and STDIN. self.select_rfds_lengths.append(2) - self.select_rfds_results.append([mock_stdin_fd, masters[0]]) + self.select_rfds_results.append([pty.STDIN_FILENO, masters[0]]) # The second call causes _MockSelectEternalWait. We expect that # STDIN is removed from the waiters as it reached EOF. self.select_rfds_lengths.append(1) From 528fc5dcc1478927cb1fda28adf649b129f60cb8 Mon Sep 17 00:00:00 2001 From: diekmann Date: Sat, 15 Apr 2017 12:50:35 +0200 Subject: [PATCH 17/26] unify code comments in PtySpawnTestBase Better terminology, clearly introducing background/slave. --- Lib/test/test_pty.py | 52 +++++++++++++++++++++----------------------- 1 file changed, 25 insertions(+), 27 deletions(-) diff --git a/Lib/test/test_pty.py b/Lib/test/test_pty.py index c931b73aec644e..c027371873719b 100644 --- a/Lib/test/test_pty.py +++ b/Lib/test/test_pty.py @@ -321,19 +321,24 @@ def test_spawn_uncommon_exit_code(self): class PtySpawnTestBase(unittest.TestCase): """A base class for the following integration test setup: A child - process is spawned with pty.spawn(). The child runs a fresh python - interpreter; its python code is passed via command line argument as - string. A background process is forked, reusing the current - instance of the python interpreter. These processes are connected - over STDIN/STDOUT pipes. The tests run fork(), select(), execlp(), - and other calls on your system. - - Starting from the parent (the main thread of this test suite), three - processes are forked for this setup. We call the spawn()-ed child - the 'slave' because it is connected to the slave side of the pty. - The background process is connected to the master side of the pty. - Internal details require a spawn_runner. The actual tests are - executed between slave and background. + process is spawned with pty.spawn() and a background process is + forked from the test suite. The two processes are connected over + STDIN/STDOUT pipes to communicate. The tests run fork(), select(), + execlp(), and other calls on your system. + + Starting from the parent (the main thread of this test suite), the + following processes are forked for this setup: + * The 'slave'. We call the pty.spawn()-ed child the 'slave' because + it is connected to the slave side of the pty. It executes python + code passed as a string. This makes the test suite very + portable. The main test code runs here. + * The 'background' process, which interacts with the slave and + drives the tests. It is connected to the master side of the pty. + Being forked from the parent, it reuses the parent's instance of + the python interpreter. + * The 'spawn_runner', an internal helper to capture the + input/output of the slave. + The actual tests are executed between slave and background. Sequence diagram of the overall test setup: @@ -370,10 +375,6 @@ class PtySpawnTestBase(unittest.TestCase): background . |< . . . . . . . . . . . . . . . . . . . . . . . . . . . . - - - The code for the spawned slave is python code in a string. This - makes the test suite very portable. """ # We introduce this generic base class for the test setup to # encapsulate multiple different types of tests in individual @@ -395,12 +396,12 @@ def _pty_spawn(stdin, stdout, args, pre_spawn_hook=''): # We cannot use the test.support.captured_output() functions # because the pty module writes to filedescriptors directly. # We cannot use test.support.script_helper because we need to - # interact with the spawned child. + # interact with the spawned slave. # We cannot monkey-patch pty.STDIN_FILENO to point to a pipe # because the fallback path of pty.fork() relies on them. # Invoking this functions creates two children: the spawn_runner # as isolated child to execute pty.spawn and capture stdout and - # its grandchild (running args) created by pty.spawn. + # its grandchild (the slave, running args) created by pty.spawn. spawn_runner = pre_spawn_hook + textwrap.dedent(""" import pty, sys; ret = pty.spawn(sys.argv[1:]); @@ -456,11 +457,8 @@ def _spawn_master_and_slave(self, master_fun, slave_src, close_stdin=False, sys.stdout.flush() sys.stderr.flush() # Without this flush, we interfere with the debug output from - # the child that will be spawned and execute master_fun. Don't - # confuse it with the child spawned by spawn(). It will work - # fine without the flush, unless you enable debug and have your - # STDOUT piped! The forked child will print to the same fd as - # we (the parent) print our debug information to. + # the background process (running master_fun). Parent and + # background process print debug information to the same fd. io_fds = (write_to_stdin_fd, read_from_stdout_fd) background_pid = self._fork_background_process(master_fun, io_fds) @@ -476,10 +474,10 @@ def _spawn_master_and_slave(self, master_fun, slave_src, close_stdin=False, debug("killing background process ({:d})".format(background_pid)) os.kill(background_pid, 9) errmsg = ["Spawned slave returned but failed.", - "The failed child code was:", - "--- BEGIN child code ---", + "The failed slave code was:", + "--- BEGIN slave code ---", slave_src, - "--- END child code ---"] + "--- END slave code ---"] rd, _, _ = select.select([read_from_stdout_fd], [], [], 0) if rd: errmsg.append("Dumping what the slave wrote last:") From aeb65859f418a32f174174666046ae4fa8865be2 Mon Sep 17 00:00:00 2001 From: diekmann Date: Sat, 15 Apr 2017 13:06:42 +0200 Subject: [PATCH 18/26] cosmetic changes --- Lib/test/test_pty.py | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-) diff --git a/Lib/test/test_pty.py b/Lib/test/test_pty.py index c027371873719b..7cf6f34f8d374e 100644 --- a/Lib/test/test_pty.py +++ b/Lib/test/test_pty.py @@ -643,16 +643,14 @@ def _wait_for_slave(to_stdin, from_stdout): raise ValueError('handshake with slave failed') debug("[background] slave ready.") + @unittest.skipUnless(sys.platform == 'linux', "ECHOCTL only supported by Linux") def _enable_echoctl(self): - if sys.platform == 'linux': - self.echoctl = True - else: - raise unittest.SkipTest('Test only available on Linux.') + self.echoctl = True _EXEC_BASE_TERMINAL_SETUP_FMT = textwrap.dedent(r""" def _base_terminal_setup(additional_lflag=0): - "Set up terminal to sane defaults (with regard to my Linux" - "system). See POSIX.1-2008, Chapter 11, General Terminal" + "Set up terminal to sane defaults (with regard to my Linux " + "system). See POSIX.1-2008, Chapter 11, General Terminal " "Interface." # Warning: ECHOCTL is not defined in POSIX. Works on @@ -710,8 +708,7 @@ def _background_process_echo(to_stdin, from_stdout): """) def test_echo(self): - """Terminals: Echoing of all characters written to the master - side and newline output translation.""" + """Echo terminal input, and translate the echoed newline""" child_code = self._EXEC_IMPORTS + \ self._EXEC_BASE_TERMINAL_SETUP_FMT.format(echoctl=False) + \ self._EXEC_CHILD_ECHO @@ -1058,8 +1055,7 @@ def test__copy_eof_on_master(self): def test__copy_eof_on_stdin(self): """Test the empty read EOF case on stdin.""" - socketpair = self._socketpair() - masters = [s.fileno() for s in socketpair] + masters = [s.fileno() for s in self._socketpair()] # Fill with dummy data os.write(masters[1], b'from master') From 57876df00be7c1efaf1c336be428fa2f09c9e8a3 Mon Sep 17 00:00:00 2001 From: diekmann Date: Sat, 15 Apr 2017 13:26:20 +0200 Subject: [PATCH 19/26] Cleanup of terminal setup code --- Lib/test/test_pty.py | 66 ++++++++++++++++++-------------------------- 1 file changed, 27 insertions(+), 39 deletions(-) diff --git a/Lib/test/test_pty.py b/Lib/test/test_pty.py index 7cf6f34f8d374e..39833f80ce58fe 100644 --- a/Lib/test/test_pty.py +++ b/Lib/test/test_pty.py @@ -645,38 +645,31 @@ def _wait_for_slave(to_stdin, from_stdout): @unittest.skipUnless(sys.platform == 'linux', "ECHOCTL only supported by Linux") def _enable_echoctl(self): - self.echoctl = True + # Warning: ECHOCTL is not defined in POSIX. Works on + # Linux 4.4 with Ubuntu GLIBC 2.23. Did not work on Mac. + return " | termios.ECHOCTL" #pass as additional lflags _EXEC_BASE_TERMINAL_SETUP_FMT = textwrap.dedent(r""" - def _base_terminal_setup(additional_lflag=0): - "Set up terminal to sane defaults (with regard to my Linux " - "system). See POSIX.1-2008, Chapter 11, General Terminal " - "Interface." - - # Warning: ECHOCTL is not defined in POSIX. Works on - # Linux 4.4 with Ubuntu GLIBC 2.23. Did not work on Mac. - if {echoctl}: - echoctl = termios.ECHOCTL - else: - echoctl = 0 + "Set up terminal to sane defaults (with regard to my Linux system). " + "See POSIX.1-2008, Chapter 11, General Terminal Interface." - terminal_fd = sys.stdin.fileno() - old = termios.tcgetattr(terminal_fd) - # don't need iflag - old[0] = 0 + terminal_fd = sys.stdin.fileno() + old = termios.tcgetattr(terminal_fd) + # don't need iflag + old[0] = 0 - # oflag: output processing: replace \n by \r\n - old[1] = termios.ONLCR | termios.OPOST + # oflag: output processing: replace \n by \r\n + old[1] = termios.ONLCR | termios.OPOST - # don't need cflag - old[2] = 0 + # don't need cflag + old[2] = 0 - # lflag: canonical mode (line-buffer), - # normal echoing, - # echoing of control chars in caret notation (for example ^C) - old[3] = termios.ICANON | termios.ECHO | echoctl | additional_lflag + # lflag: canonical mode (line-buffer), + # normal echoing, + # echoing of control chars in caret notation (for example ^C) + old[3] = termios.ICANON | termios.ECHO {add_lflags} - termios.tcsetattr(terminal_fd, termios.TCSADRAIN, old) + termios.tcsetattr(terminal_fd, termios.TCSADRAIN, old) """) @staticmethod @@ -695,7 +688,6 @@ def _background_process_echo(to_stdin, from_stdout): return 0 _EXEC_CHILD_ECHO = textwrap.dedent(r""" - _base_terminal_setup() print("slave ready!", end='', flush=True) inp = input() @@ -710,7 +702,7 @@ def _background_process_echo(to_stdin, from_stdout): def test_echo(self): """Echo terminal input, and translate the echoed newline""" child_code = self._EXEC_IMPORTS + \ - self._EXEC_BASE_TERMINAL_SETUP_FMT.format(echoctl=False) + \ + self._EXEC_BASE_TERMINAL_SETUP_FMT.format(add_lflags="") + \ self._EXEC_CHILD_ECHO self._spawn_master_and_slave(self._background_process_echo, child_code) @@ -733,7 +725,6 @@ def _background_process_bell(to_stdin, from_stdout): return 0 _EXEC_CHILD_BELL = textwrap.dedent(r""" - _base_terminal_setup() print("slave ready!", end='', flush=True) command = input() @@ -750,9 +741,9 @@ def _background_process_bell(to_stdin, from_stdout): def test_bell_echoctl(self): """Terminals: Pretty printing of the bell character in caret notation.""" - self._enable_echoctl() + lflags = self._enable_echoctl() child_code = self._EXEC_IMPORTS + \ - self._EXEC_BASE_TERMINAL_SETUP_FMT.format(echoctl=self.echoctl) + \ + self._EXEC_BASE_TERMINAL_SETUP_FMT.format(add_lflags=lflags) + \ self._EXEC_CHILD_BELL self._spawn_master_and_slave(self._background_process_bell, child_code) @@ -776,7 +767,6 @@ def _background_process_eof(to_stdin, from_stdout): return 0 _EXEC_CHILD_EOF = textwrap.dedent(""" - _base_terminal_setup() print("slave ready!", end='', flush=True) try: @@ -800,9 +790,8 @@ def _background_process_eof(to_stdin, from_stdout): def test_eof(self): """Terminals: Processing of the special EOF character.""" - self.echoctl = False child_code = self._EXEC_IMPORTS + \ - self._EXEC_BASE_TERMINAL_SETUP_FMT.format(echoctl=self.echoctl) + \ + self._EXEC_BASE_TERMINAL_SETUP_FMT.format(add_lflags="") + \ self._EXEC_CHILD_EOF self._spawn_master_and_slave(self._background_process_eof, child_code) @@ -810,9 +799,9 @@ def test_eof_echoctl(self): """Terminals: Processing of the special EOF character with ECHOCTL enabled.""" # ^D is usually not pretty printed - self._enable_echoctl() + lflags = self._enable_echoctl() child_code = self._EXEC_IMPORTS + \ - self._EXEC_BASE_TERMINAL_SETUP_FMT.format(echoctl=self.echoctl) + \ + self._EXEC_BASE_TERMINAL_SETUP_FMT.format(add_lflags=lflags) + \ self._EXEC_CHILD_EOF self._spawn_master_and_slave(self._background_process_eof, child_code) @@ -845,8 +834,6 @@ def _SIGINT_handler(a, b): signal.signal(signal.SIGINT, _SIGINT_handler) - # tell our controlling terminal to send signals on special characters - _base_terminal_setup(termios.ISIG) print("slave ready!", end='', flush=True) command = input() @@ -869,9 +856,10 @@ def test_intr(self): """Terminals: Writing a x03 char to the master side is translated to sending an INTR signal to the slave. Simulates pressing ctrl+c in master.""" - self.echoctl = False + # tell our controlling terminal to send signals on special characters + lflags = " | termios.ISIG" child_code = self._EXEC_IMPORTS + \ - self._EXEC_BASE_TERMINAL_SETUP_FMT.format(echoctl=self.echoctl) + \ + self._EXEC_BASE_TERMINAL_SETUP_FMT.format(add_lflags=lflags) + \ self._EXEC_CHILD_INTR self._spawn_master_and_slave(self._background_process_intr, child_code) From 2309d69ba0bd1f50701076aaead221953789d587 Mon Sep 17 00:00:00 2001 From: diekmann Date: Sat, 15 Apr 2017 13:33:49 +0200 Subject: [PATCH 20/26] removed test_eof_echoctl We still have test_eof. And test_bell_echoctl tests echoctl. --- Lib/test/test_pty.py | 12 +----------- 1 file changed, 1 insertion(+), 11 deletions(-) diff --git a/Lib/test/test_pty.py b/Lib/test/test_pty.py index 39833f80ce58fe..75b3e09c9d6aec 100644 --- a/Lib/test/test_pty.py +++ b/Lib/test/test_pty.py @@ -700,7 +700,7 @@ def _background_process_echo(to_stdin, from_stdout): """) def test_echo(self): - """Echo terminal input, and translate the echoed newline""" + """Echo terminal input, and translate the echoed newline.""" child_code = self._EXEC_IMPORTS + \ self._EXEC_BASE_TERMINAL_SETUP_FMT.format(add_lflags="") + \ self._EXEC_CHILD_ECHO @@ -795,16 +795,6 @@ def test_eof(self): self._EXEC_CHILD_EOF self._spawn_master_and_slave(self._background_process_eof, child_code) - def test_eof_echoctl(self): - """Terminals: Processing of the special EOF character with - ECHOCTL enabled.""" - # ^D is usually not pretty printed - lflags = self._enable_echoctl() - child_code = self._EXEC_IMPORTS + \ - self._EXEC_BASE_TERMINAL_SETUP_FMT.format(add_lflags=lflags) + \ - self._EXEC_CHILD_EOF - self._spawn_master_and_slave(self._background_process_eof, child_code) - @staticmethod def _background_process_intr(to_stdin, from_stdout): """Try to send SIGINT to child. Careful: Testsuite also watches From f93de8d5f32c78f0ad38eec6716618beffbf1957 Mon Sep 17 00:00:00 2001 From: diekmann Date: Sat, 15 Apr 2017 13:57:51 +0200 Subject: [PATCH 21/26] fixed echoing race --- Lib/test/test_pty.py | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/Lib/test/test_pty.py b/Lib/test/test_pty.py index 75b3e09c9d6aec..4e7dda3f8e3d46 100644 --- a/Lib/test/test_pty.py +++ b/Lib/test/test_pty.py @@ -807,10 +807,13 @@ def _background_process_intr(to_stdin, from_stdout): to_slave = b'This buffered stuff will be ignored'+INTR+b' Ohai slave!\n' pty._writen(to_stdin, to_slave) - expected = INTR+b' Ohai slave!\r\n' - received = _os_read_exactly(from_stdout, len(expected)) - if received != expected: - raise RuntimeError("Expecting {!r} but got {!r}".format(expected, received)) + expected = b' Ohai slave!\r\n' + # Race: The kernel may or may not echo parts before the INTR. In + # either case, due to canonical mode, the slave will only receive the + # string after INTR. + received = _os_readline(from_stdout) + if not received.endswith(expected): + raise RuntimeError("Expecting to end with {!r} but got {!r}".format(expected, received)) debug("[background] got it back") return 0 From 9cd94257c6d44581cf14d959f06461bef3a4b836 Mon Sep 17 00:00:00 2001 From: diekmann Date: Sat, 15 Apr 2017 14:03:02 +0200 Subject: [PATCH 22/26] cleaned OS X workaround comments --- Lib/test/test_pty.py | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/Lib/test/test_pty.py b/Lib/test/test_pty.py index 4e7dda3f8e3d46..8d16da25adfbf4 100644 --- a/Lib/test/test_pty.py +++ b/Lib/test/test_pty.py @@ -759,7 +759,7 @@ def _background_process_eof(to_stdin, from_stdout): # STDOUT. Tested on OS X 10.6.8 and 10.11.2. Wipe EOF # character which may remain here. c = os.read(from_stdout, 1) - if c == b'\x04': + if c == b'\x04': # ignore EOF character c = os.read(from_stdout, 1) if c != b'!': raise RuntimeError("Did not receive marker.") @@ -777,10 +777,7 @@ def _background_process_eof(to_stdin, from_stdout): # we expect an EOF here, this is good pass - # OS X leaves an EOF character in the channel which we want to - # remove. We set an exclamation mark as marker and in the - # background process, we read everything until we reach this - # marker. + # Send an exclamation mark as marker. print("!", end='', flush=True) # Send final confirmation that all went well to master. From a7e47537f3402077a1c0d402e7d93afce5eb1937 Mon Sep 17 00:00:00 2001 From: diekmann Date: Sat, 15 Apr 2017 14:28:17 +0200 Subject: [PATCH 23/26] Removed doc string from test methods. "Also, no documentation string for the method should be included. A comment (such as # Tests function returns only True or False) should be used to provide documentation for test methods. This is done because documentation strings get printed out if they exist and thus what test is being run is not stated." /Doc/build/html/library/test.html --- Lib/test/test_pty.py | 48 +++++++++++++++++++------------------------- 1 file changed, 21 insertions(+), 27 deletions(-) diff --git a/Lib/test/test_pty.py b/Lib/test/test_pty.py index 8d16da25adfbf4..4b8392b8689036 100644 --- a/Lib/test/test_pty.py +++ b/Lib/test/test_pty.py @@ -303,18 +303,18 @@ def _spawn_py_get_retcode(self, python_src): return retcode def test_spawn_exitsuccess(self): - """Spawn the python-equivalent of /bin/true.""" + # Spawn the python-equivalent of /bin/true. retcode = self._spawn_py_get_retcode('import sys; sys.exit()') self.assertEqual(retcode, 0) def test_spawn_exitfailure(self): - """Spawn the python-equivalent of /bin/false.""" + # Spawn the python-equivalent of /bin/false. retcode = self._spawn_py_get_retcode('import sys; sys.exit(1)') self.assertEqual(retcode, 1) def test_spawn_uncommon_exit_code(self): - """Test an uncommon exit code, which is less likely to be caused - by a Python exception or other failure.""" + # Test an uncommon exit code, which is less likely to be caused + # by a Python exception or other failure. retcode = self._spawn_py_get_retcode('import sys; sys.exit(81)') self.assertEqual(retcode, 81) @@ -541,8 +541,7 @@ def _background_process(to_stdin, from_stdout): return 0 # success def test_alternate_ping(self): - """Spawn a slave and fork a master background process. Let them - Ping-Pong count to 1000 by turns.""" + # Let background and slave Ping-Pong count to 1000 by turns. child_code = self._EXEC_IMPORTS + self._EXEC_CHILD self._spawn_master_and_slave(self._background_process, child_code) @@ -557,10 +556,8 @@ def _mock_disabled_osforkpty(): os.forkpty = _mock_disabled_osforkpty """.format(verbose)) - def test_alternate_ping_disable_osforkpty(self): - """Spawn a slave and fork a master background process. Let them - Ping-Pong count to 1000 by turns. Disable os.forkpty(), trigger - pty.fork() backup code.""" + def test_ping_disable_osforkpty(self): + # Disable os.forkpty(), trigger pty.fork() fallback code path. child_code = self._EXEC_IMPORTS + self._EXEC_CHILD self._spawn_master_and_slave(self._background_process, child_code, pre_spawn_hook=self._DISABLE_OS_FORKPTY) @@ -600,16 +597,14 @@ def _background_process(to_stdin, from_stdout): """) def test_read(self): - """Spawn a slave and fork a master background process. Receive - several kBytes from the slave.""" + # Receive several kBytes from the slave. child_code = self._EXEC_IMPORTS + \ self._EXEC_CHILD_FMT.format(sleeptime=0.05) debug("Test may take up to 1 second ...") self._spawn_master_and_slave(self._background_process, child_code) def test_read_close_stdin(self): - """Spawn a slave and fork a master background process. Close - STDIN and receive several kBytes from the slave.""" + # Close STDIN and receive several kBytes from the slave. # only sleep in one test to speed this up child_code = self._EXEC_IMPORTS + \ self._EXEC_CHILD_FMT.format(sleeptime=0) @@ -700,7 +695,7 @@ def _background_process_echo(to_stdin, from_stdout): """) def test_echo(self): - """Echo terminal input, and translate the echoed newline.""" + # Echo terminal input, and translate the echoed newline. child_code = self._EXEC_IMPORTS + \ self._EXEC_BASE_TERMINAL_SETUP_FMT.format(add_lflags="") + \ self._EXEC_CHILD_ECHO @@ -739,8 +734,7 @@ def _background_process_bell(to_stdin, from_stdout): """) def test_bell_echoctl(self): - """Terminals: Pretty printing of the bell character in caret - notation.""" + # Pretty printing of the bell character in caret notation. lflags = self._enable_echoctl() child_code = self._EXEC_IMPORTS + \ self._EXEC_BASE_TERMINAL_SETUP_FMT.format(add_lflags=lflags) + \ @@ -786,7 +780,7 @@ def _background_process_eof(to_stdin, from_stdout): """) def test_eof(self): - """Terminals: Processing of the special EOF character.""" + # Processing of the special EOF character. child_code = self._EXEC_IMPORTS + \ self._EXEC_BASE_TERMINAL_SETUP_FMT.format(add_lflags="") + \ self._EXEC_CHILD_EOF @@ -843,10 +837,10 @@ def _SIGINT_handler(a, b): """) def test_intr(self): - """Terminals: Writing a x03 char to the master side is - translated to sending an INTR signal to the slave. Simulates - pressing ctrl+c in master.""" - # tell our controlling terminal to send signals on special characters + # Writing a x03 char to the master side is translated to sending + # an INTR signal to the slave. Simulates pressing ctrl+c in + # master. + # Tell our controlling terminal to send signals on special characters lflags = " | termios.ISIG" child_code = self._EXEC_IMPORTS + \ self._EXEC_BASE_TERMINAL_SETUP_FMT.format(add_lflags=lflags) + \ @@ -927,7 +921,7 @@ def test__mock_stdin_stdout(self): self.assertGreater(pty.STDIN_FILENO, 2, "replaced by our mock") def test__mock_select(self): - """Test the select proxy of this test class. Meta testing.""" + # Test the select proxy of this test class. Meta testing. self.select_rfds_lengths.append(0) with self.assertRaises(AssertionError): self._mock_select([], [], []) @@ -956,7 +950,7 @@ def _socketpair(self): return socketpair def test__copy_to_each(self): - """Test the normal data case on both master_fd and stdin.""" + # Test the normal data case on both master_fd and stdin. masters = [s.fileno() for s in self._socketpair()] # Feed data. Smaller than PIPEBUF. These writes will not block. @@ -1024,15 +1018,15 @@ def _copy_eof_close_slave_helper(self, close_stdin): self.assertEqual(unconsumed, b'from stdin') def test__copy_eof_on_all(self): - """Test the empty read EOF case on both master_fd and stdin.""" + # Test the empty read EOF case on both master_fd and stdin. self._copy_eof_close_slave_helper(close_stdin=True) def test__copy_eof_on_master(self): - """Test the empty read EOF case on only master_fd.""" + # Test the empty read EOF case on only master_fd. self._copy_eof_close_slave_helper(close_stdin=False) def test__copy_eof_on_stdin(self): - """Test the empty read EOF case on stdin.""" + # Test the empty read EOF case on stdin. masters = [s.fileno() for s in self._socketpair()] # Fill with dummy data From 59447641289db65dc817dce307ec38779e837916 Mon Sep 17 00:00:00 2001 From: diekmann Date: Sat, 15 Apr 2017 17:36:38 +0200 Subject: [PATCH 24/26] Fixed comment --- 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 4b8392b8689036..0096598f449e1b 100644 --- a/Lib/test/test_pty.py +++ b/Lib/test/test_pty.py @@ -661,7 +661,7 @@ def _enable_echoctl(self): # lflag: canonical mode (line-buffer), # normal echoing, - # echoing of control chars in caret notation (for example ^C) + # optional: additional flags old[3] = termios.ICANON | termios.ECHO {add_lflags} termios.tcsetattr(terminal_fd, termios.TCSADRAIN, old) From 851c2612db4741c101e9f1305d5f5ea106d59a08 Mon Sep 17 00:00:00 2001 From: diekmann Date: Fri, 28 Jul 2017 17:19:25 +0200 Subject: [PATCH 25/26] [CONTRIBUTING.rst] Added myself to Misc/ACKS TODO: This branch also includes the patch from Chris Torek bpo-26228. --- Misc/ACKS | 1 + 1 file changed, 1 insertion(+) diff --git a/Misc/ACKS b/Misc/ACKS index e0640c114ff34f..7894547030d171 100644 --- a/Misc/ACKS +++ b/Misc/ACKS @@ -368,6 +368,7 @@ Caleb Deveraux Catherine Devlin Scott Dial Alon Diamant +Cornelius Diekmann Toby Dickenson Mark Dickinson Jack Diederich From 5dbc2495690a98635fbb373ee345733de52a5aa4 Mon Sep 17 00:00:00 2001 From: diekmann Date: Fri, 28 Jul 2017 17:50:36 +0200 Subject: [PATCH 26/26] added Chris Torek to ACKS --- Misc/ACKS | 1 + 1 file changed, 1 insertion(+) diff --git a/Misc/ACKS b/Misc/ACKS index 7894547030d171..29c1b30ffcb966 100644 --- a/Misc/ACKS +++ b/Misc/ACKS @@ -1579,6 +1579,7 @@ Eugene Toder Erik Tollerud Stephen Tonkin Matias Torchinsky +Chris Torek Sandro Tosi Richard Townsend David Townshend