Bazaar

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

  1. script-test-mv
  2. Merge into bzr.dev
Proposed by Neil Martinsen-Burrell
Status: Merged
Merged at revision: not available
Proposed branch: lp:~nmb/bzr/script-test-mv
Merge into: lp:bzr
Diff against target: 99 lines (+63/-0)
3 files modified
NEWS (+4/-0)
bzrlib/tests/script.py (+23/-0)
bzrlib/tests/test_script.py (+36/-0)
To merge this branch: bzr merge lp:~nmb/bzr/script-test-mv
Reviewer Review Type Date Requested Status
Vincent Ladeuil Approve
John A Meinel Approve
Review via email: mp+17269@code.launchpad.net
To post a comment you must log in.
Revision history for this message
Neil Martinsen-Burrell (nmb) wrote :

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! :)

Revision history for this message
Martin Pool (mbp) wrote :

That's nice.

It should be mentioned in TESTING in news.

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

Revision history for this message
John A Meinel (jameinel) wrote :

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

Revision history for this message
John A Meinel (jameinel) wrote :

-----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
Revision history for this message
Neil Martinsen-Burrell (nmb) wrote :

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.

Revision history for this message
Vincent Ladeuil (vila) wrote :

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

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'NEWS'
--- NEWS 2010-01-13 00:25:44 +0000
+++ NEWS 2010-01-13 16:22:13 +0000
@@ -180,6 +180,10 @@
180 tests that 'failed' - they're all just failures.180 tests that 'failed' - they're all just failures.
181 (Martin Pool)181 (Martin Pool)
182182
183* Shell-like tests no support the command "mv" for moving files. The
184 syntax for ``mv file1 file2``, ``mv dir1 dir2`` and ``mv file dir`` is
185 supported. (Neil Martinsen-Burrell)
186
183bzr 2.0.4 (not released yet)187bzr 2.0.4 (not released yet)
184############################188############################
185189
186190
=== 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 @@
24import glob24import glob
25import os25import os
26import shlex26import shlex
27import shutil
27from cStringIO import StringIO28from cStringIO import StringIO
2829
29from bzrlib import (30from bzrlib import (
@@ -402,6 +403,28 @@
402 retcode = 0403 retcode = 0
403 return retcode, None, err404 return retcode, None, err
404405
406 def do_mv(self, test_case, input, args):
407 err = None
408 def error(msg, path1, path2):
409 return "mv: cannot move %s to %s: %s\n" % (path1, path2, msg)
410
411 if not args or len(args) != 2:
412 raise SyntaxError("Usage: mv path1 path2")
413 path1, path2 = args
414 try:
415 shutil.move(path1, path2)
416 except OSError, e:
417 if e.errno == errno.ENOENT:
418 err = error('No such file or directory', path1)
419 else:
420 raise
421 if err:
422 retcode = 1
423 else:
424 retcode = 0
425 return retcode, None, err
426
427
405428
406class TestCaseWithMemoryTransportAndScript(tests.TestCaseWithMemoryTransport):429class TestCaseWithMemoryTransportAndScript(tests.TestCaseWithMemoryTransport):
407 """Helper class to experiment shell-like test and memory fs.430 """Helper class to experiment shell-like test and memory fs.
408431
=== 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 @@
407$ rm -r dir407$ rm -r dir
408""")408""")
409 self.failIfExists('dir')409 self.failIfExists('dir')
410
411
412class TestMv(script.TestCaseWithTransportAndScript):
413
414 def test_usage(self):
415 self.assertRaises(SyntaxError, self.run_script, '$ mv')
416 self.assertRaises(SyntaxError, self.run_script, '$ mv f')
417 self.assertRaises(SyntaxError, self.run_script, '$ mv f1 f2 f3')
418
419 def test_move_file(self):
420 self.run_script('$ echo content >file')
421 self.failUnlessExists('file')
422 self.run_script('$ mv file new_name')
423 self.failIfExists('file')
424 self.failUnlessExists('new_name')
425
426 def test_move_dir(self):
427 self.run_script("""
428$ mkdir dir
429$ echo content >dir/file
430""")
431 self.run_script('$ mv dir new_name')
432 self.failIfExists('dir')
433 self.failUnlessExists('new_name')
434 self.failUnlessExists('new_name/file')
435
436 def test_move_file_into_dir(self):
437 self.run_script("""
438$ mkdir dir
439$ echo content > file
440""")
441 self.run_script('$ mv file dir')
442 self.failUnlessExists('dir')
443 self.failIfExists('file')
444 self.failUnlessExists('dir/file')
445