Bazaar

Merge lp://staging/~nmb/bzr/update-feedback into lp://staging/bzr

  1. update-feedback
  2. Merge into bzr.dev
Proposed by Neil Martinsen-Burrell
Status: Merged
Approved by: Vincent Ladeuil
Approved revision: not available
Merged at revision: not available
Proposed branch: lp://staging/~nmb/bzr/update-feedback
Merge into: lp://staging/bzr
Diff against target: 219 lines (+55/-22)
5 files modified
NEWS (+3/-0)
bzrlib/builtins.py (+8/-2)
bzrlib/tests/blackbox/test_filtered_view_ops.py (+3/-3)
bzrlib/tests/blackbox/test_update.py (+38/-16)
bzrlib/tests/commands/test_update.py (+3/-1)
To merge this branch: bzr merge lp://staging/~nmb/bzr/update-feedback
Reviewer Review Type Date Requested Status
Vincent Ladeuil Approve
Review via email: mp+16086@code.staging.launchpad.net
To post a comment you must log in.
Revision history for this message
Neil Martinsen-Burrell (nmb) wrote :

This branch adds additional feedback for the update command on which branch the working tree is up to date with. It doesn't (currently) do anything fancy for formatting the branch URL, beyond using an appropriate encoding. Further work could be done to make it print ./... for relative paths, etc. I updated all of the tests that I could find that depend on this text, and ``bzr selftest update`` passes on my machine (Mac OS X 10.5.8).

Revision history for this message
Martin Pool (mbp) wrote :

2009/12/13 Neil Martinsen-Burrell <email address hidden>:
> Neil Martinsen-Burrell has proposed merging lp:~nmb/bzr/update-feedback into lp:bzr.
>
>    Requested reviews:
>    bzr-core (bzr-core)
>
>
> This branch adds additional feedback for the update command on which branch the working tree is up to date with.  It doesn't (currently) do anything fancy for formatting the branch URL, beyond using an appropriate encoding.  Further work could be done to make it print ./... for relative paths, etc.  I updated all of the tests that I could find that depend on this text, and ``bzr selftest update`` passes on my machine (Mac OS X 10.5.8).

Nice.

It would be better to test the paths it actually prints but that can
be a bit fiddly.

> --- bzrlib/tests/commands/test_update.py        2009-03-23 14:59:43 +0000
> +++ bzrlib/tests/commands/test_update.py        2009-12-13 03:14:14 +0000
> @@ -19,7 +19,7 @@
>     branch,
>     builtins,
>     )
> -from bzrlib.tests import transport_util
> +from bzrlib.tests import transport_util, StringIOWrapper
>
>
>  class TestUpdate(transport_util.TestCaseWithConnectionHookedTransport):
> @@ -36,6 +36,8 @@
>         self.start_logging_connections()
>
>         update = builtins.cmd_update()
> +        # update needs the encoding from outf to print URLs
> +        update.outf = StringIOWrapper()
>         # update calls it 'dir' where other commands calls it 'directory'
>         update.run(dir='local')
>         self.assertEquals(1, len(self.connections))

Hm, this is a strange way to run it rather than through run_bzr, and
needing to randomly poke in a StringIO seems to indicate a problem
with it. But maybe that's what Robert wants for tests.commands?

--
Martin <http://launchpad.net/~mbp/>

Revision history for this message
Neil Martinsen-Burrell (nmb) wrote :

On 2009-12-13 20:18 , Martin Pool wrote:
> 2009/12/13 Neil Martinsen-Burrell<email address hidden>:
>> Neil Martinsen-Burrell has proposed merging lp:~nmb/bzr/update-feedback into lp:bzr.
>>
>> Requested reviews:
>> bzr-core (bzr-core)
>>
>>
>> This branch adds additional feedback for the update command on which branch the working tree is up to date with. It doesn't (currently) do anything fancy for formatting the branch URL, beyond using an appropriate encoding. Further work could be done to make it print ./... for relative paths, etc. I updated all of the tests that I could find that depend on this text, and ``bzr selftest update`` passes on my machine (Mac OS X 10.5.8).
>
> Nice.
>
> It would be better to test the paths it actually prints but that can
> be a bit fiddly.

Agreed. If you can give me a pointer to how that might work, I'd be
glad to replace the .*'s with it.

>
>> --- bzrlib/tests/commands/test_update.py 2009-03-23 14:59:43 +0000
>> +++ bzrlib/tests/commands/test_update.py 2009-12-13 03:14:14 +0000
>> @@ -19,7 +19,7 @@
>> branch,
>> builtins,
>> )
>> -from bzrlib.tests import transport_util
>> +from bzrlib.tests import transport_util, StringIOWrapper
>>
>>
>> class TestUpdate(transport_util.TestCaseWithConnectionHookedTransport):
>> @@ -36,6 +36,8 @@
>> self.start_logging_connections()
>>
>> update = builtins.cmd_update()
>> + # update needs the encoding from outf to print URLs
>> + update.outf = StringIOWrapper()
>> # update calls it 'dir' where other commands calls it 'directory'
>> update.run(dir='local')
>> self.assertEquals(1, len(self.connections))
>
> Hm, this is a strange way to run it rather than through run_bzr, and
> needing to randomly poke in a StringIO seems to indicate a problem
> with it. But maybe that's what Robert wants for tests.commands?

This is what I found in
bzrlib/tests/commands/test_{push,pull,missing,...}.py and I just copied it.

Revision history for this message
Neil Martinsen-Burrell (nmb) wrote :

> Agreed. If you can give me a pointer to how that might work, I'd be
> glad to replace the .*'s with it.

Never mind. Got it figured out. See r4881 of the branch.

Revision history for this message
Vincent Ladeuil (vila) wrote :

Thanks for working on that.

I've cleanup the tests a bit as they were becoming a bit messy and out of date.

- I fixed some imports,

- I used assertEqualDiff instead of assertContainsRe because it make the tests clearer,
  more robust and easier to debug.

I've also removed the final '.' (and also the final '/') in the messages as it looks
like it was part of to the path ('.' is valid character in paths).
Some people heavily rely on the ability to copy/paste from there and that
was presenting an incorrect path.

Have a look at the result when it lands in bzr.dev if you're interested.

I think you addressed Martin's concerns so I will land this.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'NEWS'
2--- NEWS 2009-12-10 21:49:20 +0000
3+++ NEWS 2009-12-14 03:04:17 +0000
4@@ -112,6 +112,9 @@
5 'file://localhost/' as well as 'file:///' URLs on POSIX. (Michael
6 Hudson)
7
8+* ``bzr update`` provides feedback on which branch it is up to date with.
9+ (Neil Martinsen-Burrell)
10+
11 Documentation
12 *************
13
14
15=== modified file 'bzrlib/builtins.py'
16--- bzrlib/builtins.py 2009-12-09 08:38:02 +0000
17+++ bzrlib/builtins.py 2009-12-14 03:04:16 +0000
18@@ -1400,8 +1400,12 @@
19 possible_transports=possible_transports)
20 if master is not None:
21 tree.lock_write()
22+ branch_location = master.base
23 else:
24 tree.lock_tree_write()
25+ branch_location = tree.branch.base
26+ branch_location = urlutils.unescape_for_display(branch_location,
27+ self.outf.encoding)
28 try:
29 existing_pending_merges = tree.get_parent_ids()[1:]
30 last_rev = _mod_revision.ensure_null(tree.last_revision())
31@@ -1411,7 +1415,8 @@
32 if master is None or last_rev == _mod_revision.ensure_null(
33 master.last_revision()):
34 revno = tree.branch.revision_id_to_revno(last_rev)
35- note("Tree is up to date at revision %d." % (revno,))
36+ note("Tree is up to date at revision %d of branch %s." %
37+ (revno, branch_location))
38 return 0
39 view_info = _get_view_info_for_change_reporter(tree)
40 conflicts = tree.update(
41@@ -1419,7 +1424,8 @@
42 view_info=view_info), possible_transports=possible_transports)
43 revno = tree.branch.revision_id_to_revno(
44 _mod_revision.ensure_null(tree.last_revision()))
45- note('Updated to revision %d.' % (revno,))
46+ note('Updated to revision %d of branch %s.' %
47+ (revno, branch_location))
48 if tree.get_parent_ids()[1:] != existing_pending_merges:
49 note('Your local commits will now show as pending merges with '
50 "'bzr status', and can be committed with 'bzr commit'.")
51
52=== modified file 'bzrlib/tests/blackbox/test_filtered_view_ops.py'
53--- bzrlib/tests/blackbox/test_filtered_view_ops.py 2009-04-08 03:34:31 +0000
54+++ bzrlib/tests/blackbox/test_filtered_view_ops.py 2009-12-14 03:04:16 +0000
55@@ -189,11 +189,11 @@
56 os.chdir("tree_2")
57 self.run_bzr("bind ../tree_1")
58 out, err = self.run_bzr('update')
59- self.assertEqualDiff(
60+ self.assertStartsWith(err,
61 "Operating on whole tree but only reporting on 'my' view.\n"
62 " M a\n"
63- "All changes applied successfully.\n"
64- "Updated to revision 2.\n", err)
65+ "All changes applied successfully.\n")
66+ self.assertContainsRe(err, "Updated to revision 2 of branch .*.")
67 self.assertEqualDiff("", out)
68
69 def test_view_on_merge(self):
70
71=== modified file 'bzrlib/tests/blackbox/test_update.py'
72--- bzrlib/tests/blackbox/test_update.py 2009-03-23 14:59:43 +0000
73+++ bzrlib/tests/blackbox/test_update.py 2009-12-14 03:04:16 +0000
74@@ -18,11 +18,12 @@
75
76 """Tests for the update command of bzr."""
77
78-import os
79+import os, re
80
81 from bzrlib import branch, bzrdir
82 from bzrlib.tests.blackbox import ExternalBase
83 from bzrlib.workingtree import WorkingTree
84+from bzrlib.osutils import pathjoin
85
86
87 class TestUpdate(ExternalBase):
88@@ -30,27 +31,39 @@
89 def test_update_standalone_trivial(self):
90 self.make_branch_and_tree('.')
91 out, err = self.run_bzr('update')
92- self.assertEqual('Tree is up to date at revision 0.\n', err)
93+ self.assertContainsRe(err, 'Tree is up to date at revision 0'
94+ ' of branch %s.' % re.escape(self.test_dir))
95+ self.assertEqual('', out)
96+
97+ def test_update_quiet(self):
98+ self.make_branch_and_tree('.')
99+ out, err = self.run_bzr('update --quiet')
100+ self.assertEqual('', err)
101 self.assertEqual('', out)
102
103 def test_update_standalone_trivial_with_alias_up(self):
104 self.make_branch_and_tree('.')
105 out, err = self.run_bzr('up')
106- self.assertEqual('Tree is up to date at revision 0.\n', err)
107+ self.assertContainsRe(err, 'Tree is up to date at revision 0'
108+ ' of branch %s.' % re.escape(self.test_dir))
109 self.assertEqual('', out)
110
111 def test_update_up_to_date_light_checkout(self):
112 self.make_branch_and_tree('branch')
113 self.run_bzr('checkout --lightweight branch checkout')
114 out, err = self.run_bzr('update checkout')
115- self.assertEqual('Tree is up to date at revision 0.\n', err)
116+ branch_path = pathjoin(self.test_dir, 'branch/')
117+ self.assertContainsRe(err, 'Tree is up to date at revision 0'
118+ ' of branch %s.' % re.escape(branch_path))
119 self.assertEqual('', out)
120
121 def test_update_up_to_date_checkout(self):
122 self.make_branch_and_tree('branch')
123 self.run_bzr('checkout branch checkout')
124 out, err = self.run_bzr('update checkout')
125- self.assertEqual('Tree is up to date at revision 0.\n', err)
126+ branch_path = pathjoin(self.test_dir, 'branch/')
127+ self.assertContainsRe(err, 'Tree is up to date at revision 0'
128+ ' of branch %s.' % re.escape(branch_path))
129 self.assertEqual('', out)
130
131 def test_update_out_of_date_standalone_tree(self):
132@@ -66,8 +79,10 @@
133 out,err = self.run_bzr('update branch')
134 self.assertEqual('', out)
135 self.assertContainsRe(err, '\+N file')
136- self.assertEndsWith(err, 'All changes applied successfully.\n'
137- 'Updated to revision 1.\n')
138+ self.assertContainsRe(err, 'All changes applied successfully.')
139+ branch_path = pathjoin(self.test_dir, 'branch/')
140+ self.assertContainsRe(err, 'Updated to revision 1 of branch %s.' %
141+ re.escape(branch_path))
142 self.failUnlessExists('branch/file')
143
144 def test_update_out_of_date_light_checkout(self):
145@@ -81,8 +96,10 @@
146 # now checkout2 should be out of date
147 out,err = self.run_bzr('update checkout2')
148 self.assertContainsRe(err, '\+N file')
149- self.assertEndsWith(err, 'All changes applied successfully.\n'
150- 'Updated to revision 1.\n')
151+ self.assertContainsRe(err, r'All changes applied successfully\.')
152+ branch_path = pathjoin(self.test_dir, 'branch/')
153+ self.assertContainsRe(err, r'Updated to revision 1 of branch %s.' %
154+ re.escape(branch_path))
155 self.assertEqual('', out)
156
157 def test_update_conflicts_returns_2(self):
158@@ -105,9 +122,10 @@
159 a_file.close()
160 out,err = self.run_bzr('update checkout2', retcode=1)
161 self.assertContainsRe(err, 'M file')
162- self.assertEqual(['1 conflicts encountered.',
163- 'Updated to revision 2.'],
164- err.split('\n')[-3:-1])
165+ self.assertContainsRe(err, '1 conflicts encountered.')
166+ branch_path = pathjoin(self.test_dir, 'branch/')
167+ self.assertContainsRe(err, 'Updated to revision 2 of branch %s.' %
168+ re.escape(branch_path))
169 self.assertContainsRe(err, 'Text conflict in file\n')
170 self.assertEqual('', out)
171
172@@ -147,8 +165,10 @@
173 self.assertEqual('', out)
174 self.assertContainsRe(err, '\+N file')
175 self.assertContainsRe(err, '\+N file_b')
176- self.assertContainsRe(err, 'Updated to revision 1.\n'
177- 'Your local commits will now show as'
178+ branch_path = pathjoin(self.test_dir, 'master/')
179+ self.assertContainsRe(err, 'Updated to revision 1 of branch %s.' %
180+ re.escape(branch_path))
181+ self.assertContainsRe(err, 'Your local commits will now show as'
182 ' pending merges')
183 self.assertEqual([master_tip, child_tip], wt.get_parent_ids())
184 self.failUnlessExists('checkout/file')
185@@ -195,8 +215,10 @@
186 # merges, because they were real merges
187 out, err = self.run_bzr('update')
188 self.assertEqual('', out)
189- self.assertEndsWith(err, 'All changes applied successfully.\n'
190- 'Updated to revision 2.\n')
191+ self.assertContainsRe(err, 'All changes applied successfully.')
192+ branch_path = pathjoin(self.test_dir, 'master/')
193+ self.assertContainsRe(err, 'Updated to revision 2 of branch %s.' %
194+ re.escape(branch_path))
195 self.assertContainsRe(err, r'\+N file3')
196 # The pending merges should still be there
197 self.assertEqual(['o2'], checkout1.get_parent_ids()[1:])
198
199=== modified file 'bzrlib/tests/commands/test_update.py'
200--- bzrlib/tests/commands/test_update.py 2009-03-23 14:59:43 +0000
201+++ bzrlib/tests/commands/test_update.py 2009-12-14 03:04:16 +0000
202@@ -19,7 +19,7 @@
203 branch,
204 builtins,
205 )
206-from bzrlib.tests import transport_util
207+from bzrlib.tests import transport_util, StringIOWrapper
208
209
210 class TestUpdate(transport_util.TestCaseWithConnectionHookedTransport):
211@@ -36,6 +36,8 @@
212 self.start_logging_connections()
213
214 update = builtins.cmd_update()
215+ # update needs the encoding from outf to print URLs
216+ update.outf = StringIOWrapper()
217 # update calls it 'dir' where other commands calls it 'directory'
218 update.run(dir='local')
219 self.assertEquals(1, len(self.connections))