From 69f40723d2b0b63d9c6ec2dd60e15aca509d585f Mon Sep 17 00:00:00 2001 From: Benedek Racz Date: Fri, 31 Jan 2020 15:49:48 +0100 Subject: [PATCH] [FIX] exit status now returned from child in new-structure wexpect --- README.md | 36 ++++++++++++++++++++++++++++++++++++ tests/test_constructor.py | 1 + tests/test_interact.py | 4 ++-- tests/test_misc.py | 16 +++++++++++----- tests/test_run.py | 2 +- wexpect/__init__.py | 12 +++++++++++- wexpect/console_reader.py | 15 +++++---------- wexpect/host.py | 22 ++++++++++++++-------- 8 files changed, 81 insertions(+), 27 deletions(-) diff --git a/README.md b/README.md index 0ee590f..b2d95d2 100644 --- a/README.md +++ b/README.md @@ -41,6 +41,42 @@ child.sendline('exit') For more information see [examples](./examples) folder. +--- +## **REFACTOR** + +The original wexpect has some structural weakness, which leads me to rewrite the whole code. The +first variant of the new structure is delivered with [v3.2.0](https://pypi.org/project/wexpect/3.2.0/). +Note, that the default is the old variant (`legacy_wexpect`), to use the new you need to set the +`WEXPECT_SPAWN_CLASS` environment variable to `SpawnPipe` or `SpawnSocket`, which are the two new +structured spawn class. + +### Old vs new + +But what is the difference between the old and new and what was the problem with the old? + +Generally, wexpect (both old and new) has three processes: + + - *host* is our original pyton script/program, which want to launch the child. + - *console* is a process which started by the host, and launches the child. (This is a python script) + - *child* is the process which want to be launced. + +The child and the console has a common Windows console, distict from the host. + +The `legacy_wexpect`'s console is a thin script, almost do nothing. It initializes the Windows's +console, and monitors the host and child processes. The magic is done by the host process, which has +the switchTo() and switchBack() functions, which (de-) attaches the *child-console* Windows-console. +The host manipulates the child's console directly. This direct manipuation is the structural weakness. +The following task/usecases are hard/impossibile: + + - thread-safe multiprocessing of the host. + - logging (both console and host) + - using in grapichal IDE or with pytest + - This variant is highly depends on the pywin32 package. + +The new structure's console is a thik script. The console process do the major console manipulation, +which is controlled by the host via socket (see SpawnSocket) or named-pipe (SpawnPipe). The host +only process the except-loops. + --- ## What is it? diff --git a/tests/test_constructor.py b/tests/test_constructor.py index 58ed2b1..01ceb3a 100644 --- a/tests/test_constructor.py +++ b/tests/test_constructor.py @@ -34,6 +34,7 @@ class TestCaseConstructor(PexpectTestCase.PexpectTestCase): time.sleep(p1.delayafterterminate) p2 = wexpect.spawn('uname', ['-m', '-n', '-p', '-r', '-s', '-v'], timeout=5) p2.expect(wexpect.EOF) + time.sleep(p1.delayafterterminate) self.assertEqual(p1.before, p2.before) self.assertEqual(str(p1).splitlines()[1:9], str(p2).splitlines()[1:9]) diff --git a/tests/test_interact.py b/tests/test_interact.py index fc7c16d..5b86e22 100644 --- a/tests/test_interact.py +++ b/tests/test_interact.py @@ -34,7 +34,7 @@ from . import PexpectTestCase class InteractTestCase(PexpectTestCase.PexpectTestCase): - @unittest.skipIf(not hasattr(wexpect.spawn, 'interact'), "spawn does not support runtime interact switching.") + @unittest.skipIf(not (hasattr(wexpect, 'legacy_wexpect')) or (hasattr(wexpect.spawn, 'interact')), "spawn does not support runtime interact switching.") def test_interact(self): # Path of cmd executable: cmd_exe = 'cmd' @@ -57,7 +57,7 @@ class InteractTestCase(PexpectTestCase.PexpectTestCase): self.assertEqual('hello', p.before.splitlines()[1]) - @unittest.skipIf(not hasattr(wexpect.spawn, 'interact'), "spawn does not support runtime interact switching.") + @unittest.skipIf(not (hasattr(wexpect, 'legacy_wexpect')) or (hasattr(wexpect.spawn, 'interact')), "spawn does not support runtime interact switching.") def test_interact_dead(self): # Path of cmd executable: echo = 'echo hello' diff --git a/tests/test_misc.py b/tests/test_misc.py index 1ad981e..5519c41 100644 --- a/tests/test_misc.py +++ b/tests/test_misc.py @@ -21,10 +21,8 @@ wexpect LICENSE import unittest import sys import re -import signal -import time -import tempfile import os +import time import wexpect from . import PexpectTestCase @@ -88,7 +86,11 @@ class TestCaseMisc(PexpectTestCase.PexpectTestCase): child.readline().rstrip() self.assertEqual(child.readline().rstrip(), 'delta') child.expect(wexpect.EOF) - assert not child.isalive() + if type(child).__name__ in ['SpawnPipe', 'SpawnSocket']: + time.sleep(child.delayafterterminate) + assert not child.isalive(trust_console=False) + else: + assert not child.isalive() self.assertEqual(child.exitstatus, 0) def test_iter(self): @@ -110,7 +112,11 @@ class TestCaseMisc(PexpectTestCase.PexpectTestCase): page = ''.join(child.readlines()).replace(_CAT_EOF, '') self.assertEqual(page, '\r\nabc\r\n\r\n123\r\n') child.expect(wexpect.EOF) - assert not child.isalive() + if type(child).__name__ in ['SpawnPipe', 'SpawnSocket']: + time.sleep(child.delayafterterminate) + assert not child.isalive(trust_console=False) + else: + assert not child.isalive() self.assertEqual(child.exitstatus, 0) def test_write(self): diff --git a/tests/test_run.py b/tests/test_run.py index 2d2221b..276faac 100644 --- a/tests/test_run.py +++ b/tests/test_run.py @@ -87,7 +87,7 @@ class RunFuncTestCase(PexpectTestCase.PexpectTestCase): def test_run_bad_exitstatus(self): (the_new_way, exitstatus) = self.runfunc( 'ls -l /najoeufhdnzkxjd', withexitstatus=1) - assert exitstatus != 0 + self.assertNotEqual(exitstatus, 0) def test_run_event_as_string(self): re_flags = re.DOTALL | re.MULTILINE diff --git a/wexpect/__init__.py b/wexpect/__init__.py index b19ad73..8114ffd 100644 --- a/wexpect/__init__.py +++ b/wexpect/__init__.py @@ -1,6 +1,7 @@ # __init__.py import os +import pkg_resources try: spawn_class_name = os.environ['WEXPECT_SPAWN_CLASS'] @@ -37,6 +38,8 @@ else: from .host import SpawnSocket from .host import SpawnPipe from .host import run + from .host import searcher_string + from .host import searcher_re try: spawn = globals()[spawn_class_name] @@ -44,5 +47,12 @@ else: print(f'Error: no spawn class: {spawn_class_name}') raise + # The version is handled by the package: pbr, which derives the version from the git tags. + try: + __version__ = pkg_resources.require("wexpect")[0].version + except: # pragma: no cover + __version__ = '0.0.1.unkowndev0' + __all__ = ['split_command_line', 'join_args', 'ExceptionPexpect', 'EOF', 'TIMEOUT', - 'ConsoleReaderSocket', 'ConsoleReaderPipe', 'spawn', 'SpawnSocket', 'SpawnPipe', 'run'] + 'ConsoleReaderSocket', 'ConsoleReaderPipe', 'spawn', 'SpawnSocket', 'SpawnPipe', 'run', + 'searcher_string', 'searcher_re', '__version__'] diff --git a/wexpect/console_reader.py b/wexpect/console_reader.py index 02fd740..f927fb9 100644 --- a/wexpect/console_reader.py +++ b/wexpect/console_reader.py @@ -41,7 +41,6 @@ import time import logging import os import traceback -import pkg_resources import psutil import signal from io import StringIO @@ -66,11 +65,6 @@ screenbufferfillchar = '\4' maxconsoleY = 8000 default_port = 4321 -# The version is handled by the package: pbr, which derives the version from the git tags. -try: - __version__ = pkg_resources.require("wexpect")[0].version -except: # pragma: no cover - __version__ = '0.0.1.unkowndev0' # # Create logger: We write logs only to file. Printing out logs are dangerous, because of the deep @@ -168,10 +162,11 @@ class ConsoleReaderBase: if not self.isalive(self.host_process): logger.info('Host process has been died.') return - - if win32process.GetExitCodeProcess(self.__childProcess) != win32con.STILL_ACTIVE: - logger.info('Child finished.') - return + + self.child_exitstatus = win32process.GetExitCodeProcess(self.__childProcess) + if self.child_exitstatus != win32con.STILL_ACTIVE: + logger.info(f'Child finished with code: {self.child_exitstatus}') + return consinfo = self.consout.GetConsoleScreenBufferInfo() cursorPos = consinfo['CursorPosition'] diff --git a/wexpect/host.py b/wexpect/host.py index 364e959..2b011f4 100644 --- a/wexpect/host.py +++ b/wexpect/host.py @@ -158,10 +158,11 @@ def run (command, timeout=-1, withexitstatus=False, events=None, extra_args=None pass data to a callback function through run() through the locals dictionary passed to a callback. """ + from .__init__ import spawn if timeout == -1: - child = SpawnPipe(command, maxread=2000, logfile=logfile, cwd=cwd, env=env, **kwargs) + child = spawn(command, maxread=2000, logfile=logfile, cwd=cwd, env=env, **kwargs) else: - child = SpawnPipe(command, timeout=timeout, maxread=2000, logfile=logfile, cwd=cwd, env=env, **kwargs) + child = spawn(command, timeout=timeout, maxread=2000, logfile=logfile, cwd=cwd, env=env, **kwargs) if events is not None: patterns = list(events.keys()) responses = list(events.values()) @@ -366,7 +367,7 @@ class SpawnBase: console_class_parameters_kv_pairs = [f'{k}={v}' for k,v in self.console_class_parameters.items() ] console_class_parameters_str = ', '.join(console_class_parameters_kv_pairs) - child_class_initializator = f"wexpect.{self.console_class_name}(wexpect.join_args({args}), {console_class_parameters_str});" + child_class_initializator = f"cons = wexpect.{self.console_class_name}(wexpect.join_args({args}), {console_class_parameters_str});" commandLine = '"%s" %s "%s"' % (python_executable, ' '.join(pyargs), @@ -376,7 +377,8 @@ class SpawnBase: "import time;" "wexpect.console_reader.logger.info('loggerStart.');" f"{child_class_initializator}" - "wexpect.console_reader.logger.info('Console finished2.');" + "wexpect.console_reader.logger.info(f'Console finished2. {cons.child_exitstatus}');" + "sys.exit(cons.child_exitstatus)" ) logger.info(f'Console starter command:{commandLine}') @@ -425,6 +427,7 @@ class SpawnBase: try: self.exitstatus = self.child_process.wait(timeout=0) + logger.info(f'exitstatus: {self.exitstatus}') except psutil.TimeoutExpired: return True @@ -435,11 +438,13 @@ class SpawnBase: except psutil.NoSuchProcess as e: logger.info('Child has already died. %s', e) - def wait(self, child=True, console=True): + def wait(self, child=True, console=False): if child: - self.child_process.wait() + self.exitstatus = self.child_process.wait() + logger.info(f'exitstatus: {self.exitstatus}') if console: self.exitstatus = self.console_process.wait() + logger.info(f'exitstatus: {self.exitstatus}') return self.exitstatus def read (self, size = -1): # File-like object. @@ -834,7 +839,7 @@ class SpawnBase: class SpawnPipe(SpawnBase): def __init__(self, command, args=[], timeout=30, maxread=60000, searchwindowsize=None, - logfile=None, cwd=None, env=None, codepage=None, echo=True, port=4321, host='localhost', interact=False): + logfile=None, cwd=None, env=None, codepage=None, echo=True, interact=False, **kwargs): self.pipe = None self.console_class_name = 'ConsoleReaderPipe' self.console_class_parameters = {} @@ -948,6 +953,7 @@ class SpawnPipe(SpawnBase): try: logger.info(f'Sending kill signal: {sig}') self.send(SIGNAL_CHARS[sig]) + self.terminated = True except EOF as e: logger.info(e) @@ -955,7 +961,7 @@ class SpawnPipe(SpawnBase): class SpawnSocket(SpawnBase): def __init__(self, command, args=[], timeout=30, maxread=60000, searchwindowsize=None, - logfile=None, cwd=None, env=None, codepage=None, echo=True, port=4321, host='localhost', interact=False): + logfile=None, cwd=None, env=None, codepage=None, echo=True, port=4321, host='127.0.0.1', interact=False): self.port = port self.host = host self.sock = None