Merge lp:~sinzui/bzr-gtk/handle-patch into lp:bzr-gtk

Proposed by Curtis Hovey
Status: Merged
Approved by: Curtis Hovey
Approved revision: 776
Merged at revision: 774
Proposed branch: lp:~sinzui/bzr-gtk/handle-patch
Merge into: lp:bzr-gtk
Diff against target: 221 lines (+112/-14)
4 files modified
bzr-handle-patch (+7/-4)
diff.py (+14/-10)
tests/test_commit.py (+31/-0)
tests/test_diff.py (+60/-0)
To merge this branch: bzr merge lp:~sinzui/bzr-gtk/handle-patch
Reviewer Review Type Date Requested Status
Jelmer Vernooij (community) Approve
Review via email: mp+91514@code.launchpad.net

Description of the change

Update bzr-handle-patch to gtk3.

    Launchpad bug: https://bugs.launchpad.net/bugs/926315
    Pre-implementation: no one

bzr-handle-patch has import and type errors because it is using Gtk3,
but is calling gtk2 API.

$./bzr-handle-patch ../handle-patch/_test.patch
...
  File "...gtk/diff.py", line 473, in _get_button_bar
    hbox.pack_start(merge_button, expand=False, fill=True)
  File "/usr/lib/python2.7/dist-packages/gi/types.py", line 43, in function
    return info.invoke(*args, **kwargs)
TypeError: pack_start() takes exactly 5 non-keyword arguments (2 given)

--------------------------------------------------------------------

RULES

    * bzr-handle-patch already imports Gtk, but its two calls gtk (the
      identifier does not exist)
    * Fix the hbox.pack_start

QA

    * from the top of the tree run the following command to send
      a mock patch to bzr-handle-patch
echo "=== added file '_test.txt'
--- _test.txt 1970-01-01 00:00:00 +0000
+++ _test.txt 2012-02-03 20:00:34 +0000
@@ -0,0 +1,1 @@
+hello" | BZR_PLUGINS_AT='gtk@.' ./bzr-handle-patch -
    * Verify you are presented the diff window.

IMPLEMENTATION

Fixed the pack_start args. Added tests for the calls to construct the UI.
Updated the base class to be testible. I replaced repeated calls to show()
with show_all() which is more efficient.
  diff.py
  tests/test_diff.py

Fixed the import/calls to Gtk. Added a smoke test to verify the script
starts.
    bzr-handle-patch
    tests/test_commit.py

To post a comment you must log in.
Revision history for this message
Jelmer Vernooij (jelmer) :
review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'bzr-handle-patch'
--- bzr-handle-patch 2011-07-31 16:50:29 +0000
+++ bzr-handle-patch 2012-02-03 23:12:18 +0000
@@ -2,10 +2,9 @@
22
3from bzrlib import errors, merge_directive3from bzrlib import errors, merge_directive
4from bzrlib.plugin import load_plugins4from bzrlib.plugin import load_plugins
5load_plugins()
5from bzrlib.plugins.gtk.commands import open_display6from bzrlib.plugins.gtk.commands import open_display
67
7load_plugins()
8
9import sys8import sys
109
11if len(sys.argv) < 2:10if len(sys.argv) < 2:
@@ -32,9 +31,13 @@
32 window = controller.window31 window = controller.window
33 window.show()32 window.show()
34 Gtk = open_display()33 Gtk = open_display()
35 window.connect("destroy", gtk.main_quit)34 window.connect("destroy", Gtk.main_quit)
36except Exception, e:35except Exception, e:
37 from bzrlib.plugins.gtk.dialog import error_dialog36 from bzrlib.plugins.gtk.dialog import error_dialog
38 error_dialog('Error', str(e))37 error_dialog('Error', str(e))
39 raise38 raise
40gtk.main()39
40if len(sys.argv) == 3 and sys.argv[2] == 'test':
41 sys.exit(0)
42
43Gtk.main()
4144
=== modified file 'diff.py'
--- diff.py 2012-01-23 21:47:33 +0000
+++ diff.py 2012-02-03 23:12:18 +0000
@@ -435,10 +435,12 @@
435 differences between two revisions on a branch.435 differences between two revisions on a branch.
436 """436 """
437437
438 SHOW_WIDGETS = True
439
438 def __init__(self, parent=None, operations=None):440 def __init__(self, parent=None, operations=None):
439 super(DiffWindow, self).__init__(parent=parent)441 super(DiffWindow, self).__init__(parent=parent)
440 self.set_border_width(0)442 self.set_border_width(0)
441 self.set_title("bzrk diff")443 self.set_title("bzr diff")
442444
443 # Use two thirds of the screen by default445 # Use two thirds of the screen by default
444 screen = self.get_screen()446 screen = self.get_screen()
@@ -452,10 +454,12 @@
452 """Construct the window contents."""454 """Construct the window contents."""
453 self.vbox = Gtk.VBox()455 self.vbox = Gtk.VBox()
454 self.add(self.vbox)456 self.add(self.vbox)
455 self.vbox.show()457 if self.SHOW_WIDGETS:
458 self.vbox.show()
456 self.diff = DiffWidget()459 self.diff = DiffWidget()
457 self.vbox.pack_end(self.diff, True, True, 0)460 self.vbox.pack_end(self.diff, True, True, 0)
458 self.diff.show_all()461 if self.SHOW_WIDGETS:
462 self.diff.show_all()
459 # Build after DiffWidget to connect signals463 # Build after DiffWidget to connect signals
460 menubar = self._get_menu_bar()464 menubar = self._get_menu_bar()
461 self.vbox.pack_start(menubar, False, False, 0)465 self.vbox.pack_start(menubar, False, False, 0)
@@ -471,13 +475,11 @@
471 mb_view_wrapsource = Gtk.CheckMenuItem.new_with_mnemonic(475 mb_view_wrapsource = Gtk.CheckMenuItem.new_with_mnemonic(
472 _i18n("Wrap _Long Lines"))476 _i18n("Wrap _Long Lines"))
473 mb_view_wrapsource.connect('activate', self.diff._on_wraplines_toggled)477 mb_view_wrapsource.connect('activate', self.diff._on_wraplines_toggled)
474 mb_view_wrapsource.show()
475 mb_view_menu.append(mb_view_wrapsource)478 mb_view_menu.append(mb_view_wrapsource)
476 mb_view.show()
477 mb_view.set_submenu(mb_view_menu)479 mb_view.set_submenu(mb_view_menu)
478 mb_view.show()
479 menubar.append(mb_view)480 menubar.append(mb_view)
480 menubar.show()481 if self.SHOW_WIDGETS:
482 menubar.show_all()
481 return menubar483 return menubar
482484
483 def _get_button_bar(self, operations):485 def _get_button_bar(self, operations):
@@ -491,11 +493,13 @@
491 hbox.set_layout(Gtk.ButtonBoxStyle.START)493 hbox.set_layout(Gtk.ButtonBoxStyle.START)
492 for title, method in operations:494 for title, method in operations:
493 merge_button = Gtk.Button(title)495 merge_button = Gtk.Button(title)
494 merge_button.show()496 if self.SHOW_WIDGETS:
497 merge_button.show()
495 merge_button.set_relief(Gtk.ReliefStyle.NONE)498 merge_button.set_relief(Gtk.ReliefStyle.NONE)
496 merge_button.connect("clicked", method)499 merge_button.connect("clicked", method)
497 hbox.pack_start(merge_button, expand=False, fill=True)500 hbox.pack_start(merge_button, False, True, 0)
498 hbox.show()501 if self.SHOW_WIDGETS:
502 hbox.show()
499 return hbox503 return hbox
500504
501 def _get_merge_target(self):505 def _get_merge_target(self):
502506
=== modified file 'tests/test_commit.py'
--- tests/test_commit.py 2012-01-23 16:52:40 +0000
+++ tests/test_commit.py 2012-02-03 23:12:18 +0000
@@ -17,6 +17,8 @@
17"""Test the Commit functionality."""17"""Test the Commit functionality."""
1818
19import os19import os
20import subprocess
21from tempfile import NamedTemporaryFile
2022
21from gi.repository import Gtk23from gi.repository import Gtk
2224
@@ -1296,3 +1298,32 @@
1296 self.assertEquals(u'', self._get_commit_message())1298 self.assertEquals(u'', self._get_commit_message())
1297 self.assertEquals(u'de',1299 self.assertEquals(u'de',
1298 self._get_file_commit_messages())1300 self._get_file_commit_messages())
1301
1302
1303class BzrHandlePatchTestCase(tests.TestCase):
1304
1305 def setUp(self):
1306 top = os.path.abspath(os.path.join(
1307 os.path.dirname(__file__), os.pardir))
1308 self.script = os.path.join(top, 'bzr-handle-patch')
1309 self.env = dict(os.environ)
1310 self.env['BZR_PLUGINS_AT'] = 'gtk@%s' %top
1311 self.patch = NamedTemporaryFile()
1312 self.patch.write('\n'.join([
1313 "=== added file '_test.txt'",
1314 "--- _test.txt 1970-01-01 00:00:00 +0000",
1315 "+++ _test.txt 2012-02-03 20:00:34 +0000",
1316 "@@ -0,0 +1,1 @@",
1317 "+hello",
1318 ]))
1319 self.patch.flush()
1320 super(BzrHandlePatchTestCase, self).setUp()
1321
1322 def test_smoketest(self):
1323 # This is a smoke test to verify the process starts.
1324 bzr_notify = subprocess.Popen(
1325 [self.script, self.patch.name, 'test'],
1326 stdout=subprocess.PIPE, stderr=subprocess.PIPE, env=self.env)
1327 stdout, stderr = bzr_notify.communicate()
1328 self.assertEqual('', stdout)
1329 self.assertEqual('', stderr)
12991330
=== modified file 'tests/test_diff.py'
--- tests/test_diff.py 2012-01-23 18:10:44 +0000
+++ tests/test_diff.py 2012-02-03 23:12:18 +0000
@@ -35,6 +35,7 @@
35 DiffController,35 DiffController,
36 DiffView,36 DiffView,
37 DiffWidget,37 DiffWidget,
38 DiffWindow,
38 iter_changes_to_status,39 iter_changes_to_status,
39 MergeDirectiveController,40 MergeDirectiveController,
40 )41 )
@@ -131,6 +132,65 @@
131 self.assertFalse(widget.diff_view.show_diff.called)132 self.assertFalse(widget.diff_view.show_diff.called)
132133
133134
135class FakeDiffWindow(DiffWindow):
136
137 SHOW_WIDGETS = False
138
139
140class DiffWindowTestCase(tests.TestCaseWithTransport):
141
142 def test_init(self):
143 window = DiffWindow()
144 self.assertEqual('bzr diff', window.props.title)
145 self.assertEqual(0, window.props.border_width)
146
147 def test_init_construct_without_operations(self):
148 window = DiffWindow()
149 widgets = window.vbox.get_children()
150 self.assertEqual(2, len(widgets))
151 self.assertIsInstance(widgets[0], Gtk.MenuBar)
152 self.assertIsInstance(widgets[1], DiffWidget)
153
154 def test_init_construct_with_operations(self):
155 method = MockMethod()
156 window = DiffWindow(operations=[('title', method)])
157 widgets = window.vbox.get_children()
158 self.assertEqual(3, len(widgets))
159 self.assertIsInstance(widgets[0], Gtk.MenuBar)
160 self.assertIsInstance(widgets[1], Gtk.HButtonBox)
161 self.assertIsInstance(widgets[2], DiffWidget)
162
163 def test_get_menu_bar(self):
164 window = DiffWindow()
165 menu_bar = window._get_menu_bar()
166 self.assertIsNot(None, menu_bar)
167 menus = menu_bar.get_children()
168 self.assertEqual(1, len(menus))
169 self.assertEqual('_View', menus[0].props.label)
170 sub_menu = menus[0].get_submenu()
171 self.assertIsNot(None, sub_menu)
172 items = sub_menu.get_children()
173 self.assertEqual(1, len(items))
174 menus[0].get_submenu().get_children()[0].props.label
175 self.assertEqual('Wrap _Long Lines', items[0].props.label)
176
177 def test_get_button_bar_with_none(self):
178 window = DiffWindow()
179 self.assertIs(None, window._get_button_bar(None))
180
181 def test_get_button_bar_with_operations(self):
182 window = DiffWindow()
183 method = MockMethod()
184 button_bar = window._get_button_bar([('title', method)])
185 self.assertIsNot(None, button_bar)
186 buttons = button_bar.get_children()
187 self.assertEqual(1, len(buttons))
188 self.assertEqual('title', buttons[0].props.label)
189 buttons[0].emit('clicked')
190 self.assertIs(True, method.called)
191
192
193
134class MockDiffWidget(object):194class MockDiffWidget(object):
135195
136 def set_diff_text_sections(self, sections):196 def set_diff_text_sections(self, sections):

Subscribers

People subscribed via source and target branches

to all changes: