Merge lp:~mandel/ubuntu-sso-client/fix-broken-tests into lp:ubuntu-sso-client

[Manuel de la Peña]

- Fixed all those broken tests on windows related to the dirty reactor left by twisted.pb (LP: #960436).

- Fixed all those broken tests on windows related to the dirty reactor left by twisted.pb (LP: #960436).

This branch depends on lp:~mandel/ubuntuone-dev-tools/tcp-testcases so please ensure that either it has been merged in the ubuntuone-deb-tools trunk or that it is present in your path.

[Brian Curtin (brian.curtin)]

+1
Works IRL on Windows and seems like a reasonable approach.

review: Approve

[Natalia Bidart (nataliabidart)]

Branch looks good.

Could you please apply the following before approving:

* in ubuntu_sso/utils/tests/test_ipc.py you added a global pylint disable when is not needed globally, but only for setUp. So please remove the global disable/enable and just add on *inside* setUp.

* can you please revert the (I think unneeded) changes in these 3 diff lines:

424 + self.context_factory)
427 +
435 +

(that would be removing the added empty lines and restoring the original spacing for self.context_factory indentation).

review: Approve

[Manuel de la Peña (mandel)]

review: Abstain

[Ubuntu One Auto Pilot (otto-pilot)]

There are additional revisions which have not been approved in review. Please seek review and approval of these new revisions.

=== modified file 'ubuntu_sso/main/__init__.py'
--- ubuntu_sso/main/__init__.py	2012-04-09 17:38:24 +0000
+++ ubuntu_sso/main/__init__.py	2012-04-18 14:38:20 +0000
@@ -67,6 +67,7 @@
     from ubuntu_sso.main import linux
     source = linux
 
+UbuntuSSOClient = source.UbuntuSSOClient
 UbuntuSSOProxy = source.UbuntuSSOProxy
 get_sso_client = source.get_sso_client
 

=== modified file 'ubuntu_sso/main/linux.py'
--- ubuntu_sso/main/linux.py	2012-04-09 17:38:24 +0000
+++ ubuntu_sso/main/linux.py	2012-04-18 14:38:20 +0000
@@ -440,6 +440,10 @@
         self.sso_login = SSOLoginClient().dbus_iface
         self.cred_manager = CredentialsManagementClient().dbus_iface
 
+    def connect(self):
+        """No need to connect DBus proxy objects."""
+        return defer.succeed(None)
+
     def disconnect(self):
         """No need to disconnect DBus proxy objects."""
         return defer.succeed(None)

=== modified file 'ubuntu_sso/main/tests/__init__.py'
--- ubuntu_sso/main/tests/__init__.py	2012-04-09 17:38:24 +0000
+++ ubuntu_sso/main/tests/__init__.py	2012-04-18 14:38:20 +0000
@@ -39,11 +39,11 @@
 # pylint: disable=C0103
 
 if sys.platform == 'win32':
-    from ubuntu_sso.utils.tests.test_ipc import BaseIPCTestCase
-    BaseTestCase = BaseIPCTestCase
+    from ubuntu_sso.main.tests import windows
+    BaseTestCase = windows.BaseTestCase
 else:
-    from ubuntuone.devtools.testcases.dbus import DBusTestCase
-    BaseTestCase = DBusTestCase
+    from ubuntu_sso.main.tests import linux
+    BaseTestCase = linux.BaseTestCase
 
 # pylint: enable=C0103
 

=== added file 'ubuntu_sso/main/tests/linux.py'
--- ubuntu_sso/main/tests/linux.py	1970-01-01 00:00:00 +0000
+++ ubuntu_sso/main/tests/linux.py	2012-04-18 14:38:20 +0000
@@ -0,0 +1,37 @@
+# -*- coding: utf-8 -*-
+#
+# Copyright 2012 Canonical Ltd.
+#
+# This program is free software: you can redistribute it and/or modify it
+# under the terms of the GNU General Public License version 3, as published
+# by the Free Software Foundation.
+#
+# This program is distributed in the hope that it will be useful, but
+# WITHOUT ANY WARRANTY; without even the implied warranties of
+# MERCHANTABILITY, SATISFACTORY QUALITY, or FITNESS FOR A PARTICULAR
+# PURPOSE.  See the GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License along
+# with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+"""Utility classes used for testing main on linux."""
+
+from twisted.internet import defer
+
+from ubuntu_sso import main
+from ubuntuone.devtools.testcases.dbus import DBusTestCase
+
+
+class BaseTestCase(DBusTestCase):
+    """The base test case with platform specific support."""
+
+    @defer.inlineCallbacks
+    def setUp(self):
+        yield super(BaseTestCase, self).setUp()
+
+        self.sso_service = main.UbuntuSSOService()
+        yield self.sso_service.start()
+        self.addCleanup(self.sso_service.shutdown)
+
+        self.sso_client = yield main.get_sso_client()
+        self.addCleanup(self.sso_client.disconnect)

=== modified file 'ubuntu_sso/main/tests/test_clients.py'
--- ubuntu_sso/main/tests/test_clients.py	2012-04-09 17:38:24 +0000
+++ ubuntu_sso/main/tests/test_clients.py	2012-04-18 14:38:20 +0000
@@ -81,22 +81,17 @@
 
     @defer.inlineCallbacks
     def setUp(self):
+        # avoid putting stuff in the mainloops this MUST be done
+        # before the parent setUp is called because it will register
+        # the callbacks
+        self.patch(main.source, 'timeout_func', lambda *a: None)
+        self.patch(main.source, 'shutdown_func', lambda *a: None)
+
         yield super(AbstractTestCase, self).setUp()
         self.keyring = FakedKeyring()
         self.patch(main, 'Keyring', lambda: self.keyring)
 
-        # avoid putting stuff in the mainloops
-        self.patch(main.source, 'timeout_func', lambda *a: None)
-        self.patch(main.source, 'shutdown_func', lambda *a: None)
-
-        self.sso_service = main.UbuntuSSOService()
-        yield self.sso_service.start()
-        self.addCleanup(self.sso_service.shutdown)
-
-        self.sso_client = yield main.get_sso_client()
-        self.addCleanup(self.sso_client.disconnect)
-
-        self.client = self.patchable_backend = None
+        self.test_client = self.patchable_backend = None
 
         if self.backend_method is None:
             self.backend_method = self.method
@@ -127,19 +122,19 @@
         d = defer.Deferred()
 
         cb = lambda *a: d.callback(a)
-        match = self.client.connect_to_signal(success_signal, cb)
-        self.addCleanup(self.client.disconnect_from_signal,
+        match = self.test_client.connect_to_signal(success_signal, cb)
+        self.addCleanup(self.test_client.disconnect_from_signal,
                         success_signal, match)
 
         eb = lambda *a: d.errback(AssertionError(a))
-        match = self.client.connect_to_signal(error_signal, eb)
-        self.addCleanup(self.client.disconnect_from_signal,
+        match = self.test_client.connect_to_signal(error_signal, eb)
+        self.addCleanup(self.test_client.disconnect_from_signal,
                         error_signal, match)
 
         self.patch(self.patchable_backend, self.backend_method,
                    patched_backend_method)
 
-        yield self.client.call_method(self.method, *self.params)
+        yield self.test_client.call_method(self.method, *self.params)
 
         result = yield d
         self.assertEqual(expected_result, result)
@@ -171,7 +166,7 @@
     @defer.inlineCallbacks
     def setUp(self):
         yield super(SSOLoginProxyTestCase, self).setUp()
-        self.client = self.sso_client.sso_login
+        self.test_client = self.sso_client.sso_login
         self.patchable_backend = self.sso_service.sso_login.processor
 
 
@@ -285,7 +280,7 @@
         self.credentials = FakedCredentials()
         self.patch(main, 'Credentials', lambda *a, **kw: self.credentials)
 
-        self.client = self.sso_client.cred_manager
+        self.test_client = self.sso_client.cred_manager
         self.patchable_backend = self.credentials
 
     def _backend_succeed(self, *args, **kwargs):
@@ -309,7 +304,7 @@
         """The credentials are asked and returned in a sync call."""
         d = defer.Deferred()
 
-        self.client.call_method('find_credentials_sync',
+        self.test_client.call_method('find_credentials_sync',
                                 APP_NAME, self.args,
                                 reply_handler=d.callback,
                                 error_handler=d.errback)
@@ -324,7 +319,7 @@
         self.patch(self.credentials, 'find_credentials', self._backend_fail)
         d = defer.Deferred()
 
-        self.client.call_method('find_credentials_sync',
+        self.test_client.call_method('find_credentials_sync',
                                 APP_NAME, self.args,
                                 reply_handler=d.errback,
                                 error_handler=d.callback)

=== added file 'ubuntu_sso/main/tests/windows.py'
--- ubuntu_sso/main/tests/windows.py	1970-01-01 00:00:00 +0000
+++ ubuntu_sso/main/tests/windows.py	2012-04-18 14:38:20 +0000
@@ -0,0 +1,56 @@
+# -*- coding: utf-8 -*-
+#
+# Copyright 2012 Canonical Ltd.
+#
+# This program is free software: you can redistribute it and/or modify it
+# under the terms of the GNU General Public License version 3, as published
+# by the Free Software Foundation.
+#
+# This program is distributed in the hope that it will be useful, but
+# WITHOUT ANY WARRANTY; without even the implied warranties of
+# MERCHANTABILITY, SATISFACTORY QUALITY, or FITNESS FOR A PARTICULAR
+# PURPOSE.  See the GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License along
+# with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+"""Utility classes for testing main on windows."""
+
+from twisted.internet import defer
+
+from ubuntu_sso import main
+from ubuntu_sso.utils.tests.test_ipc import BaseIPCTestCase
+
+
+class BaseTestCase(BaseIPCTestCase):
+    """The base test case with platform specific support."""
+
+    client_class = main.UbuntuSSOClient
+    service_class = lambda *a: main.UbuntuSSOProxy(None)
+
+    @defer.inlineCallbacks
+    def setUp(self):
+
+        self.sso_service = None
+
+        def service_factory(*args):
+            """Returns the service class."""
+            self.sso_service = main.UbuntuSSOService()
+            return main.UbuntuSSOProxy(self.sso_service)
+
+        self.service_class = service_factory
+
+        yield super(BaseTestCase, self).setUp()
+
+        # set the proxy to be the service started by the parent test class
+        self.sso_service.proxy = self.service
+
+        # patch the start of the proxy since the parent setup started it
+        self.patch(self.sso_service.proxy, 'start',
+                                            lambda: defer.succeed(None))
+
+        # start the service which will perform all the required operations
+        # except the proxy start
+        yield self.sso_service.start()
+
+        self.sso_client = self.client

=== added file 'ubuntu_sso/tests/linux.py'
--- ubuntu_sso/tests/linux.py	1970-01-01 00:00:00 +0000
+++ ubuntu_sso/tests/linux.py	2012-04-18 14:38:20 +0000
@@ -0,0 +1,46 @@
+# -*- coding: utf-8 -*-
+#
+# Copyright 2011 Canonical Ltd.
+#
+# This program is free software: you can redistribute it and/or modify it
+# under the terms of the GNU General Public License version 3, as published
+# by the Free Software Foundation.
+#
+# This program is distributed in the hope that it will be useful, but
+# WITHOUT ANY WARRANTY; without even the implied warranties of
+# MERCHANTABILITY, SATISFACTORY QUALITY, or FITNESS FOR A PARTICULAR
+# PURPOSE.  See the GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License along
+# with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+"""Helpful code for testing on linux."""
+
+from twisted.internet import defer
+
+from ubuntuone.devtools.testcases.dbus import DBusTestCase
+
+# pylint complains about callables
+# pylint: disable=E1102
+
+
+class BaseServiceTestCase(DBusTestCase):
+    """Set the ipc to a random port for this instance."""
+
+    client_class = None  # the BaseClient instance
+    service_class = None  # the BaseService instance
+
+    @defer.inlineCallbacks
+    def setUp(self):
+        yield super(BaseServiceTestCase, self).setUp()
+        self.service = None
+        self.client = None
+        if self.service_class is not None:
+            self.service = self.service_class()
+            self.client = self.client_class()
+
+            # pylint: disable=E1102
+            yield self.service.start()
+            self.addCleanup(self.service.shutdown)
+            yield self.client.connect()
+            self.addCleanup(self.client.disconnect)

=== modified file 'ubuntu_sso/utils/tests/test_ipc.py'
--- ubuntu_sso/utils/tests/test_ipc.py	2012-04-09 17:38:24 +0000
+++ ubuntu_sso/utils/tests/test_ipc.py	2012-04-18 14:38:20 +0000
@@ -38,6 +38,7 @@
     NoSuchMethod,
 )
 from ubuntuone.devtools.handlers import MementoHandler
+from ubuntuone.devtools.testcases.txtcpserver import PbServerTestCase
 
 from ubuntu_sso.tests import TestCase
 from ubuntu_sso.utils import ipc
@@ -185,7 +186,7 @@
     service_cmdline = DummyService.cmdline
 
 
-class BaseIPCTestCase(TestCase):
+class BaseIPCTestCase(PbServerTestCase, TestCase):
     """Set the ipc to a random port for this instance."""
 
     timeout = 5
@@ -201,20 +202,44 @@
 
     @defer.inlineCallbacks
     def setUp(self):
+        # pylint complains about callables
+        # pylint: disable=E1102
         yield super(BaseIPCTestCase, self).setUp()
 
         self.service = None
         self.client = None
 
         if self.service_class is not None:
+
+            self.service = self.service_class()
+            self.client = self.client_class()
+
+            # patch server connection and client connection to ensure that
+            # we have clean connections
+
+            def server_listen(server_factory, service_name, activation_cmd,
+                              port, reactor=None):
+                """Connect to the local running service."""
+                self.listen_server(self.service)
+                return defer.succeed(self.listener)
+
+            self.patch(ipc, 'server_listen', server_listen)
+
+            def client_connect(client_factory, service_name,
+                               activation_cmdline, port, reactor=None):
+                """Connect the local running client."""
+                self.connect_client()
+                self.client.factory = self.client_factory
+                return defer.succeed(self.connector)
+
+            self.patch(ipc, 'client_connect', client_connect)
+
             # pylint: disable=E1102
-            self.service = self.service_class()
             yield self.service.start()
-            self.addCleanup(self.service.shutdown)
-
-            self.client = self.client_class()
             yield self.client.connect()
-            self.addCleanup(self.client.disconnect)
+
+            yield self.client_connected
+        # pylint: enable=E1102
 
     @property
     def remote_service(self):
@@ -293,6 +318,7 @@
         """
         for signal_name, args, kwargs in self.signal_mapping:
             self.assert_remote_signal(signal_name, *args, **kwargs)
+# pylint: enable=E1102
 
 
 class ListenConnectTestCase(BaseIPCTestCase):

=== modified file 'ubuntu_sso/utils/webclient/txweb.py'
--- ubuntu_sso/utils/webclient/txweb.py	2012-04-12 12:07:46 +0000
+++ ubuntu_sso/utils/webclient/txweb.py	2012-04-18 14:38:20 +0000
@@ -56,8 +56,13 @@
 class WebClient(BaseWebClient):
     """A simple web client that does not support proxies, yet."""
 
+    client_factory = None
+
     def __init__(self, connector=None, context_factory=None, **kwargs):
         """Initialize this webclient."""
+        # delay import, otherwise a default reactor gets installed
+        from twisted.web import client
+
         super(WebClient, self).__init__(**kwargs)
 
         if connector is None:
@@ -72,11 +77,17 @@
         else:
             self.context_factory = context_factory
 
+        # decide which client factory to use
+        if WebClient.client_factory is None:
+            self.client_factory = client.HTTPClientFactory
+        else:
+            self.client_factory = WebClient.client_factory
+
     @defer.inlineCallbacks
     def raw_request(self, method, uri, headers, postdata):
         """Make a raw http request."""
         # delay import, otherwise a default reactor gets installed
-        from twisted.web import client, error
+        from twisted.web import error
 
         parsed_url = urlparse.urlparse(uri)
 
@@ -89,10 +100,10 @@
         else:
             port = parsed_url.port
 
-        factory = client.HTTPClientFactory(uri, method=method,
-                                           postdata=postdata,
-                                           headers=headers,
-                                           followRedirect=False)
+        factory = self.client_factory(uri, method=method,
+                                       postdata=postdata,
+                                       headers=headers,
+                                       followRedirect=False)
         # pylint: disable=E1103
         if https:
             self.connector.connectSSL(host, port, factory,