refactor, address feedback and add more unit tests

bower · Jan 5, 2016 · a2696d2 · a2696d2
1 parent 2110148
commit a2696d2
Show file tree

Hide file tree

Showing 10 changed files with 188 additions and 11 deletions.
diff --git a/lib/commands/link.js b/lib/commands/link.js
@@ -4,6 +4,7 @@ var Q = require('q');
 var Project = require('../core/Project');
 var createLink = require('../util/createLink');
 var defaultConfig = require('../config');
+var relativeToBaseDir = require('../util/relativeToBaseDir');
 
 function link(logger, name, localName, config) {
     if (name) {
@@ -49,7 +50,7 @@ function linkTo(logger, name, localName, config) {
 
     localName = localName || name;
     src = path.join(config.storage.links, name);
-    dst = path.join(config.cwd, config.directory, localName);
+    dst = path.join(relativeToBaseDir(config.cwd)(config.directory), localName);
 
     // Delete destination folder if any
     return Q.nfcall(rimraf, dst)

diff --git a/lib/core/Manager.js b/lib/core/Manager.js
@@ -10,7 +10,7 @@ var semver = require('../util/semver');
 var copy = require('../util/copy');
 var createError = require('../util/createError');
 var scripts = require('./scripts');
-var isPathAbsolute = require('../util/isPathAbsolute');
+var relativeToBaseDir = require('../util/relativeToBaseDir');
 
 function Manager(config, logger) {
     this._config = config;
@@ -125,7 +125,7 @@ Manager.prototype.resolve = function () {
 
 Manager.prototype.preinstall = function (json) {
     var that = this;
-    var componentsDir = isPathAbsolute(this._config.directory) ? this._config.directory : path.join(this._config.cwd, this._config.directory);
+    var componentsDir = relativeToBaseDir(this._config.cwd)(this._config.directory);
 
     // If nothing to install, skip the code bellow
     if (mout.lang.isEmpty(that._dissected)) {
@@ -142,7 +142,7 @@ Manager.prototype.preinstall = function (json) {
 
 Manager.prototype.postinstall = function (json) {
     var that = this;
-    var componentsDir = isPathAbsolute(this._config.directory) ? this._config.directory : path.join(this._config.cwd, this._config.directory);
+    var componentsDir = relativeToBaseDir(this._config.cwd)(this._config.directory);
 
     // If nothing to install, skip the code bellow
     if (mout.lang.isEmpty(that._dissected)) {
@@ -172,7 +172,7 @@ Manager.prototype.install = function (json) {
         return Q.resolve({});
     }
 
-    componentsDir = isPathAbsolute(this._config.directory) ? this._config.directory : path.join(this._config.cwd, this._config.directory);
+    componentsDir = relativeToBaseDir(this._config.cwd)(this._config.directory);
     return Q.nfcall(mkdirp, componentsDir)
     .then(function () {
         var promises = [];
@@ -635,7 +635,7 @@ Manager.prototype._dissect = function () {
         }, this);
 
         // Filter only packages that need to be installed
-        componentsDir = path.resolve(that._config.cwd, that._config.directory);
+        componentsDir = relativeToBaseDir(this._config.cwd)(this._config.directory);
         this._dissected = mout.object.filter(suitables, function (decEndpoint, name) {
             var installedMeta = this._installed[name];
             var dst;

diff --git a/lib/core/Project.js b/lib/core/Project.js
@@ -13,7 +13,7 @@ var createError = require('../util/createError');
 var readJson = require('../util/readJson');
 var validLink = require('../util/validLink');
 var scripts = require('./scripts');
-var isPathAbsolute = require('../util/isPathAbsolute');
+var relativeToBaseDir = require('../util/relativeToBaseDir');
 
 function Project(config, logger) {
     this._config = config;
@@ -608,7 +608,7 @@ Project.prototype._readInstalled = function () {
 
     // Gather all folders that are actual packages by
     // looking for the package metadata file
-    componentsDir = isPathAbsolute(this._config.directory) ? this._config.directory : path.join(this._config.cwd, this._config.directory);
+    componentsDir = relativeToBaseDir(this._config.cwd)(this._config.directory);
     return this._installed = Q.nfcall(glob, '*/.bower.json', {
         cwd: componentsDir,
         dot: true
@@ -649,7 +649,7 @@ Project.prototype._readLinks = function () {
     var that = this;
 
     // Read directory, looking for links
-    componentsDir = isPathAbsolute(this._config.directory) ? this._config.directory : path.join(this._config.cwd, this._config.directory);
+    componentsDir = relativeToBaseDir(this._config.cwd)(this._config.directory);
     return Q.nfcall(fs.readdir, componentsDir)
     .then(function (filenames) {
         var promises;

diff --git a/lib/util/isPathAbsolute.js b/lib/util/isPathAbsolute.js
@@ -1,5 +1,5 @@
-function isAbsolutePath(filePath) {
+function isPathAbsolute(filePath) {
     return filePath.charAt(0) === '/';
 }
 
-module.exports = isAbsolutePath;
+module.exports = isPathAbsolute;
diff --git a/lib/util/relativeToBaseDir.js b/lib/util/relativeToBaseDir.js
@@ -0,0 +1,14 @@
+var path = require('path');
+var isPathAbsolute = require('./isPathAbsolute');
+
+function relativeToBaseDir(baseDir) {
+    return function(filePath) {
+        if(!isPathAbsolute(filePath)) {
+            return path.resolve(filePath);
+        } else {
+            return path.resolve(baseDir, filePath);
+        }
+    };
+}
+
+module.exports = relativeToBaseDir;
diff --git a/test/commands/link.js b/test/commands/link.js
@@ -1,3 +1,4 @@
+var path = require('path');
 var expect = require('expect.js');
 var helpers = require('../helpers');
 
@@ -69,6 +70,55 @@ describe('bower link', function () {
         });
     });
 
+    it('creates inter-link to relative config.directory', function () {
+        return helpers.run(link, [undefined, undefined,
+            {
+                cwd: mainPackage.path,
+                storage: {
+                    links: linksDir.path
+                }
+            }
+        ]).then(function () {
+            return helpers.run(link, ['package', undefined,
+                {
+                    cwd: otherPackage.path,
+                    directory: 'valid-extend',
+                    storage: {
+                        links: linksDir.path
+                    }
+                }
+            ]);
+        }).then(function() {
+            expect(otherPackage.read('valid-extend/package/index.js'))
+            .to.be('Hello World!');
+        });
+    });
+
+
+    it('creates inter-link to absolute config.directory', function () {
+        return helpers.run(link, [undefined, undefined,
+            {
+                cwd: mainPackage.path,
+                storage: {
+                    links: linksDir.path
+                }
+            }
+        ]).then(function () {
+            return helpers.run(link, ['package', undefined,
+                {
+                    cwd: path.join(otherPackage.path, 'invalid'),
+                    directory: path.join(otherPackage.path, 'valid-override'),
+                    storage: {
+                        links: linksDir.path
+                    }
+                }
+            ]);
+        }).then(function() {
+            expect(otherPackage.read('valid-override/package/index.js'))
+            .to.be('Hello World!');
+        });
+    });
+
     it('creates inter-link with custom local name', function () {
         return helpers.run(link, [undefined, undefined,
             {

diff --git a/test/commands/uninstall.js b/test/commands/uninstall.js
@@ -1,4 +1,5 @@
 var path = require('path');
+var mkdirp = require('mkdirp');
 var expect = require('expect.js');
 var fs = require('../../lib/util/fs');
 
@@ -53,4 +54,38 @@ describe('bower uninstall', function () {
         });
     });
 
+    it('removes dependency from relative config.directory', function () {
+        var targetPath = path.resolve(tempDir.path, 'other_directory/underscore');
+        mkdirp.sync(targetPath);
+        fs.writeFileSync(path.join(targetPath, '.bower.json'), '{ "name": "underscore" }');
+
+        return helpers.run(uninstall, [['underscore'], undefined, {
+            cwd: tempDir.path,
+            directory: 'other_directory',
+            interactive: true
+        }])
+        .then(function() {
+            expect(function() {
+                fs.statSync(targetPath);
+            }).to.throwException(/no such file or directory/);
+        });
+    });
+
+    it('removes dependency from absolute config.directory', function () {
+        var targetPath = path.resolve(tempDir.path, 'other_directory/underscore');
+        mkdirp.sync(targetPath);
+        fs.writeFileSync(path.join(targetPath, '.bower.json'), '{ "name": "underscore" }');
+
+        return helpers.run(uninstall, [['underscore'], undefined, {
+            cwd: tempDir.path,
+            directory: path.resolve(tempDir.path, 'other_directory'),
+            interactive: true
+        }])
+        .then(function() {
+            expect(function() {
+                fs.statSync(targetPath);
+            }).to.throwException(/no such file or directory/);
+        });
+    });
+
 });
diff --git a/test/util/createLink.js b/test/util/createLink.js
@@ -0,0 +1,57 @@
+var path = require('path');
+var Q = require('q');
+var fs = require('fs');
+var expect = require('expect.js');
+var helpers = require('../helpers');
+var createLink = require('../../lib/util/createLink');
+
+describe('createLink', function () {
+
+    var srcDir = new helpers.TempDir({
+        someFile: 'Hello World',
+        someDirectory: {
+            otherFile: 'Hello World'
+        }
+    });
+
+    var dstDir = new helpers.TempDir();
+
+    beforeEach(function() {
+        srcDir.prepare();
+        dstDir.prepare();
+    });
+
+    it('creates a symlink to a file', function() {
+
+        var src = path.join(srcDir.path, 'someFile'),
+            dst = path.join(dstDir.path, 'someFile');
+
+        return createLink(src, dst)
+        .then(function() {
+            return Q.nfcall(fs.readlink, dst)
+            .then(function(linkString) {
+                expect(linkString).to.be.equal(src);
+            });
+        });
+    });
+
+    it('throws an error when destination already exists', function() {
+
+        var src = path.join(srcDir.path, 'someFile'),
+            dst = path.join(dstDir.path);
+
+        var deferred = Q.defer();
+
+        createLink(src, dst)
+        .catch(function(err) {
+            expect(err.code).to.be.equal('EEXIST');
+            deferred.resolve();
+        })
+        .then(function() {
+            deferred.reject();
+        });
+
+        return deferred.promise;
+    });
+
+});
diff --git a/test/util/index.js b/test/util/index.js
@@ -3,4 +3,6 @@ describe('util', function () {
     require('./analytics');
     require('./download');
     require('./isPathAbsolute');
+    require('./relativeToBaseDir');
+    require('./createLink');
 });
diff --git a/test/util/relativeToBaseDir.js b/test/util/relativeToBaseDir.js
@@ -0,0 +1,18 @@
+var path = require('path');
+var expect = require('expect.js');
+var relativeToBaseDir = require('../../lib/util/relativeToBaseDir');
+
+describe('relativeToBaseDir', function () {
+
+    var joinOrReturnAbsolutePath = relativeToBaseDir('/tmp');
+
+    it('returns a partial function that joins paths of the partials first arguments', function() {
+        expect(joinOrReturnAbsolutePath('foo')).to.be.equal(path.resolve('/tmp/foo'));
+        expect(joinOrReturnAbsolutePath('./foo')).to.be.equal(path.resolve('/tmp/foo'));
+    });
+
+    it('returns a partial function that returns it\'s first argument when it begins with /', function() {
+        expect(joinOrReturnAbsolutePath('/foo')).to.be.equal(path.resolve('/foo'));
+        expect(joinOrReturnAbsolutePath('/foo/bar')).to.be.equal(path.resolve('/foo/bar'));
+    });
+});