Added initial tests to clarify the API design

src/shell/shell.js

@@ -4,7 +4,7 @@ var Environment = require('./environment.js');
 var async = require('../../lib/async.js');
 var Encoding = require('../encoding.js');
 var minimatch = require('minimatch');
-var ROOT_DIRECTORY_NAME = require('src/constants').ROOT_DIRECTORY_NAME;
+var ROOT_DIRECTORY_NAME = require('../constants').ROOT_DIRECTORY_NAME;
 
 function Shell(fs, options) {
   options = options || {};
@@ -431,18 +431,18 @@ Shell.prototype.mkdirp = function(path, callback) {
 /**
  * Renames `src` to `dest`, or moves `src` to `dest` if `dest` is
  * a directory. For moving multiple files, provide an array of paths
- * to be moved as `src`.
+ * to be moved as `src`. Will automatically overwrite files if conflicts
+ * arise.
  */
 Shell.prototype.mv = function(src, dest, callback) {
   var sh = this;
   var fs = sh.fs;
   var srcPath;
-  if(typeof options === 'function') {
+
-    callback = options;
+  if (!callback || typeof callback !== 'function') {
-    options = {};
+    callback(new Errors.EINVAL('Missing callback function'));
+    return;
   }
-  options = options || {};
-  callback = callback || function(){};
 
   if(!src) {
     callback(new Errors.EINVAL('Missing src argument'));
@@ -467,7 +467,7 @@ Shell.prototype.mv = function(src, dest, callback) {
       return;
     }
 
-    move(src, function(err, data) {
+    move(src, function(err) {
       if(err) {
         callback(err);
         return;
@@ -493,9 +493,8 @@ Shell.prototype.mv = function(src, dest, callback) {
     callback();
   });
 
-  // xxx - Look at "fs.rename"'s callback arguments. There's a bug here!
+  function move(srcPath, cb) {
-  function move(src, cb) {
+    fs.rename(srcPath, dest, cb);
-    fs.rename(src, dest, cb);
   }
 };
 

tests/index.js

@@ -56,6 +56,7 @@ require("./spec/shell/rm.spec");
 require("./spec/shell/env.spec");
 require("./spec/shell/mkdirp.spec");
 require("./spec/shell/find.spec");
+require("./spec/shell/mv.spec");
 
 // Ported node.js tests (filenames match names in https://github.com/joyent/node/tree/master/test)
 require("./spec/node-js/simple/test-fs-mkdir");

tests/spec/shell/mv.spec.js

@@ -0,0 +1,306 @@
+var Filer = require('../../..');
+var util = require('../../lib/test-utils.js');
+var expect = require('chai').expect;
+
+describe('FileSystemShell.mv', function() {
+  beforeEach(util.setup);
+  afterEach(util.cleanup);
+
+  it('should be a function', function() {
+    var shell = util.shell();
+    expect(shell.mv).to.be.a('function');
+  });
+
+  it('should fail when the source argument is absent', function(done) {
+    var fs = util.fs();
+    var shell = new fs.Shell();
+
+    shell.mv(null, '/destination', function(error) {
+      expect(error).to.exist;
+      expect(error.code).to.equal("EINVAL");
+      done();
+    });
+  });
+
+  it('should fail when the source argument is the root', function(done) {
+    var fs = util.fs();
+    var shell = new fs.Shell();
+
+    shell.mv('/', '/destination', function(error) {
+      expect(error).to.exist;
+      expect(error.code).to.equal("EINVAL");
+      done();
+    });
+  });
+
+  it('should fail when the destination argument is absent', function(done) {
+    var fs = util.fs();
+    var shell = new fs.Shell();
+
+    shell.mv('/source', null, function(error) {
+      expect(error).to.exist;
+      expect(error.code).to.equal("EINVAL");
+      done();
+    });
+  });
+
+  describe('on files', function() {
+    it('should move a single file when the basename of the destination is specified', function(done) {
+      var fs = util.fs();
+      var shell = new fs.Shell();
+      var contents = "a";
+
+      fs.mkdir('/dir', function(err) {
+        if(err) throw err;
+
+        fs.writeFile('/file', contents, function(err) {
+          if(err) throw err;
+
+          shell.mv('/file', '/dir/file', function(err) {
+            expect(err).not.to.exist;
+
+            fs.stat('/dir/file', function(err, stats) {
+              expect(err).not.to.exist;
+              expect(stats).to.exist;
+
+              fs.stat('/file', function(err, stats) {
+                expect(err).to.exist;
+                expect(stats).not.to.exist;
+                done();
+              });
+            });
+          });
+        });
+      });
+    });
+
+    it('should move a single file when a basename isn\'t specified in the destination', function(done) {
+      var fs = util.fs();
+      var shell = new fs.Shell();
+      var contents = "a";
+
+      fs.mkdir('/dir', function(err) {
+        if(err) throw err;
+
+        fs.writeFile('/file', contents, function(err) {
+          if(err) throw err;
+
+          shell.mv('/file', '/dir/', function(err) {
+            expect(err).not.to.exist;
+
+            fs.stat('/dir/file', function(err, stats) {
+              expect(err).not.to.exist;
+              expect(stats).to.exist;
+
+              fs.stat('/file', function(err, stats) {
+                expect(err).to.exist;
+                expect(stats).not.to.exist;
+                done();
+              });
+            });
+          });
+        });
+      });
+    });
+
+    it('should move multiple files when the destination is a path to a folder', function(done) {
+      var fs = util.fs();
+      var shell = new fs.Shell();
+      var contents = "a";
+
+      fs.mkdir('/dir', function(err) {
+        if(err) throw err;
+
+        fs.writeFile('/file', contents, function(err) {
+          if(err) throw err;
+
+          fs.writeFile('/file2', contents + 1, function(err) {
+            if(err) throw err;
+
+            shell.mv(['/file', '/file2'], '/dir/', function(err) {
+              expect(err).not.to.exist;
+
+              fs.stat('/dir/file', function(err, stats) {
+                expect(err).not.to.exist;
+                expect(stats).to.exist;
+
+                fs.stat('/dir/file2', function(err, stats) {
+                  expect(err).not.to.exist;
+                  expect(stats).to.exist;
+
+                  fs.stat('/file', function(err, stats) {
+                    expect(err).to.exist;
+                    expect(stats).not.to.exist;
+
+                    fs.stat('/file2', function(err, stats) {
+                      expect(err).to.exist;
+                      expect(stats).not.to.exist;
+                      done();
+                    });
+                  });
+                });
+              });
+            });
+          });
+        });
+      });
+    });
+
+    it('should fail to move a single file when the basename of the destination is the name of a folder', function(done) {
+      var fs = util.fs();
+      var shell = new fs.Shell();
+      var contents = "a";
+
+      fs.mkdir('/dir', function(err) {
+        if(err) throw err;
+
+        fs.writeFile('/file', contents, function(err) {
+          if(err) throw err;
+
+          shell.mv('/file', '/dir', function(err) {
+            expect(err).to.exist;
+            done();
+          });
+        });
+      });
+    });
+  });
+
+  describe('on folders', function(){
+    it('should move an empty folder when the basename of the destination is specified', function(done) {
+      var fs = util.fs();
+      var shell = new fs.Shell();
+
+      fs.mkdir('/dir', function(err) {
+        if(err) throw err;
+
+        fs.mkdir('/dir2', function(err) {
+          if(err) throw err;
+
+          shell.mv('/dir2', '/dir/dir2', function(err) {
+            expect(err).not.to.exist;
+
+            fs.stat('/dir/dir2', function(err, stats) {
+              expect(err).not.to.exist;
+              expect(stats).to.exist;
+
+              fs.stat('/dir2', function(err, stats) {
+                expect(err).to.exist;
+                expect(stats).not.to.exist;
+                done();
+              });
+            });
+          });
+        });
+      });
+    });
+
+    it('should move an empty folder when the basename of the destination isn\'t specified', function(done) {
+      var fs = util.fs();
+      var shell = new fs.Shell();
+
+      fs.mkdir('/dir', function(err) {
+        if(err) throw err;
+
+        fs.mkdir('/dir2', function(err) {
+          if(err) throw err;
+
+          shell.mv('/dir2', '/dir/', function(err) {
+            expect(err).not.to.exist;
+
+            fs.stat('/dir/dir2', function(err, stats) {
+              expect(err).not.to.exist;
+              expect(stats).to.exist;
+
+              fs.stat('/dir2', function(err, stats) {
+                expect(err).to.exist;
+                expect(stats).not.to.exist;
+                done();
+              });
+            });
+          });
+        });
+      });
+    });
+
+    it('should move a non-empty folder when the basename of the destination is specified', function(done) {
+      var fs = util.fs();
+      var shell = new fs.Shell();
+      var contents = "a";
+
+      fs.mkdir('/dir', function(err) {
+        if(err) throw err;
+
+        fs.mkdir('/dir2', contents, function(err) {
+          if(err) throw err;
+
+          fs.writeFile('/dir2/file', contents, function(err){
+            if(err) throw err;
+
+            shell.mv('/dir2', '/dir/dir2', function(err) {
+              expect(err).not.to.exist;
+
+              fs.stat('/dir/dir2', function(err, stats) {
+                expect(err).not.to.exist;
+                expect(stats).to.exist;
+
+                fs.stat('/dir/dir2/file', function(err, stats) {
+                  expect(err).not.to.exist;
+                  expect(stats).to.exist;
+
+                  fs.stat('/dir2', function(err, stats) {
+                    expect(err).to.exist;
+                    expect(stats).not.to.exist;
+                    done();
+                  });
+                })
+              });
+            });
+          });
+        });
+      });
+    });
+
+    it('should move a non-empty folder when the basename of the destination isn\'t specified', function(done) {
+      var fs = util.fs();
+      var shell = new fs.Shell();
+      var contents = "a";
+
+      fs.mkdir('/dir', function(err) {
+        if(err) throw err;
+
+        fs.mkdir('/dir2', contents, function(err) {
+          if(err) throw err;
+
+          fs.writeFile('/dir2/file', contents, function(err){
+            if(err) throw err;
+
+            shell.mv('/dir2', '/dir/', function(err) {
+              expect(err).not.to.exist;
+
+              fs.stat('/dir/dir2', function(err, stats) {
+                expect(err).not.to.exist;
+                expect(stats).to.exist;
+
+                fs.stat('/dir/dir2/file', function(err, stats) {
+                  expect(err).not.to.exist;
+                  expect(stats).to.exist;
+
+                  fs.stat('/dir2', function(err, stats) {
+                    expect(err).to.exist;
+                    expect(stats).not.to.exist;
+                    done();
+                  });
+                })
+              });
+            });
+          });
+        });
+      });
+    });
+  });
+
+  describe('in combination, deep & wide', function() {
+    // TBD
+  });
+});