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

[Curtis Hovey]

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

[Jelmer Vernooij (jelmer)]

review: Approve

=== 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 @@
 
 from bzrlib import errors, merge_directive
 from bzrlib.plugin import load_plugins
+load_plugins()
 from bzrlib.plugins.gtk.commands import open_display
 
-load_plugins()
-
 import sys
 
 if len(sys.argv) < 2:
@@ -32,9 +31,13 @@
 	window = controller.window
 	window.show()
 	Gtk = open_display()
-	window.connect("destroy", gtk.main_quit)
+	window.connect("destroy", Gtk.main_quit)
 except Exception, e:
 	from bzrlib.plugins.gtk.dialog import error_dialog
 	error_dialog('Error', str(e))
 	raise
-gtk.main()
+
+if len(sys.argv) == 3 and sys.argv[2] == 'test':
+    sys.exit(0)
+
+Gtk.main()

=== 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 @@
     differences between two revisions on a branch.
     """
 
+    SHOW_WIDGETS = True
+
     def __init__(self, parent=None, operations=None):
         super(DiffWindow, self).__init__(parent=parent)
         self.set_border_width(0)
-        self.set_title("bzrk diff")
+        self.set_title("bzr diff")
 
         # Use two thirds of the screen by default
         screen = self.get_screen()
@@ -452,10 +454,12 @@
         """Construct the window contents."""
         self.vbox = Gtk.VBox()
         self.add(self.vbox)
-        self.vbox.show()
+        if self.SHOW_WIDGETS:
+            self.vbox.show()
         self.diff = DiffWidget()
         self.vbox.pack_end(self.diff, True, True, 0)
-        self.diff.show_all()
+        if self.SHOW_WIDGETS:
+            self.diff.show_all()
         # Build after DiffWidget to connect signals
         menubar = self._get_menu_bar()
         self.vbox.pack_start(menubar, False, False, 0)
@@ -471,13 +475,11 @@
         mb_view_wrapsource = Gtk.CheckMenuItem.new_with_mnemonic(
             _i18n("Wrap _Long Lines"))
         mb_view_wrapsource.connect('activate', self.diff._on_wraplines_toggled)
-        mb_view_wrapsource.show()
         mb_view_menu.append(mb_view_wrapsource)
-        mb_view.show()
         mb_view.set_submenu(mb_view_menu)
-        mb_view.show()
         menubar.append(mb_view)
-        menubar.show()
+        if self.SHOW_WIDGETS:
+            menubar.show_all()
         return menubar
 
     def _get_button_bar(self, operations):
@@ -491,11 +493,13 @@
         hbox.set_layout(Gtk.ButtonBoxStyle.START)
         for title, method in operations:
             merge_button = Gtk.Button(title)
-            merge_button.show()
+            if self.SHOW_WIDGETS:
+                merge_button.show()
             merge_button.set_relief(Gtk.ReliefStyle.NONE)
             merge_button.connect("clicked", method)
-            hbox.pack_start(merge_button, expand=False, fill=True)
-        hbox.show()
+            hbox.pack_start(merge_button, False, True, 0)
+        if self.SHOW_WIDGETS:
+            hbox.show()
         return hbox
 
     def _get_merge_target(self):

=== 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 @@
 """Test the Commit functionality."""
 
 import os
+import subprocess
+from tempfile import NamedTemporaryFile
 
 from gi.repository import Gtk
 
@@ -1296,3 +1298,32 @@
         self.assertEquals(u'', self._get_commit_message())
         self.assertEquals(u'de',
                           self._get_file_commit_messages())
+
+
+class BzrHandlePatchTestCase(tests.TestCase):
+
+    def setUp(self):
+        top = os.path.abspath(os.path.join(
+            os.path.dirname(__file__), os.pardir))
+        self.script = os.path.join(top, 'bzr-handle-patch')
+        self.env = dict(os.environ)
+        self.env['BZR_PLUGINS_AT'] = 'gtk@%s' %top
+        self.patch = NamedTemporaryFile()
+        self.patch.write('\n'.join([
+            "=== 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",
+            ]))
+        self.patch.flush()
+        super(BzrHandlePatchTestCase, self).setUp()
+
+    def test_smoketest(self):
+        # This is a smoke test to verify the process starts.
+        bzr_notify = subprocess.Popen(
+            [self.script, self.patch.name, 'test'],
+            stdout=subprocess.PIPE, stderr=subprocess.PIPE, env=self.env)
+        stdout, stderr = bzr_notify.communicate()
+        self.assertEqual('', stdout)
+        self.assertEqual('', stderr)

=== 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 @@
     DiffController,
     DiffView,
     DiffWidget,
+    DiffWindow,
     iter_changes_to_status,
     MergeDirectiveController,
     )
@@ -131,6 +132,65 @@
         self.assertFalse(widget.diff_view.show_diff.called)
 
 
+class FakeDiffWindow(DiffWindow):
+
+    SHOW_WIDGETS = False
+
+
+class DiffWindowTestCase(tests.TestCaseWithTransport):
+
+    def test_init(self):
+        window = DiffWindow()
+        self.assertEqual('bzr diff', window.props.title)
+        self.assertEqual(0, window.props.border_width)
+
+    def test_init_construct_without_operations(self):
+        window = DiffWindow()
+        widgets = window.vbox.get_children()
+        self.assertEqual(2, len(widgets))
+        self.assertIsInstance(widgets[0], Gtk.MenuBar)
+        self.assertIsInstance(widgets[1], DiffWidget)
+
+    def test_init_construct_with_operations(self):
+        method = MockMethod()
+        window = DiffWindow(operations=[('title', method)])
+        widgets = window.vbox.get_children()
+        self.assertEqual(3, len(widgets))
+        self.assertIsInstance(widgets[0], Gtk.MenuBar)
+        self.assertIsInstance(widgets[1], Gtk.HButtonBox)
+        self.assertIsInstance(widgets[2], DiffWidget)
+
+    def test_get_menu_bar(self):
+        window = DiffWindow()
+        menu_bar = window._get_menu_bar()
+        self.assertIsNot(None, menu_bar)
+        menus = menu_bar.get_children()
+        self.assertEqual(1, len(menus))
+        self.assertEqual('_View', menus[0].props.label)
+        sub_menu = menus[0].get_submenu()
+        self.assertIsNot(None, sub_menu)
+        items = sub_menu.get_children()
+        self.assertEqual(1, len(items))
+        menus[0].get_submenu().get_children()[0].props.label
+        self.assertEqual('Wrap _Long Lines', items[0].props.label)
+
+    def test_get_button_bar_with_none(self):
+        window = DiffWindow()
+        self.assertIs(None, window._get_button_bar(None))
+
+    def test_get_button_bar_with_operations(self):
+        window = DiffWindow()
+        method = MockMethod()
+        button_bar = window._get_button_bar([('title', method)])
+        self.assertIsNot(None, button_bar)
+        buttons = button_bar.get_children()
+        self.assertEqual(1, len(buttons))
+        self.assertEqual('title', buttons[0].props.label)
+        buttons[0].emit('clicked')
+        self.assertIs(True, method.called)
+
+
+
 class MockDiffWidget(object):
 
     def set_diff_text_sections(self, sections):