Merge lp:~nataliabidart/ubuntuone-client/clean-env-aq into lp:ubuntuone-client

[Natalia Bidart]

- Fake ExternalInterface when testing the main module (LP: #823982).
- Do a proper clean on test_action_queue tests (LP: #823895).

[Roberto Alsina (ralsina)]

tests run, looks ok!

review: Approve

[Alejandro J. Cura (alecu)]

+1

review: Approve

=== modified file 'tests/syncdaemon/test_action_queue.py'
--- tests/syncdaemon/test_action_queue.py	2011-08-09 12:56:06 +0000
+++ tests/syncdaemon/test_action_queue.py	2011-08-10 14:39:36 +0000
@@ -214,6 +214,7 @@
         if hasattr(self, 'testing_deferred'):
             self.testing_deferred.callback(True)
 
+
 class TestActionQueue(ActionQueue):
     """AQ class that uses the testing protocol."""
     protocol = TestingProtocol
@@ -235,15 +236,13 @@
         self.shares = self.mktemp('shares')
         self.partials = self.mktemp('partials')
 
-        # set up Fake Main to use the testing AQ, the port will
-        # be decided later
-        self._orig_fakemain_aq = (FakeMain._fake_AQ_class,
-                                  FakeMain._fake_AQ_params)
-        FakeMain._fake_AQ_class = TestActionQueue
-        FakeMain._fake_AQ_params = ('127.0.0.1', 0, False)
+        # set up FakeMain to use the testing AQ, the port will be decided later
+        self.patch(FakeMain, '_fake_AQ_class', TestActionQueue)
+        self.patch(FakeMain, '_fake_AQ_params', ('127.0.0.1', 0, False))
 
         self.main = FakeMain(root_dir=self.root, shares_dir=self.shares,
                              data_dir=self.data, partials_dir=self.partials)
+        self.addCleanup(self.main.shutdown)
 
         self.action_queue = self.main.action_q
         self.action_queue.connection_timeout = 3
@@ -267,24 +266,14 @@
         self.handler.setLevel(logging.DEBUG)
         self._logger = logging.getLogger('ubuntuone.SyncDaemon')
         self._logger.addHandler(self.handler)
+        self.addCleanup(self._logger.removeHandler, self.handler)
+
         # setup done according to the platform
         setup_action_queue_test(self)
 
-    def _linux_setup(self):
-        import dbus
-        dbus.service.BusName.__del__ = lambda _: None
-
     @defer.inlineCallbacks
     def tearDown(self):
         """Cleanup."""
-        self._logger.removeHandler(self.handler)
-        self.main.shutdown()
-
-        # restore Fake Main settings
-        cls, params = self._orig_fakemain_aq
-        FakeMain._fake_AQ_class = cls
-        FakeMain._fake_AQ_params = params
-
         for record in self.handler.records:
             exc_info = getattr(record, 'exc_info', None)
             if exc_info is not None:
@@ -452,15 +441,14 @@
             event_queue = self.eq = FakedEventQueue()
 
         self.rq = RequestQueue(action_queue=FakeAQ())
+        self.addCleanup(self.eq.shutdown)
 
         # add a Memento handler to the logger
         self.log_handler = MementoHandler()
         self.log_handler.setLevel(logging.DEBUG)
-        logging.getLogger('ubuntuone.SyncDaemon').addHandler(self.log_handler)
-
-    def tearDown(self):
-        """Tear down."""
-        self.eq.shutdown()
+        logger = logging.getLogger('ubuntuone.SyncDaemon')
+        logger.addHandler(self.log_handler)
+        self.addCleanup(logger.removeHandler, self.log_handler)
 
     def _add_to_rq(self, *cmds):
         """Add the commands to rq.waiting and hashed_waiting."""

=== modified file 'tests/syncdaemon/test_main.py'
--- tests/syncdaemon/test_main.py	2011-08-09 12:56:06 +0000
+++ tests/syncdaemon/test_main.py	2011-08-10 14:39:36 +0000
@@ -57,6 +57,15 @@
         """Something! :)"""
 
 
+class FakedExternalInterface(object):
+    """Do nothing."""
+
+    def __init__(self, *a, **kw):
+        self._called = []
+        self.connect = lambda *a, **kw: self._called.append(('connect', a, kw))
+        self.shutdown = lambda *a, **kw: None
+
+
 class MainTests(BaseTwistedTestCase):
     """ Basic tests to check main.Main """
 
@@ -71,6 +80,8 @@
         # perform the extra setup steps according to the platform
         setup_main_test(self)
 
+        self.patch(main_mod, 'ExternalInterface', FakedExternalInterface)
+
         self.handler = MementoHandler()
         self.handler.setLevel(logging.DEBUG)
         self._logger = logging.getLogger('ubuntuone.SyncDaemon')
@@ -78,27 +89,29 @@
         self.addCleanup(self._logger.removeHandler, self.handler)
 
     def build_main(self, **kwargs):
-        """
-        Build and return a Main object, using reasonable defaults for
-        the tests, plus whatever extra kwargs are passed in.
+        """Build and return a Main object.
+
+        Use reasonable defaults for the tests, plus whatever extra kwargs are
+        passed in.
+
         """
         # get the params using the platform code to ensure they are correct
-        params =  get_main_params(self, _get_main_common_params(self))
+        params = get_main_params(self, _get_main_common_params(self))
         params.update(kwargs)
         m = main_mod.Main(**params)
+        self.addCleanup(m.shutdown)
         m.local_rescan = lambda *_: m.event_q.push('SYS_LOCAL_RESCAN_DONE')
         return m
 
     def test_main_initialization(self):
         """test that creating a Main instance works as expected."""
         main = self.build_main()
-        main.shutdown()
+        self.assertIsInstance(main, main_mod.Main)
 
     def test_main_start(self):
         """Test that Main.start works."""
         main = self.build_main()
         main.start()
-        main.shutdown()
 
     def test_main_restarts_on_critical_error(self):
         """Test that Main restarts when syncdaemon gets into UNKNOWN_ERROR."""
@@ -108,11 +121,11 @@
         main.start()
         main.event_q.push('SYS_UNKNOWN_ERROR')
         self.assertTrue(self.restarted)
-        main.shutdown()
 
     def test_shutdown_pushes_sys_quit(self):
         """When shutting down, the SYS_QUIT event is pushed."""
-        main = self.build_main()
+        params = get_main_params(self, _get_main_common_params(self))
+        main = main_mod.Main(**params)
         events = []
         self.patch(main.event_q, 'push',
                    lambda *a, **kw: events.append((a, kw)))
@@ -147,34 +160,29 @@
         main.start()
         main.event_q.push('SYS_NET_CONNECTED')
         main.event_q.push('SYS_USER_CONNECT', access_token='')
-        d0.addCallback(lambda _: main.shutdown())
         return d0
 
     def test_create_dirs_already_exists_dirs(self):
         """test that creating a Main instance works as expected."""
         link = os.path.join(self.root, 'Shared With Me')
-        self.assertFalse(path_exists(link))
+        self.assertFalse(is_link(link))
         self.assertTrue(path_exists(self.shares))
         self.assertTrue(path_exists(self.root))
         main = self.build_main()
-        self.assertTrue(path_exists(main.shares_dir_link))
         # check that the shares link is actually a link
         self.assertTrue(is_link(main.shares_dir_link))
         self.assertEquals(link, main.shares_dir_link)
-        main.shutdown()
 
     def test_create_dirs_already_exists_symlink_too(self):
         """test that creating a Main instance works as expected."""
         link = os.path.join(self.root, 'Shared With Me')
         make_link(self.shares, link)
-        self.assertTrue(path_exists(link))
         self.assertTrue(is_link(link))
         self.assertTrue(path_exists(self.shares))
         self.assertTrue(path_exists(self.root))
         main = self.build_main()
         # check that the shares link is actually a link
         self.assertTrue(is_link(main.shares_dir_link))
-        main.shutdown()
 
     def test_create_dirs_already_exists_but_not_symlink(self):
         """test that creating a Main instance works as expected."""
@@ -188,21 +196,17 @@
         # check that the shares link is actually a link
         self.assertEquals(main.shares_dir_link, link)
         self.assertFalse(is_link(main.shares_dir_link))
-        main.shutdown()
 
     def test_create_dirs_none_exists(self):
         """test that creating a Main instance works as expected."""
-        link = os.path.join(self.root, 'Shared With Me')
         # remove the existing dirs
         remove_dir(self.root)
         remove_dir(self.shares)
         main = self.build_main()
         # check that the shares link is actually a link
-        self.assertTrue(path_exists(link))
         self.assertTrue(is_link(main.shares_dir_link))
         self.assertTrue(path_exists(self.shares))
         self.assertTrue(path_exists(self.root))
-        main.shutdown()
 
     def test_connect_if_autoconnect_is_enabled(self):
         """If autoconnect option is enabled, connect the syncdaemon."""
@@ -211,15 +215,9 @@
         user_config.set_autoconnect(True)
         self.addCleanup(user_config.set_autoconnect, orig)
 
-        self.connect_called = False
-        self.patch(main_mod.ubuntuone.platform.ExternalInterface, 'connect',
-                   lambda *a, **kw: setattr(self, 'connect_called', (a, kw)))
-
         main = self.build_main()
-        self.addCleanup(main.shutdown)
-
-        self.assertEqual(self.connect_called, ((main.external,),
-                                               {'autoconnecting': True}))
+        expected = [('connect', (), {'autoconnecting': True})]
+        self.assertEqual(main.external._called, expected)
 
     def test_dont_connect_if_autoconnect_is_disabled(self):
         """If autoconnect option is disabled, do not connect the syncdaemon."""
@@ -228,14 +226,8 @@
         user_config.set_autoconnect(False)
         self.addCleanup(user_config.set_autoconnect, orig)
 
-        self.connect_called = False
-        self.patch(main_mod.ubuntuone.platform.ExternalInterface, 'connect',
-                   lambda *a, **kw: setattr(self, 'connect_called', True))
-
         main = self.build_main()
-        self.addCleanup(main.shutdown)
-
-        self.assertFalse(self.connect_called)
+        self.assertEqual(main.external._called, [])
 
     def _get_listeners(self, main):
         """Return the subscribed objects."""
@@ -247,17 +239,15 @@
 
     def test_event_logger_starts_if_available(self):
         """The event logger is started if available."""
-        self.patch(main_mod.status_listener,
+        self.patch(main_mod.event_logging,
                    "get_listener", lambda *a: FakeListener())
         main = self.build_main()
-        self.addCleanup(main.shutdown)
         self.assertIn(main.eventlog_listener, self._get_listeners(main))
 
     def test_event_logger_not_started_if_not_available(self):
         """The event logger is not started if it's not available."""
         self.patch(main_mod.event_logging, "get_listener", lambda *a: None)
         main = self.build_main()
-        self.addCleanup(main.shutdown)
         self.assertNotIn(main.eventlog_listener, self._get_listeners(main))
 
     def test_status_listener_is_installed(self):
@@ -265,43 +255,34 @@
         self.patch(main_mod.status_listener,
                    "get_listener", lambda *a: FakeListener())
         main = self.build_main()
-        self.addCleanup(main.shutdown)
         self.assertIn(main.status_listener, self._get_listeners(main))
 
     def test_status_listener_not_installed_when_disabled(self):
         """The status listener is not started if it's not available."""
         self.patch(main_mod.status_listener, "get_listener", lambda *a: None)
         main = self.build_main()
-        self.addCleanup(main.shutdown)
         self.assertNotIn(main.status_listener, self._get_listeners(main))
 
     def test_get_rootdir(self):
         """The get_rootdir returns the root dir."""
-        expected = '~/Ubuntu Test One'
+        expected = os.path.expanduser(os.path.join('~', 'Ubuntu Test One'))
         main = self.build_main(root_dir=expected)
-        self.addCleanup(main.shutdown)
-
         self.assertEqual(main.get_rootdir(), expected)
 
     def test_get_sharesdir(self):
         """The get_sharesdir returns the shares dir."""
-        expected = '~/Share it to Me'
+        expected = os.path.expanduser(os.path.join('~', 'Share it to Me'))
         main = self.build_main(shares_dir=expected)
-        self.addCleanup(main.shutdown)
-
         self.assertEqual(main.get_sharesdir(), expected)
 
     def test_get_sharesdirlink(self):
         """The get_sharesdirlink returns the shares dir link."""
         expected = 'Share it to Me'
         main = self.build_main(shares_symlink_name=expected)
-        self.addCleanup(main.shutdown)
-
         self.assertEqual(main.get_sharesdir_link(),
                          os.path.join(main.get_rootdir(), expected))
 
     def test_version_is_logged(self):
         """Test that the client version is logged."""
-        main = self.build_main()
-        self.addCleanup(main.shutdown)
+        self.build_main()
         self.assertTrue(self.handler.check_info("client version", VERSION))

=== modified file 'ubuntuone/syncdaemon/main.py'
--- ubuntuone/syncdaemon/main.py	2011-07-29 12:51:04 +0000
+++ ubuntuone/syncdaemon/main.py	2011-08-10 14:39:36 +0000
@@ -37,9 +37,7 @@
     volume_manager,
 )
 from ubuntuone import syncdaemon, clientdefs
-import ubuntuone.platform
-
-from ubuntuone.platform import event_logging
+from ubuntuone.platform import event_logging, ExternalInterface
 from ubuntuone.syncdaemon import status_listener
 from ubuntuone.syncdaemon.states import StateManager, QueueManager
 
@@ -129,7 +127,7 @@
         self.lr = local_rescan.LocalRescan(self.vm, self.fs,
                                            self.event_q, self.action_q)
 
-        self.external = ubuntuone.platform.ExternalInterface(
+        self.external = ExternalInterface(
             self, glib_loop, broadcast_events)
         self.external.oauth_credentials = oauth_credentials
         if user_config.get_autoconnect():