Merge lp:~dobey/ubuntuone-dev-tools/dbus-strict-priv into lp:ubuntuone-dev-tools

[dobey]

Be more strict about connection to the dbus service, and cleanup of connections
Just enforce disconnection from the bus, rather than trying to remove signals
Don't do inlineCallbacks for now, as it seems to cause many problems

I've gone and removed the inlineCallbacks decorator here, because it seems to create a lot more problems than it is supposed to solve, and the tests in devtools, sso-client, control-panel, and ubuntuone-client all pass without it, while having it makes the additional strictness introduced here cause failures in sso-client and control-panel, because the deferred usage seems to cause race conditions with cleanup of connections to the dbus daemon. And without the decorator, everything seems to just work, as expected, save a few minor other details in a few of the other projects. The tests all pass, but in a couple cases, there is some very weird usage of various test case imports, and MI.

And I think it is better to work everywhere now, as expected, than to just be broken everywhere, because twisted forces us to be broken; so I'm proposing this as it is.

This change is however, set up so that @inlineCallbacks can be added back to the setUp/tearDown when all the other issues are fixed, and it does in fact work correctly.

[Eric Casteleijn (thisfred)]

Looks good

review: Approve

[Tim Cole (tcole)]

review: Approve

=== modified file 'ubuntuone/devtools/testcase.py'
--- ubuntuone/devtools/testcase.py	2011-05-31 17:35:49 +0000
+++ ubuntuone/devtools/testcase.py	2011-06-01 15:39:31 +0000
@@ -27,8 +27,6 @@
 
 from functools import wraps
 
-from twisted.internet import defer
-from twisted.python import failure
 from twisted.trial.unittest import TestCase, SkipTest
 
 # DBusRunner for DBusTestCase using tests
@@ -213,7 +211,6 @@
         services.extend([DBusRunner])
         return services
 
-    @defer.inlineCallbacks
     def setUp(self):
         """Setup the infrastructure fo the test (dbus service)."""
         # Class 'BaseTestCase' has no 'setUp' member
@@ -224,59 +221,29 @@
 
         # We need to ensure DBUS_SESSION_BUS_ADDRESS is private here
         from urllib import unquote
-        bus_address = os.environ.get('DBUS_SESSION_BUS_ADDRESS', None)
+        bus_address = yield os.environ.get('DBUS_SESSION_BUS_ADDRESS', None)
         if os.path.dirname(unquote(bus_address.split(',')[0].split('=')[1])) \
                 != os.path.dirname(os.getcwd()):
             raise InvalidSessionBus('DBUS_SESSION_BUS_ADDRES is wrong.')
 
         # Set up the main loop and bus connection
-        self.loop = DBusGMainLoop(set_as_default=True)
-        self.bus = dbus.bus.BusConnection(address_or_type=bus_address,
-                                          mainloop=self.loop)
+        self.loop = yield DBusGMainLoop(set_as_default=True)
+        self.bus = yield dbus.bus.BusConnection(address_or_type=bus_address,
+                                                mainloop=self.loop)
+
+        # Check that we are on the correct bus for real
+        if len(self.bus.list_names()) > 2:
+            raise InvalidSessionBus('Too many bus connections: %s' %
+                                    len(self.bus.list_names()))
 
         # monkeypatch busName.__del__ to avoid errors on gc
         # we take care of releasing the name in shutdown
         service.BusName.__del__ = lambda _: None
-        self.bus.set_exit_on_disconnect(False)
+        yield self.bus.set_exit_on_disconnect(False)
         self.signal_receivers = set()
 
     def tearDown(self):
         """Cleanup the test."""
-        # Class 'BaseTestCase' has no 'tearDown' member
-        # pylint: disable=E1101
-        d = self.cleanup_signal_receivers(self.signal_receivers)
-        d.addBoth(self._tear_down)
-        d.addBoth(lambda _: super(DBusTestCase, self).tearDown())
-        return d
-
-    def _tear_down(self):
-        """Shutdown."""
-        self.bus.flush()
-        self.bus.close()
-
-    def error_handler(self, error):
-        """Default error handler for DBus calls."""
-        if isinstance(error, failure.Failure):
-            self.fail(error.getErrorMessage())
-
-    def cleanup_signal_receivers(self, signal_receivers):
-        """Cleanup self.signal_receivers and returns a deferred."""
-        # dbus modules will be imported by the decorator
-        # pylint: disable=E0602
-        deferreds = []
-        for match in signal_receivers:
-            d = defer.Deferred()
-
-            def callback(*args):
-                """Callback that accepts *args."""
-                if not d.called:
-                    d.callback(args)
-            self.bus.call_async(dbus.bus.BUS_DAEMON_NAME,
-                                dbus.bus.BUS_DAEMON_PATH,
-                                dbus.bus.BUS_DAEMON_IFACE, 'RemoveMatch', 's',
-                                (str(match),), callback, self.error_handler)
-            deferreds.append(d)
-        if deferreds:
-            return defer.DeferredList(deferreds)
-        else:
-            return defer.succeed(True)
+        yield self.bus.flush()
+        yield self.bus.close()
+        yield super(DBusTestCase, self).tearDown()