[MERGE] [bug #111664] bzr rm refuses to delete renamed files

[John Arbash Meinel]

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

Alexander Belchenko wrote:
> Marius Kruger ?8H5B:
>>> On 5/3/07, Marius Kruger <amanic at gmail.com> wrote:
>>>> hi
>>>>
>>>> here is the fix for the above mentioned bug,
>>>> I still need to add a test for a modified renamed file.
>>> done
> 
> === modified file bzrlib/tests/workingtree_implementations/test_remove.py
> --- bzrlib/tests/workingtree_implementations/test_remove.py
> +++ bzrlib/tests/workingtree_implementations/test_remove.py
> @@ -24,6 +24,7 @@
>      """Tests WorkingTree.remove"""
> 
>      files=['a', 'b/', 'b/c', 'd/']
> +    rfiles=['b/c', 'b', 'a', 'd']
> 
> ^-- please use spaces around =, i.e.
> 
>     files = ['a', 'b/', 'b/c', 'd/']
>     rfiles = ['b/c', 'b', 'a', 'd']
> 
> 
> 
> @@ -34,6 +35,11 @@
>          self.build_tree(TestRemove.files)
>          return tree
> 
> +    def writeFile(self, relpath, content="Some content"):
> +        f = file(relpath, 'wb')
> +        f.write(content)
> +        f.close()
> +
> 
> ^-- Martin always suggest to use try-finally block for such things, i.e.
> 
>         f = file(relpath, 'wb')
> 	try:
> 	        f.write(content)
> 	finally:
> 		f.close()
> 

It is also better to just use:

self.build_tree_content([('filename', 'contents\n')]) which does that
for you.

Or if you don't care about the content
self.build_tree(['dir/', 'file', 'dir/file'])

> 
> +    def test_remove_renamed_changed_files(self):
> +        """Check that files are not removed if they are renamed and changed."""
> +        tree = self.getTree()
> +        tree.add(TestRemove.files)
> +        tree.commit("make sure files are versioned")
> +
> +        for f in TestRemove.rfiles:
> +            tree.rename_one(f,f+'x')
> +        rfilesx = ['bx/cx', 'bx', 'ax', 'dx']
> +        self.writeFile('ax','changed and renamed!')
> +        self.writeFile('bx/cx','changed and renamed!')
> +        self.assertInWorkingTree(rfilesx)
> +        self.failUnlessExists(rfilesx)
> +
> +        try:
> +            tree.remove(rfilesx, keep_files=False)
> +            self.fail('Should throw BzrRemoveChangedFilesError')
> +        except errors.BzrRemoveChangedFilesError, e:
> +            self.assertTrue(re.match('Can\'t remove changed or unknown files:'
> +                '.*modified:.*ax.*bx/cx.*',
> +                str(e), re.DOTALL))
> +        self.assertInWorkingTree(rfilesx)
> +        self.failUnlessExists(rfilesx)
> +
> 
> ^-- I'd like to propose instead of manual try-except use something similar
>     to recipe from Ned Batchelder:
> 	http://www.nedbatchelder.com/blog/200609.html#e20060905T064418
>     But his original recipe don't use regular expressions match, only
>     strings compare.
> 
> [µ]


Our "self.assertRaises()" returns the exception object. so you can match it.

We also have "self.assertContainsRe(haystack, needle_re)". So you
shouldn't need to manually create a 'assertTrue(re.match())' statement.

John
=:->

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

iD8DBQFGO2yFJdeBCYSNAAMRApx+AJ0aIeNuWtXdTIr/DzS8ocWWY7UZwgCfd8XM
vFkeglnzZdpj77gc6cRH3Og=
=vS8V
-----END PGP SIGNATURE-----