Merge lp:~diegosarmentero/ubuntu-sso-client/qt-login-backend into lp:ubuntu-sso-client

[Diego Sarmentero]

- Adding some checks to setup_page (LP: #951461).

[lp:~diegosarmentero/ubuntu-sso-client/qt-login-backend]

911. By Diego Sarmentero on 2012-03-14

Merge

912. By Diego Sarmentero on 2012-03-14

improve in exception to avoid repeating text.

[Roberto Alsina (ralsina)]

Looks good, at least we'll get the error, I guess.

review: Approve

[Natalia Bidart (nataliabidart)]

Looks good, but the following:

        except Exception, reason:
            message = 'There was a problem trying to setup the page %r' % self
            self.show_error(message)
            log = message + ', error: %r' % reason
            logger.error(log)
            self.setEnabled(False)

should be:

        except:
            message = 'There was a problem trying to setup the page %r' % self
            self.show_error(message)
            logger.exception(message)
            self.setEnabled(False)

since logger.exception already adds the traceback when called in an except block.

review: Needs Fixing

[lp:~diegosarmentero/ubuntu-sso-client/qt-login-backend]

913. By Diego Sarmentero on 2012-03-15

Improving logs

914. By Diego Sarmentero on 2012-03-15

Reverting removed empty lines.

[Diego Sarmentero (diegosarmentero)]

> Looks good, but the following:
>
> except Exception, reason:
> message = 'There was a problem trying to setup the page %r' % self
> self.show_error(message)
> log = message + ', error: %r' % reason
> logger.error(log)
> self.setEnabled(False)
>
> should be:
>
> except:
> message = 'There was a problem trying to setup the page %r' % self
> self.show_error(message)
> logger.exception(message)
> self.setEnabled(False)
>
> since logger.exception already adds the traceback when called in an except
> block.

Done

[Natalia Bidart (nataliabidart)]

Lint issue at:

ubuntu_sso/qt/sso_wizard_page.py:
    226: [W0702, SSOWizardPage.setup_page] No exception type(s) specified

review: Needs Fixing

[lp:~diegosarmentero/ubuntu-sso-client/qt-login-backend]

915. By Diego Sarmentero on 2012-03-15

lint issue fixed.

[Natalia Bidart (nataliabidart)]

Looks good!

review: Approve

=== modified file 'ubuntu_sso/qt/sso_wizard_page.py'
--- ubuntu_sso/qt/sso_wizard_page.py	2012-03-13 02:25:12 +0000
+++ ubuntu_sso/qt/sso_wizard_page.py	2012-03-15 20:19:17 +0000
@@ -210,12 +210,25 @@
     @defer.inlineCallbacks
     def setup_page(self):
         """Setup the widget components."""
-        client = yield main.get_sso_client()
-        self.backend = client.sso_login
-        logger.info('%r - setup_page, backend is %r.', self, self.backend)
-        self._setup_signals()
-        self._set_translated_strings()
-        self._connect_ui()
+        logger.info('Starting setup_page for: %r', self)
+        # pylint: disable=W0702,W0703
+        try:
+            # Get Backend
+            client = yield main.get_sso_client()
+            self.backend = client.sso_login
+            self._set_translated_strings()
+            self._connect_ui()
+            # Call _setup_signals at the end, so we ensure that the UI
+            # is at least styled as expected if the operations with the
+            # backend fails.
+            self._setup_signals()
+        except:
+            message = 'There was a problem trying to setup the page %r' % self
+            self.show_error(message)
+            logger.exception(message)
+            self.setEnabled(False)
+        # pylint: enable=W0702,W0703
+        logger.info('%r - setup_page ends, backend is %r.', self, self.backend)
 
     def _filter_by_app_name(self, f):
         """Excecute the decorated function only for 'self.app_name'."""

=== modified file 'ubuntu_sso/qt/tests/__init__.py'
--- ubuntu_sso/qt/tests/__init__.py	2012-03-13 03:15:19 +0000
+++ ubuntu_sso/qt/tests/__init__.py	2012-03-15 20:19:17 +0000
@@ -551,7 +551,7 @@
             self.assertIn(signal, self.ui._signals)
             self.assertTrue(callable(self.ui._signals[signal]))
 
-        expected = ['_setup_signals', '_set_translated_strings', '_connect_ui']
+        expected = ['_set_translated_strings', '_connect_ui', '_setup_signals']
         self.assertEqual(expected, called)
 
 

=== modified file 'ubuntu_sso/qt/tests/test_sso_wizard_page.py'
--- ubuntu_sso/qt/tests/test_sso_wizard_page.py	2012-03-13 02:14:03 +0000
+++ ubuntu_sso/qt/tests/test_sso_wizard_page.py	2012-03-15 20:19:17 +0000
@@ -130,8 +130,8 @@
         """Test show_error with a long length string."""
         message = build_string_for_pixels(self.ui.form_errors_label,
                                           self.ui.header.max_title_width + 10)
+
         self.ui.show_error(message)
-
         self.assert_error_correct(self.ui.form_errors_label, message,
                                   self.ui.header.max_title_width)
 
@@ -140,3 +140,13 @@
         self.ui.hide_error()
 
         self.assertEqual(self.ui.form_errors_label.text(), ' ')
+
+    def test_setup_page_with_failing_backend(self):
+        """Test how the ui react with an invalid backend."""
+        self.patch(gui.main, "get_sso_client", lambda: None)
+        self.patch(self.ui, "show_error", self._set_called)
+        self.ui.setup_page()
+        reason = 'There was a problem trying to setup the page %r' % self.ui
+        expected = ((reason,), {})
+        self.assertEqual(expected, self._called)
+        self.assertFalse(self.ui.isEnabled())