Merge lp:~nataliabidart/ubuntu-sso-client/fallback-to-gtk into lp:ubuntu-sso-client

[Natalia Bidart]

- Fallback to the GTK+ UI when the specified ui_executable does not exist
  (LP: #939821).

[lp:~nataliabidart/ubuntu-sso-client/fallback-to-gtk]

890. By Natalia Bidart on 2012-02-24

Merged trunk in.

891. By Natalia Bidart on 2012-02-24

Fixed lint issue.

[Roberto Alsina (ralsina)]

+1

review: Approve

[lp:~nataliabidart/ubuntu-sso-client/fallback-to-gtk]

892. By Natalia Bidart on 2012-02-24

Typo fixed.

[dobey (dobey)]

9 + logger.debug('Fallbacking to the GTK+ UI since the given %r '

"Fallbacking" isn't a word, but "Falling back" is probably what you mean here?

58 + """The executable fallbacks to the GTK+ UI if given does not exist."""

Likewise, instead of "fallbacks" (which would be plural of fallback), you want to use "falls back to" here.

review: Needs Fixing

[lp:~nataliabidart/ubuntu-sso-client/fallback-to-gtk]

893. By Natalia Bidart on 2012-02-24

Correct english grammar.

[dobey (dobey)]

review: Approve

=== modified file 'ubuntu_sso/credentials.py'
--- ubuntu_sso/credentials.py	2012-02-17 18:43:17 +0000
+++ ubuntu_sso/credentials.py	2012-02-24 19:51:20 +0000
@@ -142,6 +142,13 @@
     def _show_ui(self, login_only):
         """Shows the UI, connect outcome signals."""
         ui_exe_path = os.path.join(get_bin_dir(), self.ui_executable)
+        if not os.path.exists(ui_exe_path):
+            logger.debug('Falling back to the GTK+ UI since the given %r '
+                         'does not exist', ui_exe_path)
+            ui_exe_path = os.path.join(get_bin_dir(), UI_EXECUTABLE_GTK)
+
+        assert os.path.exists(ui_exe_path)
+
         args = [ui_exe_path]
         for arg in ('app_name', 'help_text', 'ping_url', 'policy_url',
                     'tc_url', 'window_id'):

=== modified file 'ubuntu_sso/tests/test_credentials.py'
--- ubuntu_sso/tests/test_credentials.py	2012-02-11 19:25:01 +0000
+++ ubuntu_sso/tests/test_credentials.py	2012-02-24 19:51:20 +0000
@@ -272,8 +272,10 @@
         yield super(RegisterTestCase, self).setUp()
         self.deferred = defer.Deferred()
         self.patch_inner(self.fake_inner)
+        self.exe_path = os.path.join(utils.get_bin_dir(),
+                                     KWARGS[UI_EXECUTABLE_KEY])
         self.inner_args = [
-            os.path.join(utils.get_bin_dir(), KWARGS[UI_EXECUTABLE_KEY]),
+            self.exe_path,
             '--app_name', APP_NAME,
             '--help_text', HELP_TEXT,
             '--ping_url', PING_URL,
@@ -332,11 +334,35 @@
                    lambda kr, app: defer.succeed(None))
         self._next_inner_result = credentials.USER_SUCCESS
 
-        result = yield self.method_call(**self.kwargs)
-        self.assertEqual(result, TOKEN)
-
-        # the ui was opened and proper params were passed
-        args = yield self.deferred
+        # patch os.path.exists so it also returns True when queried for the
+        # faked exe_path
+        really_exists = os.path.exists
+        self.patch(credentials.os.path, 'exists',
+                   lambda path: path == self.exe_path or really_exists(path))
+
+        result = yield self.method_call(**self.kwargs)
+        self.assertEqual(result, TOKEN)
+
+        # the ui was opened and proper params were passed
+        args = yield self.deferred
+        self.assertEqual(self.inner_args, args)
+
+    @defer.inlineCallbacks
+    def test_ui_executable_falls_back_to_gtk(self):
+        """The executable falls back to the GTK+ UI if given does not exist."""
+        self.patch(credentials.Keyring, 'get_credentials',
+                   lambda kr, app: defer.succeed(None))
+        self._next_inner_result = credentials.USER_SUCCESS
+
+        assert not os.path.exists(self.exe_path)
+
+        result = yield self.method_call(**self.kwargs)
+        self.assertEqual(result, TOKEN)
+
+        # the ui was opened and proper params were passed
+        args = yield self.deferred
+        self.inner_args[0] = os.path.join(utils.get_bin_dir(),
+                                          credentials.UI_EXECUTABLE_GTK)
         self.assertEqual(self.inner_args, args)
 
     @defer.inlineCallbacks
@@ -436,6 +462,9 @@
         """Make the inner call fail."""
         raise SampleMiscException(kwargs)
 
+    def test_ui_executable_falls_back_to_gtk(self):
+        """This check does not apply for this test case."""
+
     def test_without_existent_token_and_return_code_cancel(self, exc=None):
         """The operation returns UserNotValidatedError."""
         exc = credentials.UserNotValidatedError