Merge lp:~ralsina/ubuntuone-control-panel/opt-parsing into lp:ubuntuone-control-panel

[Roberto Alsina]

* Cleanup the ubuntuone-control-panel-qt script moving all logic into the main module.
* Adds tests for main()
* Parse Qt options correctly
* Move some code from UniqueApp to main where it belongs, and add tests for it
* Migrate to argparse

* Cleanup the ubuntuone-control-panel-qt script moving all logic into the main module.
* Adds tests for main()
* Parse Qt options correctly
* Move some code from UniqueApp to main where it belongs, and add tests for it
* Migrate to argparse

[Diego Sarmentero (diegosarmentero)]

+1

review: Approve

[dobey (dobey)]

review: Approve

[Natalia Bidart (nataliabidart)]

Any chance you migrate to argparse while you're at this branch? :-)

[Natalia Bidart (nataliabidart)]

Thanks for this work!

Could you please re-align all the options in parse_args? The multi-line statements are misaligned, for example, lines 80 to 84 should be:

80 + result.add_argument("--switch-to", dest="switch_to",
81 + metavar="PANEL_NAME", default="",
82 + help="Start Ubuntu One in the "
83 + "PANEL_NAME tab. Possible values are: "
84 + "dashboard, volumes, devices, applications")

or (the one I prefer):

80 + result.add_argument("--switch-to", dest="switch_to",
81 + metavar="PANEL_NAME", default="",
82 + help="Start Ubuntu One in the PANEL_NAME tab. "
83 + "Possible values are: dashboard, volumes, devices, applications")

(the chosen pattern should be applied to the rest of the arguments).

review: Needs Fixing

[Natalia Bidart (nataliabidart)]

(the examples above can be properly read -regarding spacing- in the generated email with the review)

[Natalia Bidart (nataliabidart)]

Looks good!

review: Approve

[Ubuntu One Auto Pilot (otto-pilot)]

Attempt to merge into lp:ubuntuone-control-panel failed due to conflicts:

text conflict in bin/ubuntuone-control-panel-qt

[lp:~ralsina/ubuntuone-control-panel/opt-parsing]

267. By Roberto Alsina on 2012-02-28

merged trunk, resolved conflict

=== modified file 'bin/ubuntuone-control-panel-qt'
--- bin/ubuntuone-control-panel-qt	2012-02-28 14:49:21 +0000
+++ bin/ubuntuone-control-panel-qt	2012-02-28 16:47:17 +0000
@@ -25,37 +25,8 @@
     import qt4reactor
     qt4reactor.install()
 
-
-from optparse import OptionParser
-
 from ubuntuone.controlpanel.gui.qt import main
 
 
-def parser_options():
-    """Parse command line parameters."""
-    usage = "Usage: %prog [option]"
-    result = OptionParser(usage=usage)
-    result.add_option("", "--switch-to", dest="switch_to", type="string",
-                      metavar="PANEL_NAME", default="",
-                      help="Start Ubuntu One in the "
-                           "PANEL_NAME tab. Possible values are: "
-                           "dashboard, volumes, devices, applications")
-    result.add_option("-a", "--alert", dest="alert", action="store_true",
-                      default=False, help="Start Ubuntu One "
-                      "alerting the user to its presence.")
-    result.add_option("--minimized", dest="minimized", action="store_true",
-                      default=False, help="Start Ubuntu One "
-                      "only in the notification area, with no visible window. "
-                      "Implies --with-icon")
-    result.add_option("--with-icon", dest="with_icon", action="store_true",
-                      default=False, help="Start Ubuntu One "
-                      "with an icon in the notification area.")
-    return result
-
-
 if __name__ == "__main__":
-    parser = parser_options()
-    (options, args) = parser.parse_args(sys.argv)
-    main.main(switch_to=options.switch_to,
-        alert=options.alert, minimized=options.minimized,
-        with_icon=options.with_icon)
+    main.main(sys.argv)
 
=== modified file 'run-tests'
--- run-tests	2012-02-06 20:49:41 +0000
+++ run-tests	2012-02-28 16:47:17 +0000
@@ -16,7 +16,7 @@
 # You should have received a copy of the GNU General Public License along
 # with this program.  If not, see <http://www.gnu.org/licenses/>.
 
-QT_TESTS_PATH=ubuntuone/controlpanel/gui/qt/tests
+QT_TESTS_PATH="ubuntuone/controlpanel/gui/qt/tests, ubuntuone/controlpanel/gui/qt/main/tests"
 GTK_TESTS_PATH=ubuntuone/controlpanel/gui/gtk/tests
 DBUS_TESTS_PATH=ubuntuone/controlpanel/dbustests
 WINDOWS_TESTS=test_windows.py
 
=== modified file 'ubuntuone/controlpanel/gui/qt/main/__init__.py'
--- ubuntuone/controlpanel/gui/qt/main/__init__.py	2012-02-06 21:10:10 +0000
+++ ubuntuone/controlpanel/gui/qt/main/__init__.py	2012-02-28 16:47:17 +0000
@@ -16,10 +16,12 @@
 
 """Provide the correct ui main module."""
 
+import argparse
 import sys
 
 from PyQt4 import QtCore
 
+from ubuntuone.controlpanel.gui.qt.gui import start
 # Module used to include the resources into this file
 # Unused import images_rc, pylint: disable=W0611
 from ubuntuone.controlpanel.gui.qt.ui import images_rc
@@ -36,19 +38,49 @@
 # pylint: enable=C0103
 
 
-def main(switch_to='', alert=False, minimized=False, with_icon=False):
+def parser_options():
+    """Parse command line parameters."""
+    result = argparse.ArgumentParser()
+    result.add_argument("--switch-to", dest="switch_to",
+                        metavar="PANEL_NAME", default="",
+                        help="Start Ubuntu One in the "
+                             "PANEL_NAME tab. Possible values are: "
+                             "dashboard, volumes, devices, applications")
+    result.add_argument("-a", "--alert", dest="alert", action="store_true",
+                        default=False,
+                        help="Start Ubuntu One "
+                             "alerting the user to its presence.")
+    result.add_argument("--minimized", dest="minimized", action="store_true",
+                        default=False,
+                        help="Start Ubuntu One "
+                             "only in the notification area, with no "
+                             "visible window. Implies --with-icon")
+    result.add_argument("--with-icon", dest="with_icon", action="store_true",
+                        default=False,
+                        help="Start Ubuntu One "
+                             "with an icon in the notification area.")
+    return result
+
+
+def main(args):
     """Start the Qt mainloop and open the main window."""
     # The following cannot be imported outside this function
     # because u1trial already provides a reactor.
 
-    app = UniqueApplication(sys.argv, "ubuntuone-control-panel")
+    args = ['ubuntuone-installer'] + args[1:]
+    app = UniqueApplication(args, "ubuntuone-control-panel")
+    parser = parser_options()
+    # Use only the arguments that are not recognized by Qt
+    args = parser.parse_args(args=[unicode(x) for x in app.arguments()[1:]])
+    _ = args.switch_to
+    _ = args.alert
+    minimized = args.minimized
+    with_icon = args.with_icon
     source.main(app)
 
     qss = QtCore.QResource(":/ubuntuone.qss")
     app.setStyleSheet(qss.data())
 
-    from ubuntuone.controlpanel.gui.qt.gui import start
-
     # Unused variable 'window', 'icon', pylint: disable=W0612
     icon, window = start(lambda: source.main_quit(app),
                          minimized=minimized, with_icon=with_icon)
 
=== added directory 'ubuntuone/controlpanel/gui/qt/main/tests'
=== added file 'ubuntuone/controlpanel/gui/qt/main/tests/__init__.py'
--- ubuntuone/controlpanel/gui/qt/main/tests/__init__.py	1970-01-01 00:00:00 +0000
+++ ubuntuone/controlpanel/gui/qt/main/tests/__init__.py	2012-02-28 16:47:17 +0000
@@ -0,0 +1,17 @@
+# -*- coding: utf-8 -*-
+#
+# Copyright 2011-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/>.
+
+"""The test suite for the Qt UI for the control panel for Ubuntu One."""
 
=== added file 'ubuntuone/controlpanel/gui/qt/main/tests/test_main.py'
--- ubuntuone/controlpanel/gui/qt/main/tests/test_main.py	1970-01-01 00:00:00 +0000
+++ ubuntuone/controlpanel/gui/qt/main/tests/test_main.py	2012-02-28 16:47:17 +0000
@@ -0,0 +1,105 @@
+# -*- 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/>.
+
+"""Tests for the control panel's Qt main."""
+
+from twisted.internet.defer import inlineCallbacks
+
+from ubuntuone.controlpanel.gui.qt import main
+from ubuntuone.controlpanel.tests import TestCase
+
+
+class FakeApplication(object):
+
+    """A fake application."""
+
+    def __init__(self):
+        self.args = None
+        self.style = None
+
+    def __call__(self, argv, *args, **kwargs):
+        """Fake arg filtering function."""
+        if '-title' in argv:
+            argv.remove('-title')
+        self.args = (argv, args, kwargs)
+        return self
+
+    # pylint: disable=C0103
+    def setStyleSheet(self, *args, **kwargs):
+        """Fake setStyleSheet."""
+        self.style = (args, kwargs)
+    # pylint: enable=C0103
+
+    def arguments(self):
+        """Fake arguments function."""
+        return self.args[0]
+
+    def exec_(self):
+        """Fake event loop."""
+        pass
+
+
+class FakeStart(object):
+
+    """Fake start function."""
+
+    def __init__(self):
+        self.args = None
+
+    def __call__(self, *args, **kwargs):
+        self.args = (args, kwargs)
+        return None, None
+
+
+class MainTestCase(TestCase):
+
+    """Test the argument parsing and passing in main."""
+
+    @inlineCallbacks
+    def setUp(self):
+        yield super(MainTestCase, self).setUp()
+        self.app = FakeApplication()
+        self.start = FakeStart()
+        self.patch(main, "UniqueApplication", self.app)
+        self.patch(main, "start", self.start)
+
+    def test_wm_class(self):
+        """Test that we set the 1st argument, used for WM_CLASS, correctly."""
+        main.main([])
+        self.assertEqual(self.app.args,
+            (['ubuntuone-installer'], ('ubuntuone-control-panel',), {}))
+
+    def test_title_not_fail(self):
+        """Ensure -title is removed before it gets to OptParser."""
+        main.main(["blah", "-title"])
+        # Did not crash!
+
+    def test_minimized_option(self):
+        """Ensure the --minimized option is parsed and passed correctly."""
+        main.main(["blah", "--minimized"])
+        self.assertEqual(self.start.args[1],
+            {'minimized': True, 'with_icon': False})
+
+    def test_with_icon_option(self):
+        """Ensure the --minimized option is parsed and passed correctly."""
+        main.main(["blah", "--with-icon"])
+        self.assertEqual(self.start.args[1],
+            {'minimized': False, 'with_icon': True})
+
+    def test_style_loads(self):
+        """Ensure the stylesheet is loaded."""
+        main.main([])
+        self.assertTrue(self.app.style)
 
=== modified file 'ubuntuone/controlpanel/gui/qt/uniqueapp/linux.py'
--- ubuntuone/controlpanel/gui/qt/uniqueapp/linux.py	2012-02-23 19:49:25 +0000
+++ ubuntuone/controlpanel/gui/qt/uniqueapp/linux.py	2012-02-28 16:47:17 +0000
@@ -26,4 +26,4 @@
     new_instance = QtCore.pyqtSignal()
 
     def __init__(self, argv, key):
-        super(UniqueApplication, self).__init__(['ubuntuone-installer'] + argv)
+        super(UniqueApplication, self).__init__(argv)