December 2012 - tor-commits - lists.torproject.org

[stem/master] Improve tests for close_circuit exceptions
by atagar＠torproject.org 09 Dec '12

09 Dec '12

commit a8b20adb7044971935380b4170254c1c2d7f2051 Author: Sean Robinson <seankrobinson(a)gmail.com> Date: Sun Dec 9 08:37:54 2012 -0700 Improve tests for close_circuit exceptions Signed-off-by: Sean Robinson <seankrobinson(a)gmail.com> --- test/integ/control/controller.py | 3 ++- 1 files changed, 2 insertions(+), 1 deletions(-) diff --git a/test/integ/control/controller.py b/test/integ/control/controller.py index 56558ea..b2a9ffd 100644 --- a/test/integ/control/controller.py +++ b/test/integ/control/controller.py @@ -518,7 +518,8 @@ class TestController(unittest.TestCase): self.assertFalse(circ_id in circ) circ_id = controller.new_circuit() - self.assertRaises(stem.InvalidRequest, controller.close_circuit, circ_id + 1024) + self.assertRaises(stem.InvalidArguments, controller.close_circuit, circ_id + 1024) + self.assertRaises(stem.InvalidRequest, controller.close_circuit, "") def test_mapaddress(self): if test.runner.require_control(self): return

1 0

[stem/master] Raise different exceptions for different CLOSECIRCUIT errors
by atagar＠torproject.org 09 Dec '12

09 Dec '12

commit b3fe1659c0d2c7bf7c8ce282e0f858eb6350e80e Author: Sean Robinson <seankrobinson(a)gmail.com> Date: Sat Dec 8 19:57:13 2012 -0700 Raise different exceptions for different CLOSECIRCUIT errors Signed-off-by: Sean Robinson <seankrobinson(a)gmail.com> --- stem/control.py | 5 ++++- 1 files changed, 4 insertions(+), 1 deletions(-) diff --git a/stem/control.py b/stem/control.py index 8d9784e..3572987 100644 --- a/stem/control.py +++ b/stem/control.py @@ -1462,7 +1462,8 @@ class Controller(BaseController): :param str flag: optional value to modify closing, the only flag available is "IfUnused" which will not close the circuit unless it is unused - :raises: :class:`stem.InvalidArguments` if the circuit doesn't exist or not enough information is provided + :raises: :class:`stem.InvalidArguments` if the circuit is unknown + :raises: :class:`stem.InvalidRequest` if not enough information is provided """ response = self.msg("CLOSECIRCUIT %s %s"% (str(circuit_id), flag)) @@ -1470,6 +1471,8 @@ class Controller(BaseController): if not response.is_ok(): if response.code in ('512', '552'): + if response.message.startswith("Unknown circuit "): + raise stem.InvalidArguments(response.code, response.message, [circuit_id]) raise stem.InvalidRequest(response.code, response.message) else: raise stem.ProtocolError("CLOSECIRCUIT returned unexpected response code: %s" % response.code)

1 0

[stem/master] Testing event version requirement checks
by atagar＠torproject.org 09 Dec '12

09 Dec '12

commit ec13ebb581fc7b57f055c59934f5f7f41c2111bb Merge: 4cacd2f 3be3a8d Author: Damian Johnson <atagar(a)torproject.org> Date: Sun Dec 9 13:54:20 2012 -0800 Testing event version requirement checks Patches by robinson to both fix the mock_method() function and use it to test the tor version requirement checks for events. event version check: https://trac.torproject.org/7674 mock_method() fix: https://trac.torproject.org/7686 test/mocking.py | 26 +++++++++++++++++++------- test/unit/control/controller.py | 33 +++++++++++++++++++++++++++++++-- 2 files changed, 50 insertions(+), 9 deletions(-)

1 0

[stem/master] Minor revisions for the new add_event_listener tests
by atagar＠torproject.org 09 Dec '12

09 Dec '12

commit 3be3a8d94bb666b56bdc65e732f83c88ccfe0a15 Author: Damian Johnson <atagar(a)torproject.org> Date: Sun Dec 9 13:52:20 2012 -0800 Minor revisions for the new add_event_listener tests Just some minor tweaks... * Replaced 'lambda x: x' with 'mocking.no_op()'. * Dropped the 'assertIsNone()' call. The purpose of this line is to make sure that add_event_listener() doesn't raise an exception, not to make an assertion on what the function returns. * Minor import tweaks to conform with 3da47d3. --- test/unit/control/controller.py | 15 +++++++++------ 1 files changed, 9 insertions(+), 6 deletions(-) diff --git a/test/unit/control/controller.py b/test/unit/control/controller.py index a9d31d1..400d795 100644 --- a/test/unit/control/controller.py +++ b/test/unit/control/controller.py @@ -5,11 +5,12 @@ integ tests, but a few bits lend themselves to unit testing. import unittest -from stem import InvalidRequest, ProtocolError -from stem.control import _parse_circ_path, Controller, EventType import stem.socket import stem.version -import test.mocking as mocking + +from stem import InvalidRequest, ProtocolError +from stem.control import _parse_circ_path, Controller, EventType +from test import mocking class TestControl(unittest.TestCase): def setUp(self): @@ -65,12 +66,14 @@ class TestControl(unittest.TestCase): # set up for failure to create any events mocking.mock_method(Controller, "get_version", mocking.return_value(stem.version.Version('0.1.0.14'))) - self.assertRaises(InvalidRequest, self.controller.add_event_listener, lambda x: x, EventType.BW) + self.assertRaises(InvalidRequest, self.controller.add_event_listener, mocking.no_op(), EventType.BW) # set up to only fail newer events mocking.mock_method(Controller, "get_version", mocking.return_value(stem.version.Version('0.2.0.35'))) + # EventType.BW is one of the earliest events - self.assertIsNone(self.controller.add_event_listener(lambda x: x, EventType.BW)) + self.controller.add_event_listener(mocking.no_op(), EventType.BW) + # EventType.SIGNAL was added in tor version 0.2.3.1-alpha - self.assertRaises(InvalidRequest, self.controller.add_event_listener, lambda x: x, EventType.SIGNAL) + self.assertRaises(InvalidRequest, self.controller.add_event_listener, mocking.no_op(), EventType.SIGNAL)

1 0

[stem/master] Add version requirement tests for Controller.add_event_listener
by atagar＠torproject.org 09 Dec '12

09 Dec '12

commit f5e2d127581a487f9cba7edf86943c0f78445180 Author: Sean Robinson <seankrobinson(a)gmail.com> Date: Sat Dec 8 18:29:54 2012 -0700 Add version requirement tests for Controller.add_event_listener Signed-off-by: Sean Robinson <seankrobinson(a)gmail.com> --- test/unit/control/controller.py | 30 ++++++++++++++++++++++++++++-- 1 files changed, 28 insertions(+), 2 deletions(-) diff --git a/test/unit/control/controller.py b/test/unit/control/controller.py index e25fbe0..a9d31d1 100644 --- a/test/unit/control/controller.py +++ b/test/unit/control/controller.py @@ -5,10 +5,20 @@ integ tests, but a few bits lend themselves to unit testing. import unittest -from stem import ProtocolError -from stem.control import _parse_circ_path +from stem import InvalidRequest, ProtocolError +from stem.control import _parse_circ_path, Controller, EventType +import stem.socket +import stem.version +import test.mocking as mocking class TestControl(unittest.TestCase): + def setUp(self): + socket = stem.socket.ControlSocket() + self.controller = Controller(socket) + + def tearDown(self): + mocking.revert_mocking() + def test_parse_circ_path(self): """ Exercises the _parse_circ_path() helper function. @@ -47,4 +57,20 @@ class TestControl(unittest.TestCase): for test_input in malformed_inputs: self.assertRaises(ProtocolError, _parse_circ_path, test_input) + + def test_event_listening(self): + """ + Exercises the add_event_listener and remove_event_listener methods. + """ + + # set up for failure to create any events + mocking.mock_method(Controller, "get_version", mocking.return_value(stem.version.Version('0.1.0.14'))) + self.assertRaises(InvalidRequest, self.controller.add_event_listener, lambda x: x, EventType.BW) + + # set up to only fail newer events + mocking.mock_method(Controller, "get_version", mocking.return_value(stem.version.Version('0.2.0.35'))) + # EventType.BW is one of the earliest events + self.assertIsNone(self.controller.add_event_listener(lambda x: x, EventType.BW)) + # EventType.SIGNAL was added in tor version 0.2.3.1-alpha + self.assertRaises(InvalidRequest, self.controller.add_event_listener, lambda x: x, EventType.SIGNAL)

1 0

[stem/master] Use getattr()/setattr() for mocking.mock_method
by atagar＠torproject.org 09 Dec '12

09 Dec '12

commit a00e2dda071bb2d6c04b6c78ece33942ba0def53 Author: Sean Robinson <seankrobinson(a)gmail.com> Date: Sat Dec 8 18:08:39 2012 -0700 Use getattr()/setattr() for mocking.mock_method mocking.revert_mocking also needs setattr because it may be trying to revert to the original method in an object. This fixes the following error when trying to use mock_method: TypeError: 'dictproxy' object does not support item assignment Signed-off-by: Sean Robinson <seankrobinson(a)gmail.com> --- test/mocking.py | 8 ++++---- 1 files changed, 4 insertions(+), 4 deletions(-) diff --git a/test/mocking.py b/test/mocking.py index 1cf2c0a..88cbd62 100644 --- a/test/mocking.py +++ b/test/mocking.py @@ -299,7 +299,7 @@ def mock_method(target_class, method_name, mock_call): # themselves don't reference the class. This is unfortunate because it means # that we need to know both the class and method we're replacing. - target_method = target_class.__dict__[method_name] + target_method = getattr(target_class, method_name) if "mock_id" in target_method.__dict__: # we're overriding an already mocked method @@ -311,10 +311,10 @@ def mock_method(target_class, method_name, mock_call): MOCK_STATE[mocking_id] = (target_class, method_name, target_method) mock_wrapper = lambda *args: mock_call(*args) - mock_wrapper.__dict__["mock_id"] = mocking_id + setattr(mock_wrapper, "mock_id", mocking_id) # mocks the function with this wrapper - target_class.__dict__[method_name] = mock_wrapper + setattr(target_class, method_name, mock_wrapper) def revert_mocking(): """ @@ -334,7 +334,7 @@ def revert_mocking(): if module == __builtin__: setattr(__builtin__, function, impl) else: - module.__dict__[function] = impl + setattr(module, function, impl) del MOCK_STATE[mock_id]

1 0

[stem/master] Update mocking.mock_method pydocs
by atagar＠torproject.org 09 Dec '12

09 Dec '12

commit 22389a8ffd94251e857d3dd9d0abd40a01bc574e Author: Sean Robinson <seankrobinson(a)gmail.com> Date: Sat Dec 8 18:25:19 2012 -0700 Update mocking.mock_method pydocs Signed-off-by: Sean Robinson <seankrobinson(a)gmail.com> --- test/mocking.py | 18 +++++++++++++++--- 1 files changed, 15 insertions(+), 3 deletions(-) diff --git a/test/mocking.py b/test/mocking.py index 88cbd62..86acb2a 100644 --- a/test/mocking.py +++ b/test/mocking.py @@ -7,6 +7,7 @@ calling :func:`test.mocking.revert_mocking`. :: mock - replaces a function with an alternative implementation + mock_method - replaces a method with an alternative implementation revert_mocking - reverts any changes made by the mock function get_real_function - provides the non-mocked version of a function get_all_combinations - provides all combinations of attributes @@ -284,12 +285,23 @@ def mock(target, mock_call, target_module=None): def mock_method(target_class, method_name, mock_call): """ - Mocks the given class method in a similar fashion as what mock() does for - functions. + Mocks the given method in target_class in a similar fashion as mock() + does for functions. For instance... + + :: + + >>> mock_method(stem.control.Controller, "is_feature_enabled", mocking.return_true()) + >>> controller.is_feature_enabled("VERBOSE_EVENTS") + True + + :: + + "VERBOSE_EVENTS" does not exist and can never be True, but the mocked + "is_feature_enabled" will always return True, regardless. :param class target_class: class with the method we want to mock :param str method_name: name of the method to be mocked - :param functor mock_call: mocking to replace the method with + :param functor mock_call: mocking to replace the method """ # Ideally callers could call us with just the method, for instance like...

1 0

[stem/master] Dropping '--' from config options being used to launch tor
by atagar＠torproject.org 09 Dec '12

09 Dec '12

commit 4cacd2f13d1cd003db0c15addcf265fa4262ee37 Author: Damian Johnson <atagar(a)torproject.org> Date: Sun Dec 9 12:33:06 2012 -0800 Dropping '--' from config options being used to launch tor The '--' prefix in '--[config option]' commands are unnecessary. Thanks to lunar on #7666 for pointing this out. --- stem/process.py | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) diff --git a/stem/process.py b/stem/process.py index 6696821..aaa36ac 100644 --- a/stem/process.py +++ b/stem/process.py @@ -80,7 +80,7 @@ def launch_tor(tor_cmd = "tor", args = None, torrc_path = None, completion_perce runtime_args += ["-f", torrc_path] if take_ownership: - runtime_args += ["--__OwningControllerProcess", _get_pid()] + runtime_args += ["__OwningControllerProcess", _get_pid()] tor_process = subprocess.Popen(runtime_args, stdout = subprocess.PIPE, stderr = subprocess.PIPE) @@ -197,7 +197,7 @@ def launch_tor_with_config(config, tor_cmd = "tor", completion_percent = 100, in torrc_file.write("%s %s\n" % (key, value)) # prevents tor from erroring out due to a missing torrc if it gets a sighup - args = ['--__ReloadTorrcOnSIGHUP', '0'] + args = ['__ReloadTorrcOnSIGHUP', '0'] return launch_tor(tor_cmd, args, torrc_path, completion_percent, init_msg_handler, timeout, take_ownership) finally:

1 0

[ooni-probe/master] Explicitly state that reactor.stop and reactor.start should not be called from inside test methods
by art＠torproject.org 09 Dec '12

09 Dec '12

commit 53ed7f69ff809e20eeaaaa4e3797e1da155de6a2 Author: Arturo Filastò <art(a)fuffa.org> Date: Sun Dec 9 14:01:16 2012 +0100 Explicitly state that reactor.stop and reactor.start should not be called from inside test methods --- docs/source/writing_tests.rst | 4 ++++ 1 files changed, 4 insertions(+), 0 deletions(-) diff --git a/docs/source/writing_tests.rst b/docs/source/writing_tests.rst index 4ce6268..ccc529a 100644 --- a/docs/source/writing_tests.rst +++ b/docs/source/writing_tests.rst @@ -25,6 +25,10 @@ Tests are driven by inputs. For every input a new test instance is created, internally the _setUp method is called that is defined inside of test templates, then the setUp method that is overwritable by users. +Gotchas: +**never** call reactor.start of reactor.stop inside of your test method and all +will be good. + Inputs ------

1 0

[stem/master] Only using TAKEOWNERSHIP if we're on the local system
by atagar＠torproject.org 09 Dec '12

09 Dec '12

commit 120907822f06fd476f2c47b7135e816cd730b1c7 Author: Damian Johnson <atagar(a)torproject.org> Date: Sat Dec 8 23:41:21 2012 -0800 Only using TAKEOWNERSHIP if we're on the local system We were using TAKEOWNERSHIP if our pid matched tor's __OwningControllerProcess. However, doing this doesn't make sense if we're connecting remotely. Caught by Robert on... https://trac.torproject.org/7666 --- stem/control.py | 4 ++-- stem/socket.py | 16 ++++++++++++++++ test/integ/process.py | 2 +- 3 files changed, 19 insertions(+), 3 deletions(-) diff --git a/stem/control.py b/stem/control.py index be589d9..8d9784e 100644 --- a/stem/control.py +++ b/stem/control.py @@ -1511,7 +1511,7 @@ class Controller(BaseController): owning_pid = self.get_conf("__OwningControllerProcess", None) - if owning_pid == str(os.getpid()): + if owning_pid == str(os.getpid()) and self.get_socket().is_localhost(): response = self.msg("TAKEOWNERSHIP") stem.response.convert("SINGLELINE", response) @@ -1524,7 +1524,7 @@ class Controller(BaseController): except stem.ControllerError, exc: log.warn("We were unable to reset tor's __OwningControllerProcess configuration. It will continue to periodically check if our pid exists. (%s)" % response) else: - log.warn("We were unable assert ownership of tor through TAKEOWNERSHIP, despite being configured to be the owning process thrugh __OwningControllerProcess. (%s)" % response) + log.warn("We were unable assert ownership of tor through TAKEOWNERSHIP, despite being configured to be the owning process through __OwningControllerProcess. (%s)" % response) def _handle_event(self, event_message): stem.response.convert("EVENT", event_message, arrived_at = time.time()) diff --git a/stem/socket.py b/stem/socket.py index 4cbfb3e..adfc83e 100644 --- a/stem/socket.py +++ b/stem/socket.py @@ -18,6 +18,7 @@ as instances of the :class:`~stem.response.ControlMessage` class. |- send - sends a message to the socket |- recv - receives a ControlMessage from the socket |- is_alive - reports if the socket is known to be closed + |- is_localhost - returns if the socket is for the local system or not |- connect - connects a new socket |- close - shuts down the socket +- __enter__ / __exit__ - manages socket connection @@ -145,6 +146,15 @@ class ControlSocket(object): return self._is_alive + def is_localhost(self): + """ + Returns if the connection is for the local system or not. + + :returns: **bool** that's **True** if the connection is for the local host and **False** otherwise + """ + + return False + def connect(self): """ Connects to a new socket, closing our previous one if we're already @@ -300,6 +310,9 @@ class ControlPort(ControlSocket): return self._control_port + def is_localhost(self): + return self._control_addr == "127.0.0.1" + def _make_socket(self): try: control_socket = socket.socket(socket.AF_INET, socket.SOCK_STREAM) @@ -339,6 +352,9 @@ class ControlSocketFile(ControlSocket): return self._socket_path + def is_localhost(self): + return True + def _make_socket(self): try: control_socket = socket.socket(socket.AF_UNIX, socket.SOCK_STREAM) diff --git a/test/integ/process.py b/test/integ/process.py index ee21d5a..5811e37 100644 --- a/test/integ/process.py +++ b/test/integ/process.py @@ -113,7 +113,7 @@ class TestProcess(unittest.TestCase): # needing to a _get_pid() helper but after much head scratching I haven't # been able to mock os.getpid() or posix.getpid(). - sleep_process = subprocess.Popen(['sleep', '10']) + sleep_process = subprocess.Popen(['sleep', '60']) mocking.mock(stem.process._get_pid, mocking.return_value(str(sleep_process.pid))) tor_process = stem.process.launch_tor_with_config(

1 0