CB-13145 : added unit tests for mergeVariables from util.js and variable-merge.js and updated after review
Project: http://git-wip-us.apache.org/repos/asf/cordova-lib/repo Commit: http://git-wip-us.apache.org/repos/asf/cordova-lib/commit/e5b8a3c3 Tree: http://git-wip-us.apache.org/repos/asf/cordova-lib/tree/e5b8a3c3 Diff: http://git-wip-us.apache.org/repos/asf/cordova-lib/diff/e5b8a3c3 Branch: refs/heads/master Commit: e5b8a3c3cf73656552cc25bc0f3a315ac6230521 Parents: 263502d Author: Audrey So <[email protected]> Authored: Thu Aug 24 14:54:22 2017 -0700 Committer: Steve Gill <[email protected]> Committed: Tue Aug 29 22:33:11 2017 -0700 ---------------------------------------------------------------------- integration-tests/plugin.spec.js | 2 + spec/cordova/plugin/remove.spec.js | 34 +++++++++++++---- spec/cordova/plugin/util.spec.js | 53 ++++++++++++++++++++++++++- spec/plugman/variable-merge.spec.js | 63 ++++++++++++++++++++++++++++++++ src/cordova/plugin/util.js | 3 +- src/plugman/init-defaults.js | 1 - 6 files changed, 145 insertions(+), 11 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/cordova-lib/blob/e5b8a3c3/integration-tests/plugin.spec.js ---------------------------------------------------------------------- diff --git a/integration-tests/plugin.spec.js b/integration-tests/plugin.spec.js index a19f8c7..616e9e2 100644 --- a/integration-tests/plugin.spec.js +++ b/integration-tests/plugin.spec.js @@ -169,6 +169,8 @@ describe('plugin end-to-end', function () { }, 30000); it('Test 005 : should respect preference default values', function (done) { + var plugin_util = require('../src/cordova/plugin/util'); + spyOn(plugin_util, 'mergeVariables').and.returnValue({ REQUIRED: 'NO', REQUIRED_ANDROID: 'NO' }); addPlugin(path.join(pluginsDir, org_test_defaultvariables), org_test_defaultvariables, {cli_variables: { REQUIRED: 'NO', REQUIRED_ANDROID: 'NO' }}, done) .then(function () { var platformJsonPath = path.join(project, 'plugins', helpers.testPlatform + '.json'); http://git-wip-us.apache.org/repos/asf/cordova-lib/blob/e5b8a3c3/spec/cordova/plugin/remove.spec.js ---------------------------------------------------------------------- diff --git a/spec/cordova/plugin/remove.spec.js b/spec/cordova/plugin/remove.spec.js index b8e6720..6721b30 100644 --- a/spec/cordova/plugin/remove.spec.js +++ b/spec/cordova/plugin/remove.spec.js @@ -20,9 +20,8 @@ /* eslint-env jasmine */ /* globals fail */ -var remove = require('../../../src/cordova/plugin/remove'); var rewire = require('rewire'); -var platform_remove = rewire('../../../src/cordova/plugin/remove'); +var remove = rewire('../../../src/cordova/plugin/remove'); var Q = require('q'); var cordova_util = require('../../../src/cordova/util'); var metadata = require('../../../src/plugman/util/metadata'); @@ -37,8 +36,11 @@ describe('cordova/plugin/remove', function () { var projectRoot = '/some/path'; var hook_mock; var cfg_parser_mock = function () {}; - var cfg_parser_revert_mock; // eslint-disable-line no-unused-vars + var cfg_parser_revert_mock; var package_json_mock; + var plugin_info_provider_mock = function () {}; + var plugin_info_provider_revert_mock; + var plugin_info; package_json_mock = jasmine.createSpyObj('package json mock', ['cordova', 'dependencies']); package_json_mock.dependencies = {}; package_json_mock.cordova = {}; @@ -55,7 +57,18 @@ describe('cordova/plugin/remove', function () { spyOn(prepare, 'preparePlatforms').and.returnValue(true); hook_mock.fire.and.returnValue(Q()); cfg_parser_mock.prototype = jasmine.createSpyObj('config parser mock', ['write', 'removeEngine', 'addEngine', 'getHookScripts', 'removePlugin']); - cfg_parser_revert_mock = platform_remove.__set__('ConfigParser', cfg_parser_mock); + cfg_parser_revert_mock = remove.__set__('ConfigParser', cfg_parser_mock); + plugin_info_provider_mock.prototype = jasmine.createSpyObj('plugin info provider mock', ['get', 'getPreferences']); + plugin_info_provider_mock.prototype.get = function (directory) { + // id version dir getPreferences() engines engines.cordovaDependencies name versions + return plugin_info; + }; + plugin_info_provider_revert_mock = remove.__set__('PluginInfoProvider', plugin_info_provider_mock); + }); + + afterEach(function () { + cfg_parser_revert_mock(); + plugin_info_provider_revert_mock(); }); describe('error/warning conditions', function () { @@ -88,6 +101,7 @@ describe('cordova/plugin/remove', function () { }); it('should call plugman.uninstall.uninstallPlatform for each platform installed in the project and for each provided plugin', function (done) { + spyOn(plugin_util, 'mergeVariables'); remove.validatePluginId.and.returnValue('cordova-plugin-splashscreen'); var opts = {important: 'options', plugins: ['cordova-plugin-splashscreen']}; remove(projectRoot, 'cordova-plugin-splashscreen', hook_mock, opts).then(function () { @@ -101,6 +115,7 @@ describe('cordova/plugin/remove', function () { }); it('should trigger a prepare if plugman.uninstall.uninstallPlatform returned something falsy', function (done) { + spyOn(plugin_util, 'mergeVariables'); remove.validatePluginId.and.returnValue('cordova-plugin-splashscreen'); plugman.uninstall.uninstallPlatform.and.returnValue(Q(false)); var opts = {important: 'options', plugins: ['cordova-plugin-splashscreen']}; @@ -113,6 +128,7 @@ describe('cordova/plugin/remove', function () { }); it('should call plugman.uninstall.uninstallPlugin once plugin has been uninstalled for each platform', function (done) { + spyOn(plugin_util, 'mergeVariables'); remove.validatePluginId.and.returnValue('cordova-plugin-splashscreen'); var opts = {important: 'options', plugins: ['cordova-plugin-splashscreen']}; remove(projectRoot, 'cordova-plugin-splashscreen', hook_mock, opts).then(function () { @@ -125,7 +141,7 @@ describe('cordova/plugin/remove', function () { describe('when save option is provided or autosave config is on', function () { beforeEach(function () { - spyOn(platform_remove, 'validatePluginId').and.returnValue('cordova-plugin-splashscreen'); + spyOn(plugin_util, 'mergeVariables'); spyOn(plugin_util, 'saveToConfigXmlOn').and.returnValue(true); spyOn(config, 'read').and.returnValue(true); spyOn(cordova_util, 'projectConfig').and.returnValue('config.xml'); @@ -136,8 +152,9 @@ describe('cordova/plugin/remove', function () { it('should remove provided plugins from config.xml', function (done) { spyOn(cordova_util, 'requireNoCache').and.returnValue(true); fs.existsSync.and.returnValue(true); + remove.validatePluginId.and.returnValue('cordova-plugin-splashscreen'); var opts = {important: 'options', plugins: ['cordova-plugin-splashscreen']}; - platform_remove(projectRoot, 'cordova-plugin-splashscreen', hook_mock, opts).then(function () { + remove(projectRoot, 'cordova-plugin-splashscreen', hook_mock, opts).then(function () { expect(cfg_parser_mock.prototype.removePlugin).toHaveBeenCalled(); expect(cfg_parser_mock.prototype.write).toHaveBeenCalled(); expect(events.emit).toHaveBeenCalledWith('log', jasmine.stringMatching('Removing plugin cordova-plugin-splashscreen from config.xml file')); @@ -149,9 +166,10 @@ describe('cordova/plugin/remove', function () { it('should remove provided plugins from package.json (if exists)', function (done) { spyOn(cordova_util, 'requireNoCache').and.returnValue(package_json_mock); + remove.validatePluginId.and.returnValue('cordova-plugin-splashscreen'); fs.existsSync.and.returnValue(true); var opts = {important: 'options', plugins: ['cordova-plugin-splashscreen']}; - platform_remove(projectRoot, 'cordova-plugin-splashscreen', hook_mock, opts).then(function () { + remove(projectRoot, 'cordova-plugin-splashscreen', hook_mock, opts).then(function () { expect(fs.writeFileSync).toHaveBeenCalled(); expect(events.emit).toHaveBeenCalledWith('log', jasmine.stringMatching('Removing cordova-plugin-splashscreen from package.json')); }).fail(function (e) { @@ -162,6 +180,8 @@ describe('cordova/plugin/remove', function () { }); it('should remove fetch metadata from fetch.json', function (done) { + plugin_info_provider_mock.prototype.getPreferences.and.returnValue(true); + spyOn(plugin_util, 'mergeVariables'); spyOn(metadata, 'remove_fetch_metadata').and.callThrough(); remove.validatePluginId.and.returnValue('cordova-plugin-splashscreen'); var opts = {important: 'options', plugins: ['cordova-plugin-splashscreen']}; http://git-wip-us.apache.org/repos/asf/cordova-lib/blob/e5b8a3c3/spec/cordova/plugin/util.spec.js ---------------------------------------------------------------------- diff --git a/spec/cordova/plugin/util.spec.js b/spec/cordova/plugin/util.spec.js index 7a7c032..47d0667 100644 --- a/spec/cordova/plugin/util.spec.js +++ b/spec/cordova/plugin/util.spec.js @@ -20,16 +20,25 @@ var rewire = require('rewire'); var plugin_util = rewire('../../../src/cordova/plugin/util'); +var shell = require('shelljs'); +var events = require('cordova-common').events; describe('cordova/plugin/util', function () { var plugin_info_mock = function () {}; var plugin_info_revert_mock; + var cfg_parser_revert_mock; + var cfg_parser_mock = function () {}; beforeEach(function () { - plugin_info_mock.prototype = jasmine.createSpyObj('plugin info provider prototype mock', ['getAllWithinSearchPath']); + spyOn(shell, 'rm'); + spyOn(events, 'emit'); + cfg_parser_mock.prototype = jasmine.createSpyObj('config parser protytpe mock', ['getPlugin']); + cfg_parser_revert_mock = plugin_util.__set__('ConfigParser', cfg_parser_mock); + plugin_info_mock.prototype = jasmine.createSpyObj('plugin info provider prototype mock', ['getAllWithinSearchPath', 'getPreferences']); plugin_info_revert_mock = plugin_util.__set__('PluginInfoProvider', plugin_info_mock); }); afterEach(function () { plugin_info_revert_mock(); + cfg_parser_revert_mock(); }); describe('getInstalledPlugins helper method', function () { it('should return result of PluginInfoProvider\'s getAllWithinSearchPath method', function () { @@ -50,4 +59,46 @@ describe('cordova/plugin/util', function () { })).toBe(true); }); }); + describe('mergeVariables happy path', function () { + it('should return variable from cli', function () { + cfg_parser_mock.prototype.getPlugin.and.returnValue(undefined); + plugin_info_mock.prototype.getPreferences.and.returnValue({}); + var opts = { cli_variables: { FCM_VERSION: '9.0.0' } }; + expect(plugin_util.mergeVariables(plugin_info_mock.prototype, cfg_parser_mock.prototype, opts)).toEqual({FCM_VERSION: '9.0.0'}); + }); + it('should return empty object if there are no config and no cli variables', function () { + cfg_parser_mock.prototype.getPlugin.and.returnValue(undefined); + plugin_info_mock.prototype.getPreferences.and.returnValue({}); + var opts = { cli_variables: {} }; + expect(plugin_util.mergeVariables(plugin_info_mock.prototype, cfg_parser_mock.prototype, opts)).toEqual({}); + }); + it('cli variable takes precedence over config.xml', function () { + cfg_parser_mock.prototype.getPlugin.and.returnValue(undefined); + plugin_info_mock.prototype.getPreferences.and.returnValue({ + name: 'phonegap-plugin-push', + spec: '/Users/auso/cordova/phonegap-plugin-push', + variables: { FCM_VERSION: '11.0.1' } + }); + var opts = { cli_variables: {FCM_VERSION: '9.0.0'} }; + expect(plugin_util.mergeVariables(plugin_info_mock.prototype, cfg_parser_mock.prototype, opts)).toEqual({FCM_VERSION: '9.0.0'}); + }); + it('use config.xml variable if no cli variable is passed in', function () { + cfg_parser_mock.prototype.getPlugin.and.returnValue({ + name: 'phonegap-plugin-push', + spec: '/Users/auso/cordova/phonegap-plugin-push', + variables: { FCM_VERSION: '11.0.1' } + }); + plugin_info_mock.prototype.getPreferences.and.returnValue({}); + var opts = { cli_variables: {} }; + expect(plugin_util.mergeVariables(plugin_info_mock.prototype, cfg_parser_mock.prototype, opts)).toEqual({FCM_VERSION: '11.0.1'}); + }); + it('should get missed variables', function () { + cfg_parser_mock.prototype.getPlugin.and.returnValue(undefined); + plugin_info_mock.prototype.getPreferences.and.returnValue({key: 'FCM_VERSION', value: undefined}); + var opts = { cli_variables: {} }; + expect(function () { plugin_util.mergeVariables(plugin_info_mock.prototype, cfg_parser_mock.prototype, opts); }).toThrow(); + expect(shell.rm).toHaveBeenCalledWith('-rf', undefined); + expect(events.emit).toHaveBeenCalledWith('verbose', 'Removing undefined because mandatory plugin variables were missing.'); + }); + }); }); http://git-wip-us.apache.org/repos/asf/cordova-lib/blob/e5b8a3c3/spec/plugman/variable-merge.spec.js ---------------------------------------------------------------------- diff --git a/spec/plugman/variable-merge.spec.js b/spec/plugman/variable-merge.spec.js new file mode 100644 index 0000000..d333ba1 --- /dev/null +++ b/spec/plugman/variable-merge.spec.js @@ -0,0 +1,63 @@ +/** + 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 rewire = require('rewire'); +var variable_merge = rewire('../../src/plugman/variable-merge'); +var underscore = require('underscore'); + +describe('mergeVariables', function () { + var plugin_info_provider_mock = function () {}; + var plugin_info_provider_revert_mock; + var plugin_info; + + beforeEach(function () { + plugin_info = jasmine.createSpyObj('pluginInfo', ['getPreferences']); + plugin_info.dir = 'some\\plugin\\path'; + plugin_info.id = 'cordova-plugin-device'; + plugin_info.version = '1.0.0'; + plugin_info_provider_mock.prototype = jasmine.createSpyObj('plugin info provider mock', ['get']); + plugin_info_provider_mock.prototype.get = function (directory) { + return plugin_info; + }; + plugin_info_provider_revert_mock = variable_merge.__set__('PluginInfoProvider', plugin_info_provider_mock); + }); + afterEach(function () { + plugin_info_provider_revert_mock(); + }); + it('use plugin.xml if no cli/config variables', function () { + plugin_info.getPreferences.and.returnValue({FCM_VERSION: '11.0.1'}); + var opts = { cli_variables: { } }; + expect(variable_merge.mergeVariables('some/path', 'android', opts)).toEqual({FCM_VERSION: '11.0.1'}); + }); + it('cli & config variables take precedence over plugin.xml ', function () { + plugin_info.getPreferences.and.returnValue({FCM_VERSION: '11.0.1'}); + var opts = { cli_variables: {FCM_VERSION: '9.0.0'} }; + expect(variable_merge.mergeVariables('some/path', 'android', opts)).toEqual({FCM_VERSION: '9.0.0'}); + }); + it('should return no variables', function () { + plugin_info.getPreferences.and.returnValue({}); + var opts = { cli_variables: {} }; + expect(variable_merge.mergeVariables('some/path', 'android', opts)).toEqual({}); + }); + it('should throw error if variables are missing', function () { + plugin_info.getPreferences.and.returnValue({}); + spyOn(underscore, 'difference').and.returnValue(['missing variable']); + var opts = { cli_variables: {} }; + expect(function () { variable_merge.mergeVariables('some/path', 'android', opts); }).toThrow(); + }); +}); http://git-wip-us.apache.org/repos/asf/cordova-lib/blob/e5b8a3c3/src/cordova/plugin/util.js ---------------------------------------------------------------------- diff --git a/src/cordova/plugin/util.js b/src/cordova/plugin/util.js index aaf7068..6dfced9 100644 --- a/src/cordova/plugin/util.js +++ b/src/cordova/plugin/util.js @@ -21,7 +21,6 @@ var path = require('path'); var PluginInfoProvider = require('cordova-common').PluginInfoProvider; var shell = require('shelljs'); var events = require('cordova-common').events; -var Q = require('q'); var CordovaError = require('cordova-common').CordovaError; module.exports.saveToConfigXmlOn = saveToConfigXmlOn; @@ -72,7 +71,7 @@ function mergeVariables (pluginInfo, cfg, opts) { events.emit('verbose', 'Removing ' + pluginInfo.dir + ' because mandatory plugin variables were missing.'); shell.rm('-rf', pluginInfo.dir); var msg = 'Variable(s) missing (use: --variable ' + missingVariables.join('=value --variable ') + '=value).'; - return Q.reject(new CordovaError(msg)); + throw new CordovaError(msg); } return opts.cli_variables; } http://git-wip-us.apache.org/repos/asf/cordova-lib/blob/e5b8a3c3/src/plugman/init-defaults.js ---------------------------------------------------------------------- diff --git a/src/plugman/init-defaults.js b/src/plugman/init-defaults.js index 361968c..f3c5ada 100644 --- a/src/plugman/init-defaults.js +++ b/src/plugman/init-defaults.js @@ -150,7 +150,6 @@ if (!pkg.author) { : prompt('author'); } /* eslint-enable indent */ - var license = pkg.license || defaults.license || config.get('init.license') || --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
