cordova-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From a...@apache.org
Subject [17/50] [abbrv] git commit: Fix to never remove top-level plugins that are dependencies + tests.
Date Fri, 11 Apr 2014 19:46:33 GMT
Fix to never remove top-level plugins that are dependencies + tests.

github: close #58


Project: http://git-wip-us.apache.org/repos/asf/cordova-plugman/repo
Commit: http://git-wip-us.apache.org/repos/asf/cordova-plugman/commit/a4a51d7c
Tree: http://git-wip-us.apache.org/repos/asf/cordova-plugman/tree/a4a51d7c
Diff: http://git-wip-us.apache.org/repos/asf/cordova-plugman/diff/a4a51d7c

Branch: refs/heads/browserify
Commit: a4a51d7ce8200594bf90ecc524afbfc899c4e361
Parents: f58db0a
Author: jbondc <jbondc@openmv.com>
Authored: Sun Mar 9 18:31:01 2014 -0400
Committer: Andrew Grieve <agrieve@chromium.org>
Committed: Mon Mar 10 16:30:30 2014 -0400

----------------------------------------------------------------------
 spec/common.js         | 41 ++++++++++++++++-------
 spec/install.spec.js   |  9 ++---
 spec/uninstall.spec.js | 80 ++++++++++++++++++++++++++++++++++++---------
 src/uninstall.js       | 49 ++++++++++++++++++++-------
 4 files changed, 135 insertions(+), 44 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/cordova-plugman/blob/a4a51d7c/spec/common.js
----------------------------------------------------------------------
diff --git a/spec/common.js b/spec/common.js
index 2991806..6a87e23 100644
--- a/spec/common.js
+++ b/spec/common.js
@@ -29,15 +29,34 @@ if(opt.debug) {
         plugman.on('log', console.log);
 }
 
-module.exports = {
-	spy: { 
-		getInstall: function(emitSpy){
-			var install = [], i;
-			for(i in emitSpy.argsForCall) {
-				if(emitSpy.argsForCall[i][1].substr(0, 13) === 'Install start')
-					install.push(emitSpy.argsForCall[i][1]);
-			}
-			return install;
-		}
-	}
+module.exports = common = {
+    spy: { 
+        getInstall: function(emitSpy){
+            return common.spy.startsWith(emitSpy, 'Install start');
+        },
+
+        getDeleted: function(emitSpy){
+            return common.spy.startsWith(emitSpy, 'Deleted');
+        },
+
+        startsWith: function(emitSpy, string)
+        {
+            var match = [], i;
+            for(i in emitSpy.argsForCall) {
+                if(emitSpy.argsForCall[i][1].substr(0, string.length) === string)
+                    match.push(emitSpy.argsForCall[i][1]);
+            }
+            return match;
+        },
+
+        contains: function(emitSpy, string)
+        {
+            var match = [], i;
+            for(i in emitSpy.argsForCall) {
+                if(emitSpy.argsForCall[i][1].indexOf(string) >= 0)
+                    match.push(emitSpy.argsForCall[i][1]);
+            }
+            return match;
+        }
+    }
 };

http://git-wip-us.apache.org/repos/asf/cordova-plugman/blob/a4a51d7c/spec/install.spec.js
----------------------------------------------------------------------
diff --git a/spec/install.spec.js b/spec/install.spec.js
index 5a40de3..f552c49 100644
--- a/spec/install.spec.js
+++ b/spec/install.spec.js
@@ -411,17 +411,14 @@ describe('end', function() {
                          
     it('end', function() {
         done = false;
-        var finish = function(err){
+
+        promise.fin(function(err){
             if(err)
                 events.emit('error', err);
 
             shell.rm('-rf', project);
             done = true;	
-        }
-
-        promise.then( 
-
-        ).fin(finish);
+        });
 
         waitsFor(function() { return done; }, 'promise never resolved', 500);
     });

http://git-wip-us.apache.org/repos/asf/cordova-plugman/blob/a4a51d7c/spec/uninstall.spec.js
----------------------------------------------------------------------
diff --git a/spec/uninstall.spec.js b/spec/uninstall.spec.js
index 7ae8a14..bf9aa7f 100644
--- a/spec/uninstall.spec.js
+++ b/spec/uninstall.spec.js
@@ -13,9 +13,12 @@ var uninstall = require('../src/uninstall'),
     done    = false,
     srcProject = path.join(spec, 'projects', 'android_uninstall'),
     project = path.join(spec, 'projects', 'android_uninstall.test'),
+    project2 = path.join(spec, 'projects', 'android_uninstall.test2'),
 
     plugins_dir = path.join(spec, 'plugins'),
     plugins_install_dir = path.join(project, 'cordova', 'plugins'),	
+    plugins_install_dir2 = path.join(project2, 'cordova', 'plugins'),	
+
     plugins = {
         'DummyPlugin' : path.join(plugins_dir, 'DummyPlugin'),
         'A' : path.join(plugins_dir, 'dependencies', 'A'),
@@ -32,7 +35,9 @@ describe('start', function() {
 
     it('start', function() {
         shell.rm('-rf', project);
+        shell.rm('-rf', project2);
         shell.cp('-R', path.join(srcProject, '*'), project);
+        shell.cp('-R', path.join(srcProject, '*'), project2);
 
         done = false;
         promise = Q()
@@ -41,6 +46,10 @@ describe('start', function() {
         ).then(
             function(){ return install('android', project, plugins['A']) }
         ).then(
+            function(){ return install('android', project2, plugins['C']) }
+        ).then(
+            function(){ return install('android', project2, plugins['A']) }
+        ).then(
             function(){ done = true; }
         );
         waitsFor(function() { return done; }, 'promise never resolved', 500);
@@ -134,28 +143,67 @@ describe('uninstallPlatform', function() {
 });
 
 describe('uninstallPlugin', function() {							 
-    var rm, fsWrite, rmstack = [];
+    var rm, fsWrite, rmstack = [], emit;
 
     beforeEach(function() {
         fsWrite = spyOn(fs, 'writeFileSync').andReturn(true);
         rm = spyOn(shell, 'rm').andCallFake(function(f,p) { rmstack.push(p); return true});
         rmstack = [];
+        emit = spyOn(events, 'emit');
         done = false;
     });
     describe('with dependencies', function() {
 
-        it('should delete all dangling plugins', function() {
-                                              
+        it('should delete all dependent plugins', function() {                          
   
             runs(function() {
                 uninstallPromise( uninstall.uninstallPlugin('A', plugins_install_dir) );
             });
             waitsFor(function() { return done; }, 'promise never resolved', 200);
             runs(function() {
-                expect(done).toEqual(true);
-                expect(rmstack[0]).toBe( path.join(plugins_install_dir, 'C') );
-                expect(rmstack[1]).toBe( path.join(plugins_install_dir, 'D') );
-                expect(rmstack[2]).toBe( path.join(plugins_install_dir, 'A') );
-                expect(rm.calls.length).toBe(3);
+                var del = common.spy.getDeleted(emit);
+
+                expect(del).toEqual([
+                    'Deleted "C"',
+                    'Deleted "D"',
+                    'Deleted "A"'
+                ]);
+            });
+        });
+
+        it("should fail if plugin is a required dependency", function() {  
+            runs(function() {
+                uninstallPromise( uninstall.uninstallPlugin('C', plugins_install_dir) );
+            });
+            waitsFor(function() { return done; }, 'promise never resolved', 200);
+            runs(function() {
+                expect(done.message).toBe('"C" is required by (A) and cannot be removed (hint:
use -f or --force)');
+            });
+        });
+
+        it("allow forcefully removing a plugin", function() {  
+            runs(function() {
+                uninstallPromise( uninstall.uninstallPlugin('C', plugins_install_dir, {force:
true}) );
+            });
+            waitsFor(function() { return done; }, 'promise never resolved', 200);
+            runs(function() {
+                expect(done).toBe(true);
+                var del = common.spy.getDeleted(emit);
+                expect(del).toEqual(['Deleted "C"']);
+            });
+        });		
+
+        it("never remove top level plugins if they are a dependency", function() {  
+            runs(function() {
+                uninstallPromise( uninstall.uninstallPlugin('A', plugins_install_dir2) );
+            });
+            waitsFor(function() { return done; }, 'promise never resolved', 200);
+            runs(function() {
+                var del = common.spy.getDeleted(emit);
+
+                expect(del).toEqual([
+                    'Deleted "D"',
+                    'Deleted "A"'
+                ]);
             });
         });
     });
@@ -208,13 +256,6 @@ describe('end', function() {
                          
     it('end', function() {
         done = false;
-        var finish = function(err){
-            if(err)
-                plugman.emit('error', err);
-
-            shell.rm('-rf', project);
-            done = true;	
-        }
 
         promise.then( 
             function(){ 
@@ -234,7 +275,14 @@ describe('end', function() {
                 // dependencies on C,D ... should this only work with --recursive? prompt
user..?
                 return uninstall('android', project, plugins['A']) 
             }
-        ).fin(finish);
+        ).fin(function(err){
+            if(err)
+                plugman.emit('error', err);
+
+            shell.rm('-rf', project);
+            shell.rm('-rf', project2);
+            done = true;	
+        });
 
         waitsFor(function() { return done; }, 'promise never resolved', 500);
     });

http://git-wip-us.apache.org/repos/asf/cordova-plugman/blob/a4a51d7c/src/uninstall.js
----------------------------------------------------------------------
diff --git a/src/uninstall.js b/src/uninstall.js
index 18d7a18..98c57ad 100644
--- a/src/uninstall.js
+++ b/src/uninstall.js
@@ -61,23 +61,27 @@ module.exports.uninstallPlugin = function(id, plugins_dir, options) {
 
     var plugin_dir = path.join(plugins_dir, id);
 
+    // @tests - important this event is checked spec/uninstall.spec.js
+    events.emit('log', 'Removing "'+ id +'"');
+
     // If already removed, skip.
-    if (!fs.existsSync(plugin_dir)) {
+    if ( !fs.existsSync(plugin_dir) ) {
+        events.emit('verbose', 'Plugin "'+ id +'" already removed ('+ plugin_dir +')');
         return Q();
     }
 
     var xml_path  = path.join(plugin_dir, 'plugin.xml')
       , plugin_et = xml_helpers.parseElementtreeSync(xml_path);
 
-    events.emit('log', 'Deleting "'+ id +'"');
-
     var doDelete = function(id) {
         var plugin_dir = path.join(plugins_dir, id);
-        if ( !fs.existsSync(plugin_dir) ) 
-            return;
+        if ( !fs.existsSync(plugin_dir) ) {
+            events.emit('verbose', 'Plugin "'+ id +'" already removed ('+ plugin_dir +')');
+            return Q();
+        }
 
         shell.rm('-rf', plugin_dir);
-        events.emit('verbose', '"'+ id +'" deleted.');
+        events.emit('verbose', 'Deleted "'+ id +'"');
     };
 
     // We've now lost the metadata for the plugins that have been uninstalled, so we can't
use that info.
@@ -85,7 +89,11 @@ module.exports.uninstallPlugin = function(id, plugins_dir, options) {
     // anything depends on them, or if they're listed as top-level.
     // If neither, they can be deleted.
     var top_plugin_id = id;
-    var toDelete = plugin_et.findall('dependency');
+
+    // Recursively remove plugins which were installed as dependents (that are not top-level)
+    // optional?
+    var recursive = true;
+    var toDelete = recursive ? plugin_et.findall('dependency') : [];
     toDelete = toDelete && toDelete.length ? toDelete.map(function(p) { return p.attrib.id;
}) : [];
     toDelete.push(top_plugin_id);
 
@@ -95,14 +103,26 @@ module.exports.uninstallPlugin = function(id, plugins_dir, options) {
         return fs.existsSync(path.join(plugins_dir, platform + '.json'));
     });
 
+    // Can have missing plugins on some platforms when not supported..
     var dependList = {};
-    platforms.forEach(function(platform) {				   
+    platforms.forEach(function(platform) {
         var depsInfo = dependencies.generate_dependency_info(plugins_dir, platform);
         var tlps = depsInfo.top_level_plugins,
             deps, i;
 
+        // Top-level deps must always be explicitely asked to remove by user
+        tlps.forEach(function(plugin_id){
+            if(top_plugin_id == plugin_id)
+                return;
+
+            var i = toDelete.indexOf(plugin_id);
+            if(i >= 0)
+                toDelete.splice(i, 1);
+        });
+
         toDelete.forEach(function(plugin) {
             deps = dependencies.dependents(plugin, depsInfo);
+
             var i = deps.indexOf(top_plugin_id);
             if(i >= 0)
                  deps.splice(i, 1); // remove current/top-level plugin as blocking uninstall
@@ -122,8 +142,15 @@ module.exports.uninstallPlugin = function(id, plugins_dir, options) {
             if(options.force) {
                 events.emit('log', msg +' but forcing removal.');
             } else {
-                events.emit('warn', msg +' and cannot be removed (hint: use -f or --force)');
-                continue;
+                // @tests - error and event message is checked spec/uninstall.spec.js
+                msg += ' and cannot be removed (hint: use -f or --force)';
+
+                if(plugin_id == top_plugin_id) {
+                    return Q.reject( new Error(msg) );
+                } else {
+                    events.emit('warn', msg +' and cannot be removed (hint: use -f or --force)');
+                    continue;	
+                }
             }
         }
 
@@ -163,7 +190,7 @@ function runUninstallPlatform(actions, platform, project_dir, plugin_dir,
plugin
 
     var promise;
     if (deps && deps.length && danglers && danglers.length) {
-        
+
         // @tests - important this event is checked spec/uninstall.spec.js
         events.emit('log', 'Uninstalling ' + danglers.length + ' dependent plugins.');
         promise = Q.all(


Mime
View raw message