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
1=== modified file 'NEWS'
2--- NEWS 2010-01-13 00:25:44 +0000
3+++ NEWS 2010-01-13 16:22:13 +0000
4@@ -180,6 +180,10 @@
5 tests that 'failed' - they're all just failures.
6 (Martin Pool)
7
8+* Shell-like tests no support the command "mv" for moving files. The
9+ syntax for ``mv file1 file2``, ``mv dir1 dir2`` and ``mv file dir`` is
10+ supported. (Neil Martinsen-Burrell)
11+
12 bzr 2.0.4 (not released yet)
13 ############################
14
15
16=== modified file 'bzrlib/tests/script.py'
17--- bzrlib/tests/script.py 2009-12-11 05:29:35 +0000
18+++ bzrlib/tests/script.py 2010-01-13 16:22:13 +0000
19@@ -24,6 +24,7 @@
20 import glob
21 import os
22 import shlex
23+import shutil
24 from cStringIO import StringIO
25
26 from bzrlib import (
27@@ -402,6 +403,28 @@
28 retcode = 0
29 return retcode, None, err
30
31+ def do_mv(self, test_case, input, args):
32+ err = None
33+ def error(msg, path1, path2):
34+ return "mv: cannot move %s to %s: %s\n" % (path1, path2, msg)
35+
36+ if not args or len(args) != 2:
37+ raise SyntaxError("Usage: mv path1 path2")
38+ path1, path2 = args
39+ try:
40+ shutil.move(path1, path2)
41+ except OSError, e:
42+ if e.errno == errno.ENOENT:
43+ err = error('No such file or directory', path1)
44+ else:
45+ raise
46+ if err:
47+ retcode = 1
48+ else:
49+ retcode = 0
50+ return retcode, None, err
51+
52+
53
54 class TestCaseWithMemoryTransportAndScript(tests.TestCaseWithMemoryTransport):
55 """Helper class to experiment shell-like test and memory fs.
56
57=== modified file 'bzrlib/tests/test_script.py'
58--- bzrlib/tests/test_script.py 2009-11-08 20:28:36 +0000
59+++ bzrlib/tests/test_script.py 2010-01-13 16:22:12 +0000
60@@ -407,3 +407,39 @@
61 $ rm -r dir
62 """)
63 self.failIfExists('dir')
64+
65+
66+class TestMv(script.TestCaseWithTransportAndScript):
67+
68+ def test_usage(self):
69+ self.assertRaises(SyntaxError, self.run_script, '$ mv')
70+ self.assertRaises(SyntaxError, self.run_script, '$ mv f')
71+ self.assertRaises(SyntaxError, self.run_script, '$ mv f1 f2 f3')
72+
73+ def test_move_file(self):
74+ self.run_script('$ echo content >file')
75+ self.failUnlessExists('file')
76+ self.run_script('$ mv file new_name')
77+ self.failIfExists('file')
78+ self.failUnlessExists('new_name')
79+
80+ def test_move_dir(self):
81+ self.run_script("""
82+$ mkdir dir
83+$ echo content >dir/file
84+""")
85+ self.run_script('$ mv dir new_name')
86+ self.failIfExists('dir')
87+ self.failUnlessExists('new_name')
88+ self.failUnlessExists('new_name/file')
89+
90+ def test_move_file_into_dir(self):
91+ self.run_script("""
92+$ mkdir dir
93+$ echo content > file
94+""")
95+ self.run_script('$ mv file dir')
96+ self.failUnlessExists('dir')
97+ self.failIfExists('file')
98+ self.failUnlessExists('dir/file')
99+