Merge lp:~mandel/ubuntu-sso-client/proxy-creds-windows into lp:ubuntu-sso-client

[Manuel de la Peña]

- Fixed dialog on window by using show so that the QEventLoop does not block the execution of the main loop of the reactor which did not allow the deferreds to be fired (LP: #960481)

- Fixed dialog on window by using show so that the QEventLoop does not block the execution of the main loop of the reactor which did not allow the deferreds to be fired (LP: #960481)

Please test that the dialog works on both platforms in IRL by doing

PYTHONPATH=. python bin/ubuntu-sso-proxy-creds-qt --domamin test_domain

[Diego Sarmentero (diegosarmentero)]

+1

review: Approve

[Roberto Alsina (ralsina)]

There is no need to log in line 50, and am slightly concerned about having win32-specific bits here, but since this is "just a script" and has no modules where we could stash platform-specific bits, I am giving it a +1.

Maybe we can file a bug to refactor those bits away.

review: Approve

[Brian Curtin (brian.curtin)]

Refactoring post-release would be nice. IRL testing on Windows worked well.

review: Approve

[Brian Curtin (brian.curtin)]

I entered #960529 to keep track of the refactoring.

[Ubuntu One Auto Pilot (otto-pilot)]

The attempt to merge lp:~mandel/ubuntu-sso-client/proxy-creds-windows into lp:ubuntu-sso-client failed. Below is the output from the failed tests.

*** Running GTK test suite for ubuntu_sso ***
ubuntu_sso.xdg_base_directory.tests.test_common
  TestBaseDirectory
    test_load_config_paths_filter ... [OK]
    test_save_config_path ... [OK]
    test_xdg_cache_home_is_utf8_bytes ... [OK]
    test_xdg_config_dirs_are_bytes ... [OK]
    test_xdg_config_home_is_utf8_bytes ... [OK]
    test_xdg_data_dirs_are_bytes ... [OK]
    test_xdg_data_home_is_utf8_bytes ... [OK]
    test_xdg_home_is_utf8_bytes ... [OK]
twisted.trial.unittest
  TestCase
    runTest ... [OK]
ubuntu_sso.gtk.tests.test_main
  BasicTestCase
    test_main ... [OK]
twisted.trial.unittest
  TestCase
    runTest ... [OK]
ubuntu_sso.gtk.tests.test_gui
  BasicTestCase
    runTest ... [OK]
  BasicUbuntuSSOClientTestCase
    test_app_name_is_stored ... [OK]
    test_cancel_buttons_close_window ... [OK]
    test_close_callback_if_not_set ... [OK]
    test_closing_main_window_calls_close_callback ... [OK]
    test_entries_activates_default ... [OK]
    test_finish_error_shows_error_page ... [OK]
    test_finish_success_shows_success_page ... [OK]
    test_initial_text_for_entries ... [OK]
    test_main_window_is_resizable ... [OK]
    test_main_window_is_visible_at_startup ... [OK]
    test_pages_are_packed_into_container ... [OK]
    test_password_fields_are_password ... [OK]
    test_signals_are_removed ... [OK]
    test_warning_fields_are_cleared ... [OK]
    test_window_icon ... [OK]
  DefaultButtonsTestCase
    test_default_widget_can_default ... [OK]
    test_pages_have_default_widget_set ... [OK]
    test_set_current_page_grabs_focus_for_default_button ... [OK]
  EnterDetailsTestCase
    test_captcha_filename_is_different_each_time ... [OK]
    test_captcha_id_is_stored_when_captcha_is_available ... [OK]
    test_captcha_image_is_a_spinner_at_first ... [OK]
    test_captcha_image_is_not_visible_at_startup ... ...

=== modified file 'ubuntu_sso/qt/proxy_dialog.py'
--- ubuntu_sso/qt/proxy_dialog.py	2012-03-08 12:35:25 +0000
+++ ubuntu_sso/qt/proxy_dialog.py	2012-03-20 18:35:21 +0000
@@ -106,6 +106,7 @@
         except Exception, e:
             logger.exception('Could not set credentials:')
             self.done(EXCEPTION_RAISED)
+        logger.debug('Stored creds')
         # pylint: disable=W0703, W0612
         self.done(USER_SUCCESS)
 
@@ -140,13 +141,41 @@
     return parser.parse_args()
 
 
+def exit_code(return_code):
+    """Use the window result code and the sys.exit."""
+    logger.debug('exit %s', return_code)
+    QApplication.instance().exit(return_code)
+    if sys.platform == 'win32':
+        logger.debug('Stop qt reactor')
+        from twisted.internet import reactor
+        reactor.stop()
+
+
 def main():
     """Main method used to show the creds dialog."""
     # Keep ref to avoid core dump, pylint: disable=W0612
+
+    if sys.platform == 'win32':
+        import qt4reactor
+        qt4reactor.install()
+        logger.debug('Qt reactor installed.')
+
     app = QApplication(sys.argv)
     # pylint: enable=W0612
     args = parse_args()
     win = ProxyCredsDialog(domain=args.domain,
                            retry=args.retry)
-    return_code = win.exec_()
-    sys.exit(return_code)
+
+    if sys.platform == 'win32':
+        win.show()
+        win.finished.connect(exit_code)
+
+        logger.debug('Starting reactor')
+        # pylint: disable=E1101
+        from twisted.internet import reactor
+        logger.debug('QApp is %s', reactor.qApp)
+        reactor.run()
+        # pylint: enable=E1101
+    else:
+        return_code = win.exec_()
+        sys.exit(return_code)

=== modified file 'ubuntu_sso/qt/tests/test_proxy_dialog.py'
--- ubuntu_sso/qt/tests/test_proxy_dialog.py	2012-03-07 18:13:04 +0000
+++ ubuntu_sso/qt/tests/test_proxy_dialog.py	2012-03-20 18:35:21 +0000
@@ -15,7 +15,10 @@
 # with this program.  If not, see <http://www.gnu.org/licenses/>.
 """Test the proxy UI."""
 
-from twisted.internet import defer
+import qt4reactor
+
+from PyQt4.QtCore import QObject, pyqtSignal
+from twisted.internet import defer, reactor
 from twisted.trial.unittest import TestCase
 
 from ubuntu_sso.qt import proxy_dialog
@@ -301,6 +304,62 @@
         self.retry = retry
 
 
+class FakeCredsDialog(QObject):
+    """A fake dialog."""
+
+    finished = pyqtSignal(int)
+
+    def __init__(self, called, code):
+        """Create a new instance."""
+        super(FakeCredsDialog, self).__init__()
+        self.domain = None
+        self.retry = None
+        self.called = called
+        self.return_code = code
+
+    def __call__(self, domain, retry):
+        """Fake init."""
+        self.domain = domain
+        self.retry = retry
+        return self
+
+    def show(self):
+        """Show the dialog."""
+        self.called.append(('show',))
+
+    def exec_(self):
+        """Show the dialog."""
+        self.called.append(('exec_',))
+        return self.return_code
+
+
+class FakeQApplication(object):
+    """A fake QApplication."""
+
+    test_instance = None
+
+    def __init__(self, called):
+        """Create a new instance."""
+        self.called = called
+        self.args = None
+        self.code = None
+        FakeQApplication.test_instance = self
+
+    def __call__(self, args):
+        """Fake construntor."""
+        self.args = args
+
+    @classmethod
+    def instance(cls):
+        """Return the instance."""
+        return FakeQApplication.test_instance
+
+    def exit(self, code):
+        """Exit the app."""
+        self.called.append(('exit', code))
+        self.code = code
+
+
 class MainTest(TestCase):
     """Test the main method."""
 
@@ -310,29 +369,91 @@
         yield super(MainTest, self).setUp()
         self.called = []
 
+        self.return_code = 0
+        self.dialog = FakeCredsDialog(self.called, self.return_code)
+        self.patch(proxy_dialog, 'ProxyCredsDialog', self.dialog)
+
         self.args = FakeArgs(domain='domain', retry=False)
         self.patch(proxy_dialog, 'parse_args', lambda: self.args)
 
-        self.return_code = 0
-        self.patch(proxy_dialog.ProxyCredsDialog, 'exec_', lambda x:
-                self.return_code)
         self.exit_code = None
 
+        self.app = FakeQApplication(self.called)
+
+        self.patch(proxy_dialog, 'QApplication', self.app)
+
+    def _set_old_platform(self, platform):
+        """Set back the platform."""
+        proxy_dialog.sys.platform = platform
+
+    def _assert_basic_start(self):
+        """Assert the common code is actually ran."""
+        self.assertEqual(self.args.domain, self.dialog.domain)
+        self.assertEqual(self.args.retry, self.dialog.retry)
+
+    def test_linux_main(self):
+        """Test the main method on linux."""
+        old_platform = proxy_dialog.sys.platform
+        proxy_dialog.sys.platform = 'linux'
+        self.addCleanup(self._set_old_platform, old_platform)
+
         def fake_exit(code):
-            """Fake the sys.exit."""
+            """Fake sys exit."""
             self.called.append(('exit', code))
-            self.exit_code = code
 
         self.patch(proxy_dialog.sys, 'exit', fake_exit)
 
-        def fake_application(args):
-            """Fake a QApplication."""
-            self.called.append(('QApplication', args))
-
-        self.patch(proxy_dialog, 'QApplication', fake_application)
-
-    def test_main(self):
-        """Test the main method."""
-        proxy_dialog.main()
-        self.assertIn('QApplication', self.called[0])
+        proxy_dialog.main()
+
+        self._assert_basic_start()
+        self.assertIn(('exec_',), self.called)
+        self.assertIn(('exit', self.return_code), self.called)
+
+    def test_windows_main(self):
+        """Test the main method on windows."""
+        old_platform = proxy_dialog.sys.platform
+        proxy_dialog.sys.platform = 'win32'
+        self.addCleanup(self._set_old_platform, old_platform)
+
+        def fake_install():
+            """Fake the reactor installation."""
+            self.called.append(('install',))
+
+        self.patch(qt4reactor, 'install', fake_install)
+
+        def fake_start():
+            """Fake reactor.start."""
+            self.called.append(('run',))
+
+        self.patch(reactor, 'run', fake_start)
+
+        def fake_exit(code):
+            """Fake the exit method."""
+            self.called.append(('exit_code', code))
+
+        self.patch(proxy_dialog, 'exit_code', fake_exit)
+
+        proxy_dialog.main()
+
+        self._assert_basic_start()
+        self.assertIn(('show',), self.called)
+        self.assertIn(('run',), self.called)
+
+        self.dialog.finished.emit(self.return_code)
+        self.assertIn(('exit_code', self.return_code), self.called)
+
+    def test_exit_code_windows(self):
+        """"Test the exit code function."""
+        old_platform = proxy_dialog.sys.platform
+        proxy_dialog.sys.platform = 'win32'
+        self.addCleanup(self._set_old_platform, old_platform)
+
+        def fake_stop():
+            """Fake reactor.start."""
+            self.called.append(('stop',))
+
+        self.patch(reactor, 'stop', fake_stop)
+
+        proxy_dialog.exit_code(self.return_code)
+        self.assertIn(('stop',), self.called)
         self.assertIn(('exit', self.return_code), self.called)