Merge lp:~nataliabidart/ubuntu-sso-client/no-more-gobject into lp:ubuntu-sso-client

[Natalia Bidart]

No more gobject dependency on non-GUI classes! (LP: #695798).

To run the test suite, you can do:

./run-tests

To test IRL, you can run within this branch:

killall ubuntu-sso-login; DEBUG=True PYTHONPATH=. ./bin/ubuntu-sso-login

And delete your Ubuntu One token using d-feet (com.ubuntu.sso service, /com/ubuntu/sso/credentials object path, CredentialsManagemente interface, clear_credentials method) passing arguments:

'Ubuntu One', {}

Then, you can try re-adding your machine to Ubuntu One.

[Manuel de la Peña (mandel)]

I cannot see any problem with this code. +1

review: Approve

[Roberto Alsina (ralsina)]

+1 from me too.

review: Approve

[Alejandro J. Cura (alecu)]

59 + # This code is blocking, we should change this.
60 + # I've tried with deferToThread an twisted.web.client.getPage
61 + # but the returned deferred will never be fired (nataliabidart).

Neither deferToThread nor getPage will work because the twisted reactor is not used anymore in ubuntu-sso-client. Only twisted Deferreds are used in this code, but only as a control structure in order to make the sequence of callbacks cleaner, and to ease testing with trial.

I'm thinking of a few ways to have safe async http calls here: using libsoup or putting back the twisted reactor.

Using libsoup (like the ubuntuone-control-panel backend uses) would make the code dependent once again on gobject and possibly gnome, but it will make supporting all kind of proxies a one liner.

Using twisted will make the code more portable, but the backend should be moved to its own process (also just like u1cp) to avoid the problems we had when we mixed gtkwebkit and the twisted reactor.

Yet another option would be to move the ping url code to the gui layer, where each toolkit would choose the best async way to do it. (a Gnome ui would use libsoup, etc).

[Natalia Bidart (nataliabidart)]

Alecu, thanks a lot for your input, it makes perfect sense.

Currently, with my not-very-good-connection, the ping will not last more than 2 seconds, so I think we can leave the code as is for now. I filed a bug to think/resolve this in the future (bug #697628).

=== modified file 'ubuntu_sso/credentials.py'
--- ubuntu_sso/credentials.py	2010-11-24 12:48:28 +0000
+++ ubuntu_sso/credentials.py	2011-01-02 03:31:12 +0000
@@ -41,8 +41,6 @@
 
 from functools import wraps
 
-import gobject
-
 from oauth import oauth
 from twisted.internet.defer import inlineCallbacks, returnValue
 
@@ -187,13 +185,15 @@
 
     @handle_failures(msg='Problem while retrieving credentials')
     @inlineCallbacks
-    def _login_success_cb(self, dialog, app_name, email):
+    def _login_success_cb(self, app_name, email):
         """Store credentials when the login/registration succeeded.
 
         Also, open self.ping_url/email to notify about this new token. If any
         error occur, self.error_cb is called. Otherwise, self.success_cb is
         called.
 
+        Return 0 on success, and a non-zero value (or None) on error.
+
         """
         logger.info('Login/registration was successful for app %r, email %r',
                     app_name, email)
@@ -202,19 +202,21 @@
             assert len(creds) > 0, 'Creds are empty! This should not happen'
             # ping a server with the credentials if we were requested to
             if self.ping_url is not None:
-                status = self._ping_url(app_name, email, creds)
+                status = yield self._ping_url(app_name, email, creds)
                 if status is None:
                     yield self.clear_credentials()
                     return
 
             self.success_cb(creds)
+            returnValue(0)
 
-    def _auth_denial_cb(self, dialog, app_name):
+    def _auth_denial_cb(self, app_name):
         """The user decided not to allow the registration or login."""
         logger.warning('Login/registration was denied to app %r', app_name)
         self.denial_cb(app_name)
 
-    @handle_exceptions(msg='Problem opening the ping_url')
+    @handle_failures(msg='Problem opening the ping_url')
+    @inlineCallbacks
     def _ping_url(self, app_name, email, credentials):
         """Ping the self.ping_url with the email attached.
 
@@ -237,9 +239,12 @@
         request = urllib2.Request(url, headers=oauth_req.to_header())
         logger.debug('Opening the url "%s" with urllib2.urlopen.',
                      request.get_full_url())
+        # This code is blocking, we should change this.
+        # I've tried with deferToThread an twisted.web.client.getPage
+        # but the returned deferred will never be fired (nataliabidart).
         response = urllib2.urlopen(request)
         logger.debug('Url opened. Response: %s.', response.code)
-        return response.code
+        returnValue(response.code)
 
     @handle_exceptions(msg='Problem opening the Ubuntu SSO user interface')
     def _show_ui(self, login_only):
@@ -252,10 +257,9 @@
                         tc_url=self.tc_url, help_text=self.help_text,
                         window_id=self.window_id, login_only=login_only)
 
-        self.gui.connect(gui.SIG_LOGIN_SUCCEEDED, self._login_success_cb)
-        self.gui.connect(gui.SIG_REGISTRATION_SUCCEEDED,
-                         self._login_success_cb)
-        self.gui.connect(gui.SIG_USER_CANCELATION, self._auth_denial_cb)
+        self.gui.login_success_callback = self._login_success_cb
+        self.gui.registration_success_callback = self._login_success_cb
+        self.gui.user_cancellation_callback = self._auth_denial_cb
 
     @handle_failures(msg='Problem while retrieving credentials')
     @inlineCallbacks
@@ -265,33 +269,22 @@
         if token is not None and len(token) > 0:
             self.success_cb(token)
         elif token == {}:
-            gobject.idle_add(self._show_ui, login_only)
+            self._show_ui(login_only)
         else:
             # something went wrong with find_credentials, already handled.
             logger.info('_login_or_register: call to "find_credentials" went '
                         'wrong, and was already handled.')
 
     def error_cb(self, error_dict):
-        """Handle error.
-
-        Notify the caller and finish the GUI with error.
-
-        """
+        """Handle error and notify the caller."""
+        logger.error('Calling error callback at %r (error is %r).',
+                     self._error_cb, error_dict)
         self._error_cb(self.app_name, error_dict)
-        if self.gui is not None:
-            self.gui.finish_error(error=error_dict)
-        self.gui = None
 
     def success_cb(self, creds):
-        """Handle success.
-
-        Notify the caller and finish the GUI with success.
-
-        """
+        """Handle success and notify the caller."""
+        logger.debug('Calling success callback at %r.', self._success_cb)
         self._success_cb(self.app_name, creds)
-        if self.gui is not None:
-            self.gui.finish_success()
-        self.gui = None
 
     @handle_failures(msg='Problem while retrieving credentials')
     @inlineCallbacks

=== modified file 'ubuntu_sso/gtk/gui.py'
--- ubuntu_sso/gtk/gui.py	2010-11-29 16:04:26 +0000
+++ ubuntu_sso/gtk/gui.py	2011-01-02 03:31:12 +0000
@@ -30,11 +30,11 @@
 
 import dbus
 import gettext
-import gobject
 import gtk
 import xdg
 
 from dbus.mainloop.glib import DBusGMainLoop
+from twisted.internet.defer import inlineCallbacks
 
 from ubuntu_sso import (DBUS_ACCOUNT_PATH, DBUS_BUS_NAME, DBUS_IFACE_USER_NAME,
     NO_OP)
@@ -72,20 +72,6 @@
 HELP_TEXT_COLOR = gtk.gdk.Color("#bfbfbf")
 WARNING_TEXT_COLOR = gtk.gdk.Color("red")
 
-SIG_LOGIN_SUCCEEDED = 'login-succeeded'
-SIG_REGISTRATION_SUCCEEDED = 'registration-succeeded'
-SIG_USER_CANCELATION = 'user-cancelation'
-
-SIGNAL_ARGUMENTS = [
-    (SIG_LOGIN_SUCCEEDED, (gobject.TYPE_STRING, gobject.TYPE_STRING,)),
-    (SIG_REGISTRATION_SUCCEEDED, (gobject.TYPE_STRING, gobject.TYPE_STRING,)),
-    (SIG_USER_CANCELATION, (gobject.TYPE_STRING,)),
-]
-
-for sig, sig_args in SIGNAL_ARGUMENTS:
-    gobject.signal_new(sig, gtk.Window, gobject.SIGNAL_RUN_FIRST,
-                       gobject.TYPE_NONE, sig_args)
-
 
 def get_data_dir():
     """Build absolute path to  the 'data' directory."""
@@ -198,7 +184,7 @@
 
 
 class UbuntuSSOClientGUI(object):
-    """Ubuntu single sign on GUI."""
+    """Ubuntu single sign-on GUI."""
 
     CAPTCHA_SOLUTION_ENTRY = _('Type the characters above')
     CAPTCHA_LOAD_ERROR = _('There was a problem getting the captcha, '
@@ -256,7 +242,7 @@
     CAPTCHA_RELOAD_TOOLTIP = _('Reload')
 
     def __init__(self, app_name, tc_url='', help_text='',
-                 window_id=0, login_only=False, close_callback=None):
+                 window_id=0, login_only=False):
         """Create the GUI and initialize widgets."""
         gtk.link_button_set_uri_hook(NO_OP)
 
@@ -270,7 +256,12 @@
         self.tc_url = tc_url
         self.help_text = help_text
         self.login_only = login_only
-        self.close_callback = close_callback
+
+        self.close_callback = NO_OP
+        self.login_success_callback = NO_OP
+        self.registration_success_callback = NO_OP
+        self.user_cancellation_callback = NO_OP
+
         self.user_email = None
         self.user_password = None
 
@@ -730,17 +721,12 @@
                      signal_name, handler, args, kwargs)
         self.window.connect(signal_name, handler, *args, **kwargs)
 
-    def emit(self, *args, **kwargs):
-        """Emit a signal proxing the main window."""
-        logger.debug('emit: args  %r, kwargs, %r', args, kwargs)
-        self.window.emit(*args, **kwargs)
-
     def finish_success(self):
         """The whole process was completed succesfully. Show success page."""
         self._done = True
         self._set_current_page(self.success_vbox)
 
-    def finish_error(self, error):
+    def finish_error(self):
         """The whole process was not completed succesfully. Show error page."""
         self._done = True
         self._set_current_page(self.error_vbox)
@@ -766,18 +752,17 @@
         if self.window is not None:
             self.window.hide()
 
-        # process any pending events before emitting signals
+        # process any pending events before callbacking with result
         while gtk.events_pending():
             gtk.main_iteration()
 
         if not self._done:
-            self.emit(SIG_USER_CANCELATION, self.app_name)
+            self.user_cancellation_callback(self.app_name)
 
         # call user defined callback
-        if self.close_callback is not None:
-            logger.info('Calling custom close_callback %r with params %r, %r',
-                        self.close_callback, args, kwargs)
-            self.close_callback(*args, **kwargs)
+        logger.info('Calling custom close_callback %r with params %r, %r',
+                    self.close_callback, args, kwargs)
+        self.close_callback(*args, **kwargs)
 
     def on_sign_in_button_clicked(self, *args, **kwargs):
         """User wants to sign in, present the Login page."""
@@ -1096,10 +1081,15 @@
         self._set_current_page(self.enter_details_vbox, warning_text=msg)
 
     @log_call
+    @inlineCallbacks
     def on_email_validated(self, app_name, email, *args, **kwargs):
         """User email was successfully verified."""
         self._done = True
-        self.emit(SIG_REGISTRATION_SUCCEEDED, self.app_name, email)
+        result = yield self.registration_success_callback(self.app_name, email)
+        if result == 0:
+            self.finish_success()
+        else:
+            self.finish_error()
 
     @log_call
     def on_email_validation_error(self, app_name, error, *args, **kwargs):
@@ -1112,10 +1102,15 @@
         self._set_current_page(self.verify_email_vbox, warning_text=msg)
 
     @log_call
+    @inlineCallbacks
     def on_logged_in(self, app_name, email, *args, **kwargs):
         """User was successfully logged in."""
         self._done = True
-        self.emit(SIG_LOGIN_SUCCEEDED, self.app_name, email)
+        result = yield self.login_success_callback(self.app_name, email)
+        if result == 0:
+            self.finish_success()
+        else:
+            self.finish_error()
 
     @log_call
     def on_login_error(self, app_name, error, *args, **kwargs):

=== modified file 'ubuntu_sso/gtk/tests/test_gui.py'
--- ubuntu_sso/gtk/tests/test_gui.py	2010-12-09 21:04:59 +0000
+++ ubuntu_sso/gtk/tests/test_gui.py	2011-01-02 03:31:12 +0000
@@ -517,6 +517,11 @@
         self.assertTrue(self._called,
                         'close_callback was called when window was closed.')
 
+    def test_close_callback_if_not_set(self):
+        """The close_callback is a no op if not set."""
+        self.ui.on_close_clicked()
+        # no crash when close_callback is not set
+
     def test_app_name_is_stored(self):
         """The app_name is stored for further use."""
         self.assertIn(APP_NAME, self.ui.app_name)
@@ -555,18 +560,6 @@
 
         self.assertEqual(self.ui.bus.callbacks, {})
 
-    def test_close_callback(self):
-        """A close_callback parameter is called when closing the window."""
-        ui = self.gui_class(close_callback=self._set_called, **self.kwargs)
-        ui.on_close_clicked()
-        self.assertTrue(self._called, 'close_callback was called on close.')
-
-    def test_close_callback_if_none(self):
-        """A close_callback parameter is not called if is None."""
-        ui = self.gui_class(close_callback=None, **self.kwargs)
-        ui.on_close_clicked()
-        # no crash when close_callback is None
-
     def test_pages_are_packed_into_container(self):
         """All the pages are packed in the main container."""
         children = self.ui.content.get_children()
@@ -620,8 +613,7 @@
         buttons = filter(lambda name: 'cancel_button' in name or
                                       'close_button' in name, self.ui.widgets)
         for name in buttons:
-            self.ui = self.gui_class(close_callback=self._set_called,
-                                     **self.kwargs)
+            self.ui.close_callback = self._set_called
             widget = getattr(self.ui, name)
             widget.clicked()
             self.assertEqual(self._called, ((widget,), {}), msg % name)
@@ -661,7 +653,7 @@
 
     def test_finish_error_shows_error_page(self):
         """When calling 'finish_error' the error page is shown."""
-        self.ui.finish_error(error=self.error)
+        self.ui.finish_error()
         self.assert_pages_visibility(finish=True)
         self.assertEqual(self.ui.ERROR, self.ui.finish_vbox.label.get_text())
 
@@ -1164,10 +1156,10 @@
         self.click_verify_email_with_valid_data()
         self.assert_warnings_visibility()
 
-    def test_on_email_validated_shows_processing_page(self):
-        """On email validated the procesing page is still shown."""
+    def test_on_email_validated_shows_finish_page(self):
+        """On email validated the finish page is shown."""
         self.ui.on_email_validated(app_name=APP_NAME, email=EMAIL)
-        self.assert_pages_visibility(processing=True)
+        self.assert_pages_visibility(finish=True)
 
     def test_on_email_validated_does_not_clear_the_help_text(self):
         """On email validated the help text is not removed."""
@@ -1227,6 +1219,32 @@
         self.ui.verify_token_button.clicked()
         self.assertTrue(self._called)
 
+    def test_after_registration_success_finish_success(self):
+        """After REGISTRATION_SUCCESS is called, finish_success is called.
+
+        Only when REGISTRATION_SUCCESS returns 0.
+
+        """
+        self.patch(self.ui, 'finish_success', self._set_called)
+        self.ui.registration_success_callback = lambda app, email: 0
+
+        self.ui.on_email_validated(app_name=APP_NAME, email=EMAIL)
+
+        self.assertEqual(self._called, ((), {}))
+
+    def test_after_registration_error_finish_error(self):
+        """After REGISTRATION_SUCCESS is called, finish_error is called.
+
+        Only when REGISTRATION_SUCCESS returns a non-zero value.
+
+        """
+        self.patch(self.ui, 'finish_error', self._set_called)
+        self.ui.registration_success_callback = lambda app, email: -1
+
+        self.ui.on_email_validated(app_name=APP_NAME, email=EMAIL)
+
+        self.assertEqual(self._called, ((), {}))
+
 
 class VerifyEmailValidationTestCase(UbuntuSSOClientTestCase):
     """Test suite for the user registration validation (verify email page)."""
@@ -1480,11 +1498,11 @@
         self.click_connect_with_valid_data()
         self.assert_pages_visibility(processing=True)
 
-    def test_on_logged_in_morphs_to_processing_page(self):
-        """When user logged in the processing page is still shown."""
+    def test_on_logged_in_morphs_to_finish_page(self):
+        """When user logged in the finish page is shown."""
         self.click_connect_with_valid_data()
         self.ui.on_logged_in(app_name=APP_NAME, email=EMAIL)
-        self.assert_pages_visibility(processing=True)
+        self.assert_pages_visibility(finish=True)
 
     def test_on_login_error_morphs_to_login_page(self):
         """On user login error, the previous page is shown."""
@@ -1541,6 +1559,32 @@
         self.assertEqual(self.ui.user_email, EMAIL)
         self.assertEqual(self.ui.user_password, PASSWORD)
 
+    def test_after_login_success_finish_success(self):
+        """After LOGIN_SUCCESSFULL is called, finish_success is called.
+
+        Only when LOGIN_SUCCESSFULL returns 0.
+
+        """
+        self.patch(self.ui, 'finish_success', self._set_called)
+        self.ui.login_success_callback = lambda app, email: 0
+
+        self.ui.on_logged_in(app_name=APP_NAME, email=EMAIL)
+
+        self.assertEqual(self._called, ((), {}))
+
+    def test_after_login_error_finish_error(self):
+        """After LOGIN_SUCCESSFULL is called, finish_error is called.
+
+        Only when LOGIN_SUCCESSFULL returns a non-zero value.
+
+        """
+        self.patch(self.ui, 'finish_error', self._set_called)
+        self.ui.login_success_callback = lambda app, email: -1
+
+        self.ui.on_logged_in(app_name=APP_NAME, email=EMAIL)
+
+        self.assertEqual(self._called, ((), {}))
+
 
 class LoginValidationTestCase(UbuntuSSOClientTestCase):
     """Test suite for the user login validation."""
@@ -2060,119 +2104,144 @@
         self.assertEqual(self.ui.help_label.get_text(), HELP_TEXT)
 
 
-class SignalsTestCase(UbuntuSSOClientTestCase):
-    """Test the GTK signal emission."""
+class CallbacksTestCase(UbuntuSSOClientTestCase):
+    """Test the GTK callback calls."""
+
+    LOGIN_SUCCESSFULL = 'login_success_callback'
+    REGISTRATION_SUCCESS = 'registration_success_callback'
+    USER_CANCELLATION = 'user_cancellation_callback'
 
     def setUp(self):
         """Init."""
-        super(SignalsTestCase, self).setUp()
+        super(CallbacksTestCase, self).setUp()
         self._called = {}
-        for sig_name, _ in gui.SIGNAL_ARGUMENTS:
-            self.ui.connect(sig_name, self._set_called, sig_name)
-
-    def _set_called(self, widget, *args, **kwargs):
-        """Keep trace of signals emition."""
+        for name in (self.LOGIN_SUCCESSFULL,
+                     self.REGISTRATION_SUCCESS,
+                     self.USER_CANCELLATION):
+            setattr(self.ui, name, self._set_called(name))
+
+    def _set_called(self, callback_name):
+        """Keep trace of callbacks calls."""
+
         # pylint: disable=W0221
-        self._called[args[-1]] = (widget, args[:-1], kwargs)
-
-    def test_closing_main_window_sends_outcome_as_signal(self):
-        """A signal is sent when closing the main window."""
+
+        def inner(*args, **kwargs):
+            """Store arguments used in this call."""
+            self._called[callback_name] = args
+
+        return inner
+
+    def test_closing_main_window(self):
+        """When closing the main window, USER_CANCELLATION is called."""
         self.ui.window.emit('delete-event', gtk.gdk.Event(gtk.gdk.DELETE))
-        expected = (self.ui.window, (APP_NAME,), {})
-        self.assertEqual(self._called[gui.SIG_USER_CANCELATION], expected)
+        self.assertEqual(self._called[self.USER_CANCELLATION], (APP_NAME,))
 
-    def test_every_cancel_emits_proper_signal(self):
-        """Clicking on any cancel button, 'user-cancelation' signal is sent."""
-        sig = gui.SIG_USER_CANCELATION
-        msg = 'user-cancelation is emitted when "%s" is clicked.'
+    def test_every_cancel_calls_proper_callback(self):
+        """When any cancel button is clicked, USER_CANCELLATION is called."""
+        msg = 'user_cancellation_callback is called when "%s" is clicked.'
         buttons = filter(lambda name: 'cancel_button' in name, self.ui.widgets)
         for name in buttons:
-            self.ui = self.gui_class(**self.kwargs)
-            self.ui.connect(sig, self._set_called, sig)
             widget = getattr(self.ui, name)
             widget.clicked()
-            expected_args = (self.ui.window, (APP_NAME,), {})
-            self.assertEqual(self._called[sig], expected_args, msg % name)
+            self.assertEqual(self._called[self.USER_CANCELLATION], (APP_NAME,),
+                             msg % name)
             self._called = {}
 
-    def test_on_user_registration_error_proper_signal_is_emitted(self):
-        """On UserRegistrationError, SIG_USER_CANCELATION signal is sent."""
+    def test_on_user_registration_error_proper_callback_is_called(self):
+        """On UserRegistrationError, USER_CANCELLATION is called."""
         self.ui.on_user_registration_error(app_name=APP_NAME, error=self.error)
         self.ui.on_close_clicked()
-        expected = (self.ui.window, (APP_NAME,), {})
-        self.assertEqual(expected,
-                         self._called[gui.SIG_USER_CANCELATION])
-
-    def test_on_email_validated_proper_signals_is_emitted(self):
-        """On EmailValidated, 'registration-succeeded' signal is sent."""
+
+        self.assertEqual(self._called[self.USER_CANCELLATION], (APP_NAME,))
+
+    def test_on_email_validated_proper_callback_is_called(self):
+        """On EmailValidated, REGISTRATION_SUCCESS is called."""
         self.ui.on_email_validated(app_name=APP_NAME, email=EMAIL)
         self.ui.on_close_clicked()
-        self.assertEqual((self.ui.window, (APP_NAME, EMAIL), {}),
-                          self._called[gui.SIG_REGISTRATION_SUCCEEDED])
-
-    def test_on_email_validation_error_proper_signals_is_emitted(self):
-        """On EmailValidationError, SIG_USER_CANCELATION signal is sent."""
+
+        self.assertEqual(self._called[self.REGISTRATION_SUCCESS],
+                         (APP_NAME, EMAIL))
+
+    def test_on_email_validation_error_proper_callback_is_called(self):
+        """On EmailValidationError, USER_CANCELLATION is called."""
         self.ui.on_email_validation_error(app_name=APP_NAME, error=self.error)
         self.ui.on_close_clicked()
-        expected = (self.ui.window, (APP_NAME,), {})
-        self.assertEqual(expected,
-                         self._called[gui.SIG_USER_CANCELATION])
-
-    def test_on_logged_in_proper_signals_is_emitted(self):
-        """On LoggedIn, 'login-succeeded' signal is sent."""
+
+        self.assertEqual(self._called[self.USER_CANCELLATION], (APP_NAME,))
+
+    def test_on_logged_in_proper_callback_is_called(self):
+        """On LoggedIn, LOGIN_SUCCESSFULL is called."""
         self.ui.on_logged_in(app_name=APP_NAME, email=EMAIL)
         self.ui.on_close_clicked()
-        self.assertEqual((self.ui.window, (APP_NAME, EMAIL), {}),
-                          self._called[gui.SIG_LOGIN_SUCCEEDED])
-
-    def test_on_login_error_proper_signals_is_emitted(self):
-        """On LoginError, SIG_USER_CANCELATION signal is sent."""
+
+        self.assertEqual(self._called[self.LOGIN_SUCCESSFULL],
+                         (APP_NAME, EMAIL))
+
+    def test_on_login_error_proper_callback_is_called(self):
+        """On LoginError, USER_CANCELLATION is called."""
         self.click_connect_with_valid_data()
         self.ui.on_login_error(app_name=APP_NAME, error=self.error)
         self.ui.on_close_clicked()
-        expected = (self.ui.window, (APP_NAME,), {})
-        self.assertEqual(expected,
-                         self._called[gui.SIG_USER_CANCELATION])
-
-    def test_registration_successfull_even_if_prior_registration_error(self):
-        """Only one signal is sent with the final outcome."""
+
+        self.assertEqual(self._called[self.USER_CANCELLATION], (APP_NAME,))
+
+    def test_registration_success_even_if_prior_registration_error(self):
+        """Only one callback is called with the final outcome.
+
+        When the user successfully registers, REGISTRATION_SUCCESS is
+        called even if there were errors before.
+
+        """
         self.click_join_with_valid_data()
         self.ui.on_user_registration_error(app_name=APP_NAME, error=self.error)
         self.click_join_with_valid_data()
         self.ui.on_email_validated(app_name=APP_NAME, email=EMAIL)
         self.ui.on_close_clicked()
 
-        self.assertEqual(len(self._called), 1)
-        self.assertTrue(gui.SIG_REGISTRATION_SUCCEEDED in self._called)
-
-    def test_login_successfull_even_if_prior_login_error(self):
-        """Only one signal is sent with the final outcome."""
+        self.assertEqual(self._called[self.REGISTRATION_SUCCESS],
+                         (APP_NAME, EMAIL))
+
+    def test_login_success_even_if_prior_login_error(self):
+        """Only one callback is called with the final outcome.
+
+        When the user successfully logins, LOGIN_SUCCESSFULL is called even if
+        there were errors before.
+
+        """
         self.click_connect_with_valid_data()
         self.ui.on_login_error(app_name=APP_NAME, error=self.error)
         self.click_connect_with_valid_data()
         self.ui.on_logged_in(app_name=APP_NAME, email=EMAIL)
         self.ui.on_close_clicked()
 
-        self.assertEqual(len(self._called), 1)
-        self.assertTrue(gui.SIG_LOGIN_SUCCEEDED in self._called)
+        self.assertEqual(self._called[self.LOGIN_SUCCESSFULL],
+                         (APP_NAME, EMAIL))
 
     def test_user_cancelation_even_if_prior_registration_error(self):
-        """Only one signal is sent with the final outcome."""
+        """Only one callback is called with the final outcome.
+
+        When the user closes the window, USER_CANCELLATION is called even if
+        there were registration errors before.
+
+        """
         self.click_join_with_valid_data()
         self.ui.on_user_registration_error(app_name=APP_NAME, error=self.error)
         self.ui.join_cancel_button.clicked()
 
-        self.assertEqual(len(self._called), 1)
-        self.assertTrue(gui.SIG_USER_CANCELATION in self._called)
+        self.assertEqual(self._called[self.USER_CANCELLATION], (APP_NAME,))
 
     def test_user_cancelation_even_if_prior_login_error(self):
-        """Only one signal is sent with the final outcome."""
+        """Only one callback is called with the final outcome.
+
+        When the user closes the window, USER_CANCELLATION is called even if
+        there were login errors before.
+
+        """
         self.click_connect_with_valid_data()
         self.ui.on_login_error(app_name=APP_NAME, error=self.error)
         self.ui.login_cancel_button.clicked()
 
-        self.assertEqual(len(self._called), 1)
-        self.assertTrue(gui.SIG_USER_CANCELATION in self._called)
+        self.assertEqual(self._called[self.USER_CANCELLATION], (APP_NAME,))
 
 
 class DefaultButtonsTestCase(UbuntuSSOClientTestCase):

=== modified file 'ubuntu_sso/tests/test_credentials.py'
--- ubuntu_sso/tests/test_credentials.py	2010-12-20 14:29:18 +0000
+++ ubuntu_sso/tests/test_credentials.py	2011-01-02 03:31:12 +0000
@@ -21,8 +21,6 @@
 import logging
 import urllib
 
-import gobject
-
 from twisted.internet import defer
 from twisted.internet.defer import inlineCallbacks
 from twisted.trial.unittest import TestCase, FailTest
@@ -60,10 +58,6 @@
     WINDOW_ID_KEY: WINDOW_ID,
 }
 
-SIG_LOGIN_SUCCEEDED = '1'
-SIG_REGISTRATION_SUCCEEDED = '2'
-SIG_USER_CANCELATION = '3'
-
 
 class SampleMiscException(Exception):
     """An error to be used while testing."""
@@ -82,17 +76,9 @@
     def __init__(self, *args, **kwargs):
         self.args = args
         self.kwargs = kwargs
-        self.signals = []
-        self.methods = []
-        self.connect = lambda *a: self.signals.append(a)
-
-    def finish_success(self):
-        """Fake the success finish."""
-        self.methods.append('finish_success')
-
-    def finish_error(self, error):
-        """Fake the error finish."""
-        self.methods.append(('finish_error', error))
+        self.login_success_callback = None
+        self.registration_success_callback = None
+        self.user_cancellation_callback = None
 
 
 class BasicTestCase(TestCase):
@@ -100,7 +86,6 @@
 
     def setUp(self):
         """Init."""
-        self.patch(gobject, 'idle_add', lambda f, *a, **kw: f(*a, **kw))
         self._called = False  # helper
 
         self.memento = MementoHandler()
@@ -226,45 +211,21 @@
 
         self.assertEqual(getattr(self.obj, UI_MODULE_KEY), GTK_GUI_MODULE)
 
-    def test_success_cb_gui_none(self):
-        """Success callback calls the caller but not the GUI."""
-        self.obj.gui = None
-        self.obj.success_cb(creds=TOKEN)
-
-        self.assertEqual(self._called, (('success', APP_NAME, TOKEN), {}),
-                         'caller _success_cb was called.')
-
-    def test_success_cb_gui_not_none(self):
-        """Success callback calls the caller and finish the GUI."""
-        self.obj.gui = ui = FakedClientGUI(APP_NAME)
-        self.obj.success_cb(creds=TOKEN)
-
-        self.assertEqual(self._called, (('success', APP_NAME, TOKEN), {}),
-                         'caller _success_cb was called.')
-        self.assertEqual(ui.methods, ['finish_success'],
-                         'GUI finish_success was called.')
-        self.assertTrue(self.obj.gui is None, 'gui is reset to None.')
-
-    def test_error_cb_gui_none(self):
-        """Error callback calls the caller but not the GUI."""
-        self.obj.gui = None
-        error_dict = {'foo': 'bar'}
-        self.obj.error_cb(error_dict=error_dict)
-
-        self.assertEqual(self._called, (('error', APP_NAME, error_dict), {}),
-                         'caller _error_cb was called.')
-
-    def test_error_cb_gui_not_none(self):
-        """Error callback calls the caller and finish the GUI."""
-        self.obj.gui = ui = FakedClientGUI(APP_NAME)
-        error_dict = {'foo': 'bar'}
-        self.obj.error_cb(error_dict=error_dict)
-
-        self.assertEqual(self._called, (('error', APP_NAME, error_dict), {}),
-                         'caller _error_cb was called.')
-        self.assertEqual(ui.methods, [('finish_error', error_dict)],
-                         'GUI finish_error was called.')
-        self.assertTrue(self.obj.gui is None, 'gui is reset to None.')
+    def test_success_cb(self):
+        """Success callback calls the caller."""
+        self.obj.gui = None
+        self.obj.success_cb(creds=TOKEN)
+
+        self.assertEqual(self._called, (('success', APP_NAME, TOKEN), {}),
+                         'caller _success_cb was called.')
+
+    def test_error_cb(self):
+        """Error callback calls the caller."""
+        error_dict = {'foo': 'bar'}
+        self.obj.error_cb(error_dict=error_dict)
+
+        self.assertEqual(self._called, (('error', APP_NAME, error_dict), {}),
+                         'caller _error_cb was called.')
 
 
 class CredentialsLoginSuccessTestCase(CredentialsTestCase):
@@ -276,12 +237,12 @@
         self.patch(credentials.Keyring, 'get_credentials',
                    lambda kr, app: defer.succeed(None))
 
-        yield self.obj._login_success_cb(dialog=None, app_name=APP_NAME,
-                                         email=EMAIL)
+        result = yield self.obj._login_success_cb(APP_NAME, EMAIL)
 
         msg = 'Creds are empty! This should not happen'
         self.assert_error_cb_called(msg='Problem while retrieving credentials',
                                     detailed_error=AssertionError(msg))
+        self.assertEqual(result, None)
 
     @inlineCallbacks
     def test_cred_error(self):
@@ -290,11 +251,11 @@
         self.patch(credentials.Keyring, 'get_credentials',
                    lambda kr, app: defer.fail(expected_error))
 
-        yield self.obj._login_success_cb(dialog=None, app_name=APP_NAME,
-                                         email=EMAIL)
+        result = yield self.obj._login_success_cb(APP_NAME, EMAIL)
 
         msg = 'Problem while retrieving credentials'
         self.assert_error_cb_called(msg=msg, detailed_error=expected_error)
+        self.assertEqual(result, None)
         self.assertTrue(self.memento.check_exception(SampleMiscException))
 
     @inlineCallbacks
@@ -304,10 +265,10 @@
                    lambda kr, app: defer.succeed(TOKEN))
         self.patch(self.obj, '_ping_url', lambda *a, **kw: 200)
 
-        yield self.obj._login_success_cb(dialog=None, app_name=APP_NAME,
-                                         email=EMAIL)
+        result = yield self.obj._login_success_cb(APP_NAME, EMAIL)
 
         self.assertEqual(self._called, (('success', APP_NAME, TOKEN), {}))
+        self.assertEqual(result, 0)
 
     @inlineCallbacks
     def test_ping_error(self):
@@ -324,12 +285,12 @@
         self.patch(self.obj, 'clear_credentials',
                    lambda: setattr(self, '_cred_cleared', True))
 
-        yield self.obj._login_success_cb(dialog=None, app_name=APP_NAME,
-                                         email=EMAIL)
+        result = yield self.obj._login_success_cb(APP_NAME, EMAIL)
 
         # error cb called correctly
         msg = 'Problem opening the ping_url'
         self.assert_error_cb_called(msg=msg, detailed_error=FailTest(error))
+        self.assertEqual(result, None)
 
         # credentials cleared
         self.assertTrue(self._cred_cleared)
@@ -349,8 +310,7 @@
         self.patch(self.obj, '_ping_url',
                    lambda *a, **kw: setattr(self, '_url_pinged', (a, kw)))
 
-        yield self.obj._login_success_cb(dialog=None, app_name=APP_NAME,
-                                         email=EMAIL)
+        yield self.obj._login_success_cb(APP_NAME, EMAIL)
 
         self.assertEqual(self._url_pinged, ((APP_NAME, EMAIL, TOKEN), {}))
 
@@ -366,10 +326,10 @@
         self.patch(self.obj, 'clear_credentials', self._set_called)
         self.obj.ping_url = None
 
-        yield self.obj._login_success_cb(dialog=None, app_name=APP_NAME,
-                                         email=EMAIL)
+        result = yield self.obj._login_success_cb(APP_NAME, EMAIL)
 
         self.assertEqual(self._called, (('success', APP_NAME, TOKEN), {}))
+        self.assertEqual(result, 0)
 
 
 class CredentialsAuthDeniedTestCase(CredentialsTestCase):
@@ -377,10 +337,9 @@
 
     def test_auth_denial_cb(self):
         """On auth denied, self.denial_cb is called."""
-        self.obj.gui = ui = FakedClientGUI(APP_NAME)
-        self.obj._auth_denial_cb(dialog=None, app_name=APP_NAME)
+        self.obj._auth_denial_cb(app_name=APP_NAME)
+
         self.assertEqual(self._called, (('denial', APP_NAME), {}))
-        self.assertEqual(ui.methods, [], 'no GUI method was called.')
 
 
 class PingUrlTestCase(CredentialsTestCase):
@@ -401,24 +360,29 @@
 
         self.patch(credentials.urllib2, 'urlopen', faked_urlopen)
 
+    @inlineCallbacks
     def test_ping_url_if_url_is_none(self):
         """self.ping_url is opened."""
         self.patch(credentials.urllib2, 'urlopen', self.fail)
         self.obj.ping_url = None
-        self.obj._ping_url(app_name=APP_NAME, email=EMAIL, credentials=TOKEN)
+        yield self.obj._ping_url(app_name=APP_NAME, email=EMAIL,
+                                 credentials=TOKEN)
         # no failure
 
+    @inlineCallbacks
     def test_ping_url(self):
         """On auth success, self.ping_url is opened."""
-        self.obj._ping_url(app_name=APP_NAME, email=EMAIL, credentials=TOKEN)
+        yield self.obj._ping_url(app_name=APP_NAME, email=EMAIL,
+                                 credentials=TOKEN)
 
         self.assertIsInstance(self._request, credentials.urllib2.Request)
         self.assertEqual(self._request.get_full_url(),
                          self.obj.ping_url + EMAIL)
 
+    @inlineCallbacks
     def test_request_is_signed_with_credentials(self):
         """The http request to self.ping_url is signed with the credentials."""
-        result = self.obj._ping_url(APP_NAME, EMAIL, TOKEN)
+        result = yield self.obj._ping_url(APP_NAME, EMAIL, TOKEN)
         headers = self._request.headers
 
         self.assertIn('Authorization', headers)
@@ -431,12 +395,13 @@
         self.assertIn(expected, oauth_stuff)
         self.assertEqual(result, 200)
 
+    @inlineCallbacks
     def test_ping_url_error(self):
         """Exception is handled if ping fails."""
         error = 'Blu'
         self.patch(credentials.urllib2, 'urlopen', lambda r: self.fail(error))
 
-        self.obj._ping_url(APP_NAME, EMAIL, TOKEN)
+        yield self.obj._ping_url(APP_NAME, EMAIL, TOKEN)
 
         msg = 'Problem opening the ping_url'
         self.assert_error_cb_called(msg=msg, detailed_error=FailTest(error))
@@ -592,17 +557,17 @@
 
     @inlineCallbacks
     def test_connects_gui_signals(self):
-        """Outcome signals from GUI are connected to callbacks."""
+        """GUI callbacks are properly connected."""
         self.patch(credentials.Keyring, 'get_credentials',
                    lambda kr, app: defer.succeed(None))
-
-        expected = [
-            (SIG_LOGIN_SUCCEEDED, self.obj._login_success_cb),
-            (SIG_REGISTRATION_SUCCEEDED, self.obj._login_success_cb),
-            (SIG_USER_CANCELATION, self.obj._auth_denial_cb),
-        ]
         yield getattr(self.obj, self.operation)()
-        self.assertEqual(self.obj.gui.signals, expected)
+
+        self.assertEqual(self.obj.gui.login_success_callback,
+                         self.obj._login_success_cb)
+        self.assertEqual(self.obj.gui.registration_success_callback,
+                         self.obj._login_success_cb)
+        self.assertEqual(self.obj.gui.user_cancellation_callback,
+                         self.obj._auth_denial_cb)
 
     @inlineCallbacks
     def test_gui_is_created(self):

=== modified file 'ubuntu_sso/tests/test_main.py'
--- ubuntu_sso/tests/test_main.py	2011-01-02 03:31:12 +0000
+++ ubuntu_sso/tests/test_main.py	2011-01-02 03:31:12 +0000
@@ -702,6 +702,7 @@
 class CredentialsManagementTestCase(TestCase):
     """Tests for the CredentialsManagement DBus interface."""
 
+    timeout = 2
     base_args = {HELP_TEXT_KEY: HELP_TEXT, PING_URL_KEY: PING_URL,
                  TC_URL_KEY: TC_URL, WINDOW_ID_KEY: WINDOW_ID,
                  UI_CLASS_KEY: 'SuperUI', UI_MODULE_KEY: 'foo.bar.baz',
@@ -739,10 +740,8 @@
         self.assertIsInstance(self.client, ubuntu_sso.main.dbus.service.Object)
 
 
-class CredentialsManagementFindClearTestCase(CredentialsManagementTestCase):
-    """Tests for the CredentialsManagement find/clear/store methods."""
-
-    timeout = 5
+class CredentialsManagementFindTestCase(CredentialsManagementTestCase):
+    """Tests for the CredentialsManagement find method."""
 
     def test_find_credentials(self):
         """The credentials are asked and returned in signals."""
@@ -808,6 +807,10 @@
         self.client.find_credentials(APP_NAME, self.args)
         return d
 
+
+class CredentialsManagementClearTestCase(CredentialsManagementTestCase):
+    """Tests for the CredentialsManagement clear method."""
+
     def test_clear_credentials(self):
         """The credentials are removed."""
         self.create_mock_backend().clear_credentials()
@@ -840,6 +843,10 @@
         self.client.clear_credentials(APP_NAME, self.args)
         return d
 
+
+class CredentialsManagementStoreTestCase(CredentialsManagementTestCase):
+    """Tests for the CredentialsManagement store method."""
+
     def test_store_credentials(self):
         """The credentials are stored and the outcome is a signal."""
         self.create_mock_backend().store_credentials(TOKEN)