Merge lp:~sinzui/bzr-gtk/gpush-do-push into lp:bzr-gtk

Proposed by Curtis Hovey on 2012-03-11
Status: Merged
Approved by: Jelmer Vernooij on 2012-03-20
Approved revision: 799
Merged at revision: 788
Proposed branch: lp:~sinzui/bzr-gtk/gpush-do-push
Merge into: lp:bzr-gtk
Diff against target: 209 lines (+146/-31)
2 files modified
push.py (+51/-31)
tests/test_push.py (+95/-0)
To merge this branch: bzr merge lp:~sinzui/bzr-gtk/gpush-do-push
Reviewer Review Type Date Requested Status
Jelmer Vernooij (community) 2012-03-11 Approve on 2012-03-20
Review via email: mp+96928@code.launchpad.net

Description of the change

Update gpush.do_push to use fast, non-deprecated calls.

do_push() call branch.revision_history() and and PullResult.__init__
which are deprecated. Working with revision_history slows down the push.

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

RULES

    * Add tests for the current do_push().
    * Base a new do_push on bzrlib.push which uses create_clone_on_transport
      or push_branch to do the task and it is much faster.
      * Do not call deprecated methods.
    * Remove the old do_push() with the new do_push() when the tests pass for
      both.
    * Provide a better method to show messages about the push to the user.

QA

    BZR_PLUGINS_AT=gtk@./ bzr gpush

IMPLEMENTATION

The diff is a little surprising since I had the new and old do_push
method running in parallel for a time. I struggled writing tests that
setup the three main condition of the old do_push(). The methods
push_branch() and create_clone_on_transport() handle some of the
try-except and if-else rules of the old do_push method. I had to add a
method the create a PushResult to ensure consistent data was available
to make a message. The message function makes a message for the UI based
on the old do_push message and the messages that were created for the
text UI when using `bzr push`.

To post a comment you must log in.
lp:~sinzui/bzr-gtk/gpush-do-push updated on 2012-03-11
799. By Curtis Hovey on 2012-03-11

Merged trunk.

Jelmer Vernooij (jelmer) :
review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'push.py'
2--- push.py 2012-03-05 14:01:51 +0000
3+++ push.py 2012-03-11 21:44:18 +0000
4@@ -24,7 +24,8 @@
5 errors,
6 ui,
7 )
8-from bzrlib.bzrdir import BzrDir
9+from bzrlib.controldir import ControlDir
10+from bzrlib.push import PushResult
11 from bzrlib.transport import get_transport
12
13 from bzrlib.plugins.gtk.dialog import (
14@@ -140,43 +141,62 @@
15 :param overwrite: overwrite target location if it diverged
16 :return: number of revisions pushed
17 """
18- transport = get_transport(location)
19- location_url = transport.base
20-
21+ revision_id = None
22+ to_transport = get_transport(location)
23 try:
24- dir_to = BzrDir.open(location_url)
25- br_to = dir_to.open_branch()
26+ dir_to = ControlDir.open_from_transport(to_transport)
27 except errors.NotBranchError:
28- # create a branch.
29- transport = transport.clone('..')
30+ dir_to = None
31+
32+ if dir_to is None:
33 try:
34- relurl = transport.relpath(location_url)
35- transport.mkdir(relurl)
36+ br_to = br_from.create_clone_on_transport(
37+ to_transport, revision_id=revision_id)
38 except errors.NoSuchFile:
39 response = question_dialog(
40 _i18n('Non existing parent directory'),
41 _i18n("The parent directory (%s)\ndoesn't exist. Create?") %
42- location)
43+ location)
44 if response == Gtk.ResponseType.OK:
45- transport.create_prefix()
46+ br_to = br_from.create_clone_on_transport(
47+ to_transport, revision_id=revision_id, create_prefix=True)
48 else:
49- return
50- dir_to = br_from.bzrdir.clone(location_url,
51- revision_id=br_from.last_revision())
52- br_to = dir_to.open_branch()
53- count = len(br_to.revision_history())
54+ return _i18n("Push aborted.")
55+ push_result = create_push_result(br_from, br_to)
56 else:
57- br_to.revision_history()
58- try:
59- tree_to = dir_to.open_workingtree()
60- except errors.NotLocalUrl:
61- # FIXME - what to do here? how should we warn the user?
62- count = br_to.pull(br_from, overwrite)
63- except errors.NoWorkingTree:
64- count = br_to.pull(br_from, overwrite)
65- else:
66- count = tree_to.pull(br_from, overwrite)
67-
68- # The count var is either an int or a PushResult. PushResult is being
69- # coerced into an int, but the method is deprecated.
70- return _i18n("%d revision(s) pushed.") % int(count)
71+ push_result = dir_to.push_branch(br_from, revision_id, overwrite)
72+ message = create_push_message(br_from, push_result)
73+ return message
74+
75+
76+def create_push_message(br_from, push_result):
77+ """Return a mesage explaining what happened during the push."""
78+ messages = []
79+ rev_count = br_from.revno() - push_result.old_revno
80+ messages.append(_i18n("%d revision(s) pushed.") % rev_count)
81+ if push_result.stacked_on is not None:
82+ messages.append(_i18n("Stacked on %s.") % push_result.stacked_on)
83+ if push_result.workingtree_updated is False:
84+ messages.append(_i18n(
85+ "\nThe working tree was not updated:"
86+ "\nSee 'bzr help working-trees' for more information."))
87+ return '\n'.join(messages)
88+
89+
90+def create_push_result(br_from, br_to):
91+ """Return a PushResult like one created by ControlDir.push_branch()."""
92+ push_result = PushResult()
93+ push_result.source_branch = br_from
94+ push_result.target_branch = br_to
95+ push_result.branch_push_result = None
96+ push_result.master_branch = None
97+ push_result.old_revno = 0
98+ push_result.old_revid = br_to.last_revision()
99+ push_result.workingtree_updated = None # Not applicable to this case.
100+ try:
101+ push_result.stacked_on = br_to.get_stacked_on_url()
102+ except (errors.UnstackableBranchFormat,
103+ errors.UnstackableRepositoryFormat,
104+ errors.NotStacked):
105+ push_result.stacked_on = None
106+ return push_result
107
108=== modified file 'tests/test_push.py'
109--- tests/test_push.py 2012-03-04 01:54:22 +0000
110+++ tests/test_push.py 2012-03-11 21:44:18 +0000
111@@ -133,3 +133,98 @@
112 (branch, 'lp:~user/fnord/test'), push.do_push.args)
113 self.assertEqual(
114 {'overwrite': True}, push.do_push.kwargs)
115+
116+
117+class DoPushTestCase(tests.TestCaseWithTransport):
118+
119+ def setup_ui(self):
120+ set_ui_factory()
121+ progress_panel = ProgressPanel()
122+ ui.ui_factory.set_progress_bar_widget(progress_panel)
123+ MockMethod.bind(self, progress_panel.pb, 'tick')
124+ MockMethod.bind(self, progress_panel.pb, 'update')
125+ MockMethod.bind(self, progress_panel.pb, 'finished')
126+ return progress_panel
127+
128+ def make_from_branch(self):
129+ from_tree = self.make_branch_and_tree('this')
130+ self.build_tree(['this/a', 'this/b'])
131+ from_tree.add(['a', 'b'])
132+ from_tree.commit("msg")
133+ return from_tree.branch
134+
135+ def test_do_push_without_dir(self):
136+ progress_panel = self.setup_ui()
137+ from_branch = self.make_from_branch()
138+ message = push.do_push(from_branch, 'that', False)
139+ self.assertEqual('1 revision(s) pushed.', message)
140+ self.assertEqual(True, progress_panel.pb.update.called)
141+ self.assertEqual(True, progress_panel.pb.finished.called)
142+
143+ def test_do_push_without_parent_dir(self):
144+ progress_panel = self.setup_ui()
145+ from_branch = self.make_from_branch()
146+ MockMethod.bind(self, push, 'question_dialog', Gtk.ResponseType.OK)
147+ message = push.do_push(from_branch, 'that/there', False)
148+ self.assertEqual('1 revision(s) pushed.', message)
149+ self.assertEqual(True, push.question_dialog.called)
150+ self.assertEqual(True, progress_panel.pb.update.called)
151+ self.assertEqual(True, progress_panel.pb.finished.called)
152+
153+ def test_do_push_without_parent_dir_aborted(self):
154+ self.setup_ui()
155+ from_branch = self.make_from_branch()
156+ MockMethod.bind(self, push, 'question_dialog', Gtk.ResponseType.CANCEL)
157+ message = push.do_push(from_branch, 'that/there', False)
158+ self.assertEqual('Push aborted.', message)
159+
160+ def test_do_push_with_dir(self):
161+ progress_panel = self.setup_ui()
162+ self.make_branch_and_tree('that')
163+ from_branch = self.make_from_branch()
164+ message = push.do_push(from_branch, 'that', False)
165+ self.assertEqual('1 revision(s) pushed.', message)
166+ self.assertEqual(True, progress_panel.pb.update.called)
167+ self.assertEqual(True, progress_panel.pb.finished.called)
168+
169+ def test_create_push_result_unstacked(self):
170+ from_branch = object()
171+ to_branch = self.make_branch_and_tree('that').branch
172+ push_result = push.create_push_result(from_branch, to_branch)
173+ self.assertIs(from_branch, push_result.source_branch)
174+ self.assertIs(to_branch, push_result.target_branch)
175+ self.assertIs(None, push_result.branch_push_result)
176+ self.assertIs(None, push_result.master_branch)
177+ self.assertEqual(0, push_result.old_revno)
178+ self.assertEqual(to_branch.last_revision(), push_result.old_revid)
179+ self.assertIs(None, push_result.workingtree_updated)
180+ self.assertIs(None, push_result.stacked_on)
181+
182+ def test_create_push_result_stacked(self):
183+ from_branch = object()
184+ to_branch = self.make_branch_and_tree('that').branch
185+ MockMethod.bind(self, to_branch, 'get_stacked_on_url', 'lp:project')
186+ push_result = push.create_push_result(from_branch, to_branch)
187+ self.assertEqual('lp:project', push_result.stacked_on)
188+
189+ def test_create_push_message_stacked_on(self):
190+ from_branch = object()
191+ to_branch = self.make_branch_and_tree('that').branch
192+ MockMethod.bind(self, to_branch, 'get_stacked_on_url', 'lp:project')
193+ push_result = push.create_push_result(from_branch, to_branch)
194+ from_branch = self.make_from_branch()
195+ message = push.create_push_message(from_branch, push_result)
196+ self.assertEqual(
197+ '1 revision(s) pushed.\nStacked on lp:project.', message)
198+
199+ def test_create_push_message_workingtree_updated_false(self):
200+ from_branch = object()
201+ to_branch = self.make_branch_and_tree('that').branch
202+ push_result = push.create_push_result(from_branch, to_branch)
203+ push_result.workingtree_updated = False
204+ from_branch = self.make_from_branch()
205+ message = push.create_push_message(from_branch, push_result)
206+ self.assertEqual(
207+ "1 revision(s) pushed.\n\nThe working tree was not updated:\n"
208+ "See 'bzr help working-trees' for more information.",
209+ message)

Subscribers

People subscribed via source and target branches

to all changes: