diff --git a/integration-tests/pkgJson.spec.js b/integration-tests/pkgJson.spec.js index 6110b1de2..5468da059 100644 --- a/integration-tests/pkgJson.spec.js +++ b/integration-tests/pkgJson.spec.js @@ -105,13 +105,6 @@ describe('pkgJson', function () { }; } - function stringContaining (substring) { - return { - asymmetricMatch: s => typeof s === 'string' && s.includes(substring), - jasmineToString: _ => `` - }; - } - const customMatchers = { toSatisfy: () => ({ compare (version, spec) { const pass = semver.satisfies(version, spec); @@ -155,10 +148,8 @@ describe('pkgJson', function () { // Check that the plugin and spec add was successful to pkg.json. expect(getPkgJson('cordova.plugins')[pluginId]).toBeDefined(); expect(getPkgJson('dependencies')[pluginId]).tohaveMinSatisfyingVersion('1.1.2'); - // Check that the plugin and spec add was successful to config.xml. - expect(getCfg().getPlugins()).toEqual([ - { name: pluginId, spec: specWithMinSatisfyingVersion('1.1.2'), variables: {} } - ]); + + expect(getCfg().getPluginIdList()).toEqual([]); }).then(function () { // And now remove it with --save. return cordova.plugin('rm', pluginId, { save: true }); @@ -267,12 +258,6 @@ describe('pkgJson', function () { return cordova.platform('add', PLATFORM, { save: true }) .then(function () { expect(getPkgJson('cordova.platforms')).toEqual([PLATFORM]); - - // Config.xml and ios/cordova/version check. - expect(getCfg().getEngines()).toEqual([{ - name: PLATFORM, - spec: specSatisfiedBy(platformVersion(PLATFORM)) - }]); }).then(function () { return cordova.plugin('add', PLUGIN, { save: true }); }).then(function () { @@ -281,7 +266,6 @@ describe('pkgJson', function () { // Check that installed version satisfies the dependency spec const version = pluginVersion(PLUGIN); - expect(version).toSatisfy(getCfg().getPlugin(PLUGIN).spec); expect(version).toSatisfy(getPkgJson(`dependencies.${PLUGIN}`)); }); }); @@ -299,11 +283,6 @@ describe('pkgJson', function () { // Pkg.json has platform expect(getPkgJson('cordova.platforms')).toEqual([PLATFORM]); expect(getPkgJson(`dependencies.cordova-${PLATFORM}`)).toBeDefined(); - - // browser platform and spec have been added to config.xml. - expect(getCfg().getEngines()).toEqual([ - { name: PLATFORM, spec: stringContaining(platformPath) } - ]); }).then(function () { // Run cordova plugin add local path --save --fetch. return cordova.plugin('add', pluginPath, { save: true }); @@ -311,13 +290,6 @@ describe('pkgJson', function () { // Pkg.json has test plugin. expect(getPkgJson(`cordova.plugins.${PLUGIN}`)).toBeDefined(); expect(getPkgJson(`dependencies.${PLUGIN}`)).toBeDefined(); - - // Check that plugin and spec have been added to config.xml - expect(getCfg().getPlugins()).toEqual([{ - name: PLUGIN, - spec: stringContaining(pluginPath), - variables: {} - }]); }); }); }); @@ -409,11 +381,7 @@ describe('pkgJson', function () { 'cordova-browser': specWithMinSatisfyingVersion('5.0.1') }); - // Check that android and browser were added to config.xml with the correct spec. - expect(getCfg().getEngines()).toEqual([ - { name: 'android', spec: '~7.0.0' }, - { name: 'browser', spec: '~5.0.1' } - ]); + expect(getCfg().getEngines()).toEqual([]); }).then(function () { // And now remove it with --save. return cordova.platform('rm', ['android', 'browser'], { save: true }); @@ -456,9 +424,6 @@ describe('pkgJson', function () { expect(getPkgJson('cordova.platforms')).toEqual([ PLATFORM ]); // Config.xml and ios/cordova/version check. const version = platformVersion(PLATFORM); - expect(getCfg().getEngines()).toEqual([ - { name: PLATFORM, spec: specSatisfiedBy(version) } - ]); // Check that pkg.json and ios/cordova/version versions "satisfy" each other. const pkgSpec = getPkgJson(`dependencies.cordova-${PLATFORM}`); expect(version).toSatisfy(pkgSpec); @@ -491,11 +456,6 @@ describe('pkgJson', function () { }).then(function () { // pkg.json has new platform. expect(getPkgJson('cordova.platforms')).toEqual([PLATFORM]); - // Config.xml and ios/cordova/version check. - expect(getCfg().getEngines()).toEqual([{ - name: PLATFORM, - spec: specSatisfiedBy(platformVersion(PLATFORM)) - }]); }).then(function () { return cordova.plugin('add', PLUGIN, { save: true }); }).then(function () { @@ -535,11 +495,6 @@ describe('pkgJson', function () { return cordova.platform('add', `${PLATFORM}@4.5.4`, { save: true }).then(function () { // Pkg.json has ios. expect(getPkgJson('cordova.platforms')).toEqual([ PLATFORM ]); - // Config.xml and ios/cordova/version check. - expect(getCfg().getEngines()).toEqual([{ - name: PLATFORM, - spec: specSatisfiedBy(platformVersion(PLATFORM)) - }]); }).then(function () { return cordova.plugin('add', `${PLUGIN}@4.0.0`, { save: true }); }).then(function () { @@ -550,37 +505,4 @@ describe('pkgJson', function () { }); }); }); - - // No pkg.json included in test file. - describe('local path is added to config.xml without pkg.json', function () { - beforeEach(() => useProject('basePkgJson13')); - - // Test#026: has NO pkg.json. Checks if local path is added to config.xml and has no errors. - it('Test#026 : if you add a platform with local path, config.xml gets updated', function () { - const PLATFORM = 'browser'; - const platformPath = copyFixture(`platforms/cordova-${PLATFORM}`); - - return cordova.platform('add', platformPath, { save: true }) - .then(function () { - expect(getCfg().getEngines()).toContain({ - name: 'browser', spec: stringContaining(platformPath) - }); - }); - }); - - // Test#027: has NO pkg.json. Checks if local path is added to config.xml and has no errors. - it('Test#027 : if you add a plugin with local path, config.xml gets updated', function () { - const PLUGIN = 'cordova-lib-test-plugin'; - const pluginPath = copyFixture(path.join('plugins', PLUGIN)); - - return cordova.plugin('add', pluginPath, { save: true }) - .then(function () { - expect(getCfg().getPlugins()).toContain({ - name: PLUGIN, - spec: stringContaining(pluginPath), - variables: {} - }); - }); - }); - }); }); diff --git a/spec/cordova/platform/addHelper.spec.js b/spec/cordova/platform/addHelper.spec.js index e0a19939f..f69041813 100644 --- a/spec/cordova/platform/addHelper.spec.js +++ b/spec/cordova/platform/addHelper.spec.js @@ -217,14 +217,6 @@ describe('cordova/platform/addHelper', function () { }); }); - it('should write out the version of platform just added/updated to config.xml if the save option is provided', function () { - return platform_addHelper('add', hooks_mock, projectRoot, ['ios'], { save: true, restoring: true }).then(function (result) { - expect(cfg_parser_mock.prototype.removeEngine).toHaveBeenCalled(); - expect(cfg_parser_mock.prototype.addEngine).toHaveBeenCalled(); - expect(cfg_parser_mock.prototype.write).toHaveBeenCalled(); - }); - }); - describe('if the project contains a package.json', function () { it('should write out the platform just added/updated to the cordova.platforms property of package.json', function () { fs.readFileSync.and.returnValue('file'); diff --git a/spec/cordova/platform/remove.spec.js b/spec/cordova/platform/remove.spec.js index c70f9c882..9c1a7b309 100644 --- a/spec/cordova/platform/remove.spec.js +++ b/spec/cordova/platform/remove.spec.js @@ -36,7 +36,7 @@ describe('cordova/platform/remove', function () { beforeEach(function () { hooks_mock = jasmine.createSpyObj('hooksRunner mock', ['fire']); hooks_mock.fire.and.returnValue(Promise.resolve()); - cfg_parser_mock.prototype = jasmine.createSpyObj('config parser mock', ['write', 'removeEngine']); + cfg_parser_mock.prototype = jasmine.createSpyObj('config parser mock', ['write', 'getEngines', 'removeEngine']); platform_remove = rewire('../../../src/cordova/platform/remove'); platform_remove.__set__({ @@ -50,6 +50,7 @@ describe('cordova/platform/remove', function () { spyOn(cordova_util, 'removePlatformPluginsJson'); spyOn(events, 'emit'); spyOn(cordova_util, 'requireNoCache').and.returnValue({}); + cfg_parser_mock.prototype.getEngines.and.returnValue([{ name: 'atari', spec: '^0.0.1' }]); }); describe('error/warning conditions', function () { diff --git a/spec/cordova/plugin/add.spec.js b/spec/cordova/plugin/add.spec.js index b69ee2270..c135e9625 100644 --- a/spec/cordova/plugin/add.spec.js +++ b/spec/cordova/plugin/add.spec.js @@ -40,7 +40,7 @@ describe('cordova/plugin/add', function () { beforeEach(function () { hook_mock = jasmine.createSpyObj('hooks runner mock', ['fire']); hook_mock.fire.and.returnValue(Promise.resolve()); - Cfg_parser_mock.prototype = jasmine.createSpyObj('config parser prototype mock', ['getPlugin', 'removePlugin', 'addPlugin', 'write']); + Cfg_parser_mock.prototype = jasmine.createSpyObj('config parser prototype mock', ['getPlugin']); cfg_parser_revert_mock = add.__set__('ConfigParser', Cfg_parser_mock); plugin_info = jasmine.createSpyObj('pluginInfo', ['getPreferences']); plugin_info.getPreferences.and.returnValue({}); @@ -164,9 +164,6 @@ describe('cordova/plugin/add', function () { expect(add.determinePluginTarget).toHaveBeenCalledWith(jasmine.anything(), jasmine.anything(), jasmine.anything(), jasmine.objectContaining({ 'variables': cli_plugin_variables })); // check that the plugin variables from config.xml got added to cli_variables expect(plugman.install).toHaveBeenCalledWith(jasmine.anything(), jasmine.anything(), jasmine.anything(), jasmine.anything(), jasmine.objectContaining({ 'cli_variables': cli_plugin_variables })); - expect(Cfg_parser_mock.prototype.removePlugin).toHaveBeenCalledWith('cordova-plugin-device'); - expect(Cfg_parser_mock.prototype.addPlugin).toHaveBeenCalledWith(jasmine.any(Object), cli_plugin_variables); - expect(Cfg_parser_mock.prototype.write).toHaveBeenCalled(); }); }); // can't test the following due to inline require of preparePlatforms diff --git a/spec/cordova/plugin/remove.spec.js b/spec/cordova/plugin/remove.spec.js index bf26c10e3..a204763f4 100644 --- a/spec/cordova/plugin/remove.spec.js +++ b/spec/cordova/plugin/remove.spec.js @@ -52,7 +52,7 @@ describe('cordova/plugin/remove', function () { hook_mock = jasmine.createSpyObj('hooks runner mock', ['fire']); spyOn(prepare, 'preparePlatforms').and.returnValue(true); hook_mock.fire.and.returnValue(Promise.resolve()); - cfg_parser_mock.prototype = jasmine.createSpyObj('config parser mock', ['write', 'removeEngine', 'addEngine', 'getHookScripts', 'removePlugin']); + cfg_parser_mock.prototype = jasmine.createSpyObj('config parser mock', ['write', 'getPlugin', 'removePlugin']); 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) { @@ -159,6 +159,7 @@ describe('cordova/plugin/remove', function () { fs.existsSync.and.returnValue(true); remove.validatePluginId.and.returnValue('cordova-plugin-splashscreen'); var opts = { important: 'options', plugins: ['cordova-plugin-splashscreen'] }; + cfg_parser_mock.prototype.getPlugin.and.returnValue({}); return remove(projectRoot, 'cordova-plugin-splashscreen', hook_mock, opts).then(function () { expect(cfg_parser_mock.prototype.removePlugin).toHaveBeenCalled(); expect(cfg_parser_mock.prototype.write).toHaveBeenCalled(); diff --git a/spec/cordova/restore-util.spec.js b/spec/cordova/restore-util.spec.js index 2a046492e..17291e56e 100644 --- a/spec/cordova/restore-util.spec.js +++ b/spec/cordova/restore-util.spec.js @@ -149,8 +149,6 @@ describe('cordova/restore-util', () => { } function expectConsistentPlugins (plugins) { - expect(getCfg().getPlugins()).toEqual(jasmine.arrayWithExactContents(plugins)); - const unwrappedPlugins = plugins.map(p => p.sample || p); expectPluginsInPkgJson(unwrappedPlugins); @@ -160,31 +158,6 @@ describe('cordova/restore-util', () => { describe('installPlatformsFromConfigXML', () => { - it('Test#000 : should change specs in config.xml from using ~ to using ^', () => { - const PLATFORM = 'android'; - - getCfg() - .addEngine(PLATFORM, '~7.1.1') - .write(); - - setPkgJson('dependencies', { - [platformPkgName(PLATFORM)]: '^7.1.1' - }); - - return restore.installPlatformsFromConfigXML().then(() => { - // No changes to pkg.json spec for android. - const pkgJson = getPkgJson(); - expect(pkgJson.cordova.platforms).toEqual([PLATFORM]); - expect(pkgJson.dependencies[platformPkgName(PLATFORM)]).toMatch(/^\^/); - - // config.xml spec should change from '~' to '^'. - expect(getCfg().getEngines()).toEqual([{ - name: PLATFORM, - spec: jasmine.stringMatching(/^\^/) - }]); - }); - }); - it('Test#001 : should restore saved platform from package.json', () => { setPkgJson('cordova.platforms', [testPlatform]); @@ -205,7 +178,7 @@ describe('cordova/restore-util', () => { return restore.installPlatformsFromConfigXML().then(() => { // Check config.xml for spec modification. - expect(getCfg().getEngines()).toEqual([{ + expect(getCfg().getEngines()).not.toEqual([{ name: PLATFORM, spec: `git+${PLATFORM_URL}.git` }]); @@ -269,28 +242,6 @@ describe('cordova/restore-util', () => { }); }); - it('Test#007 : should update config.xml to include platforms from package.json', () => { - getCfg() - .addEngine('ios', '6.0.0') - .write(); - setPkgJson('dependencies', { - 'cordova-ios': '^4.5.4', 'cordova-browser': '^5.0.3' - }); - setPkgJson('cordova.platforms', ['ios', 'browser']); - - return restore.installPlatformsFromConfigXML().then(() => { - // Check to make sure that 'browser' spec was added properly. - expect(getCfg().getEngines()).toEqual([ - { name: 'ios', spec: '^4.5.4' }, - { name: 'browser', spec: '^5.0.3' } - ]); - // No change to pkg.json dependencies. - expect(getPkgJson('dependencies')).toEqual({ - 'cordova-ios': '^4.5.4', 'cordova-browser': '^5.0.3' - }); - }); - }); - it('Test#016 : should restore platforms & plugins and create a missing package.json', () => { getCfg() .addEngine(testPlatform) @@ -392,7 +343,7 @@ describe('cordova/restore-util', () => { }); return restore.installPluginsFromConfigXML({ save: true }).then(() => { - expectConsistentPlugins([ + expectPluginsInPkgJson([ jasmine.objectContaining({ name: 'cordova-plugin-camera', variables: { variable_1: ' ', variable_2: ' ', variable_3: 'value_3' } @@ -405,7 +356,7 @@ describe('cordova/restore-util', () => { name: 'cordova-plugin-device', variables: { variable_1: 'value_1' } }) - ]); + ].map(p => p.sample || p)); }); }); @@ -436,7 +387,7 @@ describe('cordova/restore-util', () => { }); return restore.installPluginsFromConfigXML({ save: true }).then(() => { - expectConsistentPlugins([{ + expectPluginsInPkgJson([{ name: 'cordova-plugin-camera', spec: '^2.3.0', variables: { variable_1: 'value_1', variable_2: 'value_2', variable_3: 'value_3' } @@ -448,28 +399,7 @@ describe('cordova/restore-util', () => { name: 'cordova-plugin-splashscreen', spec: undefined, variables: {} - }]); - }); - }); - - /** - * When config has no plugins and is restored, the plugins will be restored with the - * pkg.json plugins and with the spec from pkg.json dependencies. - */ - it('Test#015 : update config.xml to include all plugins/variables from pkg.json', () => { - setPkgJson('dependencies', { - 'cordova-plugin-camera': '^2.3.0' - }); - setPkgJson('cordova.plugins', { - 'cordova-plugin-camera': { variable_1: 'value_1' } - }); - - return restore.installPluginsFromConfigXML({ save: true }).then(() => { - expectConsistentPlugins([{ - name: 'cordova-plugin-camera', - spec: '^2.3.0', - variables: { variable_1: 'value_1' } - }]); + }].map(p => p.sample || p)); }); }); diff --git a/src/cordova/platform/addHelper.js b/src/cordova/platform/addHelper.js index 59435b108..2094e3fd5 100644 --- a/src/cordova/platform/addHelper.js +++ b/src/cordova/platform/addHelper.js @@ -220,13 +220,6 @@ function addHelper (cmd, hooksRunner, projectRoot, targets, opts) { // was installed. However, we save it with the "~" attribute (this allows for patch updates). spec = saveVersion ? '~' + platDetails.version : spec; - // Save target into config.xml, overriding already existing settings. - events.emit('log', '--save flag or autosave detected'); - events.emit('log', 'Saving ' + platform + '@' + spec + ' into config.xml file ...'); - cfg.removeEngine(platform); - cfg.addEngine(platform, spec); - cfg.write(); - // Save to add to pacakge.json's cordova.platforms array in the next then. platformsToSave.push(platform); } diff --git a/src/cordova/platform/remove.js b/src/cordova/platform/remove.js index 2fc0d7b11..37c946444 100644 --- a/src/cordova/platform/remove.js +++ b/src/cordova/platform/remove.js @@ -54,9 +54,11 @@ function remove (hooksRunner, projectRoot, targets, opts) { var platformName = target.split('@')[0]; var xml = cordova_util.projectConfig(projectRoot); var cfg = new ConfigParser(xml); - events.emit('log', 'Removing platform ' + target + ' from config.xml file...'); - cfg.removeEngine(platformName); - cfg.write(); + if (cfg.getEngines && cfg.getEngines().some(function (e) { return e.name === platformName; })) { + events.emit('log', 'Removing platform ' + target + ' from config.xml file...'); + cfg.removeEngine(platformName); + cfg.write(); + } // If package.json exists and contains a specified platform in cordova.platforms, it will be removed. if (pkgJson !== undefined && pkgJson.cordova !== undefined && pkgJson.cordova.platforms !== undefined) { var index = pkgJson.cordova.platforms.indexOf(platformName); diff --git a/src/cordova/plugin/add.js b/src/cordova/plugin/add.js index 6ab89ab46..2ef25da0c 100644 --- a/src/cordova/plugin/add.js +++ b/src/cordova/plugin/add.js @@ -183,13 +183,6 @@ function add (projectRoot, hooksRunner, opts) { } } } - xml = cordova_util.projectConfig(projectRoot); - cfg = new ConfigParser(xml); - cfg.removePlugin(pluginInfo.id); - cfg.addPlugin(attributes, opts.cli_variables); - cfg.write(); - - events.emit('results', 'Saved plugin info for "' + pluginInfo.id + '" to config.xml'); } }); }, Promise.resolve()); diff --git a/src/cordova/plugin/remove.js b/src/cordova/plugin/remove.js index c6b425145..e799b3861 100644 --- a/src/cordova/plugin/remove.js +++ b/src/cordova/plugin/remove.js @@ -115,12 +115,15 @@ function remove (projectRoot, targets, hooksRunner, opts) { } function persistRemovalToCfg (target) { - events.emit('log', 'Removing plugin ' + target + ' from config.xml file...'); var configPath = cordova_util.projectConfig(projectRoot); if (fs.existsSync(configPath)) { // should not happen with real life but needed for tests var configXml = new ConfigParser(configPath); - configXml.removePlugin(target); - configXml.write(); + + if (configXml.getPlugin(target)) { + events.emit('log', 'Removing plugin ' + target + ' from config.xml file...'); + configXml.removePlugin(target); + configXml.write(); + } } } diff --git a/src/cordova/restore-util.js b/src/cordova/restore-util.js index e6bfaf64e..0cf21b169 100644 --- a/src/cordova/restore-util.js +++ b/src/cordova/restore-util.js @@ -44,7 +44,6 @@ function installPlatformsFromConfigXML (platforms, opts) { var comboArray = []; var configPlatforms = []; var modifiedPkgJson = false; - var modifiedConfigXML = false; var mergedPlatformSpecs = {}; var key; var installAllPlatforms = !platforms || platforms.length === 0; @@ -159,18 +158,6 @@ function installPlatformsFromConfigXML (platforms, opts) { } mergedPlatformSpecs[item] = pkgJson.dependencies[item]; } - // First remove the engine and then add missing engine and elements to config.xml. - // Remove to avoid duplicate engines. - if (mergedPlatformSpecs[item]) { - cfg.removeEngine(item); - cfg.addEngine(item, mergedPlatformSpecs[item]); - modifiedConfigXML = true; - // If there is no spec, just remove and add the engine. - } else if (configPlatforms.indexOf(item) < 0) { - cfg.removeEngine(item); - cfg.addEngine(item); - modifiedConfigXML = true; - } }); } @@ -191,9 +178,6 @@ function installPlatformsFromConfigXML (platforms, opts) { } fs.writeFileSync(pkgJsonPath, JSON.stringify(pkgJson, null, indent), 'utf8'); } - if (modifiedConfigXML === true) { - cfg.write(); - } if (!comboArray || !comboArray.length) { return Promise.resolve('No platforms found in config.xml and/or package.json that haven\'t been added to the project'); } @@ -238,7 +222,6 @@ function installPluginsFromConfigXML (args) { var pkgJsonPath = path.join(projectRoot, 'package.json'); var pkgJson; var modifiedPkgJson = false; - var modifiedConfigXML = false; var comboObject; var mergedPluginSpecs = {}; var comboPluginIdArray; @@ -346,35 +329,6 @@ function installPluginsFromConfigXML (args) { fs.writeFileSync(pkgJsonPath, JSON.stringify(pkgJson, null, indent), 'utf8'); } } - // Write to config.xml (only if it is different from package.json in content) - comboPluginIdArray.forEach(function (plugID) { - var configXMLPlugin = cfg.getPlugin(plugID); - if (pluginIdConfig.indexOf(plugID) < 0) { - pluginIdConfig.push(plugID); - if (mergedPluginSpecs[plugID]) { - cfg.removePlugin(plugID); - cfg.addPlugin({ name: plugID, spec: mergedPluginSpecs[plugID] }, comboObject[plugID]); - modifiedConfigXML = true; - } else { - cfg.removePlugin(plugID); - cfg.addPlugin({ name: plugID }, comboObject[plugID]); - modifiedConfigXML = true; - } - - // Write only if the plugin variables or specs are different from pkgJson - } else if (((pluginIdConfig.indexOf(plugID) >= 0) && (mergedPluginSpecs[plugID]) && - (configXMLPlugin.variables !== comboObject[plugID])) || - ((mergedPluginSpecs[plugID] !== configXMLPlugin.spec) || - (configXMLPlugin.variables !== comboObject[plugID]))) { - cfg.removePlugin(plugID); - cfg.addPlugin({ name: plugID, spec: mergedPluginSpecs[plugID] }, comboObject[plugID]); - modifiedConfigXML = true; - } - }); - - if (modifiedConfigXML === true && args.save !== false) { - cfg.write(); - } // Intermediate variable to store current installing plugin name // to be able to create informative warning on plugin failure