You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cordova.apache.org by ra...@apache.org on 2019/03/30 19:34:06 UTC

[cordova-lib] branch master updated: Fix faulty Promise handling in plugman.uninstall (#759)

This is an automated email from the ASF dual-hosted git repository.

raphinesse pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/cordova-lib.git


The following commit(s) were added to refs/heads/master by this push:
     new fd9466a  Fix faulty Promise handling in plugman.uninstall (#759)
fd9466a is described below

commit fd9466af97fb17eb2fa04325496ab6575d926461
Author: Raphael von der GrĂ¼n <ra...@gmail.com>
AuthorDate: Sat Mar 30 20:34:01 2019 +0100

    Fix faulty Promise handling in plugman.uninstall (#759)
    
    * Fix unhandled rejections with dependent plugin uninstall
    
    This also includes two changes to improve the robustness of plugman_uninstall.spec:
    * Proper mock return value for npmUninstall
    * Get rid of fs-stubbing
---
 integration-tests/plugman_uninstall.spec.js |  9 ++------
 src/plugman/uninstall.js                    | 32 ++++++++++++-----------------
 2 files changed, 15 insertions(+), 26 deletions(-)

diff --git a/integration-tests/plugman_uninstall.spec.js b/integration-tests/plugman_uninstall.spec.js
index eab0734..052d36a 100644
--- a/integration-tests/plugman_uninstall.spec.js
+++ b/integration-tests/plugman_uninstall.spec.js
@@ -42,11 +42,9 @@ const plugins = {
     'C': path.join(plugins_dir, 'dependencies', 'C')
 };
 
-// Grab a reference to the original, before someone stubs it
-const { copySync } = fs;
 function setupProject (name) {
     const projectPath = path.join(projectsPath, name);
-    copySync(projectPath, project);
+    fs.copySync(projectPath, project);
 }
 
 describe('plugman/uninstall', () => {
@@ -79,11 +77,9 @@ describe('plugman/uninstall', () => {
 
     beforeEach(() => {
         uninstall = rewire('../src/plugman/uninstall');
-        uninstall.__set__('npmUninstall', jasmine.createSpy());
+        uninstall.__set__('npmUninstall', jasmine.createSpy().and.returnValue(Promise.resolve()));
 
         emit = spyOn(events, 'emit');
-        spyOn(fs, 'writeFileSync');
-        spyOn(fs, 'removeSync');
     });
 
     afterEach(() => {
@@ -102,7 +98,6 @@ describe('plugman/uninstall', () => {
             setupProject('uninstall.test');
 
             spyOn(ActionStack.prototype, 'process').and.returnValue(Promise.resolve());
-            spyOn(fs, 'copySync');
         });
 
         describe('success', function () {
diff --git a/src/plugman/uninstall.js b/src/plugman/uninstall.js
index 05d8152..7f39689 100644
--- a/src/plugman/uninstall.js
+++ b/src/plugman/uninstall.js
@@ -285,8 +285,17 @@ function runUninstallPlatform (actions, platform, project_dir, plugin_dir, plugi
     var projectRoot = cordovaUtil.isCordova();
 
     if (projectRoot) {
+        // CB-10708 This is the case when we're trying to uninstall plugin using plugman from specific
+        // platform inside of the existing CLI project. This option is usually set by cordova-lib for CLI projects
+        // but since we're running this code through plugman, we need to set it here implicitly
+        options.usePlatformWww = true;
+        options.cli_variables = variables;
+    }
 
-        // using unified hooksRunner
+    return promise.then(function () {
+        if (!projectRoot) return;
+
+        var hooksRunner = new HooksRunner(projectRoot);
         var hooksRunnerOptions = {
             cordova: { platforms: [ platform ] },
             plugin: {
@@ -296,25 +305,10 @@ function runUninstallPlatform (actions, platform, project_dir, plugin_dir, plugi
                 dir: plugin_dir
             }
         };
-
-        // CB-10708 This is the case when we're trying to uninstall plugin using plugman from specific
-        // platform inside of the existing CLI project. This option is usually set by cordova-lib for CLI projects
-        // but since we're running this code through plugman, we need to set it here implicitly
-        options.usePlatformWww = true;
-        options.cli_variables = variables;
-
-        var hooksRunner = new HooksRunner(projectRoot);
-
-        return promise.then(function () {
-            return hooksRunner.fire('before_plugin_uninstall', hooksRunnerOptions);
-        }).then(function () {
-            return handleUninstall(actions, platform, pluginInfo, project_dir, options.www_dir, plugins_dir, options.is_top_level, options);
-        });
-    } else {
-        // TODO: Need review here - this condition added for plugman install.spec.js and uninstall.spec.js passing -
-        // where should we get projectRoot - via going up from project_dir?
+        return hooksRunner.fire('before_plugin_uninstall', hooksRunnerOptions);
+    }).then(function () {
         return handleUninstall(actions, platform, pluginInfo, project_dir, options.www_dir, plugins_dir, options.is_top_level, options);
-    }
+    });
 }
 
 // Returns a promise.


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@cordova.apache.org
For additional commands, e-mail: commits-help@cordova.apache.org