[tor-commits] [stem/master] Add timeout argument to stem.util.system.call()
atagar at torproject.org
atagar at torproject.org
Sun Feb 12 05:52:41 UTC 2017
commit f3785a139eaecf4ff896315e92b89588ae3e2c9c
Author: Damian Johnson <atagar at torproject.org>
Date: Thu Feb 9 10:35:47 2017 -0800
Add timeout argument to stem.util.system.call()
While making our installer integ test more reliable I've had it take over a
minute or even fail to finish at all several times. Allowing timeouts of our
system calls is generally useful anyway so good excuse to make this addition.
---
docs/change_log.rst | 1 +
run_tests.py | 9 +++++++-
stem/util/system.py | 40 +++++++++++++++++++++++++++++++++---
test/integ/installation.py | 51 +++++++++++++++++++++++-----------------------
test/integ/util/system.py | 7 +++++++
test/unit/manual.py | 1 +
6 files changed, 80 insertions(+), 29 deletions(-)
diff --git a/docs/change_log.rst b/docs/change_log.rst
index 85619b2..338150c 100644
--- a/docs/change_log.rst
+++ b/docs/change_log.rst
@@ -55,6 +55,7 @@ The following are only available within Stem's `git repository
* **Utilities**
+ * Added timeout argument to :func:`~stem.util.system.call`
* Added :class:`~stem.util.test_tools.TimedTestRunner` and :func:`~stem.util.test_tools.test_runtimes`
.. _version_1.5:
diff --git a/run_tests.py b/run_tests.py
index 3db6280..501cbfa 100755
--- a/run_tests.py
+++ b/run_tests.py
@@ -213,9 +213,10 @@ def main():
our_version = stem.version.get_system_tor_version(args.tor_path)
skipped_targets = []
+ integ_setup_thread = None
if not args.specific_test or 'test.integ.installation'.startswith(args.specific_test):
- test.integ.installation.setup()
+ integ_setup_thread = test.integ.installation.setup()
for target in args.run_targets:
# check if we meet this target's tor version prerequisites
@@ -247,6 +248,9 @@ def main():
# We should have joined on all threads. If not then that indicates a
# leak that could both likely be a bug and disrupt further targets.
+ if integ_setup_thread:
+ integ_setup_thread.join()
+
active_threads = threading.enumerate()
if len(active_threads) > 1:
@@ -267,6 +271,9 @@ def main():
except OSError:
error_tracker.register_error()
finally:
+ if integ_setup_thread:
+ test.integ.installation.clean()
+
println()
integ_runner.stop()
println()
diff --git a/stem/util/system.py b/stem/util/system.py
index c1db6d0..4951058 100644
--- a/stem/util/system.py
+++ b/stem/util/system.py
@@ -166,6 +166,20 @@ class CallError(OSError):
return self.msg
+class CallTimeoutError(CallError):
+ """
+ Error response when making a system call that has timed out.
+
+ .. versionadded:: 1.6.0
+
+ :var float timeout: time we waited
+ """
+
+ def __init__(self, msg, command, exit_status, runtime, stdout, stderr, timeout):
+ super(CallTimeoutError, self).__init__(msg, command, exit_status, runtime, stdout, stderr)
+ self.timeout = timeout
+
+
def is_windows():
"""
Checks if we are running on Windows.
@@ -1020,7 +1034,7 @@ def files_with_suffix(base_path, suffix):
yield os.path.join(root, filename)
-def call(command, default = UNDEFINED, ignore_exit_status = False, env = None):
+def call(command, default = UNDEFINED, ignore_exit_status = False, timeout = None, env = None):
"""
call(command, default = UNDEFINED, ignore_exit_status = False)
@@ -1035,16 +1049,22 @@ def call(command, default = UNDEFINED, ignore_exit_status = False, env = None):
.. versionchanged:: 1.5.0
Added env argument.
+ .. versionchanged:: 1.6.0
+ Added timeout argument.
+
:param str,list command: command to be issued
:param object default: response if the query fails
:param bool ignore_exit_status: reports failure if our command's exit status
was non-zero
+ :param float timeout: maximum seconds to wait, blocks indefinitely if
+ **None**
:param dict env: environment variables
:returns: **list** with the lines of output from the command
- :raises: **CallError** if this fails and no default was provided, this is an
- **OSError** subclass
+ :raises:
+ * **CallError** if this fails and no default was provided
+ * **CallTimeoutError** if the timeout is reached without a default
"""
global SYSTEM_CALL_TIME
@@ -1062,6 +1082,13 @@ def call(command, default = UNDEFINED, ignore_exit_status = False, env = None):
process = subprocess.Popen(command_list, stdout = subprocess.PIPE, stderr = subprocess.PIPE, shell = is_shell_command, env = env)
+ if timeout:
+ while process.poll() is None:
+ if time.time() - start_time > timeout:
+ raise CallTimeoutError("Process didn't finish after %0.1f seconds" % timeout, ' '.join(command_list), None, timeout, '', '', timeout)
+
+ time.sleep(0.001)
+
stdout, stderr = process.communicate()
stdout, stderr = stdout.strip(), stderr.strip()
runtime = time.time() - start_time
@@ -1085,6 +1112,13 @@ def call(command, default = UNDEFINED, ignore_exit_status = False, env = None):
return stdout.decode('utf-8', 'replace').splitlines()
else:
return []
+ except CallTimeoutError as exc:
+ log.debug('System call (timeout): %s (after %0.4fs)' % (command, timeout))
+
+ if default != UNDEFINED:
+ return default
+ else:
+ raise
except OSError as exc:
log.debug('System call (failed): %s (error: %s)' % (command, exc))
diff --git a/test/integ/installation.py b/test/integ/installation.py
index cfe771a..2048ea3 100644
--- a/test/integ/installation.py
+++ b/test/integ/installation.py
@@ -15,6 +15,7 @@ import test.util
INSTALL_MISMATCH_MSG = "Running 'python setup.py sdist' doesn't match our git contents in the following way. The manifest in our setup.py may need to be updated...\n\n"
BASE_INSTALL_PATH = '/tmp/stem_test'
+DIST_PATH = os.path.join(test.util.STEM_BASE, 'dist')
SETUP_THREAD, INSTALL_FAILURE, INSTALL_PATH, SDIST_FAILURE = None, None, None, None
@@ -35,8 +36,8 @@ def setup():
try:
os.chdir(test.util.STEM_BASE)
- stem.util.system.call('%s setup.py install --prefix %s' % (sys.executable, BASE_INSTALL_PATH))
- stem.util.system.call('%s setup.py clean --all' % sys.executable) # tidy up the build directory
+ stem.util.system.call('%s setup.py install --prefix %s' % (sys.executable, BASE_INSTALL_PATH), timeout = 60)
+ stem.util.system.call('%s setup.py clean --all' % sys.executable, timeout = 60) # tidy up the build directory
site_packages_paths = glob.glob('%s/lib*/*/site-packages' % BASE_INSTALL_PATH)
if len(site_packages_paths) != 1:
@@ -46,10 +47,13 @@ def setup():
except Exception as exc:
INSTALL_FAILURE = AssertionError("Unable to install with 'python setup.py install': %s" % exc)
- try:
- stem.util.system.call('%s setup.py sdist' % sys.executable)
- except Exception as exc:
- SDIST_FAILURE = exc
+ if not os.path.exists(DIST_PATH):
+ try:
+ stem.util.system.call('%s setup.py sdist' % sys.executable, timeout = 60)
+ except Exception as exc:
+ SDIST_FAILURE = exc
+ else:
+ SDIST_FAILURE = AssertionError("%s already exists, maybe you manually ran 'python setup.py sdist'?" % DIST_PATH)
finally:
os.chdir(original_cwd)
@@ -60,6 +64,14 @@ def setup():
return SETUP_THREAD
+def clean():
+ if os.path.exists(BASE_INSTALL_PATH):
+ shutil.rmtree(BASE_INSTALL_PATH)
+
+ if os.path.exists(DIST_PATH):
+ shutil.rmtree(DIST_PATH)
+
+
def _assert_has_all_files(path):
"""
Check that all the files in the stem directory are present in the
@@ -100,17 +112,14 @@ class TestInstallation(unittest.TestCase):
install.
"""
- try:
+ if not INSTALL_PATH:
setup().join()
- if INSTALL_FAILURE:
- raise INSTALL_FAILURE
+ if INSTALL_FAILURE:
+ raise INSTALL_FAILURE
- self.assertEqual(stem.__version__, stem.util.system.call([sys.executable, '-c', "import sys;sys.path.insert(0, '%s');import stem;print(stem.__version__)" % INSTALL_PATH])[0])
- _assert_has_all_files(INSTALL_PATH)
- finally:
- if os.path.exists(BASE_INSTALL_PATH):
- shutil.rmtree(BASE_INSTALL_PATH)
+ self.assertEqual(stem.__version__, stem.util.system.call([sys.executable, '-c', "import sys;sys.path.insert(0, '%s');import stem;print(stem.__version__)" % INSTALL_PATH])[0])
+ _assert_has_all_files(INSTALL_PATH)
@test.runner.only_run_once
def test_sdist(self):
@@ -129,20 +138,12 @@ class TestInstallation(unittest.TestCase):
if SDIST_FAILURE:
raise SDIST_FAILURE
- original_cwd = os.getcwd()
- dist_path = os.path.join(test.util.STEM_BASE, 'dist')
git_contents = [line.split()[-1] for line in stem.util.system.call('git ls-tree --full-tree -r HEAD')]
- try:
- # tarball has a prefix 'stem-[verion]' directory so stipping that out
-
- dist_tar = tarfile.open(os.path.join(dist_path, 'stem-dry-run-%s.tar.gz' % stem.__version__))
- tar_contents = ['/'.join(info.name.split('/')[1:]) for info in dist_tar.getmembers() if info.isfile()]
- finally:
- if os.path.exists(dist_path):
- shutil.rmtree(dist_path)
+ # tarball has a prefix 'stem-[verion]' directory so stipping that out
- os.chdir(original_cwd)
+ dist_tar = tarfile.open(os.path.join(DIST_PATH, 'stem-dry-run-%s.tar.gz' % stem.__version__))
+ tar_contents = ['/'.join(info.name.split('/')[1:]) for info in dist_tar.getmembers() if info.isfile()]
issues = []
diff --git a/test/integ/util/system.py b/test/integ/util/system.py
index dd301f9..b6e66a0 100644
--- a/test/integ/util/system.py
+++ b/test/integ/util/system.py
@@ -563,6 +563,13 @@ class TestSystem(unittest.TestCase):
self.assertEqual(home_dir, stem.util.system.expand_path('~%s' % username))
self.assertEqual(os.path.join(home_dir, 'foo'), stem.util.system.expand_path('~%s/foo' % username))
+ def test_call_timeout(self):
+ try:
+ stem.util.system.call('sleep 1', timeout = 0.001)
+ self.fail("sleep should've timed out")
+ except stem.util.system.CallTimeoutError as exc:
+ self.assertEqual("Process didn't finish after 0.0 seconds", str(exc))
+
def test_call_time_tracked(self):
"""
Check that time taken in the call() function is tracked by SYSTEM_CALL_TIME.
diff --git a/test/unit/manual.py b/test/unit/manual.py
index 8338af7..5f939c3 100644
--- a/test/unit/manual.py
+++ b/test/unit/manual.py
@@ -105,6 +105,7 @@ EXPECTED_CONFIG_OPTIONS['MaxAdvertisedBandwidth'] = stem.manual.ConfigOption(
CACHED_MANUAL = None
+
def _cached_manual():
global CACHED_MANUAL
More information about the tor-commits
mailing list