Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Remove saving platforms/plugins to config.xml #750

Merged
merged 1 commit into from
Mar 15, 2019
Merged

Remove saving platforms/plugins to config.xml #750

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
84 changes: 3 additions & 81 deletions integration-tests/pkgJson.spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -105,13 +105,6 @@ describe('pkgJson', function () {
};
}

function stringContaining (substring) {
return {
asymmetricMatch: s => typeof s === 'string' && s.includes(substring),
jasmineToString: _ => `<stringContaining(${substring})>`
};
}

const customMatchers = {
toSatisfy: () => ({ compare (version, spec) {
const pass = semver.satisfies(version, spec);
Expand Down Expand Up @@ -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 });
Expand Down Expand Up @@ -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 () {
Expand All @@ -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}`));
});
});
Expand All @@ -299,25 +283,13 @@ 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 });
}).then(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: {}
}]);
});
});
});
Expand Down Expand Up @@ -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 });
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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 () {
Expand Down Expand Up @@ -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 () {
Expand All @@ -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'));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did you check if the fixture is still used now that this has been deleted? What is the new behavior without a package.json?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did not really look into the tests beyond trying to make them pass. 😓

My hope is to revisit these when we update the package.json handling.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well basePkgJson13 refers to a project fixture that might be unused now. I can't check it now though.


// 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: {}
});
});
});
});
});
8 changes: 0 additions & 8 deletions spec/cordova/platform/addHelper.spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand Down
3 changes: 2 additions & 1 deletion spec/cordova/platform/remove.spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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__({
Expand All @@ -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 () {
Expand Down
5 changes: 1 addition & 4 deletions spec/cordova/plugin/add.spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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({});
Expand Down Expand Up @@ -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
Expand Down
3 changes: 2 additions & 1 deletion spec/cordova/plugin/remove.spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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();
Expand Down
80 changes: 5 additions & 75 deletions spec/cordova/restore-util.spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand All @@ -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]);

Expand All @@ -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`
}]);
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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' }
Expand All @@ -405,7 +356,7 @@ describe('cordova/restore-util', () => {
name: 'cordova-plugin-device',
variables: { variable_1: 'value_1' }
})
]);
].map(p => p.sample || p));
});
});

Expand Down Expand Up @@ -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' }
Expand All @@ -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));
});
});

Expand Down
7 changes: 0 additions & 7 deletions src/cordova/platform/addHelper.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down
Loading