diff --git a/.azure-pipelines/ci.yml b/.azure-pipelines/ci.yml new file mode 100644 index 0000000..c3441df --- /dev/null +++ b/.azure-pipelines/ci.yml @@ -0,0 +1,20 @@ +steps: +- task: UsePythonVersion@0 + inputs: + versionSpec: '$(PYTHON_VERSION)' + displayName: 'Use Python $(PYTHON_VERSION)' + +- script: | + python -m pip install --upgrade pip + pip install tox + displayName: "Set up tox" + +- script: | + tox -e azure + displayName: "Run tests" + +- task: PublishTestResults@2 + condition: succeededOrFailed() + inputs: + testResultsFiles: '**/test-*.xml' + testRunTitle: 'Publish test results for Python $(PYTHON_VERSION)' diff --git a/.azure-pipelines/flake8-validation.py b/.azure-pipelines/flake8-validation.py new file mode 100644 index 0000000..89594a9 --- /dev/null +++ b/.azure-pipelines/flake8-validation.py @@ -0,0 +1,42 @@ +import os +import subprocess + +# Flake8 validation +failures = 0 +try: + flake8 = subprocess.run( + [ + "flake8", + "--exit-zero", + "--max-line-length=88", + "--select=E401,E711,E712,E713,E714,E721,E722,E901,F401,F402,F403,F405,F631,F632,F633,F811,F812,F821,F822,F841,F901,W191,W291,W292,W293,W602,W603,W604,W605,W606", + ], + capture_output=True, + check=True, + encoding="latin-1", + timeout=300, + ) +except (subprocess.CalledProcessError, subprocess.TimeoutExpired) as e: + print( + "##vso[task.logissue type=error;]flake8 validation failed with", + str(e.__class__.__name__), + ) + print(e.stdout) + print(e.stderr) + print("##vso[task.complete result=Failed;]flake8 validation failed") + exit() +for line in flake8.stdout.split("\n"): + if ":" not in line: + continue + filename, lineno, column, error = line.split(":", maxsplit=3) + errcode, error = error.strip().split(" ", maxsplit=1) + filename = os.path.normpath(filename) + failures += 1 + print( + f"##vso[task.logissue type=error;sourcepath={filename};" + f"linenumber={lineno};columnnumber={column};code={errcode};]" + error + ) + +if failures: + print(f"##vso[task.logissue type=warning]Found {failures} flake8 violation(s)") + print(f"##vso[task.complete result=Failed;]Found {failures} flake8 violation(s)") diff --git a/.azure-pipelines/syntax-validation.py b/.azure-pipelines/syntax-validation.py new file mode 100644 index 0000000..9359ad8 --- /dev/null +++ b/.azure-pipelines/syntax-validation.py @@ -0,0 +1,30 @@ +import ast +import os +import sys + +print("Python", sys.version, "\n") + +failures = 0 + +for base, _, files in os.walk("."): + for f in files: + if not f.endswith(".py"): + continue + filename = os.path.normpath(os.path.join(base, f)) + try: + with open(filename) as fh: + ast.parse(fh.read()) + except SyntaxError as se: + failures += 1 + print( + f"##vso[task.logissue type=error;sourcepath={filename};" + f"linenumber={se.lineno};columnnumber={se.offset};]" + f"SyntaxError: {se.msg}" + ) + print(" " + se.text + " " * se.offset + "^") + print(f"SyntaxError: {se.msg} in {filename} line {se.lineno}") + print() + +if failures: + print(f"##vso[task.logissue type=warning]Found {failures} syntax error(s)") + print(f"##vso[task.complete result=Failed;]Found {failures} syntax error(s)") diff --git a/.azure-pipelines.yml b/.azure-pipelines.yml new file mode 100644 index 0000000..213e36f --- /dev/null +++ b/.azure-pipelines.yml @@ -0,0 +1,124 @@ +trigger: + branches: + include: + - '*' + tags: + include: + - '*' + +stages: +- stage: static + displayName: Static Analysis + jobs: + - job: checks + displayName: static code analysis + pool: + vmImage: ubuntu-latest + steps: + # Use Python >=3.7 for syntax validation + - task: UsePythonVersion@0 + displayName: Set up python + inputs: + versionSpec: 3.7 + + # Run syntax validation on a shallow clone + - bash: | + python .azure-pipelines/syntax-validation.py + displayName: Syntax validation + + # Run flake8 validation on a shallow clone + - bash: | + pip install flake8 + python .azure-pipelines/flake8-validation.py + displayName: Flake8 validation + +- stage: tests + displayName: Run unit tests + jobs: + - job: linux + pool: + vmImage: ubuntu-latest + strategy: + matrix: + python36: + PYTHON_VERSION: 3.6 + python37: + PYTHON_VERSION: 3.7 + python38: + PYTHON_VERSION: 3.8 + python39: + PYTHON_VERSION: 3.9 + steps: + - template: .azure-pipelines/ci.yml + + - job: macOS + pool: + vmImage: macOS-latest + strategy: + matrix: + python36: + PYTHON_VERSION: 3.6 + python37: + PYTHON_VERSION: 3.7 + python38: + PYTHON_VERSION: 3.8 + python39: + PYTHON_VERSION: 3.9 + steps: + - template: .azure-pipelines/ci.yml + + - job: windows + pool: + vmImage: windows-latest + strategy: + matrix: + python36: + PYTHON_VERSION: 3.6 + python37: + PYTHON_VERSION: 3.7 + python38: + PYTHON_VERSION: 3.8 + python39: + PYTHON_VERSION: 3.9 + steps: + - template: .azure-pipelines/ci.yml + +- stage: deploy + displayName: Publish release + dependsOn: + - tests + - static + condition: and(succeeded(), startsWith(variables['Build.SourceBranch'], 'refs/tags/')) + jobs: + - job: pypi + displayName: Publish pypi release + pool: + vmImage: ubuntu-latest + steps: + - task: UsePythonVersion@0 + displayName: Set up python + inputs: + versionSpec: 3.8 + + - bash: | + python -m pip install -r requirements_dev.txt + displayName: Install dependencies + + - bash: | + python setup.py sdist bdist_wheel + ls -la dist + displayName: Build python package + + - task: PublishBuildArtifacts@1 + inputs: + pathToPublish: dist/ + artifactName: python-release + + - task: TwineAuthenticate@1 + displayName: Set up credentials + inputs: + pythonUploadServiceConnection: pypi-procrunner + + - bash: | + python -m twine upload -r pypi-procrunner --config-file $(PYPIRC_PATH) dist/*.tar.gz dist/*.whl + displayName: Publish package diff --git a/.bumpversion.cfg b/.bumpversion.cfg new file mode 100644 index 0000000..6318479 --- /dev/null +++ b/.bumpversion.cfg @@ -0,0 +1,13 @@ +[bumpversion] +current_version = 2.3.0 +commit = True +tag = True + +[bumpversion:file:setup.py] +search = version="{current_version}" +replace = version="{new_version}" + +[bumpversion:file:procrunner/__init__.py] +search = __version__ = "{current_version}" +replace = __version__ = "{new_version}" + diff --git a/.github/dependabot.yml b/.github/dependabot.yml new file mode 100644 index 0000000..55f99c9 --- /dev/null +++ b/.github/dependabot.yml @@ -0,0 +1,11 @@ +# To get started with Dependabot version updates, you'll need to specify which +# package ecosystems to update and where the package manifests are located. +# Please see the documentation for all configuration options: +# https://help.github.com/github/administering-a-repository/configuration-options-for-dependency-updates + +version: 2 +updates: + - package-ecosystem: "pip" # See documentation for possible values + directory: "/" # Location of package manifests + schedule: + interval: "monthly" diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml new file mode 100644 index 0000000..0b21e90 --- /dev/null +++ b/.pre-commit-config.yaml @@ -0,0 +1,18 @@ +repos: + +# Automatic source code formatting +- repo: https://github.com/psf/black + rev: 20.8b1 + hooks: + - id: black + args: [--safe, --quiet] + +# Syntax check and some basic flake8 +- repo: https://github.com/pre-commit/pre-commit-hooks + rev: v2.0.0 + hooks: + - id: check-ast + - id: check-yaml + - id: flake8 + args: ['--max-line-length=88', '--select=E401,E711,E712,E713,E714,E721,E722,E901,F401,F402,F403,F405,F631,F632,F633,F811,F812,F821,F822,F841,F901,W191,W291,W292,W293,W602,W603,W604,W605,W606'] + - id: check-merge-conflict diff --git a/.pyup.yml b/.pyup.yml deleted file mode 100644 index 674e27f..0000000 --- a/.pyup.yml +++ /dev/null @@ -1,5 +0,0 @@ -# autogenerated pyup.io config file -# see https://pyup.io/docs/configuration/ for all available options - -schedule: every month - diff --git a/.travis.yml b/.travis.yml index f8ff7f3..22e5511 100644 --- a/.travis.yml +++ b/.travis.yml @@ -5,15 +5,8 @@ matrix: include: - python: 3.8 - dist: xenial - sudo: true - python: 3.7 - dist: xenial - sudo: true - python: 3.6 - - python: 3.5 - - python: 2.7 - - python: pypy - os: osx language: generic env: CONDA=3.8 TOXENV=py38 @@ -23,12 +16,6 @@ - os: osx language: generic env: CONDA=3.6 TOXENV=py36 - - os: osx - language: generic - env: CONDA=3.5 TOXENV=py35 - - os: osx - language: generic - env: CONDA=2.7 TOXENV=py27 allow_failures: - env: OPTIONAL=1 @@ -38,7 +25,7 @@ before_install: | if [ ! -z "$CONDA" ]; then if [ "$TRAVIS_OS_NAME" == "osx" ]; then - curl https://repo.continuum.io/miniconda/Miniconda3-latest-MacOSX-x86_64.sh --output miniconda.sh + curl https://repo.anaconda.com/miniconda/Miniconda3-latest-MacOSX-x86_64.sh --output miniconda.sh fi chmod +x miniconda.sh ./miniconda.sh -b @@ -55,18 +42,3 @@ # Command to run tests, e.g. python setup.py test script: tox - -# Assuming you have installed the travis-ci CLI tool, after you -# create the Github repo and add it to Travis, run the -# following command to finish PyPI deployment setup: -# $ travis encrypt --add deploy.password -deploy: - provider: pypi - distributions: sdist bdist_wheel - user: mgerstel - password: - secure: qN3EYVlH22eLPZp5CSvc5Bz8bpP0l0wBZ8tcYMa6kBrqelYdIgqkuZESR/oCMu1YBA3AbcKJNwMuzuWf8RNGAFebD820OJThdP4czRMCv6LDbWABnv12lFHQLQSW1fMQVvb2arjnE/Ew7BFq70p0wOlIJJRwu6CoeOXW/sMeVYMivxdaHmgORq3cdMluFAy4amVb3Fc8i7mxAM0QGklO7x/UWJR/IEpUk2RlUbXL+HrzNoEjRtDeMxoCR84gKZTjVeUQ/iIQSuWwxlt7v1FNENj6ZEbE7+PS8/ylIVfPufbCr8tEEv8W58QcxQ5xPJC2g85ulsN5dM9/9FekhpyKa25B/4wKUNq5T8rKJ8WZ6hMiGffW8rmAfrGTmrBaojKBi0pb9VfXJ5KXUcunVXwQaAn2L80jLLHNsAo94ZxeoowD1eJox9Wh1NtNc+NiUv8K6spOIBsur7G5GY4JVA/yZ7w+DweEfQEp8/SEdVhK0vEYSYT4FnJHuAAmNgeedyAtoes4+a5bYYUM4qrz2OC78NWQWAnnsZhD4Y/TulkavWSexVCqfSePRoK3gcCs+rXwiM69XkMbL1Wgj1gNou+XMkntayH2ZDHkkyJi5F7ls4nqMH5RON9FfVygJMoHDRqh0p4RV25IzJ4FSYqKihHNBO31/URnU2ihpW7n8kM+mbM= - on: - tags: true - repo: DiamondLightSource/python-procrunner - python: 3.8 diff --git a/CONTRIBUTING.rst b/CONTRIBUTING.rst index 341c9e7..1e191be 100644 --- a/CONTRIBUTING.rst +++ b/CONTRIBUTING.rst @@ -101,10 +101,7 @@ 1. The pull request should include tests. 2. If the pull request adds functionality, the docs should be updated. Put your new functionality into a function with a docstring, and add the - feature to the list in README.rst. -3. The pull request should work for Python 2.7, 3.5, 3.6, 3.7, 3.8, and for PyPy. Check - https://travis-ci.org/DiamondLightSource/python-procrunner/pull_requests - and make sure that the tests pass for all supported Python versions. + feature to the list in HISTORY.rst/README.rst. Tips ---- diff --git a/HISTORY.rst b/HISTORY.rst index 2cee455..6097ae4 100644 --- a/HISTORY.rst +++ b/HISTORY.rst @@ -2,65 +2,89 @@ History ======= +2.3.0 (2020-10-29) +------------------ +* Add Python 3.9 support, drop Python 3.5 support +* Fix a file descriptor leak on subprocess execution + +2.2.0 (2020-09-07) +------------------ +* Calling the run() function with unnamed arguments (other than the command + list as the first argument) is now deprecated. As a number of arguments + will be removed in a future version the use of unnamed arguments will + cause future confusion. `Use explicit keyword arguments instead (#62). `_ +* `The run() function debug argument has been deprecated (#63). `_ + This is only used to debug the NonBlockingStream* classes. Those are due + to be replaced in a future release, so the argument will no longer serve + a purpose. Debugging information remains available via standard logging + mechanisms. +* Final version supporting Python 3.5 + +2.1.0 (2020-09-05) +------------------ +* `Deprecated array access on the return object (#60). `_ + The return object will become a subprocess.CompletedProcess in a future + release, which no longer allows array-based access. For a translation table + of array elements to attributes please see the pull request linked above. +* Add a `new parameter 'raise_timeout_exception' (#61). `_ + When set to 'True' a subprocess.TimeoutExpired exception is raised when the + process runtime exceeds the timeout threshold. This defaults to 'False' and + will be set to 'True' in a future release. + +2.0.0 (2020-06-24) +------------------ +* Python 3.5+ only, support for Python 2.7 has been dropped +* Deprecated function alias run_process() has been removed +* Fixed a stability issue on Windows + 1.1.0 (2019-11-04) ------------------ - * Add Python 3.8 support, drop Python 3.4 support 1.0.2 (2019-05-20) ------------------ - * Stop environment override variables leaking into the process environment 1.0.1 (2019-04-16) ------------------ - * Minor fixes on the return object (implement equality, mark as unhashable) 1.0.0 (2019-03-25) ------------------ - * Support file system path objects (PEP-519) in arguments * Change the return object to make it similar to subprocess.CompletedProcess, introduced with Python 3.5+ 0.9.1 (2019-02-22) ------------------ - * Have deprecation warnings point to correct code locations 0.9.0 (2018-12-07) ------------------ - * Trap UnicodeEncodeError when printing output. Offending characters are replaced and a warning is logged once. Hints at incorrectly set PYTHONIOENCODING. 0.8.1 (2018-12-04) ------------------ - * Fix a few deprecation warnings 0.8.0 (2018-10-09) ------------------ - * Add parameter working_directory to set the working directory of the subprocess 0.7.2 (2018-10-05) ------------------ - * Officially support Python 3.7 0.7.1 (2018-09-03) ------------------ - * Accept environment variable overriding with numeric values. 0.7.0 (2018-05-13) ------------------ - * Unicode fixes. Fix crash on invalid UTF-8 input. * Clarify that stdout/stderr values are returned as bytestrings. * Callbacks receive the data decoded as UTF-8 unicode strings @@ -70,23 +94,19 @@ 0.6.1 (2018-05-02) ------------------ - * Maintenance release to add some tests for executable resolution. 0.6.0 (2018-05-02) ------------------ - * Fix Win32 API executable resolution for commands containing a dot ('.') in addition to a file extension (say '.bat'). 0.5.1 (2018-04-27) ------------------ - * Fix Win32API dependency installation on Windows. 0.5.0 (2018-04-26) ------------------ - * New keyword 'win32resolve' which only takes effect on Windows and is enabled by default. This causes procrunner to call the Win32 API FindExecutable() function to try and lookup non-.exe files with the corresponding name. This @@ -95,21 +115,17 @@ 0.4.0 (2018-04-23) ------------------ - * Python 2.7 support on Windows. Python3 not yet supported on Windows. 0.3.0 (2018-04-17) ------------------ - * run_process() renamed to run() * Python3 compatibility fixes 0.2.0 (2018-03-12) ------------------ - * Procrunner is now Python3 3.3-3.6 compatible. 0.1.0 (2018-03-12) ------------------ - * First release on PyPI. diff --git a/README.rst b/README.rst index fb7f5d0..0fbe170 100644 --- a/README.rst +++ b/README.rst @@ -23,10 +23,6 @@ :target: https://procrunner.readthedocs.io/en/latest/?badge=latest :alt: Documentation Status -.. image:: https://pyup.io/repos/github/DiamondLightSource/python-procrunner/shield.svg - :target: https://pyup.io/repos/github/DiamondLightSource/python-procrunner/ - :alt: Updates - .. image:: https://img.shields.io/pypi/pyversions/procrunner.svg :target: https://pypi.python.org/pypi/procrunner :alt: Supported Python versions @@ -47,15 +43,16 @@ * runs an external process and waits for it to finish * does not deadlock, no matter the process stdout/stderr output behaviour * returns the exit code, stdout, stderr (separately, both as bytestrings), - and the total process runtime as a dictionary + as a subprocess.CompletedProcess object * process can run in a custom environment, either as a modification of the current environment or in a new environment from scratch -* stdin can be fed to the process, the returned dictionary contains - information how much was read by the process +* stdin can be fed to the process * stdout and stderr is printed by default, can be disabled * stdout and stderr can be passed to any arbitrary function for live processing (separately, both as unicode strings) -* optionally enforces a time limit on the process +* optionally enforces a time limit on the process, raising a + subprocess.TimeoutExpired exception if it is exceeded. + Credits ------- diff --git a/appveyor.yml b/appveyor.yml index e6e1890..0cccda9 100644 --- a/appveyor.yml +++ b/appveyor.yml @@ -5,24 +5,12 @@ # For Python versions available on Appveyor, see # http://www.appveyor.com/docs/installed-software#python - - PYTHON: "C:\\Python27" - - PYTHON: "C:\\Python35" - UNSTABLE: 1 - PYTHON: "C:\\Python36" - UNSTABLE: 1 - PYTHON: "C:\\Python37" - UNSTABLE: 1 - PYTHON: "C:\\Python38" - UNSTABLE: 1 - - PYTHON: "C:\\Python27-x64" - - PYTHON: "C:\\Python35-x64" - UNSTABLE: 1 - PYTHON: "C:\\Python36-x64" - UNSTABLE: 1 - PYTHON: "C:\\Python37-x64" - UNSTABLE: 1 - PYTHON: "C:\\Python38-x64" - UNSTABLE: 1 matrix: allow_failures: @@ -32,9 +20,6 @@ # Upgrade to the latest pip. - '%PYTHON%\\python.exe -m pip install -U pip setuptools wheel' - '%PYTHON%\\python.exe -m pip install -r requirements_dev.txt' - # Install win32api dependency. Must use the --only-binary switch explicitly - # on AppVeyor - - "%PYTHON%\\python.exe -m pip install --only-binary pywin32 pywin32" build: off @@ -42,7 +27,7 @@ # Note that you must use the environment variable %PYTHON% to refer to # the interpreter you're using - Appveyor does not do anything special # to put the Python version you want to use on PATH. - - "%PYTHON%\\python.exe setup.py test" + - "%PYTHON%\\python.exe -m pytest" after_test: # This step builds your wheels. diff --git a/debian/changelog b/debian/changelog index 308e86f..3246364 100644 --- a/debian/changelog +++ b/debian/changelog @@ -1,3 +1,9 @@ +python-procrunner (2.3.0-1) UNRELEASED; urgency=low + + * New upstream release. + + -- Debian Janitor Fri, 17 Sep 2021 11:30:37 -0000 + python-procrunner (1.1.0-1) unstable; urgency=medium * First release (Closes: #962456) diff --git a/docs/conf.py b/docs/conf.py index e850102..abce534 100755 --- a/docs/conf.py +++ b/docs/conf.py @@ -1,5 +1,4 @@ #!/usr/bin/env python -# -*- coding: utf-8 -*- # # procrunner documentation build configuration file, created by # sphinx-quickstart on Fri Jun 9 13:47:02 2017. @@ -48,9 +47,9 @@ master_doc = "index" # General information about the project. -project = u"ProcRunner" -copyright = u"2018, Markus Gerstel" -author = u"Markus Gerstel" +project = "ProcRunner" +copyright = "2020, Diamond Light Source" +author = "Diamond Light Source - Scientific Software" # The version info for the project you're documenting, acts as replacement # for |version| and |release|, also used in various other places throughout @@ -129,8 +128,8 @@ ( master_doc, "procrunner.tex", - u"ProcRunner Documentation", - u"Markus Gerstel", + "procrunner Documentation", + "Diamond Light Source - Scientific Software", "manual", ) ] @@ -140,7 +139,7 @@ # One entry per manual page. List of tuples # (source start file, name, description, authors, manual section). -man_pages = [(master_doc, "procrunner", u"ProcRunner Documentation", [author], 1)] +man_pages = [(master_doc, "procrunner", "procrunner Documentation", [author], 1)] # -- Options for Texinfo output ---------------------------------------- @@ -152,10 +151,10 @@ ( master_doc, "procrunner", - u"ProcRunner Documentation", + "procrunner Documentation", author, "procrunner", - "One line description of project.", + "Versatile utility function to run external processes", "Miscellaneous", ) ] diff --git a/docs/usage.rst b/docs/usage.rst index 8ccfe60..4b87a55 100644 --- a/docs/usage.rst +++ b/docs/usage.rst @@ -9,13 +9,14 @@ To test for successful completion:: - assert not result['exitcode'] - assert result['exitcode'] == 0 # alternatively + assert not result.returncode + assert result.returncode == 0 # alternatively + result.check_returncode() # raises subprocess.CalledProcessError() To test for no STDERR output:: - assert not result['stderr'] - assert result['stderr'] == b'' # alternatively + assert not result.stderr + assert result.stderr == b'' # alternatively To run with a specific environment variable set:: diff --git a/procrunner/__init__.py b/procrunner/__init__.py index 1a1e306..6097818 100644 --- a/procrunner/__init__.py +++ b/procrunner/__init__.py @@ -1,12 +1,10 @@ -# -*- coding: utf-8 -*- - -from __future__ import absolute_import, division, print_function - import codecs +import functools +import io import logging import os import select -import six +import shutil import subprocess import sys import time @@ -21,12 +19,11 @@ # # - runs an external process and waits for it to finish # - does not deadlock, no matter the process stdout/stderr output behaviour -# - returns the exit code, stdout, stderr (separately), and the total process -# runtime as a dictionary +# - returns the exit code, stdout, stderr (separately) as a +# subprocess.CompletedProcess object # - process can run in a custom environment, either as a modification of # the current environment or in a new environment from scratch -# - stdin can be fed to the process, the returned dictionary contains -# information how much was read by the process +# - stdin can be fed to the process # - stdout and stderr is printed by default, can be disabled # - stdout and stderr can be passed to any arbitrary function for # live processing @@ -40,25 +37,29 @@ # # Returns: # -# {'command': ['/bin/ls', '/some/path/containing spaces'], -# 'exitcode': 2, -# 'runtime': 0.12990689277648926, -# 'stderr': '/bin/ls: cannot access /some/path/containing spaces: No such file or directory\n', -# 'stdout': '', -# 'time_end': '2017-11-12 19:54:49 GMT', -# 'time_start': '2017-11-12 19:54:49 GMT', -# 'timeout': False} -# +# ReturnObject( +# args=('/bin/ls', '/some/path/containing spaces'), +# returncode=2, +# stdout=b'', +# stderr=b'/bin/ls: cannot access /some/path/containing spaces: No such file or directory\n' +# ) +# +# which also offers (albeit deprecated) +# +# result.runtime == 0.12990689277648926 +# result.time_end == '2017-11-12 19:54:49 GMT' +# result.time_start == '2017-11-12 19:54:49 GMT' +# result.timeout == False __author__ = """Markus Gerstel""" __email__ = "scientificsoftware@diamond.ac.uk" -__version__ = "1.1.0" +__version__ = "2.3.0" logger = logging.getLogger("procrunner") logger.addHandler(logging.NullHandler()) -class _LineAggregator(object): +class _LineAggregator: """ Buffer that can be filled with stream data and will aggregate complete lines. Lines can be printed or passed to an arbitrary callback function. @@ -107,12 +108,12 @@ self._buffer = "" -class _NonBlockingStreamReader(object): +class _NonBlockingStreamReader: """Reads a stream in a thread to avoid blocking/deadlocks""" def __init__(self, stream, output=True, debug=False, notify=None, callback=None): """Creates and starts a thread which reads from a stream.""" - self._buffer = six.BytesIO() + self._buffer = io.BytesIO() self._closed = False self._closing = False self._debug = debug @@ -131,6 +132,7 @@ else: if self._closing: break + self._stream.close() self._terminated = True la.flush() if self._debug: @@ -150,6 +152,7 @@ print(linedecode) if callback: callback(linedecode) + self._stream.close() self._terminated = True if self._debug: logger.debug("Stream reader terminated") @@ -184,14 +187,14 @@ if not self.has_finished(): if self._debug: logger.debug( - "NBSR join after %f seconds, underrun not resolved" - % (timeit.default_timer() - underrun_debug_timer) + "NBSR join after %f seconds, underrun not resolved", + timeit.default_timer() - underrun_debug_timer, ) raise Exception("thread did not terminate") if self._debug: logger.debug( - "NBSR underrun resolved after %f seconds" - % (timeit.default_timer() - underrun_debug_timer) + "NBSR underrun resolved after %f seconds", + timeit.default_timer() - underrun_debug_timer, ) if self._closed: raise Exception("streamreader double-closed") @@ -201,7 +204,7 @@ return data -class _NonBlockingStreamWriter(object): +class _NonBlockingStreamWriter: """Writes to a stream in a thread to avoid blocking/deadlocks""" def __init__(self, stream, data, debug=False, notify=None): @@ -209,7 +212,6 @@ self._buffer = data self._buffer_len = len(data) self._buffer_pos = 0 - self._debug = debug self._max_block_len = 4096 self._stream = stream self._terminated = False @@ -224,7 +226,7 @@ block = self._buffer[self._buffer_pos :] try: self._stream.write(block) - except IOError as e: + except OSError as e: if ( e.errno == 32 ): # broken pipe, ie. process terminated without reading entire stdin @@ -236,7 +238,7 @@ raise self._buffer_pos += len(block) if debug: - logger.debug("wrote %d bytes to stream" % len(block)) + logger.debug("wrote %d bytes to stream", len(block)) self._stream.close() self._terminated = True if notify: @@ -273,14 +275,11 @@ return obj -def _windows_resolve(command): +def _windows_resolve(command, path=None): """ Try and find the full path and file extension of the executable to run. This is so that e.g. calls to 'somescript' will point at 'somescript.cmd' without the need to set shell=True in the subprocess. - If the executable contains periods it is a special case. Here the - win32api call will fail to resolve the extension automatically, and it - has do be done explicitly. :param command: The command array to be run, with the first element being the command with or w/o path, with or w/o extension. @@ -288,79 +287,56 @@ correct extension. If the executable cannot be resolved for any reason the original command array is returned. """ - try: - import win32api - except ImportError: - if (2, 8) < sys.version_info < (3, 5): - logger.info( - "Resolving executable names only supported on Python 2.7 and 3.5+" - ) - else: - logger.warning( - "Could not resolve executable name: package win32api missing" - ) + if not command or not isinstance(command[0], str): return command - if not command or not isinstance(command[0], six.string_types): - return command - - try: - _, found_executable = win32api.FindExecutable(command[0]) + found_executable = shutil.which(command[0], path=path) + if found_executable: logger.debug("Resolved %s as %s", command[0], found_executable) - return (found_executable,) + tuple(command[1:]) - except Exception as e: - if not hasattr(e, "winerror"): - raise - # Keep this error message for later in case we fail to resolve the name - logwarning = getattr(e, "strerror", str(e)) - - if "." in command[0]: - # Special case. The win32api may not properly check file extensions, so - # try to resolve the executable explicitly. + return (found_executable, *command[1:]) + + if "\\" in command[0]: + # Special case. shutil.which may not detect file extensions if a full + # path is given, so try to resolve the executable explicitly for extension in os.getenv("PATHEXT").split(os.pathsep): - try: - _, found_executable = win32api.FindExecutable(command[0] + extension) - logger.debug("Resolved %s as %s", command[0], found_executable) - return (found_executable,) + tuple(command[1:]) - except Exception as e: - if not hasattr(e, "winerror"): - raise - - logger.warning("Error trying to resolve the executable: %s", logwarning) + found_executable = shutil.which(command[0] + extension, path=path) + if found_executable: + return (found_executable, *command[1:]) + + logger.warning("Error trying to resolve the executable: %s", command[0]) return command -if sys.version_info < (3, 5): - - class _ReturnObjectParent(object): - def check_returncode(self): - if self.returncode: - raise Exception( - "Call %r resulted in non-zero exit code %r" - % (self.args, self.returncode) - ) - - -else: - _ReturnObjectParent = subprocess.CompletedProcess - - -class ReturnObject(dict, _ReturnObjectParent): +class ReturnObject(subprocess.CompletedProcess): """ A subprocess.CompletedProcess-like object containing the executed command, stdout and stderr (both as bytestrings), and the exitcode. - Further values such as process runtime can be accessed as dictionary - values. The check_returncode() function raises an exception if the process exited with a non-zero exit code. """ - def __init__(self, *arg, **kw): - super(ReturnObject, self).__init__(*arg, **kw) - self.args = self["command"] - self.returncode = self["exitcode"] - self.stdout = self["stdout"] - self.stderr = self["stderr"] + def __init__(self, exitcode=None, command=None, stdout=None, stderr=None, **kw): + super().__init__( + args=command, returncode=exitcode, stdout=stdout, stderr=stderr + ) + self._extras = { + "timeout": kw.get("timeout"), + "runtime": kw.get("runtime"), + "time_start": kw.get("time_start"), + "time_end": kw.get("time_end"), + } + + def __getitem__(self, key): + warnings.warn( + "dictionary access to a procrunner return object is deprecated", + DeprecationWarning, + stacklevel=2, + ) + if key in self._extras: + return self._extras[key] + if not hasattr(self, key): + raise KeyError(f"Unknown attribute {key}") + return getattr(self, key) def __eq__(self, other): """Override equality operator to account for added fields""" @@ -372,15 +348,93 @@ """This object is not immutable, so mark it as unhashable""" return None - def __ne__(self, other): - """Overrides the default implementation (unnecessary in Python 3)""" - return not self.__eq__(other) - - + @property + def cmd(self): + warnings.warn( + "procrunner return object .cmd is deprecated, use .args", + DeprecationWarning, + stacklevel=2, + ) + return self.args + + @property + def command(self): + warnings.warn( + "procrunner return object .command is deprecated, use .args", + DeprecationWarning, + stacklevel=2, + ) + return self.args + + @property + def exitcode(self): + warnings.warn( + "procrunner return object .exitcode is deprecated, use .returncode", + DeprecationWarning, + stacklevel=2, + ) + return self.returncode + + @property + def timeout(self): + warnings.warn( + "procrunner return object .timeout is deprecated", + DeprecationWarning, + stacklevel=2, + ) + return self._extras["timeout"] + + @property + def runtime(self): + warnings.warn( + "procrunner return object .runtime is deprecated", + DeprecationWarning, + stacklevel=2, + ) + return self._extras["runtime"] + + @property + def time_start(self): + warnings.warn( + "procrunner return object .time_start is deprecated", + DeprecationWarning, + stacklevel=2, + ) + return self._extras["time_start"] + + @property + def time_end(self): + warnings.warn( + "procrunner return object .time_end is deprecated", + DeprecationWarning, + stacklevel=2, + ) + return self._extras["time_end"] + + def update(self, dictionary): + self._extras.update(dictionary) + + +def _deprecate_argument_calling(f): + @functools.wraps(f) + def wrapper(*args, **kwargs): + if len(args) > 1: + warnings.warn( + "Calling procrunner.run() with unnamed arguments (apart from " + "the command) is deprecated. Use keyword arguments instead.", + DeprecationWarning, + stacklevel=2, + ) + return f(*args, **kwargs) + + return wrapper + + +@_deprecate_argument_calling def run( command, timeout=None, - debug=False, + debug=None, stdin=None, print_stdout=True, print_stderr=True, @@ -390,6 +444,7 @@ environment_override=None, win32resolve=True, working_directory=None, + raise_timeout_exception=False, ): """ Run an external process. @@ -399,8 +454,8 @@ :param array command: Command line to be run, specified as array. :param timeout: Terminate program execution after this many seconds. - :param boolean debug: Enable further debug messages. - :param stdin: Optional string that is passed to command stdin. + :param boolean debug: Enable further debug messages. (deprecated) + :param stdin: Optional bytestring that is passed to command stdin. :param boolean print_stdout: Pass stdout through to sys.stdout. :param boolean print_stderr: Pass stderr through to sys.stderr. :param callback_stdout: Optional function which is called for each @@ -416,9 +471,13 @@ extension. :param string working_directory: If specified, run the executable from within this working directory. - :return: A ReturnObject() containing the executed command, stdout and stderr - (both as bytestrings), and the exitcode. Further values such as - process runtime can be accessed as dictionary values. + :param boolean raise_timeout_exception: Forward compatibility flag. If set + then a subprocess.TimeoutExpired exception is raised + instead of returning an object that can be checked + for a timeout condition. Defaults to False, will be + changed to True in a future release. + :return: The exit code, stdout, stderr (separately, as byte strings) + as a subprocess.CompletedProcess object. """ time_start = time.strftime("%Y-%m-%d %H:%M:%S GMT", time.gmtime()) @@ -429,10 +488,20 @@ else: assert sys.platform != "win32", "stdin argument not supported on Windows" stdin_pipe = subprocess.PIPE + if debug is not None: + warnings.warn( + "Use of the debug parameter is deprecated", DeprecationWarning, stacklevel=3 + ) start_time = timeit.default_timer() if timeout is not None: max_time = start_time + timeout + if not raise_timeout_exception: + warnings.warn( + "Using procrunner with timeout and without raise_timeout_exception set is deprecated", + DeprecationWarning, + stacklevel=3, + ) if environment is not None: env = {key: _path_resolve(environment[key]) for key in environment} @@ -449,11 +518,13 @@ command = tuple(_path_resolve(part) for part in command) if win32resolve and sys.platform == "win32": command = _windows_resolve(command) + if working_directory and sys.version_info < (3, 7): + working_directory = os.fspath(working_directory) p = subprocess.Popen( command, shell=False, - cwd=_path_resolve(working_directory), + cwd=working_directory, env=env, stdin=stdin_pipe, stdout=subprocess.PIPE, @@ -492,7 +563,7 @@ (timeout is None) or (timeit.default_timer() < max_time) ): if debug and timeout is not None: - logger.debug("still running (T%.2fs)" % (timeit.default_timer() - max_time)) + logger.debug("still running (T%.2fs)", timeit.default_timer() - max_time) # wait for some time or until a stream is closed try: @@ -501,10 +572,10 @@ # which could indicate that the process has terminated. try: event = thread_pipe_pool[0].poll(0.5) - except IOError as e: - # on Windows this raises "IOError: [Errno 109] The pipe has been ended" + except BrokenPipeError as e: + # on Windows this raises "BrokenPipeError: [Errno 109] The pipe has been ended" # which is for all intents and purposes equivalent to a True return value. - if e.errno != 109: + if e.winerror != 109: raise event = True if event: @@ -526,12 +597,19 @@ # timeout condition timeout_encountered = True if debug: - logger.debug("timeout (T%.2fs)" % (timeit.default_timer() - max_time)) + logger.debug("timeout (T%.2fs)", timeit.default_timer() - max_time) # send terminate signal and wait some time for buffers to be read p.terminate() if thread_pipe_pool: - thread_pipe_pool[0].poll(0.5) + try: + thread_pipe_pool[0].poll(0.5) + except BrokenPipeError as e: + # on Windows this raises "BrokenPipeError: [Errno 109] The pipe has been ended" + # which is for all intents and purposes equivalent to a True return value. + if e.winerror != 109: + raise + thread_pipe_pool.pop(0) if not stdout.has_finished() or not stderr.has_finished(): time.sleep(2) p.poll() @@ -541,7 +619,14 @@ # send kill signal and wait some more time for buffers to be read p.kill() if thread_pipe_pool: - thread_pipe_pool[0].poll(0.5) + try: + thread_pipe_pool[0].poll(0.5) + except BrokenPipeError as e: + # on Windows this raises "BrokenPipeError: [Errno 109] The pipe has been ended" + # which is for all intents and purposes equivalent to a True return value. + if e.winerror != 109: + raise + thread_pipe_pool.pop(0) if not stdout.has_finished() or not stderr.has_finished(): time.sleep(5) p.poll() @@ -552,30 +637,34 @@ runtime = timeit.default_timer() - start_time if timeout is not None: logger.debug( - "Process ended after %.1f seconds with exit code %d (T%.2fs)" - % (runtime, p.returncode, timeit.default_timer() - max_time) + "Process ended after %.1f seconds with exit code %d (T%.2fs)", + runtime, + p.returncode, + timeit.default_timer() - max_time, ) else: logger.debug( - "Process ended after %.1f seconds with exit code %d" - % (runtime, p.returncode) + "Process ended after %.1f seconds with exit code %d", runtime, p.returncode ) stdout = stdout.get_output() stderr = stderr.get_output() + + if timeout_encountered and raise_timeout_exception: + raise subprocess.TimeoutExpired( + cmd=command, timeout=timeout, output=stdout, stderr=stderr + ) + time_end = time.strftime("%Y-%m-%d %H:%M:%S GMT", time.gmtime()) - result = ReturnObject( - { - "exitcode": p.returncode, - "command": command, - "stdout": stdout, - "stderr": stderr, - "timeout": timeout_encountered, - "runtime": runtime, - "time_start": time_start, - "time_end": time_end, - } + exitcode=p.returncode, + command=command, + stdout=stdout, + stderr=stderr, + timeout=timeout_encountered, + runtime=runtime, + time_start=time_start, + time_end=time_end, ) if stdin is not None: result.update( @@ -586,44 +675,3 @@ ) return result - - -def run_process_dummy(command, **kwargs): - """ - A stand-in function that returns a valid result dictionary indicating a - successful execution. The external process is not run. - """ - warnings.warn( - "procrunner.run_process_dummy() is deprecated", DeprecationWarning, stacklevel=2 - ) - - time_start = time.strftime("%Y-%m-%d %H:%M:%S GMT", time.gmtime()) - logger.info("run_process is disabled. Requested command: %s", command) - - result = ReturnObject( - { - "exitcode": 0, - "command": command, - "stdout": "", - "stderr": "", - "timeout": False, - "runtime": 0, - "time_start": time_start, - "time_end": time_start, - } - ) - if kwargs.get("stdin") is not None: - result.update( - {"stdin_bytes_sent": len(kwargs["stdin"]), "stdin_bytes_remain": 0} - ) - return result - - -def run_process(*args, **kwargs): - """API used up to version 0.2.0.""" - warnings.warn( - "procrunner.run_process() is deprecated and has been renamed to run()", - DeprecationWarning, - stacklevel=2, - ) - return run(*args, **kwargs) diff --git a/pytest.ini b/pytest.ini new file mode 100644 index 0000000..f35fcaa --- /dev/null +++ b/pytest.ini @@ -0,0 +1,3 @@ +[pytest] +addopts = -ra +junit_family=xunit2 diff --git a/requirements.txt b/requirements.txt new file mode 100644 index 0000000..e69de29 diff --git a/requirements_dev.txt b/requirements_dev.txt index 36a6442..2ec5400 100644 --- a/requirements_dev.txt +++ b/requirements_dev.txt @@ -1,13 +1,9 @@ -bump2version==0.5.10 -coverage==4.5.4 -flake8==3.7.8 -mock==3.0.5 -pip==19.1.1 -pytest==4.5.0 # pyup: <5.0 # for Python 2.7 support -pytest-runner==5.1 -six==1.12.0 -Sphinx==1.8.5 # pyup: <2.0 # for Python 2.7 support -tox==3.13.1 -twine==1.13.0 -watchdog==0.9.0 -wheel==0.33.4 +bump2version==1.0.1 +coverage==5.3 +flake8==3.8.4 +pip==20.2.4 +pytest==6.1.2 +Sphinx==3.2.1 +tox==3.20.1 +twine==1.15.0 +wheel==0.35.1 diff --git a/setup.cfg b/setup.cfg index 38ee3f7..e9b819e 100644 --- a/setup.cfg +++ b/setup.cfg @@ -1,18 +1,8 @@ -[bumpversion] -current_version = 1.1.0 -commit = True -tag = True - -[bumpversion:file:setup.py] -search = version="{current_version}" -replace = version="{new_version}" - -[bumpversion:file:procrunner/__init__.py] -search = __version__ = "{current_version}" -replace = __version__ = "{new_version}" - -[bdist_wheel] -universal = 1 +[metadata] +project-urls = + Documentation = https://procrunner.readthedocs.io/ + GitHub = https://github.com/DiamondLightSource/python-procrunner + Bug-Tracker = https://github.com/DiamondLightSource/python-procrunner/issues [flake8] exclude = docs diff --git a/setup.py b/setup.py index 29a129a..63ed5c3 100644 --- a/setup.py +++ b/setup.py @@ -1,7 +1,6 @@ #!/usr/bin/env python # -*- coding: utf-8 -*- -import sys from setuptools import setup, find_packages with open("README.rst") as readme_file: @@ -10,17 +9,11 @@ with open("HISTORY.rst") as history_file: history = history_file.read() -requirements = [ - "six", - 'pywin32; sys_platform=="win32"', -] +requirements = [] setup_requirements = [] -needs_pytest = {"pytest", "test", "ptr"}.intersection(sys.argv) -if needs_pytest: - setup_requirements.append("pytest-runner") -test_requirements = ["mock", "pytest"] +test_requirements = ["pytest"] setup( author="Markus Gerstel", @@ -31,15 +24,11 @@ "License :: OSI Approved :: BSD License", "Natural Language :: English", "Operating System :: OS Independent", - "Programming Language :: Python :: 2", - "Programming Language :: Python :: 2.7", "Programming Language :: Python :: 3", - "Programming Language :: Python :: 3.5", "Programming Language :: Python :: 3.6", "Programming Language :: Python :: 3.7", "Programming Language :: Python :: 3.8", - "Programming Language :: Python :: Implementation :: PyPy", - "Programming Language :: Python :: Implementation :: CPython", + "Programming Language :: Python :: 3.9", "Topic :: Software Development :: Libraries :: Python Modules", ], description="Versatile utility function to run external processes", @@ -50,10 +39,11 @@ keywords="procrunner", name="procrunner", packages=find_packages(include=["procrunner"]), + python_requires=">=3.6", setup_requires=setup_requirements, test_suite="tests", tests_require=test_requirements, url="https://github.com/DiamondLightSource/python-procrunner", - version="1.1.0", + version="2.3.0", zip_safe=False, ) diff --git a/tests/test_procrunner.py b/tests/test_procrunner.py index c645968..8acec7c 100644 --- a/tests/test_procrunner.py +++ b/tests/test_procrunner.py @@ -1,11 +1,32 @@ -from __future__ import absolute_import, division, print_function - import copy -import mock +from unittest import mock import os +import pathlib import procrunner import pytest import sys + + +@mock.patch("procrunner._NonBlockingStreamReader") +@mock.patch("procrunner.time") +@mock.patch("procrunner.subprocess") +@mock.patch("procrunner.Pipe") +def test_run_command_aborts_after_timeout_legacy( + mock_pipe, mock_subprocess, mock_time, mock_streamreader +): + mock_pipe.return_value = mock.Mock(), mock.Mock() + mock_process = mock.Mock() + mock_process.returncode = None + mock_subprocess.Popen.return_value = mock_process + task = ["___"] + + with pytest.raises(RuntimeError): + with pytest.warns(DeprecationWarning, match="timeout"): + procrunner.run(task, timeout=-1, debug=False) + + assert mock_subprocess.Popen.called + assert mock_process.terminate.called + assert mock_process.kill.called @mock.patch("procrunner._NonBlockingStreamReader") @@ -22,7 +43,7 @@ task = ["___"] with pytest.raises(RuntimeError): - procrunner.run(task, -1, False) + procrunner.run(task, timeout=-1, raise_timeout_exception=True) assert mock_subprocess.Popen.called assert mock_process.terminate.called @@ -63,29 +84,32 @@ actual = procrunner.run( command, - 0.5, - False, + timeout=0.5, callback_stdout=mock.sentinel.callback_stdout, callback_stderr=mock.sentinel.callback_stderr, - working_directory=mock.sentinel.cwd, + working_directory=pathlib.Path("somecwd"), + raise_timeout_exception=True, ) assert mock_subprocess.Popen.called assert mock_subprocess.Popen.call_args[1]["env"] == os.environ - assert mock_subprocess.Popen.call_args[1]["cwd"] == mock.sentinel.cwd + assert mock_subprocess.Popen.call_args[1]["cwd"] in ( + pathlib.Path("somecwd"), + "somecwd", + ) mock_streamreader.assert_has_calls( [ mock.call( stream_stdout, output=mock.ANY, - debug=mock.ANY, + debug=None, notify=mock.ANY, callback=mock.sentinel.callback_stdout, ), mock.call( stream_stderr, output=mock.ANY, - debug=mock.ANY, + debug=None, notify=mock.ANY, callback=mock.sentinel.callback_stderr, ), @@ -95,7 +119,8 @@ assert not mock_process.terminate.called assert not mock_process.kill.called for key in expected: - assert actual[key] == expected[key] + with pytest.warns(DeprecationWarning): + assert actual[key] == expected[key] assert actual.args == tuple(command) assert actual.returncode == mock_process.returncode assert actual.stdout == mock.sentinel.proc_stdout @@ -106,8 +131,19 @@ def test_default_process_environment_is_parent_environment(mock_subprocess): mock_subprocess.Popen.side_effect = NotImplementedError() # cut calls short with pytest.raises(NotImplementedError): - procrunner.run([mock.Mock()], -1, False) + procrunner.run([mock.Mock()], timeout=-1, raise_timeout_exception=True) assert mock_subprocess.Popen.call_args[1]["env"] == os.environ + + +@mock.patch("procrunner.subprocess") +def test_using_debug_parameter_raises_warning(mock_subprocess): + mock_subprocess.Popen.side_effect = NotImplementedError() # cut calls short + with pytest.warns(DeprecationWarning, match="debug"): + with pytest.raises(NotImplementedError): + procrunner.run([mock.Mock()], debug=True) + with pytest.warns(DeprecationWarning, match="debug"): + with pytest.raises(NotImplementedError): + procrunner.run([mock.Mock()], debug=False) @mock.patch("procrunner.subprocess") @@ -116,7 +152,12 @@ mock_env = {"key": mock.sentinel.key} # Pass an environment dictionary with pytest.raises(NotImplementedError): - procrunner.run([mock.Mock()], -1, False, environment=copy.copy(mock_env)) + procrunner.run( + [mock.Mock()], + timeout=-1, + environment=copy.copy(mock_env), + raise_timeout_exception=True, + ) assert mock_subprocess.Popen.call_args[1]["env"] == mock_env @@ -129,10 +170,10 @@ with pytest.raises(NotImplementedError): procrunner.run( [mock.Mock()], - -1, - False, + timeout=-1, environment=copy.copy(mock_env1), environment_override=copy.copy(mock_env2), + raise_timeout_exception=True, ) mock_env_sum = copy.copy(mock_env1) mock_env_sum.update({key: str(mock_env2[key]) for key in mock_env2}) @@ -145,12 +186,14 @@ mock_env2 = {"keyB": str(mock.sentinel.keyB)} with pytest.raises(NotImplementedError): procrunner.run( - [mock.Mock()], -1, False, environment_override=copy.copy(mock_env2) + [mock.Mock()], + timeout=-1, + environment_override=copy.copy(mock_env2), + raise_timeout_exception=True, ) random_environment_variable = list(os.environ)[0] if random_environment_variable == list(mock_env2)[0]: random_environment_variable = list(os.environ)[1] - random_environment_value = os.getenv(random_environment_variable) assert ( random_environment_variable and random_environment_variable != list(mock_env2)[0] @@ -172,11 +215,11 @@ with pytest.raises(NotImplementedError): procrunner.run( [mock.Mock()], - -1, - False, + timeout=-1, environment_override={ random_environment_variable: "X" + random_environment_value }, + raise_timeout_exception=True, ) assert ( mock_subprocess.Popen.call_args[1]["env"][random_environment_variable] @@ -192,7 +235,7 @@ def test_nonblockingstreamreader_can_read(mock_select): import time - class _stream(object): + class _stream: def __init__(self): self.data = b"" self.closed = False @@ -263,48 +306,48 @@ def test_return_object_semantics(): ro = procrunner.ReturnObject( - { - "command": mock.sentinel.command, - "exitcode": 0, - "stdout": mock.sentinel.stdout, - "stderr": mock.sentinel.stderr, - } - ) - assert ro["command"] == mock.sentinel.command + command=mock.sentinel.command, + exitcode=0, + stdout=mock.sentinel.stdout, + stderr=mock.sentinel.stderr, + ) + with pytest.warns(DeprecationWarning): + assert ro["command"] == mock.sentinel.command assert ro.args == mock.sentinel.command - assert ro["exitcode"] == 0 + with pytest.warns(DeprecationWarning): + assert ro["exitcode"] == 0 assert ro.returncode == 0 - assert ro["stdout"] == mock.sentinel.stdout + with pytest.warns(DeprecationWarning): + assert ro["stdout"] == mock.sentinel.stdout assert ro.stdout == mock.sentinel.stdout - assert ro["stderr"] == mock.sentinel.stderr + with pytest.warns(DeprecationWarning): + assert ro["stderr"] == mock.sentinel.stderr assert ro.stderr == mock.sentinel.stderr with pytest.raises(KeyError): - ro["unknownkey"] + with pytest.warns(DeprecationWarning): + ro["unknownkey"] ro.update({"unknownkey": mock.sentinel.key}) - assert ro["unknownkey"] == mock.sentinel.key + with pytest.warns(DeprecationWarning): + assert ro["unknownkey"] == mock.sentinel.key def test_return_object_check_function_passes_on_success(): ro = procrunner.ReturnObject( - { - "command": mock.sentinel.command, - "exitcode": 0, - "stdout": mock.sentinel.stdout, - "stderr": mock.sentinel.stderr, - } + command=mock.sentinel.command, + exitcode=0, + stdout=mock.sentinel.stdout, + stderr=mock.sentinel.stderr, ) ro.check_returncode() def test_return_object_check_function_raises_on_error(): ro = procrunner.ReturnObject( - { - "command": mock.sentinel.command, - "exitcode": 1, - "stdout": mock.sentinel.stdout, - "stderr": mock.sentinel.stderr, - } + command=mock.sentinel.command, + exitcode=1, + stdout=mock.sentinel.stdout, + stderr=mock.sentinel.stderr, ) with pytest.raises(Exception) as e: ro.check_returncode() diff --git a/tests/test_procrunner_resolution.py b/tests/test_procrunner_resolution.py index 33738f8..e099be4 100644 --- a/tests/test_procrunner_resolution.py +++ b/tests/test_procrunner_resolution.py @@ -1,9 +1,6 @@ -from __future__ import absolute_import, division, print_function - import os import sys -import mock import procrunner import pytest @@ -47,11 +44,6 @@ @pytest.mark.skipif(sys.platform != "win32", reason="windows specific test only") -def test_pywin32_import(): - import win32api - - -@pytest.mark.skipif(sys.platform != "win32", reason="windows specific test only") def test_name_resolution_for_simple_exe(): command = ["cmd.exe", "/c", "echo", "hello"] @@ -66,27 +58,25 @@ @pytest.mark.skipif(sys.platform != "win32", reason="windows specific test only") -def test_name_resolution_for_complex_cases(tmpdir): - tmpdir.chdir() - +def test_name_resolution_for_complex_cases(tmp_path): bat = "simple_bat_extension" cmd = "simple_cmd_extension" exe = "simple_exe_extension" dotshort = "more_complex_filename_with_a.dot" dotlong = "more_complex_filename.withadot" - (tmpdir / bat + ".bat").ensure() - (tmpdir / cmd + ".cmd").ensure() - (tmpdir / exe + ".exe").ensure() - (tmpdir / dotshort + ".bat").ensure() - (tmpdir / dotlong + ".cmd").ensure() + (tmp_path / (bat + ".bat")).touch() + (tmp_path / (cmd + ".cmd")).touch() + (tmp_path / (exe + ".exe")).touch() + (tmp_path / (dotshort + ".bat")).touch() + (tmp_path / (dotlong + ".cmd")).touch() def is_valid(command): assert len(command) == 1 - assert os.path.exists(command[0]) + assert os.path.exists(tmp_path / command[0]) - is_valid(procrunner._windows_resolve([bat])) - is_valid(procrunner._windows_resolve([cmd])) - is_valid(procrunner._windows_resolve([exe])) - is_valid(procrunner._windows_resolve([dotshort])) - is_valid(procrunner._windows_resolve([dotlong])) + is_valid(procrunner._windows_resolve([bat], path=os.fspath(tmp_path))) + is_valid(procrunner._windows_resolve([cmd], path=os.fspath(tmp_path))) + is_valid(procrunner._windows_resolve([exe], path=os.fspath(tmp_path))) + is_valid(procrunner._windows_resolve([dotshort], path=os.fspath(tmp_path))) + is_valid(procrunner._windows_resolve([dotlong], path=os.fspath(tmp_path))) diff --git a/tests/test_procrunner_system.py b/tests/test_procrunner_system.py index 5955463..0dada84 100644 --- a/tests/test_procrunner_system.py +++ b/tests/test_procrunner_system.py @@ -1,7 +1,7 @@ -from __future__ import absolute_import, division, print_function - import os +import subprocess import sys +import timeit import procrunner import pytest @@ -36,15 +36,14 @@ else: assert result.stdout == test_string out, err = capsys.readouterr() - assert out == u"test\ufffdstring\n" - assert err == u"" + assert out == "test\ufffdstring\n" + assert err == "" -def test_running_wget(tmpdir): - tmpdir.chdir() +def test_running_wget(tmp_path): command = ["wget", "https://www.google.com", "-O", "-"] try: - result = procrunner.run(command) + result = procrunner.run(command, working_directory=tmp_path) except OSError as e: if e.errno == 2: pytest.skip("wget not available") @@ -54,15 +53,17 @@ assert b"google" in result.stdout -def test_path_object_resolution(tmpdir): +def test_path_object_resolution(tmp_path): sentinel_value = b"sentinel" - tmpdir.join("tempfile").write(sentinel_value) - tmpdir.join("reader.py").write("print(open('tempfile').read())") + tmp_path.joinpath("tempfile").write_bytes(sentinel_value) + tmp_path.joinpath("reader.py").write_text( + "with open('tempfile') as fh:\n print(fh.read())" + ) assert "LEAK_DETECTOR" not in os.environ result = procrunner.run( - [sys.executable, tmpdir.join("reader.py")], + [sys.executable, tmp_path / "reader.py"], environment_override={"PYTHONHASHSEED": "random", "LEAK_DETECTOR": "1"}, - working_directory=tmpdir, + working_directory=tmp_path, ) assert result.returncode == 0 assert not result.stderr @@ -70,3 +71,62 @@ assert ( "LEAK_DETECTOR" not in os.environ ), "overridden environment variable leaked into parent process" + + +def test_timeout_behaviour_legacy(tmp_path): + start = timeit.default_timer() + try: + with pytest.warns(DeprecationWarning, match="timeout"): + result = procrunner.run( + [sys.executable, "-c", "import time; time.sleep(5)"], + timeout=0.1, + working_directory=tmp_path, + raise_timeout_exception=False, + ) + except RuntimeError: + # This test sometimes fails with a RuntimeError. + runtime = timeit.default_timer() - start + assert runtime < 3 + return + runtime = timeit.default_timer() - start + with pytest.warns(DeprecationWarning, match="\\.timeout"): + assert result.timeout + assert runtime < 3 + assert not result.stdout + assert not result.stderr + assert result.returncode + + +def test_timeout_behaviour(tmp_path): + command = (sys.executable, "-c", "import time; time.sleep(5)") + start = timeit.default_timer() + try: + with pytest.raises(subprocess.TimeoutExpired) as te: + procrunner.run( + command, + timeout=0.1, + working_directory=tmp_path, + raise_timeout_exception=True, + ) + except RuntimeError: + # This test sometimes fails with a RuntimeError. + runtime = timeit.default_timer() - start + assert runtime < 3 + return + runtime = timeit.default_timer() - start + assert runtime < 3 + assert te.value.stdout == b"" + assert te.value.stderr == b"" + assert te.value.timeout == 0.1 + assert te.value.cmd == command + + +def test_argument_deprecation(tmp_path): + with pytest.warns(DeprecationWarning, match="keyword arguments"): + result = procrunner.run( + [sys.executable, "-V"], + None, + working_directory=tmp_path, + ) + assert not result.returncode + assert result.stderr or result.stdout diff --git a/tox.ini b/tox.ini index e6887db..3dcc731 100644 --- a/tox.ini +++ b/tox.ini @@ -1,13 +1,22 @@ [tox] -envlist = py27, py35, py36, py37, py38, flake8 +envlist = py36, py37, py38, flake8 [travis] python = 3.8: py38 3.7: py37 3.6: py36 - 3.5: py35 - 2.7: py27 + +[testenv:azure] +basepython = python +deps = + pytest-azurepipelines + pytest-cov + -r{toxinidir}/requirements_dev.txt +setenv = + PYTHONDEVMODE = 1 +commands = + pytest -ra --basetemp={envtmpdir} --cov=procrunner --cov-report=html --cov-report=xml --cov-branch [testenv:flake8] basepython = python