From 5d6e7bb0181f93925b01508b2693a1f719a0f14d Mon Sep 17 00:00:00 2001 From: Kieran Sedgwick Date: Thu, 15 Jan 2015 14:41:16 -0500 Subject: [PATCH] Refactor to fix bugs and update unit tests --- src/shell/shell.js | 49 +-- tests/spec/shell/mv.spec.js | 650 ++++++++++++++---------------------- 2 files changed, 281 insertions(+), 418 deletions(-) diff --git a/src/shell/shell.js b/src/shell/shell.js index eba196c..f809a99 100644 --- a/src/shell/shell.js +++ b/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 Constants = require('src/constants'); +var ROOT_DIRECTORY_NAME = require('../constants').ROOT_DIRECTORY_NAME; function Shell(fs, options) { options = options || {}; @@ -429,9 +429,9 @@ Shell.prototype.mkdirp = function(path, callback) { }; /** - * Moves the file or directory at the `source` path to the - * `destination` path by relinking the source to the destination - * path. + * Renames or moves `source` to `destination`, or moves each path + * in the `source` array into `destination`. Overwrites files in + * `destination` that have the same name(s) as `source` by default. */ Shell.prototype.mv = function(source, destination, callback) { var fs = this.fs; @@ -440,16 +440,17 @@ Shell.prototype.mv = function(source, destination, callback) { callback = callback || function() {}; if(!source) { - callback(new Errors.EINVAL('missing source path argument')); + callback(new Errors.EINVAL('Missing source path argument')); return; } - else if(source === Constants.ROOT_DIRECTORY_NAME) { - callback(new Errors.EINVAL('the root directory is not a valid source argument')); + + if(source === ROOT_DIRECTORY_NAME) { + callback(new Errors.EINVAL('The root directory is not a valid source argument')); return; } if(!destination) { - callback(new Errors.EINVAL('missing destination path argument')); + callback(new Errors.EINVAL('Missing destination path argument')); return; } @@ -458,21 +459,22 @@ Shell.prototype.mv = function(source, destination, callback) { destpath = Path.resolve(this.cwd, destpath); var destdir = Path.dirname(destpath); - // Recursively create any directories on the destination path which do not exist - shell.mkdirp(destdir, function(error) { + // If there is no node at the source path, error and quit + fs.lstat(sourcepath, function(error, sourcestats) { if(error) { callback(error); return; } - // If there is no node at the source path, error and quit - fs.lstat(sourcepath, function(error, sourcestats) { + // If the destination's parent directory doesn't exist, error and quit + fs.lstat(destdir, function(error) { + // If there is an error unrelated to the existence of the destination, exit if(error) { callback(error); return; } - fs.lstat(destpath, function(error, deststats) { + fs.lstat(destpath, function(error, deststats){ // If there is an error unrelated to the existence of the destination, exit if(error && error.code !== 'ENOENT') { callback(error); @@ -480,9 +482,13 @@ Shell.prototype.mv = function(source, destination, callback) { } if(deststats) { - // If the destination is a directory, new destination is destpath/source.basename - if(deststats.isDirectory()) { + if (deststats.isDirectory()) { + // If the destination is a directory, new destination is destpath/source.basename destpath = Path.join(destpath, Path.basename(sourcepath)); + } else if(sourcestats.isDirectory()) { + // If the source is a folder, but the destination isn't, error and exit + callback(new Errors.EINVAL("Destination is not a directory")); + return; } } @@ -492,14 +498,15 @@ Shell.prototype.mv = function(source, destination, callback) { callback(error); return; } - // If the source is a file, link it to destination and remove the source, then done + + // If the source isn't a directory, link it to destination and remove the source, then done if(sourcestats.isFile() || sourcestats.isSymbolicLink()) { fs.link(sourcepath, destpath, function(error) { if (error) { callback(error); return; } - shell.rm(sourcepath, {recursive:true}, function(error) { + fs.unlink(sourcepath, function(error) { if (error) { callback(error); return; @@ -507,10 +514,12 @@ Shell.prototype.mv = function(source, destination, callback) { callback(); }); }); + return; } + // If the source is a directory, create a directory at destination and then recursively // move every dir entry. - else if(sourcestats.isDirectory()) { + if(sourcestats.isDirectory()) { fs.mkdir(destpath, function(error) { if (error) { callback(error); @@ -523,8 +532,8 @@ Shell.prototype.mv = function(source, destination, callback) { return; } - // Asychronously applies move to all nodes in the source directory - async.each(entries, + // Apply the move to all nodes in the directory + async.eachSeries(entries, function(entry, callback) { move(Path.join(sourcepath, entry), Path.join(destpath, entry), function(error) { if(error) { diff --git a/tests/spec/shell/mv.spec.js b/tests/spec/shell/mv.spec.js index 7c87059..fd9767d 100644 --- a/tests/spec/shell/mv.spec.js +++ b/tests/spec/shell/mv.spec.js @@ -1,93 +1,249 @@ -define(["Filer", "util"], function(Filer, util) { - - describe('FileSystemShell.mv', function() { - beforeEach(util.setup); - afterEach(util.cleanup); +var Filer = require('../../..'); +var util = require('../../lib/test-utils.js'); +var expect = require('chai').expect; - it('should be a function', function() { - var shell = util.shell(); - expect(shell.mv).to.be.a('function'); +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 source argument is absent', function(done) { + var fs = util.fs(); + var shell = new fs.Shell(); + + shell.mv(null, null, function(error) { + expect(error).to.exist; + done(); }); + }); - it('should fail when source argument is absent', function(done) { - var fs = util.fs(); - var shell = fs.Shell(); + it('should fail when destination argument is absent', function(done) { + var fs = util.fs(); + var shell = new fs.Shell(); + var contents = "a"; - shell.mv(null, null, function(error) { + fs.writeFile('/file', contents, function(error) { + if(error) throw error; + + shell.mv('/file', null, function(error) { expect(error).to.exist; done(); }); }); + }); - it('should fail when destination argument is absent', function(done) { - var fs = util.fs(); - var shell = fs.Shell(); - var contents = "a"; + it('should fail when source argument is an empty string', function(done) { + var fs = util.fs(); + var shell = new fs.Shell(); - fs.writeFile('/file', contents, function(error) { - if(error) throw error; - - shell.mv('/file', null, function(error) { - expect(error).to.exist; - done(); - }); - }); + shell.mv('', '/file', function(error) { + expect(error).to.exist; + done(); }); + }); - it('should fail when arguments are empty strings', function(done) { - var fs = util.fs(); - var shell = fs.Shell(); + it('should fail when destination argument is an empty string', function(done) { + var fs = util.fs(); + var shell = new fs.Shell(); + var contents = 'a'; - shell.mv('', '', function(error) { + fs.writeFile('/file', contents, function(error) { + if(error) throw error; + shell.mv('/file', '', function(error) { expect(error).to.exist; done(); }); }); + }); - it('should fail when the node at source path does not exist', function(done) { - var fs = util.fs(); - var shell = fs.Shell(); - - fs.mkdir('/dir', function(error) { - expect(error).to.not.exist; - }); + it('should fail when the node at source path does not exist', function(done) { + var fs = util.fs(); + var shell = new fs.Shell(); + fs.mkdir('/dir', function(error) { + if(error) throw error; shell.mv('/file', '/dir', function(error) { expect(error).to.exist; done(); }); }); + }); - it('should fail when root is provided as source argument', function(done) { - var fs = util.fs(); - var shell = fs.Shell(); - - fs.mkdir('/dir', function(error) { - expect(error).to.not.exist; - }); + it('should fail when root is provided as source argument', function(done) { + var fs = util.fs(); + var shell = new fs.Shell(); + fs.mkdir('/dir', function(error) { + if(error) throw error; shell.mv('/', '/dir', function(error) { expect(error).to.exist; done(); }); }); + }); - it('should rename a file which is moved to the same directory under a different name', function(done) { - var fs = util.fs(); - var shell = fs.Shell(); - var contents = "a"; + it('should fail when node at source path is a folder, but node at destination is a file', function(done) { + var fs = util.fs(); + var shell = new fs.Shell(); + var contents = "a"; + + fs.mkdir ('/dir', function(error) { + if(error) throw error; fs.writeFile('/file', contents, function(error) { - expect(error).to.not.exist; - - shell.mv('/file', '/newfile', function(error) { + if(error) throw error; + + shell.mv('/dir', '/file', function(error) { + expect(error).to.exist; + expect(error.code).to.equal('EINVAL'); + done(); + }); + }); + }); + }); + + it('should rename a file which is moved to the same directory under a different name', function(done) { + var fs = util.fs(); + var shell = new fs.Shell(); + var contents = "a"; + + fs.writeFile('/file', contents, function(error) { + if(error) throw error; + + shell.mv('/file', '/newfile', function(error) { + if(error) throw error; + + fs.stat('/file', function(error, stats) { + expect(error).to.exist; + expect(stats).not.to.exist; + + fs.readFile('/newfile', 'utf8', function(error, data) { + expect(error).not.to.exist; + expect(data).to.equal(contents); + done(); + }); + }); + }); + }); + }); + + it('should rename a symlink which is moved to the same directory under a different name', function(done) { + var fs = util.fs(); + var shell = new fs.Shell(); + var contents = "a"; + + fs.writeFile('/file', contents, function(error) { + if(error) throw error; + + fs.symlink('/file', '/newfile', function(error) { + if(error) throw error; + + shell.mv('/newfile', '/newerfile', function(error) { + expect(error).not.to.exist; + + fs.stat('/file', function(error, stats) { + expect(error).not.to.exist; + + fs.stat('/newfile', function(error, stats) { + expect(error).to.exist; + + fs.readFile('/newerfile', 'utf8', function(error, data) { + expect(error).not.to.exist; + expect(data).to.equal(contents); + done(); + }); + }); + }); + }); + }); + }); + }); + + it('should move a file into an empty directory', function(done) { + var fs = util.fs(); + var shell = new fs.Shell(); + var contents = "a"; + + fs.mkdir('/dir', function(error) { + if(error) throw error; + + fs.writeFile('/file', contents, function(error) { + if(error) throw error; + + shell.mv('/file', '/dir', function(error) { expect(error).to.not.exist; - + fs.stat('/file', function(error, stats) { expect(error).to.exist; - expect(stats).to.not.exist; - - fs.stat('/newfile', function(error, stats) { + expect(error.code).to.equal('ENOENT'); + + fs.readFile('/dir/file', 'utf8', function(error, data) { + expect(error).not.to.exist; + expect(data).to.equal(contents); + done(); + }); + }); + }); + }); + }); + }); + + it('should move a file into a directory that has a file of the same name', function(done) { + var fs = util.fs(); + var shell = new fs.Shell(); + var contents = "a"; + var contents2 = "b"; + + fs.mkdir('/dir', function(error) { + if(error) throw error; + + fs.writeFile('/file', contents, function(error) { + if(error) throw error; + + fs.writeFile('/dir/file', contents2, function(error) { + if(error) throw error; + + shell.mv('/file', '/dir/file', function(error) { + expect(error).to.not.exist; + + fs.stat('/file', function(error, stats) { + expect(error).to.exist; + expect(error.code).to.equal('ENOENT'); + + fs.readFile('/dir/file', 'utf8', function(error, data) { + expect(error).not.to.exist; + expect(data).to.equal(contents); + done(); + }); + }); + }); + }); + }); + }); + }); + + it('should move an empty directory to another empty directory', function(done) { + var fs = util.fs(); + var shell = new fs.Shell(); + + fs.mkdir('/dir', function(error) { + if(error) throw error; + + fs.mkdir('/otherdir', function(error) { + if(error) throw error; + + shell.mv('/dir', '/otherdir', function(error) { + expect(error).to.not.exist; + + fs.stat('/dir', function(error, stats) { + expect(error).to.exist; + expect(error.code).to.equal('ENOENT'); + + fs.stat('/otherdir/dir', function(error, stats) { expect(error).to.not.exist; expect(stats).to.exist; done(); @@ -96,30 +252,34 @@ define(["Filer", "util"], function(Filer, util) { }); }); }); + }); - it('should rename a symlink which is moved to the same directory under a different name', function(done) { - var fs = util.fs(); - var shell = fs.Shell(); - var contents = "a"; + it('should move an empty directory to a populated directory', function(done) { + var fs = util.fs(); + var shell = new fs.Shell(); + var contents = "a"; - fs.writeFile('/file', contents, function(error) { - expect(error).to.not.exist; + fs.mkdir('/dir', function(error) { + if(error) throw error; - fs.symlink('/file', '/newfile', function(error) { - expect(error).to.not.exist; + fs.mkdir('/otherdir', function(error) { + if(error) throw error; - shell.mv('/newfile', '/newerfile', function(error) { + fs.writeFile('/otherdir/file', contents, function(error) { + if(error) throw error; + + shell.mv('/dir', '/otherdir', function(error) { expect(error).to.not.exist; - - fs.stat('/file', function(error, stats) { - expect(error).to.not.exist; - expect(stats).to.exist; - - fs.stat('/newfile', function(error, stats) { - expect(error).to.exist; - expect(stats).to.not.exist; - - fs.stat('/newerfile', function(error, stats) { + + fs.stat('/dir', function(error, stats) { + expect(error).to.exist; + expect(error.code).to.equal('ENOENT'); + + fs.stat('/otherdir/file', function(error, stats) { + expect(error).to.not.exist; + expect(stats).to.exist; + + fs.stat('/otherdir/dir', function(error, stats) { expect(error).to.not.exist; expect(stats).to.exist; done(); @@ -130,351 +290,46 @@ define(["Filer", "util"], function(Filer, util) { }); }); }); + }); - it('should move a file to a directory which does not currently exist', function(done) { - var fs = util.fs(); - var shell = fs.Shell(); - var contents = "a"; + it('should move a populated directory to a populated directory', function(done) { + var fs = util.fs(); + var shell = new fs.Shell(); + var contents = "a"; + var contents2 = "b"; - fs.writeFile('/file', contents, function(error) { - expect(error).to.not.exist; + fs.mkdir('/dir', function(error) { + if(error) throw error; - shell.mv('/file', '/dir/newfile', function(error) { - expect(error).to.not.exist; - - fs.stat('/file', function(error, stats) { - expect(error).to.exist; - expect(stats).to.not.exist; - - fs.stat('/dir/newfile', function(error, stats) { - expect(error).to.not.exist; - expect(stats).to.exist; - done(); - }); - }); - }); - }); - }); + fs.mkdir('/otherdir', function(error) { + if(error) throw error; - it('should move a file into an empty directory', function(done) { - var fs = util.fs(); - var shell = fs.Shell(); - var contents = "a"; - - fs.mkdir('/dir', function(error) { - expect(error).to.not.exist; - - fs.stat('/dir', function(error, stats) { - expect(error).to.not.exist; - expect(stats).to.exist; - - fs.writeFile('/file', contents, function(error) { - expect(error).to.not.exist; - - fs.stat('/file', function(error, stats) { - expect(error).to.not.exist; - expect(stats).to.exist; - - shell.mv('/file', '/dir', function(error) { - expect(error).to.not.exist; - - fs.stat('/file', function(error, stats) { - expect(error).to.exist; - expect(stats).to.not.exist; - - fs.stat('/dir/file', function(error, stats) { - expect(error).to.not.exist; - expect(stats).to.exist; - done(); - }); - }); - }); - }); - }); - }); - }); - }); - - it('should move a file into a directory that has a file of the same name', function(done) { - var fs = util.fs(); - var shell = fs.Shell(); - var contents = "a"; - var contents2 = "b"; - - fs.mkdir('/dir', function(error) { - expect(error).to.not.exist; - - fs.writeFile('/file', contents, function(error) { - expect(error).to.not.exist; + fs.writeFile('/otherdir/file', contents, function(error) { + if(error) throw error; fs.writeFile('/dir/file', contents2, function(error) { - expect(error).to.not.exist; - - shell.mv('/file', '/dir/file', function(error) { - expect(error).to.not.exist; - - fs.stat('/file', function(error, stats) { - expect(error).to.exist; - expect(stats).to.not.exist; - - fs.stat('/dir/file', function(error, stats) { - expect(error).to.not.exist; - expect(stats).to.exist; - - fs.readFile('/dir/file', 'utf8', function(error, data) { - expect(error).not.to.exist; - expect(data).to.equal(contents); - done(); - }); - }); - }); - }); - }); - }); - }); - }); - - it('should move an empty directory to a destination that does not currently exist', function(done) { - var fs = util.fs(); - var shell = fs.Shell(); - - fs.mkdir('/dir', function(error) { - expect(error).to.not.exist; - - shell.mv('/dir', '/newdir', function(error) { - expect(error).to.not.exist; - - fs.stat('/dir', function(error, stats) { - expect(error).to.exist; - expect(stats).to.not.exist; - - fs.stat('/newdir', function(error, stats) { - expect(error).to.not.exist; - expect(stats).to.exist; - done(); - }); - }); - }); - }); - }); - - it('should move an empty directory to another empty directory', function(done) { - var fs = util.fs(); - var shell = fs.Shell(); - - fs.mkdir('/dir', function(error) { - expect(error).to.not.exist; - - fs.mkdir('/otherdir', function(error) { - expect(error).to.not.exist; - - shell.mv('/dir', '/otherdir', function(error) { - expect(error).to.not.exist; - - fs.stat('/dir', function(error, stats) { - expect(error).to.exist; - expect(stats).to.not.exist; - - fs.stat('/otherdir/dir', function(error, stats) { - expect(error).to.not.exist; - expect(stats).to.exist; - done(); - }); - }); - }); - }); - }); - }); - - it('should move an empty directory to a populated directory', function(done) { - var fs = util.fs(); - var shell = fs.Shell(); - var contents = "a"; - - fs.mkdir('/dir', function(error) { - expect(error).to.not.exist; - - fs.mkdir('/otherdir', function(error) { - expect(error).to.not.exist; - - fs.writeFile('/otherdir/file', contents, function(error) { - expect(error).to.not.exist; + if(error) throw error; shell.mv('/dir', '/otherdir', function(error) { expect(error).to.not.exist; fs.stat('/dir', function(error, stats) { expect(error).to.exist; - expect(stats).to.not.exist; + expect(error.code).to.equal('ENOENT'); - fs.stat('/otherdir/file', function(error, stats) { + fs.readFile('/otherdir/file', 'utf8', function(error, data) { expect(error).to.not.exist; - expect(stats).to.exist; + expect(data).to.equal(contents); fs.stat('/otherdir/dir', function(error, stats) { expect(error).to.not.exist; expect(stats).to.exist; - done(); - }); - }); - }); - }); - }); - }); - }); - }); - it('should move a populated directory to a populated directory', function(done) { - var fs = util.fs(); - var shell = fs.Shell(); - var contents = "a"; - - fs.mkdir('/dir', function(error) { - expect(error).to.not.exist; - - fs.mkdir('/otherdir', function(error) { - expect(error).to.not.exist; - - fs.writeFile('/otherdir/file', contents, function(error) { - expect(error).to.not.exist; - - fs.writeFile('/dir/file', contents, function(error) { - expect(error).to.not.exist; - - shell.mv('/dir', '/otherdir', function(error) { - expect(error).to.not.exist; - - fs.stat('/dir', function(error, stats) { - expect(error).to.exist; - expect(stats).to.not.exist; - - fs.stat('/otherdir/file', function(error, stats) { - expect(error).to.not.exist; - expect(stats).to.exist; - - fs.stat('/otherdir/dir', function(error, stats) { + fs.readFile('/otherdir/dir/file', 'utf8', function(error, data) { expect(error).to.not.exist; - expect(stats).to.exist; - - fs.stat('/otherdir/dir/file', function(error, stats) { - expect(error).to.not.exist; - expect(stats).to.exist; - done(); - }) - }); - }); - }); - }); - }); - }); - }); - }); - }); - - it('should move an empty directory to another empty directory', function(done) { - var fs = util.fs(); - var shell = fs.Shell(); - - fs.mkdir('/dir', function(error) { - expect(error).to.not.exist; - - fs.mkdir('/otherdir', function(error) { - expect(error).to.not.exist; - - shell.mv('/dir', '/otherdir', function(error) { - expect(error).to.not.exist; - - fs.stat('/dir', function(error, stats) { - expect(error).to.exist; - expect(stats).to.not.exist; - - fs.stat('/otherdir/dir', function(error, stats) { - expect(error).to.not.exist; - expect(stats).to.exist; - done(); - }); - }); - }); - }); - }); - }); - - it('should move an empty directory to a populated directory', function(done) { - var fs = util.fs(); - var shell = fs.Shell(); - var contents = "a"; - - fs.mkdir('/dir', function(error) { - expect(error).to.not.exist; - - fs.mkdir('/otherdir', function(error) { - expect(error).to.not.exist; - - fs.writeFile('/otherdir/file', contents, function(error) { - expect(error).to.not.exist; - - shell.mv('/dir', '/otherdir', function(error) { - expect(error).to.not.exist; - - fs.stat('/dir', function(error, stats) { - expect(error).to.exist; - expect(stats).to.not.exist; - - fs.stat('/otherdir/file', function(error, stats) { - expect(error).to.not.exist; - expect(stats).to.exist; - - fs.stat('/otherdir/dir', function(error, stats) { - expect(error).to.not.exist; - expect(stats).to.exist; - done(); - }); - }); - }); - }); - }); - }); - }); - }); - - it('should move a populated directory to a populated directory', function(done) { - var fs = util.fs(); - var shell = fs.Shell(); - var contents = "a"; - - fs.mkdir('/dir', function(error) { - expect(error).to.not.exist; - - fs.mkdir('/otherdir', function(error) { - expect(error).to.not.exist; - - fs.writeFile('/otherdir/file', contents, function(error) { - expect(error).to.not.exist; - - fs.writeFile('/dir/file', contents, function(error) { - expect(error).to.not.exist; - - shell.mv('/dir', '/otherdir', function(error) { - expect(error).to.not.exist; - - fs.stat('/dir', function(error, stats) { - expect(error).to.exist; - expect(stats).to.not.exist; - - fs.stat('/otherdir/file', function(error, stats) { - expect(error).to.not.exist; - expect(stats).to.exist; - - fs.stat('/otherdir/dir', function(error, stats) { - expect(error).to.not.exist; - expect(stats).to.exist; - - fs.stat('/otherdir/dir/file', function(error, stats) { - expect(error).to.not.exist; - expect(stats).to.exist; - done(); - }) - }); + expect(data).to.equal(contents2); + done(); + }) }); }); }); @@ -485,4 +340,3 @@ define(["Filer", "util"], function(Filer, util) { }); }); }); -