[patch] 'bzr mv' with no arguments breaks

[John Arbash Meinel]

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()


> 
> With tree.commit('move to parent directory') in test_mv_relative uncommented
> the move results in hello.txt being unversioned. But with run_bzr, all is fine.
> 
> 
> "bzr: ERROR: u'sub1/sub2/hello.txt' is not versioned\n"

If this is what you are running into, it fits just fine, because run_bzr
is creating a new WorkingTree instance.

...

> Other than not using commits as much as in too_much I also dropped some relpath
> invocations in the conversion, it wasn't entirely sure what was being tested
> and what was auxiliary in OldTests.
> 
> Patch attached in hope of enlightenment,
> Wouter van Heyst
> 
> 
> ------------------------------------------------------------------------
> 

...

> +    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

...

> +    def test_mv_invalid(self):
> +        tree = self.make_branch_and_tree('.')
> +        self.build_tree(['test.txt', 'sub1/'])
> +        tree.add(['test.txt'])
> +
> +        out, err = self.run_bzr('rename', 'test.txt', 'sub1', retcode=3)
> +        self.assertEquals("bzr: ERROR: destination u'sub1' is not a versioned directory\n", err)
> +
> +        out, err = self.run_bzr('rename', 'test.txt', 'sub1/hello.txt', retcode=3)
> +        self.assertEquals("bzr: ERROR: can't determine destination directory id for u'sub1'\n", err)
> +        self.run_bzr('move', 'test.txt', 'sub1', retcode=3)
> +    

Here it would be nice if you check the error output. We've had a few
bugs where the command doesn't succeed, but not for the reason we
thought it was failing.


> +    def test_mv_dirs(self):
> +        tree = self.make_branch_and_tree('.')
> +        self.build_tree(['hello.txt', 'sub1/'])
> +        tree.add(['hello.txt', 'sub1'])
> +
> +        self.run_bzr('rename', 'sub1', 'sub2')
> +        self.run_bzr('move', 'hello.txt', 'sub2')
> +
> +        self.failUnlessExists("sub2")
> +        self.failUnlessExists("sub2/hello.txt")
> +        self.failIfExists("sub1")
> +        self.failIfExists("hello.txt")
> +
> +        #tree.commit('commit with some things moved to subdirs', allow_pointless=False)
> +        #self.run_bzr('commit', '-m', 'move somethings')

This is probably where you need to reload the WorkingTree.
Either that, or if you don't really want to commit, then remove these lines.

> +
> +        self.build_tree(['sub1/'])
> +        tree.add(['sub1'])
> +        self.run_bzr('move', 'sub2/hello.txt', 'sub1')
> +        self.failIfExists('sub2/hello.txt')
> +        self.failUnlessExists('sub1/hello.txt')
> +        self.run_bzr('move', 'sub2', 'sub1')
> +        self.failIfExists('sub2')
> +        self.failUnlessExists('sub1/sub2')

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.

John
=:->

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 254 bytes
Desc: OpenPGP digital signature
Url : https://lists.ubuntu.com/archives/bazaar/attachments/20060705/f521a61f/attachment.pgp