Code review comment for lp://staging/~nmb/bzr/update-feedback

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.

« Back to merge proposal