diff --git a/procrunner/__init__.py b/procrunner/__init__.py index 8940ea7..60f24f4 100644 --- a/procrunner/__init__.py +++ b/procrunner/__init__.py @@ -8,6 +8,7 @@ import sys import time import timeit +import warnings from multiprocessing import Pipe from threading import Thread @@ -346,6 +347,7 @@ environment_override=None, win32resolve=True, working_directory=None, + raise_timeout_exception=False, ): """ Run an external process. @@ -372,6 +374,11 @@ extension. :param string working_directory: If specified, run the executable from within this working directory. + :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: 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. @@ -389,6 +396,12 @@ 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=2, + ) if environment is not None: env = {key: _path_resolve(environment[key]) for key in environment} @@ -519,8 +532,13 @@ 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, diff --git a/tests/test_procrunner.py b/tests/test_procrunner.py index b174d1d..9221a56 100644 --- a/tests/test_procrunner.py +++ b/tests/test_procrunner.py @@ -4,6 +4,28 @@ 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, -1, False) + + assert mock_subprocess.Popen.called + assert mock_process.terminate.called + assert mock_process.kill.called @mock.patch("procrunner._NonBlockingStreamReader") @@ -20,7 +42,7 @@ task = ["___"] with pytest.raises(RuntimeError): - procrunner.run(task, -1, False) + procrunner.run(task, -1, False, raise_timeout_exception=True) assert mock_subprocess.Popen.called assert mock_process.terminate.called @@ -66,6 +88,7 @@ callback_stdout=mock.sentinel.callback_stdout, callback_stderr=mock.sentinel.callback_stderr, working_directory=mock.sentinel.cwd, + raise_timeout_exception=True, ) assert mock_subprocess.Popen.called @@ -104,7 +127,7 @@ 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()], -1, False, raise_timeout_exception=True) assert mock_subprocess.Popen.call_args[1]["env"] == os.environ @@ -114,7 +137,13 @@ 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()], + -1, + False, + environment=copy.copy(mock_env), + raise_timeout_exception=True, + ) assert mock_subprocess.Popen.call_args[1]["env"] == mock_env @@ -131,6 +160,7 @@ False, 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}) @@ -143,7 +173,11 @@ 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()], + -1, + False, + 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]: @@ -174,6 +208,7 @@ environment_override={ random_environment_variable: "X" + random_environment_value }, + raise_timeout_exception=True, ) assert ( mock_subprocess.Popen.call_args[1]["env"][random_environment_variable] diff --git a/tests/test_procrunner_system.py b/tests/test_procrunner_system.py index 7442deb..afe4501 100644 --- a/tests/test_procrunner_system.py +++ b/tests/test_procrunner_system.py @@ -1,5 +1,7 @@ import os +import subprocess import sys +import timeit import procrunner import pytest @@ -68,3 +70,42 @@ 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() + 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, + ) + runtime = timeit.default_timer() - start + if hasattr(result, "timeout"): + with pytest.warns(DeprecationWarning, match="\\.timeout"): + assert result.timeout + else: + 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() + with pytest.raises(subprocess.TimeoutExpired) as te: + procrunner.run( + command, + timeout=0.1, + working_directory=tmp_path, + raise_timeout_exception=True, + ) + 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