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

Proposed by Roberto Alsina
Status: Merged
Approved by: Roberto Alsina
Approved revision: 267
Merged at revision: 267
Proposed branch: lp:~ralsina/ubuntuone-control-panel/opt-parsing
Merge into: lp:ubuntuone-control-panel
Diff against target: 269 lines (+161/-36)
6 files modified
bin/ubuntuone-control-panel-qt (+1/-30)
run-tests (+1/-1)
ubuntuone/controlpanel/gui/qt/main/__init__.py (+36/-4)
ubuntuone/controlpanel/gui/qt/main/tests/__init__.py (+17/-0)
ubuntuone/controlpanel/gui/qt/main/tests/test_main.py (+105/-0)
ubuntuone/controlpanel/gui/qt/uniqueapp/linux.py (+1/-1)
To merge this branch: bzr merge lp:~ralsina/ubuntuone-control-panel/opt-parsing
Reviewer Review Type Date Requested Status
Natalia Bidart Approve
dobey (community) Approve
Diego Sarmentero (community) Approve
Review via email: mp+94599@code.launchpad.net

Commit message

* 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

Description of the change

* 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

To post a comment you must log in.
Revision history for this message
Diego Sarmentero (diegosarmentero) wrote :

+1

review: Approve
Revision history for this message
dobey (dobey) :
review: Approve
Revision history for this message
Natalia Bidart (nataliabidart) wrote :

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

Revision history for this message
Natalia Bidart (nataliabidart) wrote :

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
Revision history for this message
Natalia Bidart (nataliabidart) wrote :

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

Revision history for this message
Natalia Bidart (nataliabidart) wrote :

Looks good!

review: Approve
Revision history for this message
Ubuntu One Auto Pilot (otto-pilot) wrote :

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

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

267. By Roberto Alsina

merged trunk, resolved conflict

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'bin/ubuntuone-control-panel-qt'
2--- bin/ubuntuone-control-panel-qt 2012-02-28 14:49:21 +0000
3+++ bin/ubuntuone-control-panel-qt 2012-02-28 16:47:17 +0000
4@@ -25,37 +25,8 @@
5 import qt4reactor
6 qt4reactor.install()
7
8-
9-from optparse import OptionParser
10-
11 from ubuntuone.controlpanel.gui.qt import main
12
13
14-def parser_options():
15- """Parse command line parameters."""
16- usage = "Usage: %prog [option]"
17- result = OptionParser(usage=usage)
18- result.add_option("", "--switch-to", dest="switch_to", type="string",
19- metavar="PANEL_NAME", default="",
20- help="Start Ubuntu One in the "
21- "PANEL_NAME tab. Possible values are: "
22- "dashboard, volumes, devices, applications")
23- result.add_option("-a", "--alert", dest="alert", action="store_true",
24- default=False, help="Start Ubuntu One "
25- "alerting the user to its presence.")
26- result.add_option("--minimized", dest="minimized", action="store_true",
27- default=False, help="Start Ubuntu One "
28- "only in the notification area, with no visible window. "
29- "Implies --with-icon")
30- result.add_option("--with-icon", dest="with_icon", action="store_true",
31- default=False, help="Start Ubuntu One "
32- "with an icon in the notification area.")
33- return result
34-
35-
36 if __name__ == "__main__":
37- parser = parser_options()
38- (options, args) = parser.parse_args(sys.argv)
39- main.main(switch_to=options.switch_to,
40- alert=options.alert, minimized=options.minimized,
41- with_icon=options.with_icon)
42+ main.main(sys.argv)
43
44=== modified file 'run-tests'
45--- run-tests 2012-02-06 20:49:41 +0000
46+++ run-tests 2012-02-28 16:47:17 +0000
47@@ -16,7 +16,7 @@
48 # You should have received a copy of the GNU General Public License along
49 # with this program. If not, see <http://www.gnu.org/licenses/>.
50
51-QT_TESTS_PATH=ubuntuone/controlpanel/gui/qt/tests
52+QT_TESTS_PATH="ubuntuone/controlpanel/gui/qt/tests, ubuntuone/controlpanel/gui/qt/main/tests"
53 GTK_TESTS_PATH=ubuntuone/controlpanel/gui/gtk/tests
54 DBUS_TESTS_PATH=ubuntuone/controlpanel/dbustests
55 WINDOWS_TESTS=test_windows.py
56
57=== modified file 'ubuntuone/controlpanel/gui/qt/main/__init__.py'
58--- ubuntuone/controlpanel/gui/qt/main/__init__.py 2012-02-06 21:10:10 +0000
59+++ ubuntuone/controlpanel/gui/qt/main/__init__.py 2012-02-28 16:47:17 +0000
60@@ -16,10 +16,12 @@
61
62 """Provide the correct ui main module."""
63
64+import argparse
65 import sys
66
67 from PyQt4 import QtCore
68
69+from ubuntuone.controlpanel.gui.qt.gui import start
70 # Module used to include the resources into this file
71 # Unused import images_rc, pylint: disable=W0611
72 from ubuntuone.controlpanel.gui.qt.ui import images_rc
73@@ -36,19 +38,49 @@
74 # pylint: enable=C0103
75
76
77-def main(switch_to='', alert=False, minimized=False, with_icon=False):
78+def parser_options():
79+ """Parse command line parameters."""
80+ result = argparse.ArgumentParser()
81+ result.add_argument("--switch-to", dest="switch_to",
82+ metavar="PANEL_NAME", default="",
83+ help="Start Ubuntu One in the "
84+ "PANEL_NAME tab. Possible values are: "
85+ "dashboard, volumes, devices, applications")
86+ result.add_argument("-a", "--alert", dest="alert", action="store_true",
87+ default=False,
88+ help="Start Ubuntu One "
89+ "alerting the user to its presence.")
90+ result.add_argument("--minimized", dest="minimized", action="store_true",
91+ default=False,
92+ help="Start Ubuntu One "
93+ "only in the notification area, with no "
94+ "visible window. Implies --with-icon")
95+ result.add_argument("--with-icon", dest="with_icon", action="store_true",
96+ default=False,
97+ help="Start Ubuntu One "
98+ "with an icon in the notification area.")
99+ return result
100+
101+
102+def main(args):
103 """Start the Qt mainloop and open the main window."""
104 # The following cannot be imported outside this function
105 # because u1trial already provides a reactor.
106
107- app = UniqueApplication(sys.argv, "ubuntuone-control-panel")
108+ args = ['ubuntuone-installer'] + args[1:]
109+ app = UniqueApplication(args, "ubuntuone-control-panel")
110+ parser = parser_options()
111+ # Use only the arguments that are not recognized by Qt
112+ args = parser.parse_args(args=[unicode(x) for x in app.arguments()[1:]])
113+ _ = args.switch_to
114+ _ = args.alert
115+ minimized = args.minimized
116+ with_icon = args.with_icon
117 source.main(app)
118
119 qss = QtCore.QResource(":/ubuntuone.qss")
120 app.setStyleSheet(qss.data())
121
122- from ubuntuone.controlpanel.gui.qt.gui import start
123-
124 # Unused variable 'window', 'icon', pylint: disable=W0612
125 icon, window = start(lambda: source.main_quit(app),
126 minimized=minimized, with_icon=with_icon)
127
128=== added directory 'ubuntuone/controlpanel/gui/qt/main/tests'
129=== added file 'ubuntuone/controlpanel/gui/qt/main/tests/__init__.py'
130--- ubuntuone/controlpanel/gui/qt/main/tests/__init__.py 1970-01-01 00:00:00 +0000
131+++ ubuntuone/controlpanel/gui/qt/main/tests/__init__.py 2012-02-28 16:47:17 +0000
132@@ -0,0 +1,17 @@
133+# -*- coding: utf-8 -*-
134+#
135+# Copyright 2011-2012 Canonical Ltd.
136+#
137+# This program is free software: you can redistribute it and/or modify it
138+# under the terms of the GNU General Public License version 3, as published
139+# by the Free Software Foundation.
140+#
141+# This program is distributed in the hope that it will be useful, but
142+# WITHOUT ANY WARRANTY; without even the implied warranties of
143+# MERCHANTABILITY, SATISFACTORY QUALITY, or FITNESS FOR A PARTICULAR
144+# PURPOSE. See the GNU General Public License for more details.
145+#
146+# You should have received a copy of the GNU General Public License along
147+# with this program. If not, see <http://www.gnu.org/licenses/>.
148+
149+"""The test suite for the Qt UI for the control panel for Ubuntu One."""
150
151=== added file 'ubuntuone/controlpanel/gui/qt/main/tests/test_main.py'
152--- ubuntuone/controlpanel/gui/qt/main/tests/test_main.py 1970-01-01 00:00:00 +0000
153+++ ubuntuone/controlpanel/gui/qt/main/tests/test_main.py 2012-02-28 16:47:17 +0000
154@@ -0,0 +1,105 @@
155+# -*- coding: utf-8 -*-
156+#
157+# Copyright 2012 Canonical Ltd.
158+#
159+# This program is free software: you can redistribute it and/or modify it
160+# under the terms of the GNU General Public License version 3, as published
161+# by the Free Software Foundation.
162+#
163+# This program is distributed in the hope that it will be useful, but
164+# WITHOUT ANY WARRANTY; without even the implied warranties of
165+# MERCHANTABILITY, SATISFACTORY QUALITY, or FITNESS FOR A PARTICULAR
166+# PURPOSE. See the GNU General Public License for more details.
167+#
168+# You should have received a copy of the GNU General Public License along
169+# with this program. If not, see <http://www.gnu.org/licenses/>.
170+
171+"""Tests for the control panel's Qt main."""
172+
173+from twisted.internet.defer import inlineCallbacks
174+
175+from ubuntuone.controlpanel.gui.qt import main
176+from ubuntuone.controlpanel.tests import TestCase
177+
178+
179+class FakeApplication(object):
180+
181+ """A fake application."""
182+
183+ def __init__(self):
184+ self.args = None
185+ self.style = None
186+
187+ def __call__(self, argv, *args, **kwargs):
188+ """Fake arg filtering function."""
189+ if '-title' in argv:
190+ argv.remove('-title')
191+ self.args = (argv, args, kwargs)
192+ return self
193+
194+ # pylint: disable=C0103
195+ def setStyleSheet(self, *args, **kwargs):
196+ """Fake setStyleSheet."""
197+ self.style = (args, kwargs)
198+ # pylint: enable=C0103
199+
200+ def arguments(self):
201+ """Fake arguments function."""
202+ return self.args[0]
203+
204+ def exec_(self):
205+ """Fake event loop."""
206+ pass
207+
208+
209+class FakeStart(object):
210+
211+ """Fake start function."""
212+
213+ def __init__(self):
214+ self.args = None
215+
216+ def __call__(self, *args, **kwargs):
217+ self.args = (args, kwargs)
218+ return None, None
219+
220+
221+class MainTestCase(TestCase):
222+
223+ """Test the argument parsing and passing in main."""
224+
225+ @inlineCallbacks
226+ def setUp(self):
227+ yield super(MainTestCase, self).setUp()
228+ self.app = FakeApplication()
229+ self.start = FakeStart()
230+ self.patch(main, "UniqueApplication", self.app)
231+ self.patch(main, "start", self.start)
232+
233+ def test_wm_class(self):
234+ """Test that we set the 1st argument, used for WM_CLASS, correctly."""
235+ main.main([])
236+ self.assertEqual(self.app.args,
237+ (['ubuntuone-installer'], ('ubuntuone-control-panel',), {}))
238+
239+ def test_title_not_fail(self):
240+ """Ensure -title is removed before it gets to OptParser."""
241+ main.main(["blah", "-title"])
242+ # Did not crash!
243+
244+ def test_minimized_option(self):
245+ """Ensure the --minimized option is parsed and passed correctly."""
246+ main.main(["blah", "--minimized"])
247+ self.assertEqual(self.start.args[1],
248+ {'minimized': True, 'with_icon': False})
249+
250+ def test_with_icon_option(self):
251+ """Ensure the --minimized option is parsed and passed correctly."""
252+ main.main(["blah", "--with-icon"])
253+ self.assertEqual(self.start.args[1],
254+ {'minimized': False, 'with_icon': True})
255+
256+ def test_style_loads(self):
257+ """Ensure the stylesheet is loaded."""
258+ main.main([])
259+ self.assertTrue(self.app.style)
260
261=== modified file 'ubuntuone/controlpanel/gui/qt/uniqueapp/linux.py'
262--- ubuntuone/controlpanel/gui/qt/uniqueapp/linux.py 2012-02-23 19:49:25 +0000
263+++ ubuntuone/controlpanel/gui/qt/uniqueapp/linux.py 2012-02-28 16:47:17 +0000
264@@ -26,4 +26,4 @@
265 new_instance = QtCore.pyqtSignal()
266
267 def __init__(self, argv, key):
268- super(UniqueApplication, self).__init__(['ubuntuone-installer'] + argv)
269+ super(UniqueApplication, self).__init__(argv)

Subscribers

People subscribed via source and target branches