Merge lp:~sinzui/bzr-gtk/precise-commit-0 into lp:bzr-gtk

[Curtis Hovey]

Define _last_selected_file in init so that async signal callbacks can use it.

I occasionally see the following traceback when committing use bzr-gtk
r769 on precise:

  File "/home/curtis/.bazaar/plugins/gtk/commit.py", line 579, in _on_treeview_files_cursor_changed
    self._update_per_file_info(selection)
  File "/home/curtis/.bazaar/plugins/gtk/commit.py", line 618, in _update_per_file_info
    self._save_current_file_message()
  File "/home/curtis/.bazaar/plugins/gtk/commit.py", line 605, in _save_current_file_message
    if self._last_selected_file is None:
AttributeError: 'CommitDialog' object has no attribute '_last_selected_file'

The commit is successful. Looking at the order of events,
_construct_file_list() connects _on_treeview_files_cursor_changed() to
the cursor-changed signal. _on_treeview_files_cursor_changed() presumes
_last_selected_file() was created, but it will not be created until
_fill_in_per_file_info() is called (which is two lines before
_construct_file_list())

The cursor-changed signal may have been called after _fill_in_per_file_info(),
but it is often called before with Gtk3.

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

RULES

    * Ensure that CommitDialog._last_selected_file is defined before
      any signal callback is connected.

QA

While I do know the steps that I see the error, I do not always see it. Maybe
the issue only happens when there is a large tree of changes?
    * Run gcommit under precice
    * Type a few characters
    * Cancel the action
    * Save the message
    * Verify there is no AttributeError on strout

[Jelmer Vernooij (jelmer)]

Thanks for adding tests, bzr-gtk has far too few. :-)

review: Approve

=== modified file 'commit.py'
--- commit.py	2011-12-20 16:47:38 +0000
+++ commit.py	2012-01-22 14:44:24 +0000
@@ -114,6 +114,7 @@
         self._enable_per_file_commits = True
         self._commit_all_changes = True
         self.committed_revision_id = None # Nothing has been committed yet
+        self._last_selected_file = None
         self._saved_commit_messages_manager = SavedCommitMessagesManager(
             self._wt, self._wt.branch)
 
 
=== modified file 'tests/test_commit.py'
--- tests/test_commit.py	2011-12-20 16:47:38 +0000
+++ tests/test_commit.py	2012-01-22 14:44:24 +0000
@@ -35,6 +35,7 @@
     commit,
     commitmsgs,
     )
+from bzrlib.plugins.gtk.commitmsgs import SavedCommitMessagesManager
 
 
 # TODO: All we need is basic ancestry code to test this, we shouldn't need a
@@ -142,8 +143,47 @@
         pass # With no widgets, there are no widgets to fill out
 
 
+class MockMethod():
+
+    @classmethod
+    def bind(klass, test_instance, obj, method_name):
+        original_method = getattr(obj, method_name)
+        test_instance.addCleanup(setattr, obj, method_name, original_method)
+        setattr(obj, method_name, klass())
+
+    def __init__(self):
+        self.called = False
+        self.args = None
+        self.kwargs = None
+
+    def __call__(self, *args, **kwargs):
+        self.called = True
+        self.args = args
+        self.kwargs = kwargs
+
+
 class TestCommitDialogSimple(tests.TestCaseWithTransport):
 
+    def test_init(self):
+        MockMethod.bind(self, CommitDialogNoWidgets, 'setup_params')
+        MockMethod.bind(self, CommitDialogNoWidgets, 'construct')
+        MockMethod.bind(self, CommitDialogNoWidgets, 'fill_in_data')
+
+        tree = self.make_branch_and_tree('tree')
+        rev_id = tree.commit('first')
+        dlg = CommitDialogNoWidgets(tree)
+        self.assertIs(tree, dlg._wt)
+        self.assertIs(None, dlg._selected)
+        self.assertTrue(dlg._enable_per_file_commits)
+        self.assertTrue(dlg._commit_all_changes)
+        self.assertIs(None, dlg.committed_revision_id)
+        self.assertIs(None, dlg._last_selected_file)
+        self.assertIsInstance(
+            dlg._saved_commit_messages_manager, SavedCommitMessagesManager)
+        self.assertTrue(CommitDialogNoWidgets.setup_params.called)
+        self.assertTrue(CommitDialogNoWidgets.construct.called)
+        self.assertTrue(CommitDialogNoWidgets.fill_in_data.called)
+
     def test_setup_parameters_no_pending(self):
         tree = self.make_branch_and_tree('tree')
         rev_id = tree.commit('first')