Repository: cordova-lib Updated Branches: refs/heads/master 983e9b8f9 -> 2877e3343
CB-11022 Respect result returned by plugin installation and skip prepare if it is truthy This closes #423 Project: http://git-wip-us.apache.org/repos/asf/cordova-lib/repo Commit: http://git-wip-us.apache.org/repos/asf/cordova-lib/commit/2877e334 Tree: http://git-wip-us.apache.org/repos/asf/cordova-lib/tree/2877e334 Diff: http://git-wip-us.apache.org/repos/asf/cordova-lib/diff/2877e334 Branch: refs/heads/master Commit: 2877e334337da86dd876005bbc3b606b8e1c31bd Parents: 983e9b8 Author: Vladimir Kotikov <[email protected]> Authored: Thu Apr 7 13:56:49 2016 +0300 Committer: Vladimir Kotikov <[email protected]> Committed: Thu Apr 14 10:50:22 2016 +0300 ---------------------------------------------------------------------- cordova-lib/spec-cordova/plugin.spec.js | 555 +++++++++++++----------- cordova-lib/spec-plugman/install.spec.js | 32 +- cordova-lib/spec-plugman/uninstall.spec.js | 38 ++ cordova-lib/src/cordova/plugin.js | 29 +- cordova-lib/src/plugman/install.js | 17 +- cordova-lib/src/plugman/uninstall.js | 7 +- 6 files changed, 409 insertions(+), 269 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/cordova-lib/blob/2877e334/cordova-lib/spec-cordova/plugin.spec.js ---------------------------------------------------------------------- diff --git a/cordova-lib/spec-cordova/plugin.spec.js b/cordova-lib/spec-cordova/plugin.spec.js index a7e46eb..ac6f6b9 100644 --- a/cordova-lib/spec-cordova/plugin.spec.js +++ b/cordova-lib/spec-cordova/plugin.spec.js @@ -1,254 +1,301 @@ -/** - Licensed to the Apache Software Foundation (ASF) under one - or more contributor license agreements. See the NOTICE file - distributed with this work for additional information - regarding copyright ownership. The ASF licenses this file - to you under the Apache License, Version 2.0 (the - "License"); you may not use this file except in compliance - with the License. You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - - Unless required by applicable law or agreed to in writing, - software distributed under the License is distributed on an - "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - KIND, either express or implied. See the License for the - specific language governing permissions and limitations - under the License. -*/ - -var helpers = require('./helpers'), - path = require('path'), - Q = require('q'), - shell = require('shelljs'), - events = require('cordova-common').events, - cordova = require('../src/cordova/cordova'), - plugman = require('../src/plugman/plugman'), - registry = require('../src/plugman/registry/registry'); - -var util = require('../src/cordova/util'); - -var tmpDir = helpers.tmpDir('plugin_test'); -var project = path.join(tmpDir, 'project'); -var pluginsDir = path.join(__dirname, 'fixtures', 'plugins'); - -var pluginId = 'org.apache.cordova.fakeplugin1'; -var org_test_defaultvariables = 'org.test.defaultvariables'; - -// This plugin is published to npm and defines cordovaDependencies -// in its package.json. Based on the dependencies and the version of -// cordova-android installed in our test project, the CLI should -// select version 1.1.2 of the plugin. We don't actually fetch from -// npm, but we do check the npm info. -var npmInfoTestPlugin = 'cordova-lib-test-plugin'; -var npmInfoTestPluginVersion = '1.1.2'; - -var testGitPluginRepository = 'https://github.com/apache/cordova-plugin-device.git'; -var testGitPluginId = 'cordova-plugin-device'; - -var results; - -// Runs: list, add, list -function addPlugin(target, id, options) { - // Check there are no plugins yet. - return cordova.raw.plugin('list').then(function() { - expect(results).toMatch(/No plugins added/gi); - }).then(function() { - // Add a fake plugin from fixtures. - return cordova.raw.plugin('add', target, options); - }).then(function() { - expect(path.join(project, 'plugins', id, 'plugin.xml')).toExist(); - }).then(function() { - return cordova.raw.plugin('ls'); - }).then(function() { - expect(results).toContain(id); - }); -} - -// Runs: remove, list -function removePlugin(id) { - return cordova.raw.plugin('rm', id) - .then(function() { - // The whole dir should be gone. - expect(path.join(project, 'plugins', id)).not.toExist(); - }).then(function() { - return cordova.raw.plugin('ls'); - }).then(function() { - expect(results).toMatch(/No plugins added/gi); - }); -} - -var errorHandler = { - errorCallback: function(error) { - // We want the error to be printed by jasmine - expect(error).toBeUndefined(); - } -}; - -// We can't call add with a searchpath or else we will conflict with other tests -// that use a searchpath. See loadLocalPlugins() in plugman/fetch.js for details. -// The searchpath behavior gets tested in the plugman spec -function mockPluginFetch(id, dir) { - spyOn(plugman.raw, 'fetch').andCallFake(function(target, pluginPath, fetchOptions) { - var dest = path.join(project, 'plugins', id); - var src = path.join(dir, 'plugin.xml'); - - shell.mkdir(dest); - shell.cp(src, dest); - return Q(dest); - }); -} - -describe('plugin end-to-end', function() { - events.on('results', function(res) { results = res; }); - - beforeEach(function() { - shell.rm('-rf', project); - - // cp then mv because we need to copy everything, but that means it'll copy the whole directory. - // Using /* doesn't work because of hidden files. - shell.cp('-R', path.join(__dirname, 'fixtures', 'base'), tmpDir); - shell.mv(path.join(tmpDir, 'base'), project); - // Copy some platform to avoid working on a project with no platforms. - shell.cp('-R', path.join(__dirname, 'fixtures', 'platforms', helpers.testPlatform), path.join(project, 'platforms')); - process.chdir(project); - - // Reset origCwd before each spec to respect chdirs - util._resetOrigCwd(); - delete process.env.PWD; - - spyOn(errorHandler, 'errorCallback').andCallThrough(); - }); - - afterEach(function() { - process.chdir(path.join(__dirname, '..')); // Needed to rm the dir on Windows. - shell.rm('-rf', tmpDir); - expect(errorHandler.errorCallback).not.toHaveBeenCalled(); - }); - - it('should successfully add and remove a plugin with no options', function(done) { - addPlugin(path.join(pluginsDir, 'fake1'), pluginId, {}, done) - .then(function() { - return removePlugin(pluginId); - }) - .fail(errorHandler.errorCallback) - .fin(done); - }); - - it('should successfully add a plugin using relative path when running from subdir inside of project', function(done) { - // Copy plugin to subdir inside of the project. This is required since path.relative - // returns an absolute path when source and dest are on different drives - var plugindir = path.join(project, 'custom-plugins/some-plugin-inside-subfolder'); - shell.mkdir('-p', plugindir); - shell.cp('-r', path.join(pluginsDir, 'fake1/*'), plugindir); - - // Create a subdir, where we're going to run cordova from - var subdir = path.join(project, 'bin'); - shell.mkdir('-p', subdir); - shell.cd(subdir); - - // Add plugin using relative path - addPlugin(path.relative(subdir, plugindir), pluginId, {}, done) - .then(function() { - return removePlugin(pluginId); - }) - .fail(errorHandler.errorCallback) - .fin(done); - }); - - it('should successfully add a plugin when specifying CLI variables', function(done) { - addPlugin(path.join(pluginsDir, org_test_defaultvariables), org_test_defaultvariables, {cli_variables: { REQUIRED:'yes', REQUIRED_ANDROID:'yes'}}, done) - .fail(errorHandler.errorCallback) - .fin(done); - }); - - it('should not check npm info when using the searchpath flag', function(done) { - mockPluginFetch(npmInfoTestPlugin, path.join(pluginsDir, npmInfoTestPlugin)); - - spyOn(registry, 'info'); - addPlugin(npmInfoTestPlugin, npmInfoTestPlugin, {searchpath: pluginsDir}, done) - .then(function() { - expect(registry.info).not.toHaveBeenCalled(); - - var fetchOptions = plugman.raw.fetch.mostRecentCall.args[2]; - expect(fetchOptions.searchpath).toBeDefined(); - }) - .fail(errorHandler.errorCallback) - .fin(done); - }); - - it('should not check npm info when using the noregistry flag', function(done) { - mockPluginFetch(npmInfoTestPlugin, path.join(pluginsDir, npmInfoTestPlugin)); - - spyOn(registry, 'info'); - addPlugin(npmInfoTestPlugin, npmInfoTestPlugin, {noregistry:true}, done) - .then(function() { - expect(registry.info).not.toHaveBeenCalled(); - - var fetchOptions = plugman.raw.fetch.mostRecentCall.args[2]; - expect(fetchOptions.noregistry).toBeTruthy(); - }) - .fail(errorHandler.errorCallback) - .fin(done); - }); - - it('should not check npm info when fetching from a Git repository', function(done) { - spyOn(registry, 'info'); - addPlugin(testGitPluginRepository, testGitPluginId, {}, done) - .then(function() { - expect(registry.info).not.toHaveBeenCalled(); - }) - .fail(errorHandler.errorCallback) - .fin(done); - }); - - it('should select the plugin version based on npm info when fetching from npm', function(done) { - mockPluginFetch(npmInfoTestPlugin, path.join(pluginsDir, npmInfoTestPlugin)); - - spyOn(registry, 'info').andCallThrough(); - addPlugin(npmInfoTestPlugin, npmInfoTestPlugin, {}, done) - .then(function() { - expect(registry.info).toHaveBeenCalled(); - - var fetchTarget = plugman.raw.fetch.mostRecentCall.args[0]; - expect(fetchTarget).toEqual(npmInfoTestPlugin + '@' + npmInfoTestPluginVersion); - }) - .fail(errorHandler.errorCallback) - .fin(done); - }); - - it('should handle scoped npm packages', function(done) { - var scopedPackage = '@testscope/' + npmInfoTestPlugin; - mockPluginFetch(npmInfoTestPlugin, path.join(pluginsDir, npmInfoTestPlugin)); - - spyOn(registry, 'info').andReturn(Q({})); - addPlugin(scopedPackage, npmInfoTestPlugin, {}, done) - .then(function() { - // Check to make sure that we are at least trying to get the correct package. - // This package is not published to npm, so we can't truly do end-to-end tests - - expect(registry.info).toHaveBeenCalledWith([scopedPackage]); - - var fetchTarget = plugman.raw.fetch.mostRecentCall.args[0]; - expect(fetchTarget).toEqual(scopedPackage); - }) - .fail(errorHandler.errorCallback) - .fin(done); - }); - - it('should handle scoped npm packages with given version tags', function(done) { - var scopedPackage = '@testscope/' + npmInfoTestPlugin + '@latest'; - mockPluginFetch(npmInfoTestPlugin, path.join(pluginsDir, npmInfoTestPlugin)); - - spyOn(registry, 'info'); - addPlugin(scopedPackage, npmInfoTestPlugin, {}, done) - .then(function() { - expect(registry.info).not.toHaveBeenCalled(); - - var fetchTarget = plugman.raw.fetch.mostRecentCall.args[0]; - expect(fetchTarget).toEqual(scopedPackage); - }) - .fail(errorHandler.errorCallback) - .fin(done); - }); -}); +/** + Licensed to the Apache Software Foundation (ASF) under one + or more contributor license agreements. See the NOTICE file + distributed with this work for additional information + regarding copyright ownership. The ASF licenses this file + to you under the Apache License, Version 2.0 (the + "License"); you may not use this file except in compliance + with the License. You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, + software distributed under the License is distributed on an + "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + KIND, either express or implied. See the License for the + specific language governing permissions and limitations + under the License. +*/ + +var helpers = require('./helpers'), + path = require('path'), + Q = require('q'), + shell = require('shelljs'), + events = require('cordova-common').events, + cordova = require('../src/cordova/cordova'), + prepare = require('../src/cordova/prepare'), + platforms = require('../src/platforms/platforms'), + plugman = require('../src/plugman/plugman'), + registry = require('../src/plugman/registry/registry'); + +var util = require('../src/cordova/util'); + +var tmpDir = helpers.tmpDir('plugin_test'); +var project = path.join(tmpDir, 'project'); +var pluginsDir = path.join(__dirname, 'fixtures', 'plugins'); + +var pluginId = 'org.apache.cordova.fakeplugin1'; +var org_test_defaultvariables = 'org.test.defaultvariables'; + +// This plugin is published to npm and defines cordovaDependencies +// in its package.json. Based on the dependencies and the version of +// cordova-android installed in our test project, the CLI should +// select version 1.1.2 of the plugin. We don't actually fetch from +// npm, but we do check the npm info. +var npmInfoTestPlugin = 'cordova-lib-test-plugin'; +var npmInfoTestPluginVersion = '1.1.2'; + +var testGitPluginRepository = 'https://github.com/apache/cordova-plugin-device.git'; +var testGitPluginId = 'cordova-plugin-device'; + +var results; + +// Runs: list, add, list +function addPlugin(target, id, options) { + // Check there are no plugins yet. + return cordova.raw.plugin('list').then(function() { + expect(results).toMatch(/No plugins added/gi); + }).then(function() { + // Add a fake plugin from fixtures. + return cordova.raw.plugin('add', target, options); + }).then(function() { + expect(path.join(project, 'plugins', id, 'plugin.xml')).toExist(); + }).then(function() { + return cordova.raw.plugin('ls'); + }).then(function() { + expect(results).toContain(id); + }); +} + +// Runs: remove, list +function removePlugin(id) { + return cordova.raw.plugin('rm', id) + .then(function() { + // The whole dir should be gone. + expect(path.join(project, 'plugins', id)).not.toExist(); + }).then(function() { + return cordova.raw.plugin('ls'); + }).then(function() { + expect(results).toMatch(/No plugins added/gi); + }); +} + +var errorHandler = { + errorCallback: function(error) { + // We want the error to be printed by jasmine + expect(error).toBeUndefined(); + } +}; + +// We can't call add with a searchpath or else we will conflict with other tests +// that use a searchpath. See loadLocalPlugins() in plugman/fetch.js for details. +// The searchpath behavior gets tested in the plugman spec +function mockPluginFetch(id, dir) { + spyOn(plugman.raw, 'fetch').andCallFake(function(target, pluginPath, fetchOptions) { + var dest = path.join(project, 'plugins', id); + var src = path.join(dir, 'plugin.xml'); + + shell.mkdir(dest); + shell.cp(src, dest); + return Q(dest); + }); +} + +function setupPlatformApiSpies() { + var api = platforms.getPlatformApi(helpers.testPlatform, path.join(project, 'platforms', helpers.testPlatform)); + var addPluginOrig = api.addPlugin; + var removePluginOrig = api.removePlugin; + + spyOn(api, 'addPlugin').andCallFake(function () { + return addPluginOrig.apply(api, arguments) + .thenResolve(true); + }); + + spyOn(api, 'removePlugin').andCallFake(function () { + return removePluginOrig.apply(api, arguments) + .thenResolve(true); + }); +} + +describe('plugin end-to-end', function() { + events.on('results', function(res) { results = res; }); + + beforeEach(function() { + shell.rm('-rf', project); + + // cp then mv because we need to copy everything, but that means it'll copy the whole directory. + // Using /* doesn't work because of hidden files. + shell.cp('-R', path.join(__dirname, 'fixtures', 'base'), tmpDir); + shell.mv(path.join(tmpDir, 'base'), project); + // Copy some platform to avoid working on a project with no platforms. + shell.cp('-R', path.join(__dirname, 'fixtures', 'platforms', helpers.testPlatform), path.join(project, 'platforms')); + process.chdir(project); + + // Reset origCwd before each spec to respect chdirs + util._resetOrigCwd(); + delete process.env.PWD; + + spyOn(prepare, 'preparePlatforms').andCallThrough(); + spyOn(errorHandler, 'errorCallback').andCallThrough(); + }); + + afterEach(function() { + process.chdir(path.join(__dirname, '..')); // Needed to rm the dir on Windows. + shell.rm('-rf', tmpDir); + expect(errorHandler.errorCallback).not.toHaveBeenCalled(); + }); + + it('should successfully add and remove a plugin with no options', function(done) { + addPlugin(path.join(pluginsDir, 'fake1'), pluginId, {}, done) + .then(function() { + return removePlugin(pluginId); + }) + .fail(errorHandler.errorCallback) + .fin(done); + }); + + it('should run prepare after plugin installation/removal by default', function(done) { + addPlugin(path.join(pluginsDir, 'fake1'), pluginId, {}) + .then(function() { + expect(prepare.preparePlatforms).toHaveBeenCalled(); + prepare.preparePlatforms.reset(); + return removePlugin(pluginId); + }) + .then(function () { + expect(prepare.preparePlatforms).toHaveBeenCalled(); + }) + .fail(errorHandler.errorCallback) + .fin(done); + }); + + it('should not run prepare after plugin installation/removal if platform return non-falsy value', function(done) { + setupPlatformApiSpies(); + addPlugin(path.join(pluginsDir, 'fake1'), pluginId, {}) + .then(function() { + expect(prepare.preparePlatforms).not.toHaveBeenCalled(); + return removePlugin(pluginId); + }) + .then(function () { + expect(prepare.preparePlatforms).not.toHaveBeenCalled(); + }) + .fail(errorHandler.errorCallback) + .fin(done); + }); + + it('should successfully add a plugin using relative path when running from subdir inside of project', function(done) { + // Copy plugin to subdir inside of the project. This is required since path.relative + // returns an absolute path when source and dest are on different drives + var plugindir = path.join(project, 'custom-plugins/some-plugin-inside-subfolder'); + shell.mkdir('-p', plugindir); + shell.cp('-r', path.join(pluginsDir, 'fake1/*'), plugindir); + + // Create a subdir, where we're going to run cordova from + var subdir = path.join(project, 'bin'); + shell.mkdir('-p', subdir); + shell.cd(subdir); + + // Add plugin using relative path + addPlugin(path.relative(subdir, plugindir), pluginId, {}, done) + .then(function() { + return removePlugin(pluginId); + }) + .fail(errorHandler.errorCallback) + .fin(done); + }); + + it('should successfully add a plugin when specifying CLI variables', function(done) { + addPlugin(path.join(pluginsDir, org_test_defaultvariables), org_test_defaultvariables, {cli_variables: { REQUIRED:'yes', REQUIRED_ANDROID:'yes'}}, done) + .fail(errorHandler.errorCallback) + .fin(done); + }); + + it('should not check npm info when using the searchpath flag', function(done) { + mockPluginFetch(npmInfoTestPlugin, path.join(pluginsDir, npmInfoTestPlugin)); + + spyOn(registry, 'info'); + addPlugin(npmInfoTestPlugin, npmInfoTestPlugin, {searchpath: pluginsDir}, done) + .then(function() { + expect(registry.info).not.toHaveBeenCalled(); + + var fetchOptions = plugman.raw.fetch.mostRecentCall.args[2]; + expect(fetchOptions.searchpath).toBeDefined(); + }) + .fail(errorHandler.errorCallback) + .fin(done); + }); + + it('should not check npm info when using the noregistry flag', function(done) { + mockPluginFetch(npmInfoTestPlugin, path.join(pluginsDir, npmInfoTestPlugin)); + + spyOn(registry, 'info'); + addPlugin(npmInfoTestPlugin, npmInfoTestPlugin, {noregistry:true}, done) + .then(function() { + expect(registry.info).not.toHaveBeenCalled(); + + var fetchOptions = plugman.raw.fetch.mostRecentCall.args[2]; + expect(fetchOptions.noregistry).toBeTruthy(); + }) + .fail(errorHandler.errorCallback) + .fin(done); + }); + + it('should not check npm info when fetching from a Git repository', function(done) { + spyOn(registry, 'info'); + addPlugin(testGitPluginRepository, testGitPluginId, {}, done) + .then(function() { + expect(registry.info).not.toHaveBeenCalled(); + }) + .fail(errorHandler.errorCallback) + .fin(done); + }); + + it('should select the plugin version based on npm info when fetching from npm', function(done) { + mockPluginFetch(npmInfoTestPlugin, path.join(pluginsDir, npmInfoTestPlugin)); + + spyOn(registry, 'info').andCallThrough(); + addPlugin(npmInfoTestPlugin, npmInfoTestPlugin, {}, done) + .then(function() { + expect(registry.info).toHaveBeenCalled(); + + var fetchTarget = plugman.raw.fetch.mostRecentCall.args[0]; + expect(fetchTarget).toEqual(npmInfoTestPlugin + '@' + npmInfoTestPluginVersion); + }) + .fail(errorHandler.errorCallback) + .fin(done); + }); + + it('should handle scoped npm packages', function(done) { + var scopedPackage = '@testscope/' + npmInfoTestPlugin; + mockPluginFetch(npmInfoTestPlugin, path.join(pluginsDir, npmInfoTestPlugin)); + + spyOn(registry, 'info').andReturn(Q({})); + addPlugin(scopedPackage, npmInfoTestPlugin, {}, done) + .then(function() { + // Check to make sure that we are at least trying to get the correct package. + // This package is not published to npm, so we can't truly do end-to-end tests + + expect(registry.info).toHaveBeenCalledWith([scopedPackage]); + + var fetchTarget = plugman.raw.fetch.mostRecentCall.args[0]; + expect(fetchTarget).toEqual(scopedPackage); + }) + .fail(errorHandler.errorCallback) + .fin(done); + }); + + it('should handle scoped npm packages with given version tags', function(done) { + var scopedPackage = '@testscope/' + npmInfoTestPlugin + '@latest'; + mockPluginFetch(npmInfoTestPlugin, path.join(pluginsDir, npmInfoTestPlugin)); + + spyOn(registry, 'info'); + addPlugin(scopedPackage, npmInfoTestPlugin, {}, done) + .then(function() { + expect(registry.info).not.toHaveBeenCalled(); + + var fetchTarget = plugman.raw.fetch.mostRecentCall.args[0]; + expect(fetchTarget).toEqual(scopedPackage); + }) + .fail(errorHandler.errorCallback) + .fin(done); + }); +}); http://git-wip-us.apache.org/repos/asf/cordova-lib/blob/2877e334/cordova-lib/spec-plugman/install.spec.js ---------------------------------------------------------------------- diff --git a/cordova-lib/spec-plugman/install.spec.js b/cordova-lib/spec-plugman/install.spec.js index a5895f2..0e1e5cd 100644 --- a/cordova-lib/spec-plugman/install.spec.js +++ b/cordova-lib/spec-plugman/install.spec.js @@ -27,6 +27,7 @@ var install = require('../src/plugman/install'), events = require('cordova-common').events, plugman = require('../src/plugman/plugman'), platforms = require('../src/plugman/platforms/common'), + knownPlatforms = require('../src/platforms/platforms'), common = require('./common'), fs = require('fs'), os = require('os'), @@ -127,10 +128,21 @@ describe('start', function() { return origParseElementtreeSync(path); }); }); + it('start', function() { shell.rm('-rf', project); shell.cp('-R', path.join(srcProject, '*'), project); + // Every time when addPlugin is called it will return some truthy value + var returnValueIndex = 0; + var returnValues = [true, {}, [], 'foo', function(){}]; + var api = knownPlatforms.getPlatformApi('android', project); + var addPluginOrig = api.addPlugin; + spyOn(api, 'addPlugin').andCallFake(function () { + return addPluginOrig.apply(api, arguments) + .thenResolve(returnValues[returnValueIndex++]); + }); + done = false; promise = Q() .then( @@ -138,7 +150,8 @@ describe('start', function() { return install('android', project, plugins['org.test.plugins.dummyplugin']); } ).then( - function(){ + function(result){ + expect(result).toBeTruthy(); results['actions_callCount'] = actions_push.callCount; results['actions_create'] = ca.argsForCall[0]; results['config_add'] = config_queue_add.argsForCall[0]; @@ -146,22 +159,28 @@ describe('start', function() { return Q(); } ).then( - function(){ return install('android', project, plugins['com.cordova.engine']); } - ).then( function(){ + return install('android', project, plugins['com.cordova.engine']); + } + ).then( + function(result){ + expect(result).toBeTruthy(); emit = spyOn(events, 'emit'); return install('android', project, plugins['org.test.plugins.childbrowser']); } ).then( - function(){ + function(result){ + expect(result).toBeTruthy(); return install('android', project, plugins['com.adobe.vars'], plugins_install_dir, { cli_variables:{API_KEY:'batman'} }); } ).then( - function(){ + function(result){ + expect(result).toBeTruthy(); return install('android', project, plugins['org.test.defaultvariables'], plugins_install_dir, { cli_variables:{API_KEY:'batman'} }); } ).then( - function(){ + function(result){ + expect(result).toBeTruthy(); done = true; results['emit_results'] = []; @@ -323,6 +342,7 @@ describe('install', function() { .fin(done); }); }); + it('should not check custom engine version that is not supported for platform', function() { var spy = spyOn(semver, 'satisfies').andReturn(true); runs(function() { http://git-wip-us.apache.org/repos/asf/cordova-lib/blob/2877e334/cordova-lib/spec-plugman/uninstall.spec.js ---------------------------------------------------------------------- diff --git a/cordova-lib/spec-plugman/uninstall.spec.js b/cordova-lib/spec-plugman/uninstall.spec.js index b6e2f5c..c1c3a71 100644 --- a/cordova-lib/spec-plugman/uninstall.spec.js +++ b/cordova-lib/spec-plugman/uninstall.spec.js @@ -22,6 +22,7 @@ var uninstall = require('../src/plugman/uninstall'), install = require('../src/plugman/install'), actions = require('cordova-common').ActionStack, + PluginInfo = require('cordova-common').PluginInfo, events = require('cordova-common').events, plugman = require('../src/plugman/plugman'), common = require('./common'), @@ -52,6 +53,8 @@ var uninstall = require('../src/plugman/uninstall'), promise, dummy_id = 'org.test.plugins.dummyplugin'; +var dummyPluginInfo = new PluginInfo(plugins['org.test.plugins.dummyplugin']); + var TEST_XML = '<?xml version="1.0" encoding="UTF-8"?>\n' + '<widget xmlns = "http://www.w3.org/ns/widgets"\n' + ' xmlns:cdv = "http://cordova.apache.org/ns/1.0"\n' + @@ -139,6 +142,41 @@ describe('uninstallPlatform', function() { }).fin(done); }); + it('should return propagate value returned by PlatformApi removePlugin method', function(done) { + var platformApi = { removePlugin: jasmine.createSpy('removePlugin') }; + spyOn(platforms, 'getPlatformApi').andReturn(platformApi); + + var existsSyncOrig = fs.existsSync; + spyOn(fs, 'existsSync').andCallFake(function (file) { + if (file.indexOf(dummy_id) >= 0) return true; + return existsSyncOrig.call(fs, file); + }); + + var fakeProvider = jasmine.createSpyObj('fakeProvider', ['get']); + fakeProvider.get.andReturn(dummyPluginInfo); + + function validateReturnedResultFor(values, expectedResult) { + return values.reduce(function (promise, value) { + return promise.then(function () { + platformApi.removePlugin.andReturn(Q(value)); + return uninstall.uninstallPlatform('android', project, dummy_id, null, + { pluginInfoProvider: fakeProvider, platformVersion: '9.9.9' }); + }) + .then(function(result) { + expect(!!result).toEqual(expectedResult); + }, function(err) { + expect(err).toBeUndefined(); + }); + }, Q()); + } + + validateReturnedResultFor([ true, {}, [], 'foo', function(){} ], true) + .then(function () { + return validateReturnedResultFor([ false, null, undefined, '' ], false); + }) + .fin(done); + }); + describe('with dependencies', function() { var emit; beforeEach(function() { http://git-wip-us.apache.org/repos/asf/cordova-lib/blob/2877e334/cordova-lib/src/cordova/plugin.js ---------------------------------------------------------------------- diff --git a/cordova-lib/src/cordova/plugin.js b/cordova-lib/src/cordova/plugin.js index 89ecee4..fd75458 100644 --- a/cordova-lib/src/cordova/plugin.js +++ b/cordova-lib/src/cordova/plugin.js @@ -97,6 +97,9 @@ module.exports = function plugin(command, targets, opts) { } } + // Assume we don't need to run prepare by default + var shouldRunPrepare = false; + switch(command) { case 'add': if (!targets || !targets.length) { @@ -183,7 +186,12 @@ module.exports = function plugin(command, targets, opts) { }; events.emit('verbose', 'Calling plugman.install on plugin "' + pluginInfo.dir + '" for platform "' + platform); - return plugman.raw.install(platform, platformRoot, path.basename(pluginInfo.dir), pluginPath, options); + return plugman.raw.install(platform, platformRoot, path.basename(pluginInfo.dir), pluginPath, options) + .then(function (didPrepare) { + // If platform does not returned anything we'll need + // to trigger a prepare after all plugins installed + if (!didPrepare) shouldRunPrepare = true; + }); }) .thenResolve(pluginInfo); }) @@ -217,8 +225,13 @@ module.exports = function plugin(command, targets, opts) { events.emit('results', 'Saved plugin info for "' + pluginInfo.id + '" to config.xml'); } }); - }, Q()); // end Q.all + }, Q()); }).then(function() { + // CB-11022 We do not need to run prepare after plugin install until shouldRunPrepare flag is set to true + if (!shouldRunPrepare) { + return Q(); + } + // Need to require right here instead of doing this at the beginning of file // otherwise tests are failing without any real reason. return require('./prepare').preparePlatforms(platformList, projectRoot, opts); @@ -250,7 +263,12 @@ module.exports = function plugin(command, targets, opts) { return soFar.then(function() { var platformRoot = path.join(projectRoot, 'platforms', platform); events.emit('verbose', 'Calling plugman.uninstall on plugin "' + target + '" for platform "' + platform + '"'); - return plugman.raw.uninstall.uninstallPlatform(platform, platformRoot, target, pluginPath); + return plugman.raw.uninstall.uninstallPlatform(platform, platformRoot, target, pluginPath) + .then(function (didPrepare) { + // If platform does not returned anything we'll need + // to trigger a prepare after all plugins installed + if (!didPrepare) shouldRunPrepare = true; + }); }); }, Q()) .then(function() { @@ -275,6 +293,11 @@ module.exports = function plugin(command, targets, opts) { }); }, Q()); }).then(function () { + // CB-11022 We do not need to run prepare after plugin install until shouldRunPrepare flag is set to true + if (!shouldRunPrepare) { + return Q(); + } + return require('./prepare').preparePlatforms(platformList, projectRoot, opts); }).then(function() { opts.cordova = { plugins: cordova_util.findPlugins(pluginPath) }; http://git-wip-us.apache.org/repos/asf/cordova-lib/blob/2877e334/cordova-lib/src/plugman/install.js ---------------------------------------------------------------------- diff --git a/cordova-lib/src/plugman/install.js b/cordova-lib/src/plugman/install.js index efbbda1..07a8e8f 100644 --- a/cordova-lib/src/plugman/install.js +++ b/cordova-lib/src/plugman/install.js @@ -313,7 +313,10 @@ function runInstall(actions, platform, project_dir, plugin_dir, plugins_dir, opt } else { events.emit('log', 'Dependent plugin "' + pluginInfo.id + '" already installed on ' + platform + '.'); } - return Q(); + + // CB-11022 return true always in this case since if the plugin is installed + // we don't need to call prepare in any way + return Q(true); } events.emit('log', 'Installing "' + pluginInfo.id + '" for ' + platform); @@ -402,8 +405,10 @@ function runInstall(actions, platform, project_dir, plugin_dir, plugins_dir, opt return hooksRunner.fire('before_plugin_install', hookOptions).then(function() { return handleInstall(actions, pluginInfo, platform, project_dir, plugins_dir, install_plugin_dir, filtered_variables, options); - }).then(function(){ - return hooksRunner.fire('after_plugin_install', hookOptions); + }).then(function(installResult){ + return hooksRunner.fire('after_plugin_install', hookOptions) + // CB-11022 Propagate install result to caller to be able to avoid unnecessary prepare + .thenResolve(installResult); }); } else { return handleInstall(actions, pluginInfo, platform, project_dir, plugins_dir, install_plugin_dir, filtered_variables, options); @@ -604,7 +609,8 @@ function handleInstall(actions, pluginInfo, platform, project_dir, plugins_dir, return platform_modules.getPlatformApi(platform, project_dir) .addPlugin(pluginInfo, options) - .then (function() { + .then (function(result) { + events.emit('verbose', 'Install complete for ' + pluginInfo.id + ' on ' + platform + '.'); // Add plugin to installed list. This already done in platform, // but need to be duplicated here to manage dependencies properly. @@ -638,6 +644,9 @@ function handleInstall(actions, pluginInfo, platform, project_dir, plugins_dir, info_strings.forEach( function(info) { events.emit('results', interp_vars(filtered_variables, info)); }); + + // Propagate value, returned by platform's addPlugin method to caller + return Q(result); }); } http://git-wip-us.apache.org/repos/asf/cordova-lib/blob/2877e334/cordova-lib/src/plugman/uninstall.js ---------------------------------------------------------------------- diff --git a/cordova-lib/src/plugman/uninstall.js b/cordova-lib/src/plugman/uninstall.js index ae88889..1bbc1bf 100644 --- a/cordova-lib/src/plugman/uninstall.js +++ b/cordova-lib/src/plugman/uninstall.js @@ -234,7 +234,7 @@ function runUninstallPlatform(actions, platform, project_dir, plugin_dir, plugin var pluginInfoProvider = options.pluginInfoProvider; // If this plugin is not really installed, return (CB-7004). if (!fs.existsSync(plugin_dir)) { - return Q(); + return Q(true); } var pluginInfo = pluginInfoProvider.get(plugin_dir); @@ -332,7 +332,7 @@ function handleUninstall(actions, platform, pluginInfo, project_dir, www_dir, pl options.usePlatformWww = true; return platform_modules.getPlatformApi(platform, project_dir) .removePlugin(pluginInfo, options) - .then(function() { + .then(function(result) { // Remove plugin from installed list. This already done in platform, // but need to be duplicated here to remove plugin entry from project's // plugin list to manage dependencies properly. @@ -359,5 +359,8 @@ function handleUninstall(actions, platform, pluginInfo, project_dir, www_dir, pl buildModule.prepBuildFiles(); } } + + // CB-11022 propagate `removePlugin` result to the caller + return Q(result); }); } --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
