[patch] 'bzr mv' with no arguments breaks

[John Arbash Meinel]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Wouter van Heyst wrote:
> On Wed, Jul 05, 2006 at 08:50:26AM -0500, John Arbash Meinel wrote:
>> Wouter van Heyst wrote:
>>> Moshe Zadka reported bzr mv failing with 
>>> bzr: ERROR: exceptions.TypeError: len() of unsized object
>>>
>>> The issue itself is easy to fix, but in writing a test I decided to
>>> extract old move tests from too_much.OldTests. Ouch.
>>>
>>> I'm not really happy with the result, one problem I don't understand is that
>>> commit on a workingtree works differently than self.run_bzr('commit')
>> One thing you have to be aware of, is that a WorkingTree object won't
>> necessarily reload its state if you use run_bzr behind its back.
>>
>> So doing:
>>
>> wt = self.make_branch_and_tree('.')
>> ...
>> self.run_bzr('mv', 'a', 'b')
>>
>> You may need to do one of:
>> self.wt.read_inventory()
>>
>> or
>> self.wt = self.wt.bzrdir.open_workingtree()
> 
> Ah, that certainly got me. Doing a tree.read_working_inventory() after
> moving around files fixes the problem for me.
> 
> ...

Well, as Robert said, this probably means there are some bugs in
WorkingTree, since it is caching things longer than it should.

> 
>>> +    def test_mv_unversioned(self):
>>> +        self.build_tree(['unversioned.txt'])
>>> +        out, err = self.run_bzr('mv', 'unversioned.txt', 'elsewhere', retcode=3)
>>> +        self.assertContainsRe(err, "^bzr: ERROR: can't rename: old name .* is not versioned\n")
>>> +
>> You can use the new: self.run_bzr_error() function, which takes a list
>> of regexes to match on the error output. This would make it:
>> self.run_bzr_error(["^bzr: ERROR: can't rename: old name .* ..."],
>> 		   'mv', 'unversioned.txt', 'elsewhere')
>>
>> It defaults to retcode=3
> 
> That is certainly useful. I've now also replaced the assertEquals, but
> I'm not sure that is a good idea. This way I'm not asserting that the
> error is all the output there is, and you can easily exceed 80 columns
> if you don't want to split the error string. 


Well, you can also grab out,err from run_bzr_error.

out, err = self.run_bzr_error(...

It is just a helper for when you want a regex. A lot of times there are
specific details that can change (like the exact
revision_id/username/etc). But you really want to match a regex.

Do what makes the most sense to you.

> 
>> The rest all looks good. It would be nice if you could figure out why
>> committing doesn't do what you want, and do a little bit of cleanup. Let
>> me know if you find run_bzr_error() ugly for any reason. I'd like it to
>> be a helpful function. But otherwise +1 from me.
> 
> I've cleaned the patch up a bit, I'll see if I can find out where the
> bug in workingtree is, but rereading the inventory works for now.
> 
> Wouter van Heyst
> 

I'll try to review your patch later. But I wanted to give you a little
feedback right away.

John
=:->

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.2.2 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iD8DBQFEr8OiJdeBCYSNAAMRAs11AJ9Rq+CWm9sW2LAe5l/20FBcE4483wCeIxGh
cFVMmFYR8Y06ZO7OlIbAiRU=
=EgLn
-----END PGP SIGNATURE-----