Merge lp:~nmb/bzr/script-test-mv into lp:bzr

[Neil Martinsen-Burrell]

[Neil Martinsen-Burrell (nmb)]

This branch adds an mv command to the shell-like tests in bzrlib/tests/script.py, along with three tests modeled on those of the rm command. It only implements the simplest ``mv name1 name2`` syntax, not the more compltete ``mv file1 file2 ... dir`` command. Scratch your own itch! :)

[Martin Pool (mbp)]

That's nice.

It should be mentioned in TESTING in news.

I wonder if it should use a transport operation rather than shutil?

[John A Meinel (jameinel)]

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

Martin Pool wrote:
> That's nice.
>
> It should be mentioned in TESTING in news.
>
> I wonder if it should use a transport operation rather than shutil?

If you use shutil.move() it *does* support "path ... dir". It may not
support multiple paths, but 'move' will notice the target is a directory
an move things into it.

transport.rename() won't (being based on top of os.rename), etc.

If shutil is meant to be used on something like MemoryTransport, then we
obviously have to go there. If it is only meant to be used on disk, then
'shutil.move' is probably fine.

John
=:->

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

iEYEARECAAYFAktN1C0ACgkQJdeBCYSNAAMl1QCeLIHpBjx3Z9r2ermpLEcBEIC/
R/IAnA2BuJ6SH0UIifPMVE9pIsLDe2XU
=eKG2
-----END PGP SIGNATURE-----

[John A Meinel (jameinel)]

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

Neil Martinsen-Burrell wrote:
> Neil Martinsen-Burrell has proposed merging lp:~nmb/bzr/script-test-mv into lp:bzr.
>
> Requested reviews:
> bzr-core (bzr-core)
>
>
> This branch adds an mv command to the shell-like tests in bzrlib/tests/script.py, along with three tests modeled on those of the rm command. It only implements the simplest ``mv name1 name2`` syntax, not the more compltete ``mv file1 file2 ... dir`` command. Scratch your own itch! :)
>

I would probably add a test that "mv file dir" has the expected
"dir/file" result. Otherwise:

 review: approve

Let me know if you need help finishing this.
John
=:->

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

iEYEARECAAYFAktN1FkACgkQJdeBCYSNAAMlqQCg1RLOCyGU/ehIiJzyVfUThEdk
/4EAoIURyAfCvZBKpWIE7lzwKe8buOWe
=+uen
-----END PGP SIGNATURE-----

review: Approve

[lp:~nmb/bzr/script-test-mv]

4955. By Neil Martinsen-Burrell on 2010-01-13

added a test for "mv file dir" and a NEWS entry

[Neil Martinsen-Burrell (nmb)]

shutil (actually os) was what I saw being used in the other commands so I went with it. If these are limited to disk-based transports, then perhaps we should just document this fact.

Added one more test and a NEWS entry in the branch.

[Vincent Ladeuil (vila)]

Using os.rename() instead of shutil.move() will make the limitations more obvious.

The long term plan is to move to a transport-based implementation where moving
across file systems will not be supported anyway.

A final test to check the behaviour when the file does not exist... reveals untested code
(error was called with a single path) :)

I'll tweak and merge.

review: Approve

=== modified file 'NEWS'
--- NEWS	2010-01-13 00:25:44 +0000
+++ NEWS	2010-01-13 16:22:13 +0000
@@ -180,6 +180,10 @@
   tests that 'failed' - they're all just failures.
   (Martin Pool)
 
+* Shell-like tests no support the command "mv" for moving files.  The
+  syntax for ``mv file1 file2``, ``mv dir1 dir2`` and ``mv file dir`` is
+  supported.  (Neil Martinsen-Burrell)
+
 bzr 2.0.4 (not released yet)
 ############################
 

=== modified file 'bzrlib/tests/script.py'
--- bzrlib/tests/script.py	2009-12-11 05:29:35 +0000
+++ bzrlib/tests/script.py	2010-01-13 16:22:13 +0000
@@ -24,6 +24,7 @@
 import glob
 import os
 import shlex
+import shutil
 from cStringIO import StringIO
 
 from bzrlib import (
@@ -402,6 +403,28 @@
             retcode = 0
         return retcode, None, err
 
+    def do_mv(self, test_case, input, args):
+        err = None
+        def error(msg, path1, path2):
+            return "mv: cannot move %s to %s: %s\n" % (path1, path2, msg)
+
+        if not args or len(args) != 2:
+            raise SyntaxError("Usage: mv path1 path2")
+        path1, path2 = args
+        try:
+            shutil.move(path1, path2)
+        except OSError, e:
+            if e.errno == errno.ENOENT:
+                err = error('No such file or directory', path1)
+            else:
+                raise
+        if err:
+            retcode = 1
+        else:
+            retcode = 0
+        return retcode, None, err
+
+
 
 class TestCaseWithMemoryTransportAndScript(tests.TestCaseWithMemoryTransport):
     """Helper class to experiment shell-like test and memory fs.

=== modified file 'bzrlib/tests/test_script.py'
--- bzrlib/tests/test_script.py	2009-11-08 20:28:36 +0000
+++ bzrlib/tests/test_script.py	2010-01-13 16:22:12 +0000
@@ -407,3 +407,39 @@
 $ rm -r dir
 """)
         self.failIfExists('dir')
+
+
+class TestMv(script.TestCaseWithTransportAndScript):
+
+    def test_usage(self):
+        self.assertRaises(SyntaxError, self.run_script, '$ mv')
+        self.assertRaises(SyntaxError, self.run_script, '$ mv f')
+        self.assertRaises(SyntaxError, self.run_script, '$ mv f1 f2 f3')
+
+    def test_move_file(self):
+        self.run_script('$ echo content >file')
+        self.failUnlessExists('file')
+        self.run_script('$ mv file new_name')
+        self.failIfExists('file')
+        self.failUnlessExists('new_name')
+
+    def test_move_dir(self):
+        self.run_script("""
+$ mkdir dir
+$ echo content >dir/file
+""")
+        self.run_script('$ mv dir new_name')
+        self.failIfExists('dir')
+        self.failUnlessExists('new_name')
+        self.failUnlessExists('new_name/file')
+        
+    def test_move_file_into_dir(self):
+        self.run_script("""
+$ mkdir dir
+$ echo content > file
+""")
+        self.run_script('$ mv file dir')
+        self.failUnlessExists('dir')
+        self.failIfExists('file')
+        self.failUnlessExists('dir/file')
+