commit ed63439d9c0ed846ea0d526dc7fdc92569aa26c5 Author: Damian Johnson atagar@torproject.org Date: Mon Jan 20 09:32:19 2014 -0800
Only check for extra tor processes once
Well, that was unintuitive. Our test class' constructor is called once for each test, so self.is_extra_tor_running was always None for our setup block.
Moving this out into a global so we only check this once. This has a couple advantages...
1. Fewer system calls so our integ tests run faster. This amounts to about 0.5s on my system.
2. Less confusing test output. Our jenkins tests had a handful of failures that was probably due to having multiple tor processes running...
https://jenkins.torproject.org/job/stem-tor-ci/448/console
The weird thing was that some tests properly skipped with 'multiple tor instances' while others didn't and in turn failed. Now we should either always skip or always run.
One thing that has me concerned is that if a new tor instance spins up in the middle of our test run then we won't detect it. We might need to change this later to always check if we last detected only a single process. Even if we do this we'll get most of the runtime benefits since this now resides in a helper function rather than setup (so it's only triggered when relevant). --- test/integ/util/system.py | 57 ++++++++++++++++++++++++--------------------- 1 file changed, 31 insertions(+), 26 deletions(-)
diff --git a/test/integ/util/system.py b/test/integ/util/system.py index df389b1..0f37e90 100644 --- a/test/integ/util/system.py +++ b/test/integ/util/system.py @@ -19,6 +19,9 @@ except ImportError: from mock import Mock, patch
+IS_EXTRA_TOR_RUNNING = None + + def filter_system_call(prefixes): """ Provides a functor that passes calls on to the stem.util.system.call() @@ -45,26 +48,6 @@ def _has_port():
class TestSystem(unittest.TestCase): - is_extra_tor_running = None - - def setUp(self): - # Try to figure out if there's more than one tor instance running. This - # check will fail if pgrep is unavailable (for instance on bsd) but this - # isn't the end of the world. It's just used to skip tests if they should - # legitemately fail. - - if self.is_extra_tor_running is None: - if stem.util.system.is_windows(): - # TODO: not sure how to check for this on windows - self.is_extra_tor_running = False - elif not stem.util.system.is_bsd(): - pgrep_results = stem.util.system.call(stem.util.system.GET_PID_BY_NAME_PGREP % "tor") - self.is_extra_tor_running = len(pgrep_results) > 1 - else: - ps_results = stem.util.system.call(stem.util.system.GET_PID_BY_NAME_PS_BSD) - results = [r for r in ps_results if r.endswith(" tor")] - self.is_extra_tor_running = len(results) > 1 - def test_is_available(self): """ Checks the stem.util.system.is_available function. @@ -100,7 +83,7 @@ class TestSystem(unittest.TestCase): if stem.util.system.is_windows(): test.runner.skip(self, "(unavailable on windows)") return - elif self.is_extra_tor_running: + elif self._is_extra_tor_running(): test.runner.skip(self, "(multiple tor instances)") return
@@ -113,7 +96,7 @@ class TestSystem(unittest.TestCase): Tests the get_pid_by_name function with a pgrep response. """
- if self.is_extra_tor_running: + if self._is_extra_tor_running(): test.runner.skip(self, "(multiple tor instances)") return elif not stem.util.system.is_available("pgrep"): @@ -135,7 +118,7 @@ class TestSystem(unittest.TestCase): Tests the get_pid_by_name function with a pidof response. """
- if self.is_extra_tor_running: + if self._is_extra_tor_running(): test.runner.skip(self, "(multiple tor instances)") return elif not stem.util.system.is_available("pidof"): @@ -157,7 +140,7 @@ class TestSystem(unittest.TestCase): Tests the get_pid_by_name function with the linux variant of ps. """
- if self.is_extra_tor_running: + if self._is_extra_tor_running(): test.runner.skip(self, "(multiple tor instances)") return elif not stem.util.system.is_available("ps"): @@ -182,7 +165,7 @@ class TestSystem(unittest.TestCase): Tests the get_pid_by_name function with the bsd variant of ps. """
- if self.is_extra_tor_running: + if self._is_extra_tor_running(): test.runner.skip(self, "(multiple tor instances)") return elif not stem.util.system.is_available("ps"): @@ -208,7 +191,7 @@ class TestSystem(unittest.TestCase): """
runner = test.runner.get_runner() - if self.is_extra_tor_running: + if self._is_extra_tor_running(): test.runner.skip(self, "(multiple tor instances)") return elif not stem.util.system.is_available("lsof"): @@ -555,3 +538,25 @@ class TestSystem(unittest.TestCase): self.assertEqual("stem_integ", stem.util.system.get_process_name()) finally: stem.util.system.set_process_name(initial_name) + + def _is_extra_tor_running(self): + # Try to figure out if there's more than one tor instance running. This + # check will fail if pgrep is unavailable (for instance on bsd) but this + # isn't the end of the world. It's just used to skip tests if they should + # legitemately fail. + + global IS_EXTRA_TOR_RUNNING + + if IS_EXTRA_TOR_RUNNING is None: + if stem.util.system.is_windows(): + # TODO: not sure how to check for this on windows + IS_EXTRA_TOR_RUNNING = False + elif not stem.util.system.is_bsd(): + pgrep_results = stem.util.system.call(stem.util.system.GET_PID_BY_NAME_PGREP % "tor") + IS_EXTRA_TOR_RUNNING = len(pgrep_results) > 1 + else: + ps_results = stem.util.system.call(stem.util.system.GET_PID_BY_NAME_PS_BSD) + results = [r for r in ps_results if r.endswith(" tor")] + IS_EXTRA_TOR_RUNNING = len(results) > 1 + + return IS_EXTRA_TOR_RUNNING