From 577027461231460862daff1b8bee18b05efb702f Mon Sep 17 00:00:00 2001 From: Steven Silvester <steven.silvester@ieee.org> Date: Thu, 17 Feb 2022 17:12:32 -0600 Subject: [PATCH 1/2] test that terminate causes EOFError try with sigterm try with sigkill too update tests add loop test add explicit EOF another attempt cleanup try again don't skip the test skip control tests for now use new event loop reinstate control tests fix handling of event loop follow docs try skipping the loop test try conpty only remove more winpty see if removing the extra read works try without extra write and no loop test back to original code try winpty only timeout after 15 minutes try with conpty only remove eof handling try without capture run pytest directly pin pytest fix shell quoting use 10 minute timeout --- .github/workflows/windows_build.yml | 5 ++- runtests.py | 2 +- winpty/ptyprocess.py | 16 +++++-- winpty/tests/test_ptyprocess.py | 70 ++++++++++++++++++++++++++++- 4 files changed, 86 insertions(+), 7 deletions(-) diff --git a/.github/workflows/windows_build.yml b/.github/workflows/windows_build.yml index f2a599d9..a178dd7d 100644 --- a/.github/workflows/windows_build.yml +++ b/.github/workflows/windows_build.yml @@ -11,6 +11,7 @@ jobs: windows: name: Windows Py${{ matrix.PYTHON_VERSION }} runs-on: windows-latest + timeout-minutes: 10 env: PYTHON_VERSION: ${{ matrix.PYTHON_VERSION }} RUNNER_OS: "windows" @@ -45,13 +46,13 @@ jobs: run: conda install -y winpty - name: Install build/test dependencies shell: bash -l {0} - run: pip install maturin toml pytest pytest-lazy-fixture flaky + run: pip install maturin toml "pytest<7" pytest-lazy-fixture flaky - name: Build pywinpty shell: bash -l {0} run: maturin develop - name: Run tests shell: pwsh - run: python runtests.py + run: python -m pytest -v -x -s # Enable this to get RDP access to the worker. # - name: Download # # if: ${{ failure() }} diff --git a/runtests.py b/runtests.py index 220f7ba7..4c4b4fbd 100644 --- a/runtests.py +++ b/runtests.py @@ -12,7 +12,7 @@ def run_pytest(extra_args=None): - pytest_args = ['-v', '-x'] + pytest_args = ['-v', '-x', '-s'] # Allow user to pass a custom test path to pytest to e.g. run just one test if extra_args: diff --git a/winpty/ptyprocess.py b/winpty/ptyprocess.py index 8f7f904d..7cf4c8b8 100644 --- a/winpty/ptyprocess.py +++ b/winpty/ptyprocess.py @@ -241,7 +241,7 @@ def terminate(self, force=False): if not self.isalive(): return True if force: - self.kill(signal.SIGKILL) + self.kill(signal.SIGTERM) time.sleep(self.delayafterterminate) if not self.isalive(): return True @@ -337,14 +337,24 @@ def _read_in_thread(address, pty, blocking): client.connect(address) call = 0 + while 1: try: data = pty.read(4096, blocking=blocking) or b'0011Ignore' - if data or not blocking: + try: + client.send(bytes(data, 'utf-8')) + except socket.error: + break + + # Handle end of file. + if pty.iseof(): try: - client.send(bytes(data, 'utf-8')) + client.send(b'') except socket.error: + pass + finally: break + call += 1 except Exception as e: break diff --git a/winpty/tests/test_ptyprocess.py b/winpty/tests/test_ptyprocess.py index 71259bcd..5456dbe3 100644 --- a/winpty/tests/test_ptyprocess.py +++ b/winpty/tests/test_ptyprocess.py @@ -2,6 +2,7 @@ """winpty wrapper tests.""" # Standard library imports +import asyncio import os import signal import time @@ -17,6 +18,7 @@ from winpty.ptyprocess import PtyProcess, which + @pytest.fixture(scope='module', params=['WinPTY', 'ConPTY']) def pty_fixture(request): backend = request.param @@ -160,13 +162,79 @@ def test_exit_status(pty_fixture): assert pty.exitstatus == 1 -def test_kill(pty_fixture): +@pytest.mark.timeout(30) +def test_kill_sigterm(pty_fixture): pty = pty_fixture() + pty.write('echo \"foo\"\r\nsleep 1000\r\n') + pty.read() pty.kill(signal.SIGTERM) + + while True: + try: + pty.read() + except EOFError: + break + assert not pty.isalive() assert pty.exitstatus == signal.SIGTERM +@pytest.mark.timeout(30) +def test_terminate(pty_fixture): + pty = pty_fixture() + pty.write('echo \"foo\"\r\nsleep 1000\r\n') + pty.read() + pty.terminate() + + while True: + try: + pty.read() + except EOFError: + break + + assert not pty.isalive() + + +@pytest.mark.timeout(30) +def test_terminate_force(pty_fixture): + pty = pty_fixture() + pty.write('echo \"foo\"\r\nsleep 1000\r\n') + pty.read() + pty.terminate(force=True) + + while True: + try: + pty.read() + except EOFError: + break + + assert not pty.isalive() + + +def test_terminate_loop(pty_fixture): + pty = pty_fixture() + loop = asyncio.SelectorEventLoop() + asyncio.set_event_loop(loop) + + def reader(): + try: + data = pty.read() + except EOFError: + loop.remove_reader(pty.fd) + loop.stop() + + loop.add_reader(pty.fd, reader) + loop.call_soon(pty.write, 'echo \"foo\"\r\nsleep 1000\r\n') + loop.call_soon(pty.terminate, True) + + try: + loop.run_forever() + finally: + loop.close() + + assert not pty.isalive() + + def test_getwinsize(pty_fixture): pty = pty_fixture() assert pty.getwinsize() == (24, 80) From 336fe01b7f1d997cd81b491e8f8e629d49b67739 Mon Sep 17 00:00:00 2001 From: Steven Silvester <steven.silvester@ieee.org> Date: Wed, 2 Mar 2022 12:54:26 -0600 Subject: [PATCH 2/2] xfail on some tests with conpty fix backend parameter add another skip add another skip revert changes to test running remove pytest pin --- .github/workflows/windows_build.yml | 4 ++-- runtests.py | 2 +- winpty/tests/test_ptyprocess.py | 11 +++++++++++ 3 files changed, 14 insertions(+), 3 deletions(-) diff --git a/.github/workflows/windows_build.yml b/.github/workflows/windows_build.yml index a178dd7d..bd74d43e 100644 --- a/.github/workflows/windows_build.yml +++ b/.github/workflows/windows_build.yml @@ -46,13 +46,13 @@ jobs: run: conda install -y winpty - name: Install build/test dependencies shell: bash -l {0} - run: pip install maturin toml "pytest<7" pytest-lazy-fixture flaky + run: pip install maturin toml pytest pytest-lazy-fixture flaky - name: Build pywinpty shell: bash -l {0} run: maturin develop - name: Run tests shell: pwsh - run: python -m pytest -v -x -s + run: python runtests.py # Enable this to get RDP access to the worker. # - name: Download # # if: ${{ failure() }} diff --git a/runtests.py b/runtests.py index 4c4b4fbd..220f7ba7 100644 --- a/runtests.py +++ b/runtests.py @@ -12,7 +12,7 @@ def run_pytest(extra_args=None): - pytest_args = ['-v', '-x', '-s'] + pytest_args = ['-v', '-x'] # Allow user to pass a custom test path to pytest to e.g. run just one test if extra_args: diff --git a/winpty/tests/test_ptyprocess.py b/winpty/tests/test_ptyprocess.py index 5456dbe3..a688c747 100644 --- a/winpty/tests/test_ptyprocess.py +++ b/winpty/tests/test_ptyprocess.py @@ -34,6 +34,7 @@ def pty_fixture(request): def _pty_factory(cmd=None, env=None): cmd = cmd or 'cmd' return PtyProcess.spawn(cmd, env=env, backend=backend) + _pty_factory.backend = request.param return _pty_factory @@ -125,12 +126,16 @@ def test_flush(pty_fixture): def test_intr(pty_fixture): + if pty_fixture.backend == 'ConPTY': + pytest.xfail('Not supported on ConPTY') pty = pty_fixture(cmd=[sys.executable, 'import time; time.sleep(10)']) pty.sendintr() assert pty.wait() != 0 def test_send_control(pty_fixture): + if pty_fixture.backend == 'ConPTY': + pytest.xfail('Not supported on ConPTY') pty = pty_fixture(cmd=[sys.executable, 'import time; time.sleep(10)']) pty.sendcontrol('d') assert pty.wait() != 0 @@ -138,6 +143,8 @@ def test_send_control(pty_fixture): @pytest.mark.skipif(which('cat') is None, reason="Requires cat on the PATH") def test_send_eof(pty_fixture): + if pty_fixture.backend == 'ConPTY': + pytest.xfail('Not supported on ConPTY') cat = pty_fixture('cat') cat.sendeof() assert cat.wait() == 0 @@ -151,6 +158,8 @@ def test_isatty(pty_fixture): def test_wait(pty_fixture): + if pty_fixture.backend == 'ConPTY': + pytest.xfail('Not supported on ConPTY') pty = pty_fixture(cmd=[sys.executable, '--version']) assert pty.wait() == 0 @@ -212,6 +221,8 @@ def test_terminate_force(pty_fixture): def test_terminate_loop(pty_fixture): + if pty_fixture.backend == 'ConPTY': + pytest.xfail('Not supported on ConPTY') pty = pty_fixture() loop = asyncio.SelectorEventLoop() asyncio.set_event_loop(loop)