[GitHub] cordova-lib issue #599: CB-13463 : prevent package.json update with --nosave...

2017-10-25 Thread stevengill
Github user stevengill commented on the issue:

https://github.com/apache/cordova-lib/pull/599
  
merge!


---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org



[GitHub] cordova-lib issue #599: CB-13463 : prevent package.json update with --nosave...

2017-10-24 Thread stevengill
Github user stevengill commented on the issue:

https://github.com/apache/cordova-lib/pull/599
  
LGTM, but the ci failures 😨 


---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org



[GitHub] cordova-lib issue #601: CB-13485: Test with Node8

2017-10-23 Thread stevengill
Github user stevengill commented on the issue:

https://github.com/apache/cordova-lib/pull/601
  
yeah i ran into this last time i tried to do it. 

npm 5 doesn't handle `npm link` properly. Doesn't work with our current 
link all the cordova deps strategy 


---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org



[GitHub] cordova-lib issue #597: CB-13451 (all platforms) "pkg not defined" exception...

2017-10-20 Thread stevengill
Github user stevengill commented on the issue:

https://github.com/apache/cordova-lib/pull/597
  
merged! Thanks again @OminousWater!


---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org



[GitHub] cordova-lib issue #597: CB-13451 (all platforms) "pkg not defined" exception...

2017-10-20 Thread stevengill
Github user stevengill commented on the issue:

https://github.com/apache/cordova-lib/pull/597
  
No worries @OminousWater! That is what reviews are for :)

CI gets trigged with new commits or updated commits. Looks like your PR 
triggered it. I'll wait and see how they go


---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org



[GitHub] cordova-lib pull request #597: CB-13451 (all platforms) "pkg not defined" ex...

2017-10-20 Thread stevengill
Github user stevengill commented on a diff in the pull request:

https://github.com/apache/cordova-lib/pull/597#discussion_r146039059
  
--- Diff: src/plugman/init-defaults.js ---
@@ -55,25 +55,26 @@ function readDeps (test) {
 };
 }
 
-var name = pkg.name || defaults.id || basename;
+/* eslint-disable */
--- End diff --

Do you need this `/* eslint-disable */`


---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org



[GitHub] cordova-lib pull request #597: CB-13451 (all platforms) "pkg not defined" ex...

2017-10-20 Thread stevengill
Github user stevengill commented on a diff in the pull request:

https://github.com/apache/cordova-lib/pull/597#discussion_r146038992
  
--- Diff: .eslintignore ---
@@ -1,4 +1 @@
-spec/cordova/fixtures/**
-spec/plugman/projects/**
-spec/plugman/plugins/**
-spec/cordova/temp/**
+**/init-defaults.js
--- End diff --

Why did you remove the other items from `.eslintignore`?


---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org



[GitHub] cordova-lib pull request #597: CB-13451 (all platforms) "pkg not defined" ex...

2017-10-20 Thread stevengill
Github user stevengill commented on a diff in the pull request:

https://github.com/apache/cordova-lib/pull/597#discussion_r146039159
  
--- Diff: src/plugman/init-defaults.js ---
@@ -149,10 +149,11 @@ if (!pkg.author) {
 }
 : prompt('author');
 }
-/* eslint-enable indent */
-var license = pkg.license ||
+var license = package.license ||
   defaults.license ||
   config.get('init.license') ||
   config.get('init-license') ||
   'ISC';
+/* eslint-enable */
--- End diff --

is this needed? `/* eslint-enable */`


---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org



[GitHub] cordova-lib issue #597: CB-13451 (all platforms) "pkg not defined" exception...

2017-10-19 Thread stevengill
Github user stevengill commented on the issue:

https://github.com/apache/cordova-lib/pull/597
  
Thanks for all your work on this @OminousWater! 


---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org



[GitHub] cordova-lib issue #597: CB-13451 (all platforms) "pkg not defined" exception...

2017-10-18 Thread stevengill
Github user stevengill commented on the issue:

https://github.com/apache/cordova-lib/pull/597
  
I think turning it back to `package` in plugman is the way to go. We can 
add an eslint exception for it I hope


---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org



[GitHub] cordova-coho pull request #161: Update tools-release-process.md

2017-10-06 Thread stevengill
GitHub user stevengill opened a pull request:

https://github.com/apache/cordova-coho/pull/161

Update tools-release-process.md



### Platforms affected


### What does this PR do?


### What testing has been done on this change?


### Checklist
- [ ] [Reported an issue](http://cordova.apache.org/contribute/issues.html) 
in the JIRA database
- [ ] Commit message follows the format: "CB-3232: (android) Fix bug with 
resolving file paths", where CB- is the JIRA ID & "android" is the platform 
affected.
- [ ] Added automated test coverage as appropriate for this change.


You can merge this pull request into a Git repository by running:

$ git pull https://github.com/stevengill/cordova-coho patch-61

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/cordova-coho/pull/161.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #161


commit 707fbd3a10b83c60b80c2fad2e809f336ab396d9
Author: Steve Gill <stevengil...@gmail.com>
Date:   2017-10-06T22:38:17Z

Update tools-release-process.md




---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org



[GitHub] cordova-node-xcode issue #2: CB-12844 Update pbxProject.js

2017-10-04 Thread stevengill
Github user stevengill commented on the issue:

https://github.com/apache/cordova-node-xcode/pull/2
  
Hey @sruthakeerthikotla!

Could you add a test to the pr? 


---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org



[GitHub] cordova-cli pull request #287: CB-13303 : added noprod/production as an opti...

2017-10-03 Thread stevengill
Github user stevengill commented on a diff in the pull request:

https://github.com/apache/cordova-cli/pull/287#discussion_r142530184
  
--- Diff: src/cli.js ---
@@ -475,8 +491,11 @@ function cli (inputArgs) {
 link: args.link || false,
 save: args.save,
 shrinkwrap: args.shrinkwrap || false,
-force: args.force || false
+force: args.force || false,
+production: args.production,
+noprod: args.noprod || false
--- End diff --

no need to pass noprod, it is only used to figure out production. Which you 
do on L454


---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org



[GitHub] cordova-cli pull request #287: CB-13303 : added noprod/production as an opti...

2017-10-03 Thread stevengill
Github user stevengill commented on a diff in the pull request:

https://github.com/apache/cordova-cli/pull/287#discussion_r142529980
  
--- Diff: src/cli.js ---
@@ -465,6 +473,14 @@ function cli (inputArgs) {
 // User explicitly did not pass in searchpath
 args.searchpath = conf.get('searchpath');
 }
+if (args.production === undefined) {
+// User explicitly did not pass in noprod
+args.production = conf.get('production');
+}
+if (args.noprod === undefined) {
--- End diff --

lets not offer `noprod` in config. Just `args.production`. We only need 
noprod like you have in line 454. Used to figure out of args.production is true 
or false. 


---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org



[GitHub] cordova-cli pull request #286: CB-13353 : added save-exact option

2017-10-03 Thread stevengill
Github user stevengill commented on a diff in the pull request:

https://github.com/apache/cordova-cli/pull/286#discussion_r142484800
  
--- Diff: src/cli.js ---
@@ -449,6 +450,10 @@ function cli (inputArgs) {
 args.save = true;
 }
 
+if (args['save-exact']) {
--- End diff --

what happens if you don't have lines 453-456? Are they needed?


---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org



[GitHub] cordova-cli pull request #286: CB-13353 : added save-exact option

2017-10-03 Thread stevengill
Github user stevengill commented on a diff in the pull request:

https://github.com/apache/cordova-cli/pull/286#discussion_r142475924
  
--- Diff: src/cli.js ---
@@ -443,12 +444,16 @@ function cli (inputArgs) {
 });
 }
 
-if (args.nosave) {
+if (args.nosave || args['save-exact']) {
--- End diff --

this change is probably not needed since we handle the save or save-exact 
or no-save flags in cordova-fetch. 


---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org



[GitHub] cordova-cli issue #285: CB-13274 : removed .jshintignore file and removed un...

2017-09-26 Thread stevengill
Github user stevengill commented on the issue:

https://github.com/apache/cordova-cli/pull/285
  
Merge it!


---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org



[GitHub] cordova-js pull request #149: CB-12895 : added eslint to repo

2017-09-26 Thread stevengill
Github user stevengill commented on a diff in the pull request:

https://github.com/apache/cordova-js/pull/149#discussion_r141133636
  
--- Diff: package.json ---
@@ -15,6 +15,7 @@
 "url": "https://issues.apache.org/jira/browse/CB;
   },
   "scripts": {
+"eslint": "eslint src test",
--- End diff --

we want to keep `pkg` in `.gitignore`. What happens if we run `eslint` on a 
folder that doesn't exist?

We need to figure out how to fix those errors. Grr. I can take a look with 
you. Manually fixing those errors is not an option. Are they all fixable with 
`eslint --fix`?



---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org



[GitHub] cordova-js pull request #149: CB-12895 : added eslint to repo

2017-09-25 Thread stevengill
Github user stevengill commented on a diff in the pull request:

https://github.com/apache/cordova-js/pull/149#discussion_r140927739
  
--- Diff: package.json ---
@@ -15,6 +15,7 @@
 "url": "https://issues.apache.org/jira/browse/CB;
   },
   "scripts": {
+"eslint": "eslint src test",
 "test": "grunt test",
--- End diff --

can you add `npm run eslint` to the test script here


---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org



[GitHub] cordova-js pull request #149: CB-12895 : added eslint to repo

2017-09-25 Thread stevengill
Github user stevengill commented on a diff in the pull request:

https://github.com/apache/cordova-js/pull/149#discussion_r140927689
  
--- Diff: package.json ---
@@ -15,6 +15,7 @@
 "url": "https://issues.apache.org/jira/browse/CB;
   },
   "scripts": {
+"eslint": "eslint src test",
--- End diff --

can you also get it to run on `pkg`

`"eslint src test pkg"`


---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org



[GitHub] cordova-plugin-device-motion issue #57: CB-13068: Updated README to point to...

2017-09-17 Thread stevengill
Github user stevengill commented on the issue:

https://github.com/apache/cordova-plugin-device-motion/pull/57
  
Merged this in, Added `info` tag. 

@maverickmishra can you close this issue?


---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org



[GitHub] cordova-plugin-device-motion pull request #57: CB-13068: Updated README to p...

2017-09-17 Thread stevengill
Github user stevengill commented on a diff in the pull request:


https://github.com/apache/cordova-plugin-device-motion/pull/57#discussion_r139333464
  
--- Diff: README.md ---
@@ -27,6 +27,18 @@ description: Access accelerometer data.
 
 # cordova-plugin-device-motion
 
+# Deprecation Notice
+
+With the [W3C Device Motion and Orientation 
API](https://www.w3.org/TR/2016/CR-orientation-event-20160818/) now
+being supported on iOS, Android and Windows devices, this plugin is not 
needed any more. Migrating from this
+plugin to the [W3C Device Motion and Orientation 
API](https://www.w3.org/TR/2016/CR-orientation-event-20160818/)
+is explained in this [PhoneGap blog 
post](https://blog.phonegap.com/migrating-from-the-cordova-device-motion-plugin-ddd8176632ed).
--- End diff --

The plugins release blog post will talk about the deprecation. 


---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org



[GitHub] cordova-docs issue #729: Fixed up 'deployment' docs in README.md.

2017-09-13 Thread stevengill
Github user stevengill commented on the issue:

https://github.com/apache/cordova-docs/pull/729
  
LGTM. 

I personally prefer `npm run serve` over `node_modules/.bin/gulp build 
--prod` but can deal with it.


---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org



[GitHub] cordova-lib pull request #584: CB-12361 : added tests for plugin/save.js

2017-09-06 Thread stevengill
Github user stevengill commented on a diff in the pull request:

https://github.com/apache/cordova-lib/pull/584#discussion_r137442765
  
--- Diff: spec/cordova/plugin/save.spec.js ---
@@ -54,15 +62,131 @@ describe('cordova/plugin/save', function () {
 });
 });
 describe('happy path', function () {
-it('should remove all plugins from config.xml and re-add new ones 
based on those retrieved from fetch.json');
-it('should only add top-level plugins to config.xml');
-it('should write individual plugin specs to config.xml');
-it('should write individual plugin variables to config.xml');
+it('check that existing plugins are getting removed', function 
(done) {
+save(projectRoot).then(function () {
+
expect(cfg_parser_mock.prototype.removePlugin).toHaveBeenCalledWith('VRPlugin');
+
expect(cfg_parser_mock.prototype.removePlugin).toHaveBeenCalledWith('MastodonSocialPlugin');
+}).fail(function (e) {
+expect(e).toBeUndefined();
+fail('did not expect fail handler to be invoked');
+}).done(done);
+});
+
+it('plugins are being removed first and then only top level 
plugins are being restored', function (done) {
+var fake_fetch_json =
+{'VRPlugin': {'source': {
+'type': 'registry',
+'id': 'id'
+},
+'is_top_level': true
+},
+'MastodonSocialPlugin': { 'source': {
+'type': 'registry',
+'id': 'id'
+},
+'is_top_level': false }};
+
+
fs.readFileSync.and.returnValue(JSON.stringify(fake_fetch_json));
+save(projectRoot).then(function () {
+
expect(cfg_parser_mock.prototype.removePlugin).toHaveBeenCalledWith('VRPlugin');
+
expect(cfg_parser_mock.prototype.removePlugin).toHaveBeenCalledWith('MastodonSocialPlugin');
+
expect(cfg_parser_mock.prototype.addPlugin).toHaveBeenCalledWith(Object({ name: 
'VRPlugin' }), [ ]);
+
expect(cfg_parser_mock.prototype.addPlugin).not.toHaveBeenCalledWith(Object({ 
name: 'MastodonSocialPlugin' }), [ ]);
+expect(cfg_parser_mock.prototype.write).toHaveBeenCalled();
+}).fail(function (e) {
+expect(e).toBeUndefined();
+fail('did not expect fail handler to be invoked');
+}).done(done);
+});
+
+it('should write individual plugin specs to config.xml', function 
(done) {
+var fake_fetch_json =
+{'VRPlugin': {'source': {
+'type': 'registry',
+'id': 'id'
+},
+'is_top_level': true }};
+
fs.readFileSync.and.returnValue(JSON.stringify(fake_fetch_json));
+spyOn(save, 'getSpec').and.returnValue('1.0.0');
+save(projectRoot).then(function () {
+
expect(cfg_parser_mock.prototype.addPlugin).toHaveBeenCalledWith(Object({ name: 
'VRPlugin', spec: '1.0.0' }), jasmine.any(Object));
+expect(cfg_parser_mock.prototype.write).toHaveBeenCalled();
+}).fail(function (e) {
+expect(e).toBeUndefined();
+fail('did not expect fail handler to be invoked');
+}).done(done);
+});
+
+it('should write individual plugin variables to config.xml', 
function (done) {
+var fake_fetch_json =
+{'VRPlugin': {'source': {
+'type': 'registry',
+'id': 'id'
+},
+'is_top_level': true,
+'variables': {
+'var 1': ' '
+}}};
+
fs.readFileSync.and.returnValue(JSON.stringify(fake_fetch_json));
+save(projectRoot).then(function () {
+
expect(cfg_parser_mock.prototype.addPlugin).toHaveBeenCalledWith(jasmine.any(Object),
 [ Object({ name: 'var 1', value: ' ' }) ]);
+expect(cfg_parser_mock.prototype.write).toHaveBeenCalled();
+}).fail(function (e) {
+expect(e).toBeUndefined();
+fail('did not expect fail handler to be invoked');
+}).done(done);
+});
 });
 describe('getSpec helper method', function () {
-it('should return a plugin source\'s url or path property 
immediately');
-it('should return a version if a version was provided to plugin 
id

[GitHub] cordova-lib pull request #584: CB-12361 : added tests for plugin/save.js

2017-09-06 Thread stevengill
Github user stevengill commented on a diff in the pull request:

https://github.com/apache/cordova-lib/pull/584#discussion_r137442686
  
--- Diff: spec/cordova/plugin/save.spec.js ---
@@ -54,15 +62,131 @@ describe('cordova/plugin/save', function () {
 });
 });
 describe('happy path', function () {
-it('should remove all plugins from config.xml and re-add new ones 
based on those retrieved from fetch.json');
-it('should only add top-level plugins to config.xml');
-it('should write individual plugin specs to config.xml');
-it('should write individual plugin variables to config.xml');
+it('check that existing plugins are getting removed', function 
(done) {
+save(projectRoot).then(function () {
+
expect(cfg_parser_mock.prototype.removePlugin).toHaveBeenCalledWith('VRPlugin');
+
expect(cfg_parser_mock.prototype.removePlugin).toHaveBeenCalledWith('MastodonSocialPlugin');
+}).fail(function (e) {
+expect(e).toBeUndefined();
+fail('did not expect fail handler to be invoked');
+}).done(done);
+});
+
+it('plugins are being removed first and then only top level 
plugins are being restored', function (done) {
+var fake_fetch_json =
+{'VRPlugin': {'source': {
+'type': 'registry',
+'id': 'id'
+},
+'is_top_level': true
+},
+'MastodonSocialPlugin': { 'source': {
+'type': 'registry',
+'id': 'id'
+},
+'is_top_level': false }};
+
+
fs.readFileSync.and.returnValue(JSON.stringify(fake_fetch_json));
+save(projectRoot).then(function () {
+
expect(cfg_parser_mock.prototype.removePlugin).toHaveBeenCalledWith('VRPlugin');
+
expect(cfg_parser_mock.prototype.removePlugin).toHaveBeenCalledWith('MastodonSocialPlugin');
+
expect(cfg_parser_mock.prototype.addPlugin).toHaveBeenCalledWith(Object({ name: 
'VRPlugin' }), [ ]);
+
expect(cfg_parser_mock.prototype.addPlugin).not.toHaveBeenCalledWith(Object({ 
name: 'MastodonSocialPlugin' }), [ ]);
+expect(cfg_parser_mock.prototype.write).toHaveBeenCalled();
+}).fail(function (e) {
+expect(e).toBeUndefined();
+fail('did not expect fail handler to be invoked');
+}).done(done);
+});
+
+it('should write individual plugin specs to config.xml', function 
(done) {
+var fake_fetch_json =
+{'VRPlugin': {'source': {
+'type': 'registry',
+'id': 'id'
+},
+'is_top_level': true }};
+
fs.readFileSync.and.returnValue(JSON.stringify(fake_fetch_json));
+spyOn(save, 'getSpec').and.returnValue('1.0.0');
+save(projectRoot).then(function () {
+
expect(cfg_parser_mock.prototype.addPlugin).toHaveBeenCalledWith(Object({ name: 
'VRPlugin', spec: '1.0.0' }), jasmine.any(Object));
+expect(cfg_parser_mock.prototype.write).toHaveBeenCalled();
+}).fail(function (e) {
+expect(e).toBeUndefined();
+fail('did not expect fail handler to be invoked');
+}).done(done);
+});
+
+it('should write individual plugin variables to config.xml', 
function (done) {
+var fake_fetch_json =
+{'VRPlugin': {'source': {
+'type': 'registry',
+'id': 'id'
+},
+'is_top_level': true,
+'variables': {
+'var 1': ' '
+}}};
+
fs.readFileSync.and.returnValue(JSON.stringify(fake_fetch_json));
+save(projectRoot).then(function () {
+
expect(cfg_parser_mock.prototype.addPlugin).toHaveBeenCalledWith(jasmine.any(Object),
 [ Object({ name: 'var 1', value: ' ' }) ]);
+expect(cfg_parser_mock.prototype.write).toHaveBeenCalled();
+}).fail(function (e) {
+expect(e).toBeUndefined();
+fail('did not expect fail handler to be invoked');
+}).done(done);
+});
 });
 describe('getSpec helper method', function () {
-it('should return a plugin source\'s url or path property 
immediately');
-it('should return a version if a version was provided to plugin 
id

[GitHub] cordova-lib pull request #584: CB-12361 : added tests for plugin/save.js

2017-09-06 Thread stevengill
Github user stevengill commented on a diff in the pull request:

https://github.com/apache/cordova-lib/pull/584#discussion_r137442253
  
--- Diff: spec/cordova/plugin/save.spec.js ---
@@ -54,15 +62,131 @@ describe('cordova/plugin/save', function () {
 });
 });
 describe('happy path', function () {
-it('should remove all plugins from config.xml and re-add new ones 
based on those retrieved from fetch.json');
-it('should only add top-level plugins to config.xml');
-it('should write individual plugin specs to config.xml');
-it('should write individual plugin variables to config.xml');
+it('check that existing plugins are getting removed', function 
(done) {
+save(projectRoot).then(function () {
+
expect(cfg_parser_mock.prototype.removePlugin).toHaveBeenCalledWith('VRPlugin');
+
expect(cfg_parser_mock.prototype.removePlugin).toHaveBeenCalledWith('MastodonSocialPlugin');
+}).fail(function (e) {
+expect(e).toBeUndefined();
+fail('did not expect fail handler to be invoked');
+}).done(done);
+});
+
+it('plugins are being removed first and then only top level 
plugins are being restored', function (done) {
+var fake_fetch_json =
+{'VRPlugin': {'source': {
+'type': 'registry',
+'id': 'id'
+},
+'is_top_level': true
+},
+'MastodonSocialPlugin': { 'source': {
+'type': 'registry',
+'id': 'id'
+},
+'is_top_level': false }};
+
+
fs.readFileSync.and.returnValue(JSON.stringify(fake_fetch_json));
+save(projectRoot).then(function () {
+
expect(cfg_parser_mock.prototype.removePlugin).toHaveBeenCalledWith('VRPlugin');
+
expect(cfg_parser_mock.prototype.removePlugin).toHaveBeenCalledWith('MastodonSocialPlugin');
+
expect(cfg_parser_mock.prototype.addPlugin).toHaveBeenCalledWith(Object({ name: 
'VRPlugin' }), [ ]);
+
expect(cfg_parser_mock.prototype.addPlugin).not.toHaveBeenCalledWith(Object({ 
name: 'MastodonSocialPlugin' }), [ ]);
+expect(cfg_parser_mock.prototype.write).toHaveBeenCalled();
+}).fail(function (e) {
+expect(e).toBeUndefined();
+fail('did not expect fail handler to be invoked');
+}).done(done);
+});
+
+it('should write individual plugin specs to config.xml', function 
(done) {
+var fake_fetch_json =
+{'VRPlugin': {'source': {
+'type': 'registry',
+'id': 'id'
+},
+'is_top_level': true }};
+
fs.readFileSync.and.returnValue(JSON.stringify(fake_fetch_json));
+spyOn(save, 'getSpec').and.returnValue('1.0.0');
+save(projectRoot).then(function () {
+
expect(cfg_parser_mock.prototype.addPlugin).toHaveBeenCalledWith(Object({ name: 
'VRPlugin', spec: '1.0.0' }), jasmine.any(Object));
+expect(cfg_parser_mock.prototype.write).toHaveBeenCalled();
+}).fail(function (e) {
+expect(e).toBeUndefined();
+fail('did not expect fail handler to be invoked');
+}).done(done);
+});
+
+it('should write individual plugin variables to config.xml', 
function (done) {
+var fake_fetch_json =
+{'VRPlugin': {'source': {
+'type': 'registry',
+'id': 'id'
+},
+'is_top_level': true,
+'variables': {
+'var 1': ' '
+}}};
+
fs.readFileSync.and.returnValue(JSON.stringify(fake_fetch_json));
+save(projectRoot).then(function () {
+
expect(cfg_parser_mock.prototype.addPlugin).toHaveBeenCalledWith(jasmine.any(Object),
 [ Object({ name: 'var 1', value: ' ' }) ]);
+expect(cfg_parser_mock.prototype.write).toHaveBeenCalled();
+}).fail(function (e) {
+expect(e).toBeUndefined();
+fail('did not expect fail handler to be invoked');
+}).done(done);
+});
 });
 describe('getSpec helper method', function () {
-it('should return a plugin source\'s url or path property 
immediately');
-it('should return a version if a version was provided to plugin 
id

[GitHub] cordova-lib pull request #584: CB-12361 : added tests for plugin/save.js

2017-09-06 Thread stevengill
Github user stevengill commented on a diff in the pull request:

https://github.com/apache/cordova-lib/pull/584#discussion_r137442083
  
--- Diff: spec/cordova/plugin/save.spec.js ---
@@ -54,15 +62,131 @@ describe('cordova/plugin/save', function () {
 });
 });
 describe('happy path', function () {
-it('should remove all plugins from config.xml and re-add new ones 
based on those retrieved from fetch.json');
-it('should only add top-level plugins to config.xml');
-it('should write individual plugin specs to config.xml');
-it('should write individual plugin variables to config.xml');
+it('check that existing plugins are getting removed', function 
(done) {
+save(projectRoot).then(function () {
+
expect(cfg_parser_mock.prototype.removePlugin).toHaveBeenCalledWith('VRPlugin');
+
expect(cfg_parser_mock.prototype.removePlugin).toHaveBeenCalledWith('MastodonSocialPlugin');
+}).fail(function (e) {
+expect(e).toBeUndefined();
+fail('did not expect fail handler to be invoked');
+}).done(done);
+});
+
+it('plugins are being removed first and then only top level 
plugins are being restored', function (done) {
+var fake_fetch_json =
+{'VRPlugin': {'source': {
+'type': 'registry',
+'id': 'id'
+},
+'is_top_level': true
+},
+'MastodonSocialPlugin': { 'source': {
+'type': 'registry',
+'id': 'id'
+},
+'is_top_level': false }};
+
+
fs.readFileSync.and.returnValue(JSON.stringify(fake_fetch_json));
+save(projectRoot).then(function () {
+
expect(cfg_parser_mock.prototype.removePlugin).toHaveBeenCalledWith('VRPlugin');
+
expect(cfg_parser_mock.prototype.removePlugin).toHaveBeenCalledWith('MastodonSocialPlugin');
+
expect(cfg_parser_mock.prototype.addPlugin).toHaveBeenCalledWith(Object({ name: 
'VRPlugin' }), [ ]);
+
expect(cfg_parser_mock.prototype.addPlugin).not.toHaveBeenCalledWith(Object({ 
name: 'MastodonSocialPlugin' }), [ ]);
+expect(cfg_parser_mock.prototype.write).toHaveBeenCalled();
+}).fail(function (e) {
+expect(e).toBeUndefined();
+fail('did not expect fail handler to be invoked');
+}).done(done);
+});
+
+it('should write individual plugin specs to config.xml', function 
(done) {
+var fake_fetch_json =
+{'VRPlugin': {'source': {
+'type': 'registry',
+'id': 'id'
+},
+'is_top_level': true }};
+
fs.readFileSync.and.returnValue(JSON.stringify(fake_fetch_json));
+spyOn(save, 'getSpec').and.returnValue('1.0.0');
+save(projectRoot).then(function () {
+
expect(cfg_parser_mock.prototype.addPlugin).toHaveBeenCalledWith(Object({ name: 
'VRPlugin', spec: '1.0.0' }), jasmine.any(Object));
+expect(cfg_parser_mock.prototype.write).toHaveBeenCalled();
+}).fail(function (e) {
+expect(e).toBeUndefined();
+fail('did not expect fail handler to be invoked');
+}).done(done);
+});
+
+it('should write individual plugin variables to config.xml', 
function (done) {
+var fake_fetch_json =
+{'VRPlugin': {'source': {
+'type': 'registry',
+'id': 'id'
+},
+'is_top_level': true,
+'variables': {
+'var 1': ' '
+}}};
+
fs.readFileSync.and.returnValue(JSON.stringify(fake_fetch_json));
+save(projectRoot).then(function () {
+
expect(cfg_parser_mock.prototype.addPlugin).toHaveBeenCalledWith(jasmine.any(Object),
 [ Object({ name: 'var 1', value: ' ' }) ]);
+expect(cfg_parser_mock.prototype.write).toHaveBeenCalled();
+}).fail(function (e) {
+expect(e).toBeUndefined();
+fail('did not expect fail handler to be invoked');
+}).done(done);
+});
 });
 describe('getSpec helper method', function () {
-it('should return a plugin source\'s url or path property 
immediately');
-it('should return a version if a version was provided to plugin 
id

[GitHub] cordova-docs pull request #724: CB-13231 : added cordova-common@2.1.0 releas...

2017-09-05 Thread stevengill
Github user stevengill commented on a diff in the pull request:

https://github.com/apache/cordova-docs/pull/724#discussion_r137072952
  
--- Diff: www/_posts/2017-09-01-cordova-common-2.1.0.md ---
@@ -0,0 +1,40 @@
+---
+layout: post
+author:
+name: Audrey So
+url: https://twitter.com/aud_rey_so
+title:  "Cordova-Common@2.1.0 Released!"
+date: 2017-09-01
+categories: news
+tags: release tools cordova-common
+---
+
+We just released some changes to `cordova-common`!
+
+* [cordova@2.1.0](https://www.npmjs.com/package/cordova-common)
+
+Release Highlights:
+* Support added for `` in `config.xml`
+* `allows-arbitrary-loads-for-media` attribute parsing added for 
`getAccesses`
+* Added variable replacing the `framework` tag
+* `JSON` uses 2 spaces for indentation
+
+To update your cordova CLI:
--- End diff --

Remove the to update part. It isn't really useful to consumers until after 
it has been included in next lib + platform releases


---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org



[GitHub] cordova-docs pull request #724: CB-13231 : added cordova-common@2.1.0 releas...

2017-09-05 Thread stevengill
Github user stevengill commented on a diff in the pull request:

https://github.com/apache/cordova-docs/pull/724#discussion_r137072825
  
--- Diff: www/_posts/2017-09-01-cordova-common-2.1.0.md ---
@@ -0,0 +1,40 @@
+---
+layout: post
+author:
+name: Audrey So
+url: https://twitter.com/aud_rey_so
+title:  "Cordova-Common@2.1.0 Released!"
+date: 2017-09-01
+categories: news
+tags: release tools cordova-common
+---
+
+We just released some changes to `cordova-common`!
+
+* [cordova@2.1.0](https://www.npmjs.com/package/cordova-common)
--- End diff --

I would update this to 
```[cordova-common@2.1.0](https://www.npmjs.com/package/cordova-common)```


---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org



[GitHub] cordova-docs pull request #724: CB-13231 : added cordova-common@2.1.0 releas...

2017-09-05 Thread stevengill
Github user stevengill commented on a diff in the pull request:

https://github.com/apache/cordova-docs/pull/724#discussion_r137073157
  
--- Diff: www/_posts/2017-09-01-cordova-common-2.1.0.md ---
@@ -0,0 +1,40 @@
+---
+layout: post
+author:
+name: Audrey So
+url: https://twitter.com/aud_rey_so
+title:  "Cordova-Common@2.1.0 Released!"
+date: 2017-09-01
+categories: news
+tags: release tools cordova-common
+---
+
+We just released some changes to `cordova-common`!
+
+* [cordova@2.1.0](https://www.npmjs.com/package/cordova-common)
+
+Release Highlights:
+* Support added for `` in `config.xml`
+* `allows-arbitrary-loads-for-media` attribute parsing added for 
`getAccesses`
+* Added variable replacing the `framework` tag
+* `JSON` uses 2 spaces for indentation
+
+To update your cordova CLI:
--- End diff --

Instead of the update, mention that `watch for this release to start 
rolling into upcoming platform and cordova-cli releases`


---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org



[GitHub] cordova-lib issue #584: CB-12361 : added tests for plugin/save.js

2017-08-31 Thread stevengill
Github user stevengill commented on the issue:

https://github.com/apache/cordova-lib/pull/584
  
Make sure to run `npm run cover` to see the coverage report for save.js


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org



[GitHub] cordova-lib pull request #584: CB-12361 : added tests for plugin/save.js

2017-08-31 Thread stevengill
Github user stevengill commented on a diff in the pull request:

https://github.com/apache/cordova-lib/pull/584#discussion_r136486774
  
--- Diff: spec/cordova/plugin/save.spec.js ---
@@ -54,15 +61,145 @@ describe('cordova/plugin/save', function () {
 });
 });
 describe('happy path', function () {
-it('should remove all plugins from config.xml and re-add new ones 
based on those retrieved from fetch.json');
-it('should only add top-level plugins to config.xml');
-it('should write individual plugin specs to config.xml');
-it('should write individual plugin variables to config.xml');
+it('should remove all plugins from config.xml and re-add new ones 
based on those retrieved from fetch.json', function (done) {
--- End diff --

This test checks that the plugins get removed but not if the new ones get 
re-added (based on description). I think the description should be updated to 
just check that existing plugins are getting removed


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org



[GitHub] cordova-lib pull request #584: CB-12361 : added tests for plugin/save.js

2017-08-31 Thread stevengill
Github user stevengill commented on a diff in the pull request:

https://github.com/apache/cordova-lib/pull/584#discussion_r136490386
  
--- Diff: spec/cordova/plugin/save.spec.js ---
@@ -54,15 +61,145 @@ describe('cordova/plugin/save', function () {
 });
 });
 describe('happy path', function () {
-it('should remove all plugins from config.xml and re-add new ones 
based on those retrieved from fetch.json');
-it('should only add top-level plugins to config.xml');
-it('should write individual plugin specs to config.xml');
-it('should write individual plugin variables to config.xml');
+it('should remove all plugins from config.xml and re-add new ones 
based on those retrieved from fetch.json', function (done) {
+save(projectRoot).then(function () {
+
expect(cfg_parser_mock.prototype.removePlugin).toHaveBeenCalledWith('VRPlugin');
+
expect(cfg_parser_mock.prototype.removePlugin).toHaveBeenCalledWith('MastodonSocialPlugin');
+}).fail(function (e) {
+expect(e).toBeUndefined();
+fail('did not expect fail handler to be invoked');
+}).done(done);
+});
+
+it('should only add top-level plugins to config.xml', function 
(done) {
+var fake_fetch_json =
+{'VRPlugin': {'source': {
+'type': 'registry',
+'id': 'id'
+},
+'is_top_level': true
+},
+'MastodonSocialPlugin': { 'source': {
+'type': 'registry',
+'id': 'id'
+},
+'is_top_level': false }};
+
+
fs.readFileSync.and.returnValue(JSON.stringify(fake_fetch_json));
+save(projectRoot).then(function () {
+
expect(cfg_parser_mock.prototype.addPlugin).toHaveBeenCalledWith(Object({ name: 
'VRPlugin' }), [ ]);
+expect(cfg_parser_mock.prototype.write).toHaveBeenCalled();
+}).fail(function (e) {
+expect(e).toBeUndefined();
+fail('did not expect fail handler to be invoked');
+}).done(done);
+});
+
+it('should write individual plugin specs to config.xml', function 
(done) {
+var fake_fetch_json =
+{'VRPlugin': {'source': {
+'type': 'registry',
+'id': 'id'
+},
+'is_top_level': true }};
+
fs.readFileSync.and.returnValue(JSON.stringify(fake_fetch_json));
+spyOn(save, 'getSpec').and.returnValue('1.0.0');
+save(projectRoot).then(function () {
+
expect(cfg_parser_mock.prototype.addPlugin).toHaveBeenCalledWith(Object({ name: 
'VRPlugin', spec: '1.0.0' }), jasmine.any(Object));
+expect(cfg_parser_mock.prototype.write).toHaveBeenCalled();
+}).fail(function (e) {
+expect(e).toBeUndefined();
+fail('did not expect fail handler to be invoked');
+}).done(done);
+});
+
+it('should write individual plugin variables to config.xml', 
function (done) {
+var fake_fetch_json =
+{'VRPlugin': {'source': {
+'type': 'registry',
+'id': 'id'
+},
+'is_top_level': true,
+'variables': {
+'var 1': ' '
+}}};
+
fs.readFileSync.and.returnValue(JSON.stringify(fake_fetch_json));
+save(projectRoot).then(function () {
+
expect(cfg_parser_mock.prototype.addPlugin).toHaveBeenCalledWith(jasmine.any(Object),
 [ Object({ name: 'var 1', value: ' ' }) ]);
+expect(cfg_parser_mock.prototype.write).toHaveBeenCalled();
+}).fail(function (e) {
+expect(e).toBeUndefined();
+fail('did not expect fail handler to be invoked');
+}).done(done);
+});
 });
 describe('getSpec helper method', function () {
-it('should return a plugin source\'s url or path property 
immediately');
-it('should return a version if a version was provided to plugin 
id');
-it('should return a version that includes scope if scope was part 
of plugin id');
-it('should fall back to using PluginInfoProvider to retrieve a 
version as last resort');
+it('should return a plugin source\'s url or path property 
immediately', function (done) {
+var fake_fetch_json

[GitHub] cordova-lib pull request #584: CB-12361 : added tests for plugin/save.js

2017-08-31 Thread stevengill
Github user stevengill commented on a diff in the pull request:

https://github.com/apache/cordova-lib/pull/584#discussion_r136489451
  
--- Diff: spec/cordova/plugin/save.spec.js ---
@@ -54,15 +61,145 @@ describe('cordova/plugin/save', function () {
 });
 });
 describe('happy path', function () {
-it('should remove all plugins from config.xml and re-add new ones 
based on those retrieved from fetch.json');
-it('should only add top-level plugins to config.xml');
-it('should write individual plugin specs to config.xml');
-it('should write individual plugin variables to config.xml');
+it('should remove all plugins from config.xml and re-add new ones 
based on those retrieved from fetch.json', function (done) {
+save(projectRoot).then(function () {
+
expect(cfg_parser_mock.prototype.removePlugin).toHaveBeenCalledWith('VRPlugin');
+
expect(cfg_parser_mock.prototype.removePlugin).toHaveBeenCalledWith('MastodonSocialPlugin');
+}).fail(function (e) {
+expect(e).toBeUndefined();
+fail('did not expect fail handler to be invoked');
+}).done(done);
+});
+
+it('should only add top-level plugins to config.xml', function 
(done) {
+var fake_fetch_json =
+{'VRPlugin': {'source': {
+'type': 'registry',
+'id': 'id'
+},
+'is_top_level': true
+},
+'MastodonSocialPlugin': { 'source': {
+'type': 'registry',
+'id': 'id'
+},
+'is_top_level': false }};
+
+
fs.readFileSync.and.returnValue(JSON.stringify(fake_fetch_json));
+save(projectRoot).then(function () {
+
expect(cfg_parser_mock.prototype.addPlugin).toHaveBeenCalledWith(Object({ name: 
'VRPlugin' }), [ ]);
+expect(cfg_parser_mock.prototype.write).toHaveBeenCalled();
+}).fail(function (e) {
+expect(e).toBeUndefined();
+fail('did not expect fail handler to be invoked');
+}).done(done);
+});
+
+it('should write individual plugin specs to config.xml', function 
(done) {
+var fake_fetch_json =
+{'VRPlugin': {'source': {
+'type': 'registry',
+'id': 'id'
+},
+'is_top_level': true }};
+
fs.readFileSync.and.returnValue(JSON.stringify(fake_fetch_json));
+spyOn(save, 'getSpec').and.returnValue('1.0.0');
+save(projectRoot).then(function () {
+
expect(cfg_parser_mock.prototype.addPlugin).toHaveBeenCalledWith(Object({ name: 
'VRPlugin', spec: '1.0.0' }), jasmine.any(Object));
+expect(cfg_parser_mock.prototype.write).toHaveBeenCalled();
+}).fail(function (e) {
+expect(e).toBeUndefined();
+fail('did not expect fail handler to be invoked');
+}).done(done);
+});
+
+it('should write individual plugin variables to config.xml', 
function (done) {
+var fake_fetch_json =
+{'VRPlugin': {'source': {
+'type': 'registry',
+'id': 'id'
+},
+'is_top_level': true,
+'variables': {
+'var 1': ' '
+}}};
+
fs.readFileSync.and.returnValue(JSON.stringify(fake_fetch_json));
+save(projectRoot).then(function () {
+
expect(cfg_parser_mock.prototype.addPlugin).toHaveBeenCalledWith(jasmine.any(Object),
 [ Object({ name: 'var 1', value: ' ' }) ]);
+expect(cfg_parser_mock.prototype.write).toHaveBeenCalled();
+}).fail(function (e) {
+expect(e).toBeUndefined();
+fail('did not expect fail handler to be invoked');
+}).done(done);
+});
 });
 describe('getSpec helper method', function () {
-it('should return a plugin source\'s url or path property 
immediately');
-it('should return a version if a version was provided to plugin 
id');
-it('should return a version that includes scope if scope was part 
of plugin id');
-it('should fall back to using PluginInfoProvider to retrieve a 
version as last resort');
+it('should return a plugin source\'s url or path property 
immediately', function (done) {
+var fake_fetch_json

[GitHub] cordova-lib pull request #584: CB-12361 : added tests for plugin/save.js

2017-08-31 Thread stevengill
Github user stevengill commented on a diff in the pull request:

https://github.com/apache/cordova-lib/pull/584#discussion_r136488898
  
--- Diff: spec/cordova/plugin/save.spec.js ---
@@ -54,15 +61,145 @@ describe('cordova/plugin/save', function () {
 });
 });
 describe('happy path', function () {
-it('should remove all plugins from config.xml and re-add new ones 
based on those retrieved from fetch.json');
-it('should only add top-level plugins to config.xml');
-it('should write individual plugin specs to config.xml');
-it('should write individual plugin variables to config.xml');
+it('should remove all plugins from config.xml and re-add new ones 
based on those retrieved from fetch.json', function (done) {
+save(projectRoot).then(function () {
+
expect(cfg_parser_mock.prototype.removePlugin).toHaveBeenCalledWith('VRPlugin');
+
expect(cfg_parser_mock.prototype.removePlugin).toHaveBeenCalledWith('MastodonSocialPlugin');
+}).fail(function (e) {
+expect(e).toBeUndefined();
+fail('did not expect fail handler to be invoked');
+}).done(done);
+});
+
+it('should only add top-level plugins to config.xml', function 
(done) {
+var fake_fetch_json =
+{'VRPlugin': {'source': {
+'type': 'registry',
+'id': 'id'
+},
+'is_top_level': true
+},
+'MastodonSocialPlugin': { 'source': {
+'type': 'registry',
+'id': 'id'
+},
+'is_top_level': false }};
+
+
fs.readFileSync.and.returnValue(JSON.stringify(fake_fetch_json));
+save(projectRoot).then(function () {
+
expect(cfg_parser_mock.prototype.addPlugin).toHaveBeenCalledWith(Object({ name: 
'VRPlugin' }), [ ]);
+expect(cfg_parser_mock.prototype.write).toHaveBeenCalled();
+}).fail(function (e) {
+expect(e).toBeUndefined();
+fail('did not expect fail handler to be invoked');
+}).done(done);
+});
+
+it('should write individual plugin specs to config.xml', function 
(done) {
+var fake_fetch_json =
+{'VRPlugin': {'source': {
+'type': 'registry',
+'id': 'id'
+},
+'is_top_level': true }};
+
fs.readFileSync.and.returnValue(JSON.stringify(fake_fetch_json));
+spyOn(save, 'getSpec').and.returnValue('1.0.0');
+save(projectRoot).then(function () {
+
expect(cfg_parser_mock.prototype.addPlugin).toHaveBeenCalledWith(Object({ name: 
'VRPlugin', spec: '1.0.0' }), jasmine.any(Object));
+expect(cfg_parser_mock.prototype.write).toHaveBeenCalled();
+}).fail(function (e) {
+expect(e).toBeUndefined();
+fail('did not expect fail handler to be invoked');
+}).done(done);
+});
+
+it('should write individual plugin variables to config.xml', 
function (done) {
+var fake_fetch_json =
+{'VRPlugin': {'source': {
+'type': 'registry',
+'id': 'id'
+},
+'is_top_level': true,
+'variables': {
+'var 1': ' '
+}}};
+
fs.readFileSync.and.returnValue(JSON.stringify(fake_fetch_json));
+save(projectRoot).then(function () {
+
expect(cfg_parser_mock.prototype.addPlugin).toHaveBeenCalledWith(jasmine.any(Object),
 [ Object({ name: 'var 1', value: ' ' }) ]);
+expect(cfg_parser_mock.prototype.write).toHaveBeenCalled();
+}).fail(function (e) {
+expect(e).toBeUndefined();
+fail('did not expect fail handler to be invoked');
+}).done(done);
+});
 });
 describe('getSpec helper method', function () {
-it('should return a plugin source\'s url or path property 
immediately');
-it('should return a version if a version was provided to plugin 
id');
-it('should return a version that includes scope if scope was part 
of plugin id');
-it('should fall back to using PluginInfoProvider to retrieve a 
version as last resort');
+it('should return a plugin source\'s url or path property 
immediately', function (done) {
+var fake_fetch_json

[GitHub] cordova-lib pull request #584: CB-12361 : added tests for plugin/save.js

2017-08-31 Thread stevengill
Github user stevengill commented on a diff in the pull request:

https://github.com/apache/cordova-lib/pull/584#discussion_r136490187
  
--- Diff: spec/cordova/plugin/save.spec.js ---
@@ -54,15 +61,145 @@ describe('cordova/plugin/save', function () {
 });
 });
 describe('happy path', function () {
-it('should remove all plugins from config.xml and re-add new ones 
based on those retrieved from fetch.json');
-it('should only add top-level plugins to config.xml');
-it('should write individual plugin specs to config.xml');
-it('should write individual plugin variables to config.xml');
+it('should remove all plugins from config.xml and re-add new ones 
based on those retrieved from fetch.json', function (done) {
+save(projectRoot).then(function () {
+
expect(cfg_parser_mock.prototype.removePlugin).toHaveBeenCalledWith('VRPlugin');
+
expect(cfg_parser_mock.prototype.removePlugin).toHaveBeenCalledWith('MastodonSocialPlugin');
+}).fail(function (e) {
+expect(e).toBeUndefined();
+fail('did not expect fail handler to be invoked');
+}).done(done);
+});
+
+it('should only add top-level plugins to config.xml', function 
(done) {
+var fake_fetch_json =
+{'VRPlugin': {'source': {
+'type': 'registry',
+'id': 'id'
+},
+'is_top_level': true
+},
+'MastodonSocialPlugin': { 'source': {
+'type': 'registry',
+'id': 'id'
+},
+'is_top_level': false }};
+
+
fs.readFileSync.and.returnValue(JSON.stringify(fake_fetch_json));
+save(projectRoot).then(function () {
+
expect(cfg_parser_mock.prototype.addPlugin).toHaveBeenCalledWith(Object({ name: 
'VRPlugin' }), [ ]);
+expect(cfg_parser_mock.prototype.write).toHaveBeenCalled();
+}).fail(function (e) {
+expect(e).toBeUndefined();
+fail('did not expect fail handler to be invoked');
+}).done(done);
+});
+
+it('should write individual plugin specs to config.xml', function 
(done) {
+var fake_fetch_json =
+{'VRPlugin': {'source': {
+'type': 'registry',
+'id': 'id'
+},
+'is_top_level': true }};
+
fs.readFileSync.and.returnValue(JSON.stringify(fake_fetch_json));
+spyOn(save, 'getSpec').and.returnValue('1.0.0');
+save(projectRoot).then(function () {
+
expect(cfg_parser_mock.prototype.addPlugin).toHaveBeenCalledWith(Object({ name: 
'VRPlugin', spec: '1.0.0' }), jasmine.any(Object));
+expect(cfg_parser_mock.prototype.write).toHaveBeenCalled();
+}).fail(function (e) {
+expect(e).toBeUndefined();
+fail('did not expect fail handler to be invoked');
+}).done(done);
+});
+
+it('should write individual plugin variables to config.xml', 
function (done) {
+var fake_fetch_json =
+{'VRPlugin': {'source': {
+'type': 'registry',
+'id': 'id'
+},
+'is_top_level': true,
+'variables': {
+'var 1': ' '
+}}};
+
fs.readFileSync.and.returnValue(JSON.stringify(fake_fetch_json));
+save(projectRoot).then(function () {
+
expect(cfg_parser_mock.prototype.addPlugin).toHaveBeenCalledWith(jasmine.any(Object),
 [ Object({ name: 'var 1', value: ' ' }) ]);
+expect(cfg_parser_mock.prototype.write).toHaveBeenCalled();
+}).fail(function (e) {
+expect(e).toBeUndefined();
+fail('did not expect fail handler to be invoked');
+}).done(done);
+});
 });
 describe('getSpec helper method', function () {
--- End diff --

So with all of these `getSpec` tests, you want to test getSpec directly. 
Right now, you are doing `save(projectRoot)` but instead you want to do 
`save.getSpec({ url: 
'https://git-wip-us.apache.org/repos/asf/cordova-plugin-camera.git' }, 
'/some/path', 'VRPlugin')`


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled

[GitHub] cordova-lib pull request #584: CB-12361 : added tests for plugin/save.js

2017-08-31 Thread stevengill
Github user stevengill commented on a diff in the pull request:

https://github.com/apache/cordova-lib/pull/584#discussion_r136487019
  
--- Diff: spec/cordova/plugin/save.spec.js ---
@@ -54,15 +61,145 @@ describe('cordova/plugin/save', function () {
 });
 });
 describe('happy path', function () {
-it('should remove all plugins from config.xml and re-add new ones 
based on those retrieved from fetch.json');
-it('should only add top-level plugins to config.xml');
-it('should write individual plugin specs to config.xml');
-it('should write individual plugin variables to config.xml');
+it('should remove all plugins from config.xml and re-add new ones 
based on those retrieved from fetch.json', function (done) {
+save(projectRoot).then(function () {
+
expect(cfg_parser_mock.prototype.removePlugin).toHaveBeenCalledWith('VRPlugin');
+
expect(cfg_parser_mock.prototype.removePlugin).toHaveBeenCalledWith('MastodonSocialPlugin');
+}).fail(function (e) {
+expect(e).toBeUndefined();
+fail('did not expect fail handler to be invoked');
+}).done(done);
+});
+
+it('should only add top-level plugins to config.xml', function 
(done) {
+var fake_fetch_json =
+{'VRPlugin': {'source': {
+'type': 'registry',
+'id': 'id'
+},
+'is_top_level': true
+},
+'MastodonSocialPlugin': { 'source': {
+'type': 'registry',
+'id': 'id'
+},
+'is_top_level': false }};
+
+
fs.readFileSync.and.returnValue(JSON.stringify(fake_fetch_json));
+save(projectRoot).then(function () {
+
expect(cfg_parser_mock.prototype.addPlugin).toHaveBeenCalledWith(Object({ name: 
'VRPlugin' }), [ ]);
--- End diff --

Maybe add another expect to show that the non top level plugin didn't get 
installed. Something like 
```
expect(cfg_parser_mock.prototype.addPlugin).toHaveBeenCalledWith(Object({ 
name: 'MastodonSocialPlugin' }), [ ]);```


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org



[GitHub] cordova-lib pull request #584: CB-12361 : added tests for plugin/save.js

2017-08-31 Thread stevengill
Github user stevengill commented on a diff in the pull request:

https://github.com/apache/cordova-lib/pull/584#discussion_r136487749
  
--- Diff: spec/cordova/plugin/save.spec.js ---
@@ -54,15 +61,145 @@ describe('cordova/plugin/save', function () {
 });
 });
 describe('happy path', function () {
-it('should remove all plugins from config.xml and re-add new ones 
based on those retrieved from fetch.json');
-it('should only add top-level plugins to config.xml');
-it('should write individual plugin specs to config.xml');
-it('should write individual plugin variables to config.xml');
+it('should remove all plugins from config.xml and re-add new ones 
based on those retrieved from fetch.json', function (done) {
+save(projectRoot).then(function () {
+
expect(cfg_parser_mock.prototype.removePlugin).toHaveBeenCalledWith('VRPlugin');
+
expect(cfg_parser_mock.prototype.removePlugin).toHaveBeenCalledWith('MastodonSocialPlugin');
+}).fail(function (e) {
+expect(e).toBeUndefined();
+fail('did not expect fail handler to be invoked');
+}).done(done);
+});
+
+it('should only add top-level plugins to config.xml', function 
(done) {
+var fake_fetch_json =
+{'VRPlugin': {'source': {
+'type': 'registry',
+'id': 'id'
+},
+'is_top_level': true
+},
+'MastodonSocialPlugin': { 'source': {
+'type': 'registry',
+'id': 'id'
+},
+'is_top_level': false }};
+
+
fs.readFileSync.and.returnValue(JSON.stringify(fake_fetch_json));
+save(projectRoot).then(function () {
+
expect(cfg_parser_mock.prototype.addPlugin).toHaveBeenCalledWith(Object({ name: 
'VRPlugin' }), [ ]);
--- End diff --

Maybe for this test, you can redo the checks from the first test. That is 
confirm that these two expects are run before the addPlugin one

```

expect(cfg_parser_mock.prototype.removePlugin).toHaveBeenCalledWith('VRPlugin');

expect(cfg_parser_mock.prototype.removePlugin).toHaveBeenCalledWith('MastodonSocialPlugin');
```

I think they should pass. If they do, you can also update the description 
of this test to reflect that the plugins are being removed first and then only 
top level plugins are being restored. 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org



[GitHub] cordova-js pull request #148: CB-13232: added test for cordova's unquie loca...

2017-08-30 Thread stevengill
GitHub user stevengill opened a pull request:

https://github.com/apache/cordova-js/pull/148

CB-13232: added test for cordova's unquie local style require

This is in response to https://github.com/apache/cordova-js/pull/146

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/stevengill/cordova-js CB-13232

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/cordova-js/pull/148.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #148


commit 133850782a8166ad1b51e5a248ac5cc6869de726
Author: Steve Gill <stevengil...@gmail.com>
Date:   2017-08-31T00:47:09Z

CB-13232: added test for cordova's unquie local style require




---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org



[GitHub] cordova-js issue #146: CB-13163: fix using relative paths in calls to requir...

2017-08-30 Thread stevengill
Github user stevengill commented on the issue:

https://github.com/apache/cordova-js/pull/146
  
If you checkout https://github.com/apache/cordova-ios/pull/333, you can see 
that I fixed the relative require for console. 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org



[GitHub] cordova-lib issue #590: CB-13145: Create playservices version preference in ...

2017-08-30 Thread stevengill
Github user stevengill commented on the issue:

https://github.com/apache/cordova-lib/pull/590
  
This has been merged. Can you close it


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org



[GitHub] cordova-docs issue #723: CB-13214: added browser and serve release blog post

2017-08-29 Thread stevengill
Github user stevengill commented on the issue:

https://github.com/apache/cordova-docs/pull/723
  
Let me know if you have any feedback!


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org



[GitHub] cordova-docs pull request #723: CB-13214: added browser and serve release bl...

2017-08-29 Thread stevengill
GitHub user stevengill opened a pull request:

https://github.com/apache/cordova-docs/pull/723

CB-13214: added browser and serve release blog post



### Platforms affected


### What does this PR do?


### What testing has been done on this change?


### Checklist
- [ ] [Reported an issue](http://cordova.apache.org/contribute/issues.html) 
in the JIRA database
- [ ] Commit message follows the format: "CB-3232: (android) Fix bug with 
resolving file paths", where CB- is the JIRA ID & "android" is the platform 
affected.
- [ ] Added automated test coverage as appropriate for this change.


You can merge this pull request into a Git repository by running:

$ git pull https://github.com/stevengill/cordova-docs CB-13214

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/cordova-docs/pull/723.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #723


commit 1c939f6abc90baf128903ad8185b8d7c86a4bd42
Author: Steve Gill <stevengil...@gmail.com>
Date:   2017-08-30T00:41:05Z

CB-13214: added browser and serve release blog post




---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org



[GitHub] cordova-lib pull request #590: CB-13145: Create playservices version prefere...

2017-08-28 Thread stevengill
Github user stevengill commented on a diff in the pull request:

https://github.com/apache/cordova-lib/pull/590#discussion_r135664127
  
--- Diff: src/cordova/plugin/util.js ---
@@ -35,3 +40,39 @@ function saveToConfigXmlOn (config_json, options) {
 var autosave = config_json.auto_save_plugins || false;
 return autosave || options.save;
 }
+
+/*
+ * Merges cli and config.xml variables.
+ *
+ * @param   {object}pluginInfo
+ * @param   {object}config.xml
+ * @param   {object}options
+ *
+ * @return  {object}object containing the new merged variables
+ */
+
+function mergeVariables (pluginInfo, cfg, opts) {
+// Validate top-level required variables
+var pluginVariables = pluginInfo.getPreferences();
+opts.cli_variables = opts.cli_variables || {};
+var pluginEntry = cfg.getPlugin(pluginInfo.id);
+// Get variables from config.xml
+var configVariables = pluginEntry ? pluginEntry.variables : {};
+// Add config variable if it's missing in cli_variables
+Object.keys(configVariables).forEach(function (variable) {
+opts.cli_variables[variable] = opts.cli_variables[variable] || 
configVariables[variable];
+});
+var missingVariables = Object.keys(pluginVariables)
+.filter(function (variableName) {
+// discard variables with default value
+return !(pluginVariables[variableName] || 
opts.cli_variables[variableName]);
+});
+
+if (missingVariables.length) {
+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));
--- End diff --

get rid of Q dep


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org



[GitHub] cordova-lib pull request #590: CB-13145: Create playservices version prefere...

2017-08-28 Thread stevengill
Github user stevengill commented on a diff in the pull request:

https://github.com/apache/cordova-lib/pull/590#discussion_r135664112
  
--- Diff: src/cordova/plugin/util.js ---
@@ -35,3 +40,39 @@ function saveToConfigXmlOn (config_json, options) {
 var autosave = config_json.auto_save_plugins || false;
 return autosave || options.save;
 }
+
+/*
+ * Merges cli and config.xml variables.
+ *
+ * @param   {object}pluginInfo
+ * @param   {object}config.xml
+ * @param   {object}options
+ *
+ * @return  {object}object containing the new merged variables
+ */
+
+function mergeVariables (pluginInfo, cfg, opts) {
+// Validate top-level required variables
+var pluginVariables = pluginInfo.getPreferences();
+opts.cli_variables = opts.cli_variables || {};
+var pluginEntry = cfg.getPlugin(pluginInfo.id);
+// Get variables from config.xml
+var configVariables = pluginEntry ? pluginEntry.variables : {};
+// Add config variable if it's missing in cli_variables
+Object.keys(configVariables).forEach(function (variable) {
+opts.cli_variables[variable] = opts.cli_variables[variable] || 
configVariables[variable];
+});
+var missingVariables = Object.keys(pluginVariables)
+.filter(function (variableName) {
+// discard variables with default value
+return !(pluginVariables[variableName] || 
opts.cli_variables[variableName]);
+});
+
+if (missingVariables.length) {
+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));
--- End diff --

Change this to `throw new CordovaError(msg)`


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org



[GitHub] cordova-lib pull request #569: CB-12361: added main function unit tests for ...

2017-08-25 Thread stevengill
Github user stevengill commented on a diff in the pull request:

https://github.com/apache/cordova-lib/pull/569#discussion_r135331136
  
--- Diff: spec/cordova/plugin/add.spec.js ---
@@ -21,16 +21,82 @@
 /* globals fail */
 
 var Q = require('q');
-var add = require('../../../src/cordova/plugin/add');
+var rewire = require('rewire');
+var add = rewire('../../../src/cordova/plugin/add');
+var plugman = require('../../../src/plugman/plugman');
+var cordova_util = require('../../../src/cordova/util');
+var path = require('path');
+var fs = require('fs');
+var config = require('../../../src/cordova/config');
+var events = require('cordova-common').events;
+var registry = require('../../../src/plugman/registry/registry');
+var plugin_util = require('../../../src/cordova/plugin/util');
 
-describe('cordova/plugin/add', function () {
+fdescribe('cordova/plugin/add', function () {
 var projectRoot = '/some/path';
 var hook_mock;
+var Cfg_parser_mock = function () {};
+var cfg_parser_revert_mock;
+var plugin_info_provider_mock = function () {};
+var plugin_info_provider_revert_mock;
+var plugin_info;
+var package_json_mock;
+
 beforeEach(function () {
 hook_mock = jasmine.createSpyObj('hooks runner mock', ['fire']);
 hook_mock.fire.and.returnValue(Q());
+Cfg_parser_mock.prototype = jasmine.createSpyObj('config parser 
prototype mock', ['getPlugin', 'removePlugin', 'addPlugin', 'write']);
+/* eslint-disable */
+Cfg_parser_mock.prototype.getPlugin;
+Cfg_parser_mock.prototype.getPlugin;
--- End diff --

good catch!


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org



[GitHub] cordova-js issue #146: CB-13163: fix using relative paths in calls to requir...

2017-08-24 Thread stevengill
Github user stevengill commented on the issue:

https://github.com/apache/cordova-js/pull/146
  
I had to revert this as it broke all of our plugins that use local require 
syntax (ex. cordova device-motion at 
https://github.com/apache/cordova-plugin-device-motion/blob/master/www/accelerometer.js#L29)

cordova's require handles local require very differently than node's local 
require.

Cordova require would convert `require('./Acceleration')` into 
`cordova-plugin-device-motion.Acceleration`. Then is uses some sort of mapping 
to grab and include the code (I haven't dug into it)

This PR wants `require(./logger)` to become 
`require('cordova/plugin/ios/logger')` instead of 
`cordova-plugin-console.logger`. (Once again, I haven't dug into why the 
mapping isn't working now that the plugin is local. I think it is because the 
console plugin code is baked into cordova.js instead of being included during 
runtime like plugin js code).

Instead of this PR, I'm going to fix the require call at 
https://github.com/apache/cordova-ios/blob/master/cordova-js-src/plugin/ios/console.js#L24.

It will change from `require('./logger')` into 
`require('cordova/plugin/ios/logger')`. Now the local require is working for 
logger and cordova's local require continues working as it has been for the 
last few years.

Ultimately, cordova.js & how we load plugins needs to rethought and 
refactored. 






---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org



[GitHub] cordova-docs issue #719: CB-13076,CB-13068 : Removed references of cordova d...

2017-08-21 Thread stevengill
Github user stevengill commented on the issue:

https://github.com/apache/cordova-docs/pull/719
  
hmm, @filmaj screen shot makes it seem like he is testing on dev and not on 
7.x. Maybe there are other references that still need to be removed. 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org



[GitHub] cordova-lib issue #574: CB-12838 : prevented sorting and alphabetizing platf...

2017-08-16 Thread stevengill
Github user stevengill commented on the issue:

https://github.com/apache/cordova-lib/pull/574
  
LGTM


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org



[GitHub] cordova-lib pull request #574: CB-12838 : prevented sorting and alphabetizin...

2017-08-11 Thread stevengill
Github user stevengill commented on a diff in the pull request:

https://github.com/apache/cordova-lib/pull/574#discussion_r132781446
  
--- Diff: src/cordova/restore-util.js ---
@@ -341,19 +340,29 @@ function installPluginsFromConfigXML (args) {
 fs.writeFileSync(pkgJsonPath, JSON.stringify(pkgJson, null, 
2), 'utf8');
 }
 }
-// Write config.xml (only if plugins exist in package.json).
+// 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);
-}
-cfg.removePlugin(plugID);
-if (mergedPluginSpecs[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) ||
--- End diff --

so to confirm, either `((pluginIdConfig.indexOf(plugID) > 0) && 
(mergedPluginSpecs[plugID]) &&(configXMLPlugin.variables !== 
comboObject[plugID]))` or `((mergedPluginSpecs[plugID] !== 
configXMLPlugin.spec) || (configXMLPlugin.variables !== comboObject[plugID]))` 
need to be true


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org



[GitHub] cordova-lib pull request #574: CB-12838 : prevented sorting and alphabetizin...

2017-08-11 Thread stevengill
Github user stevengill commented on a diff in the pull request:

https://github.com/apache/cordova-lib/pull/574#discussion_r132780726
  
--- Diff: src/cordova/restore-util.js ---
@@ -341,19 +340,29 @@ function installPluginsFromConfigXML (args) {
 fs.writeFileSync(pkgJsonPath, JSON.stringify(pkgJson, null, 
2), 'utf8');
 }
 }
-// Write config.xml (only if plugins exist in package.json).
+// 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);
-}
-cfg.removePlugin(plugID);
-if (mergedPluginSpecs[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]))) ||
--- End diff --

I think you have an extra bracket around this one


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org



[GitHub] cordova-lib pull request #574: CB-12838 : prevented sorting and alphabetizin...

2017-08-11 Thread stevengill
Github user stevengill commented on a diff in the pull request:

https://github.com/apache/cordova-lib/pull/574#discussion_r132780476
  
--- Diff: src/cordova/restore-util.js ---
@@ -341,19 +340,29 @@ function installPluginsFromConfigXML (args) {
 fs.writeFileSync(pkgJsonPath, JSON.stringify(pkgJson, null, 
2), 'utf8');
 }
 }
-// Write config.xml (only if plugins exist in package.json).
+// 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);
-}
-cfg.removePlugin(plugID);
-if (mergedPluginSpecs[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]) &&
--- End diff --

so this if statement, `(pluginIdConfig.indexOf(plugID) > 0)`, what if 
`plugID` is in the 0th index? I think you want `>=`


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org



[GitHub] cordova-lib pull request #581: CB-12361 : added plugin remove tests

2017-08-11 Thread stevengill
Github user stevengill commented on a diff in the pull request:

https://github.com/apache/cordova-lib/pull/581#discussion_r132758854
  
--- Diff: spec/cordova/plugin/remove.spec.js ---
@@ -32,18 +66,123 @@ describe('cordova/plugin/remove', function () {
 expect(e.message).toContain('No plugin specified');
 }).done(done);
 });
-it('should require that a provided plugin be installed in the 
current project');
+
+it('should require that a provided plugin be installed in the 
current project', function (done) {
+var opts = { plugins: [ undefined ] };
+remove(projectRoot, 'plugin', hook_mock, opts).then(function 
() {
+fail('success handler unexpectedly invoked');
+}).fail(function (e) {
+expect(e.message).toContain('is not present in the 
project');
+}).done(done);
+});
 });
 describe('happy path', function () {
-it('should fire the before_plugin_rm hook');
-it('should call plugman.uninstall.uninstallPlatform for each 
platform installed in the project and for each provided plugin');
-it('should trigger a prepare if 
plugman.uninstall.uninstallPlatform returned something falsy');
-it('should call plugman.uninstall.uninstallPlugin once plugin has 
been uninstalled for each platform');
+it('should fire the before_plugin_rm hook', function (done) {
+var opts = { important: 'options', plugins: [] };
+remove(projectRoot, 'cordova-plugin-splashscreen', hook_mock, 
opts).then(function () {
+
expect(hook_mock.fire).toHaveBeenCalledWith('before_plugin_rm', opts);
+}).fail(function (e) {
+fail('fail handler unexpectedly invoked');
+console.error(e);
+}).done(done);
+});
+
+it('should call plugman.uninstall.uninstallPlatform for each 
platform installed in the project and for each provided plugin', function 
(done) {
+
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 () {
+
expect(plugman.uninstall.uninstallPlatform).toHaveBeenCalled();
+expect(events.emit).toHaveBeenCalledWith('verbose', 
jasmine.stringMatching('plugman.uninstall on plugin 
"cordova-plugin-splashscreen" for platform "ios"'));
+expect(events.emit).toHaveBeenCalledWith('verbose', 
jasmine.stringMatching('plugman.uninstall on plugin 
"cordova-plugin-splashscreen" for platform "android"'));
+}).fail(function (e) {
+fail('fail handler unexpectedly invoked');
+console.error(e);
+}).done(done);
+});
+
+it('should trigger a prepare if 
plugman.uninstall.uninstallPlatform returned something falsy', function (done) {
+
remove.validatePluginId.and.returnValue('cordova-plugin-splashscreen');
+plugman.uninstall.uninstallPlatform.and.returnValue(Q(false));
+var opts = {important: 'options', plugins: 
['cordova-plugin-splashscreen']};
+remove(projectRoot, 'cordova-plugin-splashscreen', hook_mock, 
opts).then(function () {
+
expect(plugman.uninstall.uninstallPlatform).toHaveBeenCalled();
+expect(events.emit).toHaveBeenCalledWith('verbose', 
'Calling prepare.');
--- End diff --

i'd suggest removing this expect event due to my other comment


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org



[GitHub] cordova-lib pull request #577: CB-12361 : added tests for platform/list.js

2017-08-11 Thread stevengill
Github user stevengill commented on a diff in the pull request:

https://github.com/apache/cordova-lib/pull/577#discussion_r132738598
  
--- Diff: spec/cordova/platform/list.spec.js ---
@@ -0,0 +1,75 @@
+/**
+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 events = require('cordova-common').events;
+var Q = require('q');
+var rewire = require('rewire');
+var platform_list = rewire('../../../src/cordova/platform/list');
--- End diff --

i don't think you use rewire anywhere in this file


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org



[GitHub] cordova-lib pull request #577: CB-12361 : added tests for platform/list.js

2017-08-11 Thread stevengill
Github user stevengill commented on a diff in the pull request:

https://github.com/apache/cordova-lib/pull/577#discussion_r132742737
  
--- Diff: spec/cordova/platform/list.spec.js ---
@@ -0,0 +1,75 @@
+/**
+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 events = require('cordova-common').events;
+var Q = require('q');
+var rewire = require('rewire');
+var platform_list = rewire('../../../src/cordova/platform/list');
+var platform_metadata = require('../../../src/cordova/platform_metadata');
+var cordova_util = require('../../../src/cordova/util');
+var fail;
+
+describe('cordova/platform/list', function () {
+var hooks_mock;
+var projectRoot = '/some/path';
+
+beforeEach(function () {
+hooks_mock = jasmine.createSpyObj('hooksRunner mock', ['fire']);
+hooks_mock.fire.and.returnValue(Q());
+spyOn(cordova_util, 
'getInstalledPlatformsWithVersions').and.callThrough();
+spyOn(events, 'emit');
+spyOn(platform_metadata, 'save');
+spyOn(cordova_util, 'requireNoCache').and.returnValue({});
+});
+
+it('should fire the before_platform_ls hook', function () {
+platform_list(hooks_mock, projectRoot, {save: true});
+expect(hooks_mock.fire).toHaveBeenCalledWith('before_platform_ls', 
Object({ save: true }));
+});
+
+it('should file the after_platform_ls hook', function (done) {
--- End diff --

type. Should be fire


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org



[GitHub] cordova-lib pull request #582: CB-12361 : added tests for plugin/search.js

2017-08-11 Thread stevengill
Github user stevengill commented on a diff in the pull request:

https://github.com/apache/cordova-lib/pull/582#discussion_r132736159
  
--- Diff: spec/cordova/plugin/search.spec.js ---
@@ -44,7 +44,36 @@ describe('cordova/plugin/search', function () {
 console.error(e);
 }).done(done);
 });
-it('should open a link to cordova.apache.org/plugins if no plugins are 
provided as parameter');
-it('should open a link to cordova.apache.org/plugins, providing the 
plugins passed in as a query-string parameter');
-it('should fire the after_plugin_search hook');
+
+it('should open a link to cordova.apache.org/plugins if no plugins are 
provided as parameter', function (done) {
+var opts = {important: 'options', plugins: []};
+search(hook_mock, opts).then(function () {
+expect(opener_mock).toHaveBeenCalled();
+}).fail(function (e) {
+console.log(e);
+fail('fail handler unexpectedly invoked');
+console.error(e);
+}).done(done);
+});
+
+it('should open a link to cordova.apache.org/plugins, providing the 
plugins passed in as a query-string parameter', function (done) {
+var opts = {important: 'options', plugins: 
['cordova-plugin-camera', 'cordova-plugin-splashscreen']};
+search(hook_mock, opts).then(function () {
+expect(opener_mock).toHaveBeenCalled();
+}).fail(function (e) {
+console.log(e);
--- End diff --

same


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org



[GitHub] cordova-lib pull request #582: CB-12361 : added tests for plugin/search.js

2017-08-11 Thread stevengill
Github user stevengill commented on a diff in the pull request:

https://github.com/apache/cordova-lib/pull/582#discussion_r132736117
  
--- Diff: spec/cordova/plugin/search.spec.js ---
@@ -44,7 +44,36 @@ describe('cordova/plugin/search', function () {
 console.error(e);
 }).done(done);
 });
-it('should open a link to cordova.apache.org/plugins if no plugins are 
provided as parameter');
-it('should open a link to cordova.apache.org/plugins, providing the 
plugins passed in as a query-string parameter');
-it('should fire the after_plugin_search hook');
+
+it('should open a link to cordova.apache.org/plugins if no plugins are 
provided as parameter', function (done) {
+var opts = {important: 'options', plugins: []};
+search(hook_mock, opts).then(function () {
+expect(opener_mock).toHaveBeenCalled();
+}).fail(function (e) {
+console.log(e);
--- End diff --

Don't think you need console.log(e) since you console.error two lines below


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org



[GitHub] cordova-cli issue #284: Update readme.md - Proposal of corrections

2017-08-09 Thread stevengill
Github user stevengill commented on the issue:

https://github.com/apache/cordova-cli/pull/284
  
Changes look good to me


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org



[GitHub] cordova-cli pull request #284: Update readme.md - Proposal of corrections

2017-08-09 Thread stevengill
Github user stevengill commented on a diff in the pull request:

https://github.com/apache/cordova-cli/pull/284#discussion_r132319941
  
--- Diff: doc/readme.md ---
@@ -374,7 +374,7 @@ based on the following criteria (listed in order of 
precedence):
 
 ### Examples
 
-- Add `cordova-plugin-camera` and `cordova-plugin-file` to the project and 
it be be saved to `config.xml` & `package.json`. Use `../plugins` directory to 
search for the plugins.
+- Add `cordova-plugin-camera` and `cordova-plugin-file` to the project and 
save it to `config.xml` & `package.json`. Use `../plugins` directory to search 
for the plugins.
--- End diff --

The example below doesn't need the `--save` flag anymore. Somehow this 
example got missed when all of the others got updated. 

`cordova plugin add cordova-plugin-camera cordova-plugin-file --searchpath 
../plugins`


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org



[GitHub] cordova-lib pull request #578: CB-12361 : added unit-tests for getPlatformDe...

2017-07-27 Thread stevengill
Github user stevengill commented on a diff in the pull request:

https://github.com/apache/cordova-lib/pull/578#discussion_r129996355
  
--- Diff: spec/cordova/platform/getPlatformDetailsFromDir.spec.js ---
@@ -0,0 +1,79 @@
+/**
+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 path = require('path');
+var fs = require('fs');
+var Q = require('q');
+var rewire = require('rewire');
+var cordova_util = require('../../../src/cordova/util');
+var platform_getPlatformDetails = 
rewire('../../../src/cordova/platform/getPlatformDetailsFromDir');
+var events = require('cordova-common').events;
+var fail;
+
+describe('cordova/platform/getPlatformDetailsFromDir', function () {
+var package_json_mock;
+package_json_mock = jasmine.createSpyObj('package json mock', 
['cordova', 'dependencies']);
+package_json_mock.name = 'io.cordova.hellocordova';
+package_json_mock.version = '1.0.0';
+
+beforeEach(function () {
+spyOn(Q, 'reject');
+spyOn(fs, 'existsSync');
+spyOn(cordova_util, 'requireNoCache');
+spyOn(events, 'emit');
+});
+
+it('should throw if no config.xml or pkgJson', function (done) {
+platform_getPlatformDetails('dir', ['ios']);
+expect(Q.reject).toHaveBeenCalledWith(jasmine.stringMatching(/does 
not seem to contain a valid package.json or a valid Cordova platform/));
+done();
+});
+
+it('should throw if no platform is provided', function (done) {
+cordova_util.requireNoCache.and.returnValue({});
+platform_getPlatformDetails('dir');
+expect(Q.reject).toHaveBeenCalledWith(jasmine.stringMatching(/does 
not seem to contain a Cordova platform:/));
+done();
+});
+
+it('should return a promise with platform and version', function 
(done) {
+fs.existsSync.and.callFake(function(filePath) {
+if(path.basename(filePath) === 'package.json') {
+return true;
+} else {
+return false;
+}
+});
+cordova_util.requireNoCache.and.returnValue(package_json_mock);
+platform_getPlatformDetails('dir', ['cordova-android'])
+.then(function(result) {
+expect(result.platform).toBe('io.cordova.hellocordova');
+expect(result.version).toBe('1.0.0');
+expect(Q.reject).not.toHaveBeenCalled();
+}).fail(function (err) {
+fail('unexpected failure handler invoked!');
+console.error(err);
+}).done(done);
+});
+
+it('should remove the cordova- prefix from the platform name for known 
platforms', function (done) {
+platform_getPlatformDetails.platformFromName('cordova-ios');
+expect(events.emit).toHaveBeenCalledWith('verbose', 
jasmine.stringMatching(/Removing "cordova-" prefix/));
+
expect(platform_getPlatformDetails.platformFromName('cordova-ios')).toBe('ios');
--- End diff --

yup


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org



[GitHub] cordova-lib issue #580: CB-12895 : Replaced jshint with eslint

2017-07-27 Thread stevengill
Github user stevengill commented on the issue:

https://github.com/apache/cordova-lib/pull/580
  
Huge PR! haha LGTM. I did a quick scan. Looks like spacing formatting for 
the most part. A few unused vars that you added exceptions for. 

Merge it


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org



[GitHub] cordova-lib pull request #579: CB-12361 : added tests for save.js

2017-07-27 Thread stevengill
Github user stevengill commented on a diff in the pull request:

https://github.com/apache/cordova-lib/pull/579#discussion_r129981636
  
--- Diff: spec/cordova/platform/save.spec.js ---
@@ -0,0 +1,71 @@
+/**
+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 Q = require('q');
+var rewire = require('rewire');
+var platform_save = rewire('../../../src/cordova/platform/save');
+var platform_metadata = require('../../../src/cordova/platform_metadata');
+var fail;
+var semver = require('semver');
+
+describe('cordova/platform/save', function () {
+var hooks_mock;
+var projectRoot = '/some/path';
+var cfg_parser_mock = function () {};
+var cfg_parser_revert_mock;
+
+beforeEach(function () {
+spyOn(semver, 'valid');
+cfg_parser_mock.prototype = jasmine.createSpyObj('config parser 
mock', ['write', 'removeEngine', 'addEngine','getEngines']);
+cfg_parser_revert_mock = platform_save.__set__('ConfigParser', 
cfg_parser_mock);
+cfg_parser_mock.prototype.getEngines.and.returnValue(['android']);
+});
+
+afterEach(function () {
+cfg_parser_revert_mock();
+});
+
+it('should first remove platforms already in config.xml', function 
(done) {
+platform_save(hooks_mock, projectRoot, {save : true})
+.then(function(res){
+
expect(cfg_parser_mock.prototype.getEngines).toHaveBeenCalled();
+
expect(cfg_parser_mock.prototype.removeEngine).toHaveBeenCalled();
+}).fail(function (err) {
+fail('unexpected failure handler invoked!');
+console.error(err);
+}).done(done);
+});
+
+it('add and write to config.xml', function (done) {
+spyOn(platform_metadata, 
'getPlatformVersions').and.returnValue(Q(['6.3.0']));
--- End diff --

so getPlatformVersions returns in the format of `{platform: platform, 
version: version}`. So instead of returning `Q([6.3.0])`, you could return 
`Q({platform: 'android', version: 6.3.0})`. That way the first argument for 
line 58 won't be undefined. 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org



[GitHub] cordova-lib pull request #579: CB-12361 : added tests for save.js

2017-07-27 Thread stevengill
Github user stevengill commented on a diff in the pull request:

https://github.com/apache/cordova-lib/pull/579#discussion_r129981726
  
--- Diff: spec/cordova/platform/save.spec.js ---
@@ -0,0 +1,71 @@
+/**
+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 Q = require('q');
+var rewire = require('rewire');
+var platform_save = rewire('../../../src/cordova/platform/save');
+var platform_metadata = require('../../../src/cordova/platform_metadata');
+var fail;
+var semver = require('semver');
+
+describe('cordova/platform/save', function () {
+var hooks_mock;
+var projectRoot = '/some/path';
+var cfg_parser_mock = function () {};
+var cfg_parser_revert_mock;
+
+beforeEach(function () {
+spyOn(semver, 'valid');
+cfg_parser_mock.prototype = jasmine.createSpyObj('config parser 
mock', ['write', 'removeEngine', 'addEngine','getEngines']);
+cfg_parser_revert_mock = platform_save.__set__('ConfigParser', 
cfg_parser_mock);
+cfg_parser_mock.prototype.getEngines.and.returnValue(['android']);
+});
+
+afterEach(function () {
+cfg_parser_revert_mock();
+});
+
+it('should first remove platforms already in config.xml', function 
(done) {
+platform_save(hooks_mock, projectRoot, {save : true})
+.then(function(res){
+
expect(cfg_parser_mock.prototype.getEngines).toHaveBeenCalled();
+
expect(cfg_parser_mock.prototype.removeEngine).toHaveBeenCalled();
+}).fail(function (err) {
+fail('unexpected failure handler invoked!');
+console.error(err);
+}).done(done);
+});
+
+it('add and write to config.xml', function (done) {
+spyOn(platform_metadata, 
'getPlatformVersions').and.returnValue(Q(['6.3.0']));
+semver.valid.and.returnValue('6.0.0');
+platform_save(hooks_mock, projectRoot, {save : true})
+.then(function(result) {
+
expect(cfg_parser_mock.prototype.addEngine).toHaveBeenCalledWith(undefined, 
'~6.0.0');
+expect(cfg_parser_mock.prototype.write).toHaveBeenCalled();
+}).fail(function (err) {
+fail('unexpected failure handler invoked!');
+console.error(err);
+}).done(done);
+});
+
+it('should first remove platforms already in config.xml', function 
(done) {
--- End diff --

I think you forgot to update the description here when you copied the first 
test :)


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org



[GitHub] cordova-lib pull request #578: CB-12361 : added unit-tests for getPlatformDe...

2017-07-27 Thread stevengill
Github user stevengill commented on a diff in the pull request:

https://github.com/apache/cordova-lib/pull/578#discussion_r129926425
  
--- Diff: spec/cordova/platform/getPlatformDetailsFromDir.spec.js ---
@@ -0,0 +1,79 @@
+/**
+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 path = require('path');
+var fs = require('fs');
+var Q = require('q');
+var rewire = require('rewire');
+var cordova_util = require('../../../src/cordova/util');
+var platform_getPlatformDetails = 
rewire('../../../src/cordova/platform/getPlatformDetailsFromDir');
+var events = require('cordova-common').events;
+var fail;
+
+describe('cordova/platform/getPlatformDetailsFromDir', function () {
+var package_json_mock;
+package_json_mock = jasmine.createSpyObj('package json mock', 
['cordova', 'dependencies']);
+package_json_mock.name = 'io.cordova.hellocordova';
+package_json_mock.version = '1.0.0';
+
+beforeEach(function () {
+spyOn(Q, 'reject');
+spyOn(fs, 'existsSync');
+spyOn(cordova_util, 'requireNoCache');
+spyOn(events, 'emit');
+});
+
+it('should throw if no config.xml or pkgJson', function (done) {
+platform_getPlatformDetails('dir', ['ios']);
+expect(Q.reject).toHaveBeenCalledWith(jasmine.stringMatching(/does 
not seem to contain a valid package.json or a valid Cordova platform/));
+done();
+});
+
+it('should throw if no platform is provided', function (done) {
+cordova_util.requireNoCache.and.returnValue({});
+platform_getPlatformDetails('dir');
+expect(Q.reject).toHaveBeenCalledWith(jasmine.stringMatching(/does 
not seem to contain a Cordova platform:/));
+done();
+});
+
+it('should return a promise with platform and version', function 
(done) {
+fs.existsSync.and.callFake(function(filePath) {
+if(path.basename(filePath) === 'package.json') {
+return true;
+} else {
+return false;
+}
+});
+cordova_util.requireNoCache.and.returnValue(package_json_mock);
+platform_getPlatformDetails('dir', ['cordova-android'])
+.then(function(result) {
+expect(result.platform).toBe('io.cordova.hellocordova');
+expect(result.version).toBe('1.0.0');
+expect(Q.reject).not.toHaveBeenCalled();
+}).fail(function (err) {
+fail('unexpected failure handler invoked!');
+console.error(err);
+}).done(done);
+});
+
+it('should remove the cordova- prefix from the platform name for known 
platforms', function (done) {
+platform_getPlatformDetails.platformFromName('cordova-ios');
+expect(events.emit).toHaveBeenCalledWith('verbose', 
jasmine.stringMatching(/Removing "cordova-" prefix/));
+
expect(platform_getPlatformDetails.platformFromName('cordova-ios')).toBe('ios');
--- End diff --

You can take the expect from line 76 and combine it with line 74 and delete 
76. So line 74 gets replaced by line 76. Line 75 should still pass


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org



[GitHub] cordova-lib pull request #577: CB-12361 : added tests for platform/list.js

2017-07-26 Thread stevengill
Github user stevengill commented on a diff in the pull request:

https://github.com/apache/cordova-lib/pull/577#discussion_r129730654
  
--- Diff: spec/cordova/platform/list.spec.js ---
@@ -0,0 +1,66 @@
+/**
+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 events = require('cordova-common').events;
+var Q = require('q');
+var events = require('cordova-common').events;
+var rewire = require('rewire');
+var platform_list = rewire('../../../src/cordova/platform/list');
+var platform_metadata = require('../../../src/cordova/platform_metadata');
+var cordova_util = require('../../../src/cordova/util');
+var fail;
+
+describe('cordova/platform/list', function () {
+var hooks_mock;
+var projectRoot = '/some/path';
+
+beforeEach(function () {
+hooks_mock = jasmine.createSpyObj('hooksRunner mock', ['fire']);
+hooks_mock.fire.and.returnValue(Q());
+spyOn(cordova_util, 
'getInstalledPlatformsWithVersions').and.callThrough();
+spyOn(events, 'emit');
+spyOn(platform_metadata, 'save');
+spyOn(cordova_util, 'requireNoCache').and.returnValue({});
+});
+
+it('should fire the before_platform_ls hook', function () {
+platform_list(hooks_mock, projectRoot, {save : true});
+expect(hooks_mock.fire).toHaveBeenCalledWith('before_platform_ls', 
Object({ save: true }));
+});
+
+it('should file the after_platform_ls hook',function (done) {
+platform_list(hooks_mock, projectRoot, {save : true})
+.then(function(result) {
+
expect(hooks_mock.fire).toHaveBeenCalledWith('after_platform_ls', Object({ 
save: true }));
+}).fail(function (err) {
+console.log(err.message);
+fail('unexpected failure handler invoked!');
+console.error(err);
+}).done(done);
+});
+
+it('should print results of available platforms',function (done) {
+platform_list(hooks_mock, projectRoot, {save : true})
+.then(function(result) {
+expect(events.emit).toHaveBeenCalledWith('results', 
jasmine.stringMatching(/Installed platforms:/));
+}).fail(function (err) {
+console.log(err.message);
--- End diff --

I don't think your fail function should `console.log(err.message)` and 
`console.log(err)`. Just `console.log(err)` is sufficient. 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org



[GitHub] cordova-lib pull request #577: CB-12361 : added tests for platform/list.js

2017-07-26 Thread stevengill
Github user stevengill commented on a diff in the pull request:

https://github.com/apache/cordova-lib/pull/577#discussion_r129730691
  
--- Diff: spec/cordova/platform/list.spec.js ---
@@ -0,0 +1,66 @@
+/**
+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 events = require('cordova-common').events;
+var Q = require('q');
+var events = require('cordova-common').events;
+var rewire = require('rewire');
+var platform_list = rewire('../../../src/cordova/platform/list');
+var platform_metadata = require('../../../src/cordova/platform_metadata');
+var cordova_util = require('../../../src/cordova/util');
+var fail;
+
+describe('cordova/platform/list', function () {
+var hooks_mock;
+var projectRoot = '/some/path';
+
+beforeEach(function () {
+hooks_mock = jasmine.createSpyObj('hooksRunner mock', ['fire']);
+hooks_mock.fire.and.returnValue(Q());
+spyOn(cordova_util, 
'getInstalledPlatformsWithVersions').and.callThrough();
+spyOn(events, 'emit');
+spyOn(platform_metadata, 'save');
+spyOn(cordova_util, 'requireNoCache').and.returnValue({});
+});
+
+it('should fire the before_platform_ls hook', function () {
+platform_list(hooks_mock, projectRoot, {save : true});
+expect(hooks_mock.fire).toHaveBeenCalledWith('before_platform_ls', 
Object({ save: true }));
+});
+
+it('should file the after_platform_ls hook',function (done) {
+platform_list(hooks_mock, projectRoot, {save : true})
+.then(function(result) {
+
expect(hooks_mock.fire).toHaveBeenCalledWith('after_platform_ls', Object({ 
save: true }));
+}).fail(function (err) {
+console.log(err.message);
+fail('unexpected failure handler invoked!');
+console.error(err);
+}).done(done);
+});
+
+it('should print results of available platforms',function (done) {
+platform_list(hooks_mock, projectRoot, {save : true})
+.then(function(result) {
+expect(events.emit).toHaveBeenCalledWith('results', 
jasmine.stringMatching(/Installed platforms:/));
+}).fail(function (err) {
+console.log(err.message);
--- End diff --

This is the same case as the previous test too


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org



[GitHub] cordova-lib pull request #576: CB-12361 : added unit tests for remove platfo...

2017-07-26 Thread stevengill
Github user stevengill commented on a diff in the pull request:

https://github.com/apache/cordova-lib/pull/576#discussion_r129713116
  
--- Diff: spec/cordova/platform/remove.spec.js ---
@@ -0,0 +1,160 @@
+/**
+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 path = require('path');
+var fs = require('fs');
+var Q = require('q');
+var events = require('cordova-common').events;
+var rewire = require('rewire');
+var platform_remove = rewire('../../../src/cordova/platform/remove');
+var platform_metadata = require('../../../src/cordova/platform_metadata');
+var cordova_util = require('../../../src/cordova/util');
+var promiseutil = require('../../../src/util/promise-util');
+var fail;
+
+describe('cordova/platform/remove', function () {
+var projectRoot = '/some/path';
+var cfg_parser_mock = function () {};
+var cfg_parser_revert_mock;
+var hooks_mock;
+var package_json_mock;
+package_json_mock = jasmine.createSpyObj('package json mock', 
['cordova', 'dependencies']);
+package_json_mock.dependencies = {};
+package_json_mock.cordova = {};
+
+var hooksRunnerRevert;
+
+beforeEach(function () {
+hooks_mock = jasmine.createSpyObj('hooksRunner mock', ['fire']);
+hooks_mock.fire.and.returnValue(Q());
+hooksRunnerRevert = platform_remove.__set__('HooksRunner', 
function () {});
+cfg_parser_mock.prototype = jasmine.createSpyObj('config parser 
mock', ['write', 'removeEngine']);
+cfg_parser_revert_mock = platform_remove.__set__('ConfigParser', 
cfg_parser_mock);
+spyOn(fs, 'existsSync').and.returnValue(false);
+spyOn(fs, 'writeFileSync');
+spyOn(cordova_util, 'removePlatformPluginsJson');
+spyOn(events, 'emit');
+spyOn(cordova_util, 'requireNoCache').and.returnValue({});
+});
+afterEach(function () {
+cfg_parser_revert_mock();
+hooksRunnerRevert();
+});
+describe('error/warning conditions', function () {
+it('should require specifying at least one platform', function 
(done) {
+platform_remove('remove', hooks_mock).then(function () {
+fail('remove success handler unexpectedly invoked');
+}).fail(function (e) {
+expect(e.message).toContain('No platform(s) specified.');
+}).done(done);
+});
+});
+describe('happy path (success conditions)', function () {
+it('should fire the before_platform_* hook', function () {
--- End diff --

for some reason, when i run this test I get `Parsing false failed`. But the 
test passes


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org



[GitHub] cordova-test-platform pull request #1: Doc requirements for platform api exp...

2017-07-21 Thread stevengill
Github user stevengill commented on a diff in the pull request:

https://github.com/apache/cordova-test-platform/pull/1#discussion_r128869649
  
--- Diff: PlatformRequirements.md ---
@@ -0,0 +1,144 @@
+
+# New Platform Checklist
+ 
+## Stand-alone scripts
+ 
+bin/create scripts
+- bin/create _(typically a node script)_
+- bin/create.bat for windows
+- windows .bat file typically just calls bin/create with node
+ 
+bin/update
+- not entirely sure this code is run, or needs to exist with newish 
non-destructive platform updates
+ 
+## Package Expectations
+ 
+- Platforms must have a package.json in their root.
+- Package.json exports a 'main', usually `"main": "src/cordova/Api.js"`
+- This allows other modules to simply require() the path to this 
platform and get access to the Api.
+ 
+## Api (Platform) Expectations
+- The PlatformApi class
+- The PlatformApi class is an abstraction around a particular platform 
that exposes all the actions, properties, and methods for this platform so they 
are accessible programmatically.
+- It can install & uninstall plugins with all source files, web assets 
and js files.
+- It exposes a single 'prepare' method to provide a way for 
cordova-lib to apply a project's setting/www content to the platform.
+- The PlatformApi class should be implemented by each platform that 
wants to use the new API flow. For those platforms, which don't provide their 
own PlatformApi, there will be a polyfill in cordova-lib.
+- Platforms that implement their own PlatformApi instance should 
implement all prototype methods of this class to be fully compatible with 
cordova-lib.
+- The PlatformApi instance should define the following field:
+- platform : This is a String that defines a platform name.
+ 
+- Api.js exports static functions
+- there is currently a requirement that the file be called Api.js 
(todo:change that)
+ 
+ 
+- Api.js exports static function `updatePlatform(destination, options, 
events);`
+- PlatformApi.updatePlatform = function (cordovaProject, options) {};
+- The `updatePlatform` method is equal to the bin/update script. It 
should update an already installed platform. It should accept a CordovaProject 
instance, that defines a project structure and configuration, that should be 
applied to the new platform, and an options object.
+- cordovaProject: This is a CordovaProject instance that defines a 
project structure and configuration, that should be applied to the new 
platform. This argument is optional and if not defined, that platform is used 
as a standalone project and not as part of a Cordova project.
+- options : This is an options object. The most common options are :
+- options.customTemplate : This is a path to custom template, that 
should override the default one from platform.
+- options.link : This is a flag that should indicate that the 
platform's sources will be linked to the installed platform instead of copying.
+ - The `updatePlatform` method must return a promise, which is either 
fulfilled with a PlatformApi instance or rejected with a CordovaError.
+ 
+- Api.js exports static function `createPlatform(destination, cfg, 
options, events);`
+- PlatformApi.createPlatform = function(cordovaProject, options) {};
+- The `createPlatform method` is equal to the bin/create script. It 
should install the platform to a specified directory and create a platform 
project. It should accept a CordovaProject instance, that defines a project 
structure and configuration, that should be applied to the new platform, and an 
options object.
+- cordovaProject : This is a CordovaProject instance that defines a 
project structure and configuration, that should be applied to the new 
platform. This argument is optional and if not defined, that platform is used 
as a standalone project and not as part of a Cordova project.
+- options : This is an options object. The most common options are :
+- options.customTemplate : This is a path to custom template, that 
should override the default one from the platform.
--- End diff --

maybe just link to the template documentation? 
https://cordova.apache.org/docs/en/latest/guide/cli/template.html


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org



[GitHub] cordova-test-platform pull request #1: Doc requirements for platform api exp...

2017-07-21 Thread stevengill
Github user stevengill commented on a diff in the pull request:

https://github.com/apache/cordova-test-platform/pull/1#discussion_r128869395
  
--- Diff: PlatformRequirements.md ---
@@ -0,0 +1,144 @@
+
+# New Platform Checklist
+ 
+## Stand-alone scripts
+ 
+bin/create scripts
+- bin/create _(typically a node script)_
+- bin/create.bat for windows
+- windows .bat file typically just calls bin/create with node
+ 
+bin/update
+- not entirely sure this code is run, or needs to exist with newish 
non-destructive platform updates
+ 
+## Package Expectations
+ 
+- Platforms must have a package.json in their root.
+- Package.json exports a 'main', usually `"main": "src/cordova/Api.js"`
+- This allows other modules to simply require() the path to this 
platform and get access to the Api.
+ 
+## Api (Platform) Expectations
+- The PlatformApi class
+- The PlatformApi class is an abstraction around a particular platform 
that exposes all the actions, properties, and methods for this platform so they 
are accessible programmatically.
+- It can install & uninstall plugins with all source files, web assets 
and js files.
+- It exposes a single 'prepare' method to provide a way for 
cordova-lib to apply a project's setting/www content to the platform.
+- The PlatformApi class should be implemented by each platform that 
wants to use the new API flow. For those platforms, which don't provide their 
own PlatformApi, there will be a polyfill in cordova-lib.
--- End diff --

yup. polyfill is on the way out


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org



[GitHub] cordova-lib issue #558: CB-12766 Consistently write JSON with 2 spaces inden...

2017-07-17 Thread stevengill
Github user stevengill commented on the issue:

https://github.com/apache/cordova-lib/pull/558
  
Hey @G-Rath 

CB-12810 is very much still open :). 
https://issues.apache.org/jira/browse/CB-12810

Thanks for your comment. This is something we will hopefully get to soon.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org



[GitHub] cordova-lib pull request #573: CB-12361 : updated addHelper tests

2017-07-12 Thread stevengill
Github user stevengill commented on a diff in the pull request:

https://github.com/apache/cordova-lib/pull/573#discussion_r127055093
  
--- Diff: spec/cordova/platform/addHelper.spec.js ---
@@ -16,34 +16,439 @@
 */
 /* eslint-env jasmine */
 
+var path = require('path');
+var fs = require('fs');
+var Q = require('q');
+var shell = require('shelljs');
+var events = require('cordova-common').events;
+var rewire = require('rewire');
+var platform_addHelper = rewire('../../../src/cordova/platform/addHelper');
+var platform_module = require('../../../src/cordova/platform');
+var platform_metadata = require('../../../src/cordova/platform_metadata');
+var cordova_util = require('../../../src/cordova/util');
+var cordova_config = require('../../../src/cordova/config');
+var plugman = require('../../../src/plugman/plugman');
+var fetch_metadata = require('../../../src/plugman/util/metadata');
+var lazy_load = require('../../../src/cordova/lazy_load');
+var prepare = require('../../../src/cordova/prepare');
+var gitclone = require('../../../src/gitclone');
+var fail;
+
 describe('cordova/platform/addHelper', function () {
+var projectRoot = '/some/path';
+// These _mock and _revert_mock objects use rewire as the modules 
these mocks replace
+// during testing all return functions, which we cannot spy on using 
jasmine.
+// Thus, we replace these modules inside the scope of addHelper.js 
using rewire, and shim
+// in these _mock test dummies. The test dummies themselves are 
constructed using
+// jasmine.createSpy inside the first beforeEach.
+var cfg_parser_mock = function () {};
+var cfg_parser_revert_mock;
+var hooks_mock;
+var platform_api_mock;
+var fetch_mock;
+var fetch_revert_mock;
+var prepare_mock;
+var prepare_revert_mock;
+var fake_platform = {
+'platform': 'atari'
+};
+var package_json_mock;
+package_json_mock = jasmine.createSpyObj('package json mock', 
['cordova', 'dependencies']);
+package_json_mock.dependencies = {};
+package_json_mock.cordova = {};
+
+beforeEach(function () {
+hooks_mock = jasmine.createSpyObj('hooksRunner mock', ['fire']);
+hooks_mock.fire.and.returnValue(Q());
+cfg_parser_mock.prototype = jasmine.createSpyObj('config parser 
mock', ['write', 'removeEngine', 'addEngine', 'getHookScripts']);
+cfg_parser_revert_mock = 
platform_addHelper.__set__('ConfigParser', cfg_parser_mock);
+fetch_mock = jasmine.createSpy('fetch mock').and.returnValue(Q());
+fetch_revert_mock = platform_addHelper.__set__('fetch', 
fetch_mock);
+prepare_mock = jasmine.createSpy('prepare 
mock').and.returnValue(Q());
+prepare_mock.preparePlatforms = 
jasmine.createSpy('preparePlatforms mock').and.returnValue(Q());
+prepare_revert_mock = platform_addHelper.__set__('prepare', 
prepare_mock);
+spyOn(shell, 'mkdir');
+spyOn(fs, 'existsSync').and.returnValue(false);
+spyOn(fs, 'writeFileSync');
+spyOn(cordova_util, 
'projectConfig').and.returnValue(path.join(projectRoot, 'config.xml'));
+spyOn(cordova_util, 'isDirectory').and.returnValue(false);
+spyOn(cordova_util, 'fixRelativePath').and.callFake(function 
(input) { return input; });
+spyOn(cordova_util, 'isUrl').and.returnValue(false);
+spyOn(cordova_util, 'hostSupports').and.returnValue(true);
+spyOn(cordova_util, 'removePlatformPluginsJson');
+spyOn(cordova_config, 'read').and.returnValue({});
+spyOn(events, 'emit');
+// Fake platform details we will use for our mocks, returned by 
either
+// getPlatfromDetailsFromDir (in the local-directory case), or
+// downloadPlatform (in every other case)
+spyOn(platform_module, 
'getPlatformDetailsFromDir').and.returnValue(Q(fake_platform));
+spyOn(platform_addHelper, 
'downloadPlatform').and.returnValue(Q(fake_platform));
+spyOn(platform_addHelper, 
'getVersionFromConfigFile').and.returnValue(false);
+spyOn(platform_addHelper, 
'installPluginsForNewPlatform').and.returnValue(Q());
+platform_api_mock = jasmine.createSpyObj('platform api mock', 
['createPlatform', 'updatePlatform']);
+platform_api_mock.createPlatform.and.returnValue(Q());
+platform_api_mock.updatePlatform.and.returnValue(Q());
+spyOn(cordova_util, 
'getPlatformApiFunction').and.returnValue(platform_api_mock);
+spyOn(platform_metadata, 'save');
+spyOn(cordova_util, 'requireNoCache').and.returnValue({});
+});
+afterEach(function () {
+cfg_parser_revert_mock

[GitHub] cordova-lib pull request #573: CB-12361 : updated addHelper tests

2017-07-12 Thread stevengill
Github user stevengill commented on a diff in the pull request:

https://github.com/apache/cordova-lib/pull/573#discussion_r127032404
  
--- Diff: spec/cordova/platform/addHelper.spec.js ---
@@ -16,34 +16,439 @@
 */
 /* eslint-env jasmine */
 
+var path = require('path');
+var fs = require('fs');
+var Q = require('q');
+var shell = require('shelljs');
+var events = require('cordova-common').events;
+var rewire = require('rewire');
+var platform_addHelper = rewire('../../../src/cordova/platform/addHelper');
+var platform_module = require('../../../src/cordova/platform');
+var platform_metadata = require('../../../src/cordova/platform_metadata');
+var cordova_util = require('../../../src/cordova/util');
+var cordova_config = require('../../../src/cordova/config');
+var plugman = require('../../../src/plugman/plugman');
+var fetch_metadata = require('../../../src/plugman/util/metadata');
+var lazy_load = require('../../../src/cordova/lazy_load');
+var prepare = require('../../../src/cordova/prepare');
+var gitclone = require('../../../src/gitclone');
+var fail;
+
 describe('cordova/platform/addHelper', function () {
+var projectRoot = '/some/path';
+// These _mock and _revert_mock objects use rewire as the modules 
these mocks replace
+// during testing all return functions, which we cannot spy on using 
jasmine.
+// Thus, we replace these modules inside the scope of addHelper.js 
using rewire, and shim
+// in these _mock test dummies. The test dummies themselves are 
constructed using
+// jasmine.createSpy inside the first beforeEach.
+var cfg_parser_mock = function () {};
+var cfg_parser_revert_mock;
+var hooks_mock;
+var platform_api_mock;
+var fetch_mock;
+var fetch_revert_mock;
+var prepare_mock;
+var prepare_revert_mock;
+var fake_platform = {
+'platform': 'atari'
+};
+var package_json_mock;
+package_json_mock = jasmine.createSpyObj('package json mock', 
['cordova', 'dependencies']);
+package_json_mock.dependencies = {};
+package_json_mock.cordova = {};
+
+beforeEach(function () {
+hooks_mock = jasmine.createSpyObj('hooksRunner mock', ['fire']);
+hooks_mock.fire.and.returnValue(Q());
+cfg_parser_mock.prototype = jasmine.createSpyObj('config parser 
mock', ['write', 'removeEngine', 'addEngine', 'getHookScripts']);
+cfg_parser_revert_mock = 
platform_addHelper.__set__('ConfigParser', cfg_parser_mock);
+fetch_mock = jasmine.createSpy('fetch mock').and.returnValue(Q());
+fetch_revert_mock = platform_addHelper.__set__('fetch', 
fetch_mock);
+prepare_mock = jasmine.createSpy('prepare 
mock').and.returnValue(Q());
+prepare_mock.preparePlatforms = 
jasmine.createSpy('preparePlatforms mock').and.returnValue(Q());
+prepare_revert_mock = platform_addHelper.__set__('prepare', 
prepare_mock);
+spyOn(shell, 'mkdir');
+spyOn(fs, 'existsSync').and.returnValue(false);
+spyOn(fs, 'writeFileSync');
+spyOn(cordova_util, 
'projectConfig').and.returnValue(path.join(projectRoot, 'config.xml'));
+spyOn(cordova_util, 'isDirectory').and.returnValue(false);
+spyOn(cordova_util, 'fixRelativePath').and.callFake(function 
(input) { return input; });
+spyOn(cordova_util, 'isUrl').and.returnValue(false);
+spyOn(cordova_util, 'hostSupports').and.returnValue(true);
+spyOn(cordova_util, 'removePlatformPluginsJson');
+spyOn(cordova_config, 'read').and.returnValue({});
+spyOn(events, 'emit');
+// Fake platform details we will use for our mocks, returned by 
either
+// getPlatfromDetailsFromDir (in the local-directory case), or
+// downloadPlatform (in every other case)
+spyOn(platform_module, 
'getPlatformDetailsFromDir').and.returnValue(Q(fake_platform));
+spyOn(platform_addHelper, 
'downloadPlatform').and.returnValue(Q(fake_platform));
+spyOn(platform_addHelper, 
'getVersionFromConfigFile').and.returnValue(false);
+spyOn(platform_addHelper, 
'installPluginsForNewPlatform').and.returnValue(Q());
+platform_api_mock = jasmine.createSpyObj('platform api mock', 
['createPlatform', 'updatePlatform']);
+platform_api_mock.createPlatform.and.returnValue(Q());
+platform_api_mock.updatePlatform.and.returnValue(Q());
+spyOn(cordova_util, 
'getPlatformApiFunction').and.returnValue(platform_api_mock);
+spyOn(platform_metadata, 'save');
+spyOn(cordova_util, 'requireNoCache').and.returnValue({});
+});
+afterEach(function () {
+cfg_parser_revert_mock

[GitHub] cordova-lib pull request #573: CB-12361 : updated addHelper tests

2017-07-11 Thread stevengill
Github user stevengill commented on a diff in the pull request:

https://github.com/apache/cordova-lib/pull/573#discussion_r126843121
  
--- Diff: spec/cordova/platform/addHelper.spec.js ---
@@ -16,34 +16,439 @@
 */
 /* eslint-env jasmine */
 
+var path = require('path');
+var fs = require('fs');
+var Q = require('q');
+var shell = require('shelljs');
+var events = require('cordova-common').events;
+var rewire = require('rewire');
+var platform_addHelper = rewire('../../../src/cordova/platform/addHelper');
+var platform_module = require('../../../src/cordova/platform');
+var platform_metadata = require('../../../src/cordova/platform_metadata');
+var cordova_util = require('../../../src/cordova/util');
+var cordova_config = require('../../../src/cordova/config');
+var plugman = require('../../../src/plugman/plugman');
+var fetch_metadata = require('../../../src/plugman/util/metadata');
+var lazy_load = require('../../../src/cordova/lazy_load');
+var prepare = require('../../../src/cordova/prepare');
+var gitclone = require('../../../src/gitclone');
+var fail;
+
 describe('cordova/platform/addHelper', function () {
+var projectRoot = '/some/path';
+// These _mock and _revert_mock objects use rewire as the modules 
these mocks replace
+// during testing all return functions, which we cannot spy on using 
jasmine.
+// Thus, we replace these modules inside the scope of addHelper.js 
using rewire, and shim
+// in these _mock test dummies. The test dummies themselves are 
constructed using
+// jasmine.createSpy inside the first beforeEach.
+var cfg_parser_mock = function () {};
+var cfg_parser_revert_mock;
+var hooks_mock;
+var platform_api_mock;
+var fetch_mock;
+var fetch_revert_mock;
+var prepare_mock;
+var prepare_revert_mock;
+var fake_platform = {
+'platform': 'atari'
+};
+var package_json_mock;
+package_json_mock = jasmine.createSpyObj('package json mock', 
['cordova', 'dependencies']);
+package_json_mock.dependencies = {};
+package_json_mock.cordova = {};
+
+beforeEach(function () {
+hooks_mock = jasmine.createSpyObj('hooksRunner mock', ['fire']);
+hooks_mock.fire.and.returnValue(Q());
+cfg_parser_mock.prototype = jasmine.createSpyObj('config parser 
mock', ['write', 'removeEngine', 'addEngine', 'getHookScripts']);
+cfg_parser_revert_mock = 
platform_addHelper.__set__('ConfigParser', cfg_parser_mock);
+fetch_mock = jasmine.createSpy('fetch mock').and.returnValue(Q());
+fetch_revert_mock = platform_addHelper.__set__('fetch', 
fetch_mock);
+prepare_mock = jasmine.createSpy('prepare 
mock').and.returnValue(Q());
+prepare_mock.preparePlatforms = 
jasmine.createSpy('preparePlatforms mock').and.returnValue(Q());
+prepare_revert_mock = platform_addHelper.__set__('prepare', 
prepare_mock);
+spyOn(shell, 'mkdir');
+spyOn(fs, 'existsSync').and.returnValue(false);
+spyOn(fs, 'writeFileSync');
+spyOn(cordova_util, 
'projectConfig').and.returnValue(path.join(projectRoot, 'config.xml'));
+spyOn(cordova_util, 'isDirectory').and.returnValue(false);
+spyOn(cordova_util, 'fixRelativePath').and.callFake(function 
(input) { return input; });
+spyOn(cordova_util, 'isUrl').and.returnValue(false);
+spyOn(cordova_util, 'hostSupports').and.returnValue(true);
+spyOn(cordova_util, 'removePlatformPluginsJson');
+spyOn(cordova_config, 'read').and.returnValue({});
+spyOn(events, 'emit');
+// Fake platform details we will use for our mocks, returned by 
either
+// getPlatfromDetailsFromDir (in the local-directory case), or
+// downloadPlatform (in every other case)
+spyOn(platform_module, 
'getPlatformDetailsFromDir').and.returnValue(Q(fake_platform));
+spyOn(platform_addHelper, 
'downloadPlatform').and.returnValue(Q(fake_platform));
+spyOn(platform_addHelper, 
'getVersionFromConfigFile').and.returnValue(false);
+spyOn(platform_addHelper, 
'installPluginsForNewPlatform').and.returnValue(Q());
+platform_api_mock = jasmine.createSpyObj('platform api mock', 
['createPlatform', 'updatePlatform']);
+platform_api_mock.createPlatform.and.returnValue(Q());
+platform_api_mock.updatePlatform.and.returnValue(Q());
+spyOn(cordova_util, 
'getPlatformApiFunction').and.returnValue(platform_api_mock);
+spyOn(platform_metadata, 'save');
+spyOn(cordova_util, 'requireNoCache').and.returnValue({});
+});
+afterEach(function () {
+cfg_parser_revert_mock

[GitHub] cordova-lib pull request #573: CB-12361 : updated addHelper tests

2017-07-11 Thread stevengill
Github user stevengill commented on a diff in the pull request:

https://github.com/apache/cordova-lib/pull/573#discussion_r126842836
  
--- Diff: spec/cordova/platform/addHelper.spec.js ---
@@ -16,34 +16,439 @@
 */
 /* eslint-env jasmine */
 
+var path = require('path');
+var fs = require('fs');
+var Q = require('q');
+var shell = require('shelljs');
+var events = require('cordova-common').events;
+var rewire = require('rewire');
+var platform_addHelper = rewire('../../../src/cordova/platform/addHelper');
+var platform_module = require('../../../src/cordova/platform');
+var platform_metadata = require('../../../src/cordova/platform_metadata');
+var cordova_util = require('../../../src/cordova/util');
+var cordova_config = require('../../../src/cordova/config');
+var plugman = require('../../../src/plugman/plugman');
+var fetch_metadata = require('../../../src/plugman/util/metadata');
+var lazy_load = require('../../../src/cordova/lazy_load');
+var prepare = require('../../../src/cordova/prepare');
+var gitclone = require('../../../src/gitclone');
+var fail;
+
 describe('cordova/platform/addHelper', function () {
+var projectRoot = '/some/path';
+// These _mock and _revert_mock objects use rewire as the modules 
these mocks replace
+// during testing all return functions, which we cannot spy on using 
jasmine.
+// Thus, we replace these modules inside the scope of addHelper.js 
using rewire, and shim
+// in these _mock test dummies. The test dummies themselves are 
constructed using
+// jasmine.createSpy inside the first beforeEach.
+var cfg_parser_mock = function () {};
+var cfg_parser_revert_mock;
+var hooks_mock;
+var platform_api_mock;
+var fetch_mock;
+var fetch_revert_mock;
+var prepare_mock;
+var prepare_revert_mock;
+var fake_platform = {
+'platform': 'atari'
+};
+var package_json_mock;
+package_json_mock = jasmine.createSpyObj('package json mock', 
['cordova', 'dependencies']);
+package_json_mock.dependencies = {};
+package_json_mock.cordova = {};
+
+beforeEach(function () {
+hooks_mock = jasmine.createSpyObj('hooksRunner mock', ['fire']);
+hooks_mock.fire.and.returnValue(Q());
+cfg_parser_mock.prototype = jasmine.createSpyObj('config parser 
mock', ['write', 'removeEngine', 'addEngine', 'getHookScripts']);
+cfg_parser_revert_mock = 
platform_addHelper.__set__('ConfigParser', cfg_parser_mock);
+fetch_mock = jasmine.createSpy('fetch mock').and.returnValue(Q());
+fetch_revert_mock = platform_addHelper.__set__('fetch', 
fetch_mock);
+prepare_mock = jasmine.createSpy('prepare 
mock').and.returnValue(Q());
+prepare_mock.preparePlatforms = 
jasmine.createSpy('preparePlatforms mock').and.returnValue(Q());
+prepare_revert_mock = platform_addHelper.__set__('prepare', 
prepare_mock);
+spyOn(shell, 'mkdir');
+spyOn(fs, 'existsSync').and.returnValue(false);
+spyOn(fs, 'writeFileSync');
+spyOn(cordova_util, 
'projectConfig').and.returnValue(path.join(projectRoot, 'config.xml'));
+spyOn(cordova_util, 'isDirectory').and.returnValue(false);
+spyOn(cordova_util, 'fixRelativePath').and.callFake(function 
(input) { return input; });
+spyOn(cordova_util, 'isUrl').and.returnValue(false);
+spyOn(cordova_util, 'hostSupports').and.returnValue(true);
+spyOn(cordova_util, 'removePlatformPluginsJson');
+spyOn(cordova_config, 'read').and.returnValue({});
+spyOn(events, 'emit');
+// Fake platform details we will use for our mocks, returned by 
either
+// getPlatfromDetailsFromDir (in the local-directory case), or
+// downloadPlatform (in every other case)
+spyOn(platform_module, 
'getPlatformDetailsFromDir').and.returnValue(Q(fake_platform));
+spyOn(platform_addHelper, 
'downloadPlatform').and.returnValue(Q(fake_platform));
+spyOn(platform_addHelper, 
'getVersionFromConfigFile').and.returnValue(false);
+spyOn(platform_addHelper, 
'installPluginsForNewPlatform').and.returnValue(Q());
+platform_api_mock = jasmine.createSpyObj('platform api mock', 
['createPlatform', 'updatePlatform']);
+platform_api_mock.createPlatform.and.returnValue(Q());
+platform_api_mock.updatePlatform.and.returnValue(Q());
+spyOn(cordova_util, 
'getPlatformApiFunction').and.returnValue(platform_api_mock);
+spyOn(platform_metadata, 'save');
+spyOn(cordova_util, 'requireNoCache').and.returnValue({});
+});
+afterEach(function () {
+cfg_parser_revert_mock

[GitHub] cordova-lib pull request #573: CB-12361 : updated addHelper tests

2017-07-11 Thread stevengill
Github user stevengill commented on a diff in the pull request:

https://github.com/apache/cordova-lib/pull/573#discussion_r126844076
  
--- Diff: spec/cordova/platform/addHelper.spec.js ---
@@ -16,34 +16,439 @@
 */
 /* eslint-env jasmine */
 
+var path = require('path');
+var fs = require('fs');
+var Q = require('q');
+var shell = require('shelljs');
+var events = require('cordova-common').events;
+var rewire = require('rewire');
+var platform_addHelper = rewire('../../../src/cordova/platform/addHelper');
+var platform_module = require('../../../src/cordova/platform');
+var platform_metadata = require('../../../src/cordova/platform_metadata');
+var cordova_util = require('../../../src/cordova/util');
+var cordova_config = require('../../../src/cordova/config');
+var plugman = require('../../../src/plugman/plugman');
+var fetch_metadata = require('../../../src/plugman/util/metadata');
+var lazy_load = require('../../../src/cordova/lazy_load');
+var prepare = require('../../../src/cordova/prepare');
+var gitclone = require('../../../src/gitclone');
+var fail;
+
 describe('cordova/platform/addHelper', function () {
+var projectRoot = '/some/path';
+// These _mock and _revert_mock objects use rewire as the modules 
these mocks replace
+// during testing all return functions, which we cannot spy on using 
jasmine.
+// Thus, we replace these modules inside the scope of addHelper.js 
using rewire, and shim
+// in these _mock test dummies. The test dummies themselves are 
constructed using
+// jasmine.createSpy inside the first beforeEach.
+var cfg_parser_mock = function () {};
+var cfg_parser_revert_mock;
+var hooks_mock;
+var platform_api_mock;
+var fetch_mock;
+var fetch_revert_mock;
+var prepare_mock;
+var prepare_revert_mock;
+var fake_platform = {
+'platform': 'atari'
+};
+var package_json_mock;
+package_json_mock = jasmine.createSpyObj('package json mock', 
['cordova', 'dependencies']);
+package_json_mock.dependencies = {};
+package_json_mock.cordova = {};
+
+beforeEach(function () {
+hooks_mock = jasmine.createSpyObj('hooksRunner mock', ['fire']);
+hooks_mock.fire.and.returnValue(Q());
+cfg_parser_mock.prototype = jasmine.createSpyObj('config parser 
mock', ['write', 'removeEngine', 'addEngine', 'getHookScripts']);
+cfg_parser_revert_mock = 
platform_addHelper.__set__('ConfigParser', cfg_parser_mock);
+fetch_mock = jasmine.createSpy('fetch mock').and.returnValue(Q());
+fetch_revert_mock = platform_addHelper.__set__('fetch', 
fetch_mock);
+prepare_mock = jasmine.createSpy('prepare 
mock').and.returnValue(Q());
+prepare_mock.preparePlatforms = 
jasmine.createSpy('preparePlatforms mock').and.returnValue(Q());
+prepare_revert_mock = platform_addHelper.__set__('prepare', 
prepare_mock);
+spyOn(shell, 'mkdir');
+spyOn(fs, 'existsSync').and.returnValue(false);
+spyOn(fs, 'writeFileSync');
+spyOn(cordova_util, 
'projectConfig').and.returnValue(path.join(projectRoot, 'config.xml'));
+spyOn(cordova_util, 'isDirectory').and.returnValue(false);
+spyOn(cordova_util, 'fixRelativePath').and.callFake(function 
(input) { return input; });
+spyOn(cordova_util, 'isUrl').and.returnValue(false);
+spyOn(cordova_util, 'hostSupports').and.returnValue(true);
+spyOn(cordova_util, 'removePlatformPluginsJson');
+spyOn(cordova_config, 'read').and.returnValue({});
+spyOn(events, 'emit');
+// Fake platform details we will use for our mocks, returned by 
either
+// getPlatfromDetailsFromDir (in the local-directory case), or
+// downloadPlatform (in every other case)
+spyOn(platform_module, 
'getPlatformDetailsFromDir').and.returnValue(Q(fake_platform));
+spyOn(platform_addHelper, 
'downloadPlatform').and.returnValue(Q(fake_platform));
+spyOn(platform_addHelper, 
'getVersionFromConfigFile').and.returnValue(false);
+spyOn(platform_addHelper, 
'installPluginsForNewPlatform').and.returnValue(Q());
+platform_api_mock = jasmine.createSpyObj('platform api mock', 
['createPlatform', 'updatePlatform']);
+platform_api_mock.createPlatform.and.returnValue(Q());
+platform_api_mock.updatePlatform.and.returnValue(Q());
+spyOn(cordova_util, 
'getPlatformApiFunction').and.returnValue(platform_api_mock);
+spyOn(platform_metadata, 'save');
+spyOn(cordova_util, 'requireNoCache').and.returnValue({});
+});
+afterEach(function () {
+cfg_parser_revert_mock

[GitHub] cordova-lib pull request #573: CB-12361 : updated addHelper tests

2017-06-29 Thread stevengill
Github user stevengill commented on a diff in the pull request:

https://github.com/apache/cordova-lib/pull/573#discussion_r124893454
  
--- Diff: spec/cordova/platform/addHelper.spec.js ---
@@ -16,34 +16,459 @@
 */
 /* eslint-env jasmine */
 
+var path = require('path');
+var fs = require('fs');
+var Q = require('q');
+var shell = require('shelljs');
+var events = require('cordova-common').events;
+var rewire = require('rewire');
+var platform_addHelper = rewire('../../../src/cordova/platform/addHelper');
+var platform_module = require('../../../src/cordova/platform');
+var platform_metadata = require('../../../src/cordova/platform_metadata');
+var cordova_util = require('../../../src/cordova/util');
+var cordova_config = require('../../../src/cordova/config');
+var plugman = require('../../../src/plugman/plugman');
+var fetch_metadata = require('../../../src/plugman/util/metadata');
+var lazy_load = require('../../../src/cordova/lazy_load');
+// require module here
+// spy on it and return 
+var cordova = require('../../../src/cordova/cordova');
+var prepare = require('../../../src/cordova/prepare');
+var gitclone = require('../../../src/gitclone');
+var fail;
+
 describe('cordova/platform/addHelper', function () {
+var projectRoot = '/some/path';
+// These _mock and _revert_mock objects use rewire as the modules 
these mocks replace
+// during testing all return functions, which we cannot spy on using 
jasmine.
+// Thus, we replace these modules inside the scope of addHelper.js 
using rewire, and shim
+// in these _mock test dummies. The test dummies themselves are 
constructed using
+// jasmine.createSpy inside the first beforeEach.
+var cfg_parser_mock = function () {};
+var cfg_parser_revert_mock;
+var hooks_mock;
+var platform_api_mock;
+var fetch_mock;
+var fetch_revert_mock;
+var prepare_mock;
+var prepare_revert_mock;
+var fake_platform = {
+'platform': 'atari'
+};
+beforeEach(function () {
+hooks_mock = jasmine.createSpyObj('hooksRunner mock', ['fire']);
+hooks_mock.fire.and.returnValue(Q());
+cfg_parser_mock.prototype = jasmine.createSpyObj('config parser 
mock', ['write', 'removeEngine', 'addEngine', 'getHookScripts']);
+cfg_parser_revert_mock = 
platform_addHelper.__set__('ConfigParser', cfg_parser_mock);
+fetch_mock = jasmine.createSpy('fetch mock').and.returnValue(Q());
+fetch_revert_mock = platform_addHelper.__set__('fetch', 
fetch_mock);
+prepare_mock = jasmine.createSpy('prepare 
mock').and.returnValue(Q());
+prepare_mock.preparePlatforms = 
jasmine.createSpy('preparePlatforms mock').and.returnValue(Q());
+prepare_revert_mock = platform_addHelper.__set__('prepare', 
prepare_mock);
+spyOn(shell, 'mkdir');
+spyOn(fs, 'existsSync').and.returnValue(false);
+spyOn(fs, 'writeFileSync');
+spyOn(cordova_util, 
'projectConfig').and.returnValue(path.join(projectRoot, 'config.xml'));
+spyOn(cordova_util, 'isDirectory').and.returnValue(false);
+spyOn(cordova_util, 'fixRelativePath').and.callFake(function 
(input) { return input; });
+spyOn(cordova_util, 'isUrl').and.returnValue(false);
+spyOn(cordova_util, 'hostSupports').and.returnValue(true);
+spyOn(cordova_util, 'removePlatformPluginsJson');
+spyOn(cordova_config, 'read').and.returnValue({});
+// Fake platform details we will use for our mocks, returned by 
either
+// getPlatfromDetailsFromDir (in the local-directory case), or
+// downloadPlatform (in every other case)
+spyOn(platform_module, 
'getPlatformDetailsFromDir').and.returnValue(Q(fake_platform));
+spyOn(platform_addHelper, 
'downloadPlatform').and.returnValue(Q(fake_platform));
+spyOn(platform_addHelper, 
'getVersionFromConfigFile').and.returnValue(false);
+spyOn(platform_addHelper, 
'installPluginsForNewPlatform').and.returnValue(Q());
+platform_api_mock = jasmine.createSpyObj('platform api mock', 
['createPlatform', 'updatePlatform']);
+platform_api_mock.createPlatform.and.returnValue(Q());
+platform_api_mock.updatePlatform.and.returnValue(Q());
+spyOn(cordova_util, 
'getPlatformApiFunction').and.returnValue(platform_api_mock);
+spyOn(platform_metadata, 'save');
+});
+afterEach(function () {
+cfg_parser_revert_mock();
+fetch_revert_mock();
+prepare_revert_mock();
+});
 describe('error/warning conditions', function () {
-it('should require specifying at least one platform

[GitHub] cordova-lib pull request #573: CB-12361 : updated addHelper tests

2017-06-29 Thread stevengill
Github user stevengill commented on a diff in the pull request:

https://github.com/apache/cordova-lib/pull/573#discussion_r124892113
  
--- Diff: spec/cordova/platform/addHelper.spec.js ---
@@ -16,34 +16,459 @@
 */
 /* eslint-env jasmine */
 
+var path = require('path');
+var fs = require('fs');
+var Q = require('q');
+var shell = require('shelljs');
+var events = require('cordova-common').events;
+var rewire = require('rewire');
+var platform_addHelper = rewire('../../../src/cordova/platform/addHelper');
+var platform_module = require('../../../src/cordova/platform');
+var platform_metadata = require('../../../src/cordova/platform_metadata');
+var cordova_util = require('../../../src/cordova/util');
+var cordova_config = require('../../../src/cordova/config');
+var plugman = require('../../../src/plugman/plugman');
+var fetch_metadata = require('../../../src/plugman/util/metadata');
+var lazy_load = require('../../../src/cordova/lazy_load');
+// require module here
+// spy on it and return 
+var cordova = require('../../../src/cordova/cordova');
+var prepare = require('../../../src/cordova/prepare');
+var gitclone = require('../../../src/gitclone');
+var fail;
+
 describe('cordova/platform/addHelper', function () {
+var projectRoot = '/some/path';
+// These _mock and _revert_mock objects use rewire as the modules 
these mocks replace
+// during testing all return functions, which we cannot spy on using 
jasmine.
+// Thus, we replace these modules inside the scope of addHelper.js 
using rewire, and shim
+// in these _mock test dummies. The test dummies themselves are 
constructed using
+// jasmine.createSpy inside the first beforeEach.
+var cfg_parser_mock = function () {};
+var cfg_parser_revert_mock;
+var hooks_mock;
+var platform_api_mock;
+var fetch_mock;
+var fetch_revert_mock;
+var prepare_mock;
+var prepare_revert_mock;
+var fake_platform = {
+'platform': 'atari'
+};
+beforeEach(function () {
+hooks_mock = jasmine.createSpyObj('hooksRunner mock', ['fire']);
+hooks_mock.fire.and.returnValue(Q());
+cfg_parser_mock.prototype = jasmine.createSpyObj('config parser 
mock', ['write', 'removeEngine', 'addEngine', 'getHookScripts']);
+cfg_parser_revert_mock = 
platform_addHelper.__set__('ConfigParser', cfg_parser_mock);
+fetch_mock = jasmine.createSpy('fetch mock').and.returnValue(Q());
+fetch_revert_mock = platform_addHelper.__set__('fetch', 
fetch_mock);
+prepare_mock = jasmine.createSpy('prepare 
mock').and.returnValue(Q());
+prepare_mock.preparePlatforms = 
jasmine.createSpy('preparePlatforms mock').and.returnValue(Q());
+prepare_revert_mock = platform_addHelper.__set__('prepare', 
prepare_mock);
+spyOn(shell, 'mkdir');
+spyOn(fs, 'existsSync').and.returnValue(false);
+spyOn(fs, 'writeFileSync');
+spyOn(cordova_util, 
'projectConfig').and.returnValue(path.join(projectRoot, 'config.xml'));
+spyOn(cordova_util, 'isDirectory').and.returnValue(false);
+spyOn(cordova_util, 'fixRelativePath').and.callFake(function 
(input) { return input; });
+spyOn(cordova_util, 'isUrl').and.returnValue(false);
+spyOn(cordova_util, 'hostSupports').and.returnValue(true);
+spyOn(cordova_util, 'removePlatformPluginsJson');
+spyOn(cordova_config, 'read').and.returnValue({});
+// Fake platform details we will use for our mocks, returned by 
either
+// getPlatfromDetailsFromDir (in the local-directory case), or
+// downloadPlatform (in every other case)
+spyOn(platform_module, 
'getPlatformDetailsFromDir').and.returnValue(Q(fake_platform));
+spyOn(platform_addHelper, 
'downloadPlatform').and.returnValue(Q(fake_platform));
+spyOn(platform_addHelper, 
'getVersionFromConfigFile').and.returnValue(false);
+spyOn(platform_addHelper, 
'installPluginsForNewPlatform').and.returnValue(Q());
+platform_api_mock = jasmine.createSpyObj('platform api mock', 
['createPlatform', 'updatePlatform']);
+platform_api_mock.createPlatform.and.returnValue(Q());
+platform_api_mock.updatePlatform.and.returnValue(Q());
+spyOn(cordova_util, 
'getPlatformApiFunction').and.returnValue(platform_api_mock);
+spyOn(platform_metadata, 'save');
+});
+afterEach(function () {
+cfg_parser_revert_mock();
+fetch_revert_mock();
+prepare_revert_mock();
+});
 describe('error/warning conditions', function () {
-it('should require specifying at least one platform

[GitHub] cordova-lib pull request #573: CB-12361 : updated addHelper tests

2017-06-29 Thread stevengill
Github user stevengill commented on a diff in the pull request:

https://github.com/apache/cordova-lib/pull/573#discussion_r124880319
  
--- Diff: spec/cordova/platform/addHelper.spec.js ---
@@ -16,34 +16,459 @@
 */
 /* eslint-env jasmine */
 
+var path = require('path');
+var fs = require('fs');
+var Q = require('q');
+var shell = require('shelljs');
+var events = require('cordova-common').events;
+var rewire = require('rewire');
+var platform_addHelper = rewire('../../../src/cordova/platform/addHelper');
+var platform_module = require('../../../src/cordova/platform');
+var platform_metadata = require('../../../src/cordova/platform_metadata');
+var cordova_util = require('../../../src/cordova/util');
+var cordova_config = require('../../../src/cordova/config');
+var plugman = require('../../../src/plugman/plugman');
+var fetch_metadata = require('../../../src/plugman/util/metadata');
+var lazy_load = require('../../../src/cordova/lazy_load');
+// require module here
+// spy on it and return 
+var cordova = require('../../../src/cordova/cordova');
+var prepare = require('../../../src/cordova/prepare');
+var gitclone = require('../../../src/gitclone');
+var fail;
+
 describe('cordova/platform/addHelper', function () {
+var projectRoot = '/some/path';
+// These _mock and _revert_mock objects use rewire as the modules 
these mocks replace
+// during testing all return functions, which we cannot spy on using 
jasmine.
+// Thus, we replace these modules inside the scope of addHelper.js 
using rewire, and shim
+// in these _mock test dummies. The test dummies themselves are 
constructed using
+// jasmine.createSpy inside the first beforeEach.
+var cfg_parser_mock = function () {};
+var cfg_parser_revert_mock;
+var hooks_mock;
+var platform_api_mock;
+var fetch_mock;
+var fetch_revert_mock;
+var prepare_mock;
+var prepare_revert_mock;
+var fake_platform = {
+'platform': 'atari'
+};
+beforeEach(function () {
+hooks_mock = jasmine.createSpyObj('hooksRunner mock', ['fire']);
+hooks_mock.fire.and.returnValue(Q());
+cfg_parser_mock.prototype = jasmine.createSpyObj('config parser 
mock', ['write', 'removeEngine', 'addEngine', 'getHookScripts']);
+cfg_parser_revert_mock = 
platform_addHelper.__set__('ConfigParser', cfg_parser_mock);
+fetch_mock = jasmine.createSpy('fetch mock').and.returnValue(Q());
+fetch_revert_mock = platform_addHelper.__set__('fetch', 
fetch_mock);
+prepare_mock = jasmine.createSpy('prepare 
mock').and.returnValue(Q());
+prepare_mock.preparePlatforms = 
jasmine.createSpy('preparePlatforms mock').and.returnValue(Q());
+prepare_revert_mock = platform_addHelper.__set__('prepare', 
prepare_mock);
+spyOn(shell, 'mkdir');
+spyOn(fs, 'existsSync').and.returnValue(false);
+spyOn(fs, 'writeFileSync');
+spyOn(cordova_util, 
'projectConfig').and.returnValue(path.join(projectRoot, 'config.xml'));
+spyOn(cordova_util, 'isDirectory').and.returnValue(false);
+spyOn(cordova_util, 'fixRelativePath').and.callFake(function 
(input) { return input; });
+spyOn(cordova_util, 'isUrl').and.returnValue(false);
+spyOn(cordova_util, 'hostSupports').and.returnValue(true);
+spyOn(cordova_util, 'removePlatformPluginsJson');
+spyOn(cordova_config, 'read').and.returnValue({});
+// Fake platform details we will use for our mocks, returned by 
either
+// getPlatfromDetailsFromDir (in the local-directory case), or
+// downloadPlatform (in every other case)
+spyOn(platform_module, 
'getPlatformDetailsFromDir').and.returnValue(Q(fake_platform));
+spyOn(platform_addHelper, 
'downloadPlatform').and.returnValue(Q(fake_platform));
+spyOn(platform_addHelper, 
'getVersionFromConfigFile').and.returnValue(false);
+spyOn(platform_addHelper, 
'installPluginsForNewPlatform').and.returnValue(Q());
+platform_api_mock = jasmine.createSpyObj('platform api mock', 
['createPlatform', 'updatePlatform']);
+platform_api_mock.createPlatform.and.returnValue(Q());
+platform_api_mock.updatePlatform.and.returnValue(Q());
+spyOn(cordova_util, 
'getPlatformApiFunction').and.returnValue(platform_api_mock);
+spyOn(platform_metadata, 'save');
+});
+afterEach(function () {
+cfg_parser_revert_mock();
+fetch_revert_mock();
+prepare_revert_mock();
+});
 describe('error/warning conditions', function () {
-it('should require specifying at least one platform

[GitHub] cordova-lib pull request #573: CB-12361 : updated addHelper tests

2017-06-29 Thread stevengill
Github user stevengill commented on a diff in the pull request:

https://github.com/apache/cordova-lib/pull/573#discussion_r124881373
  
--- Diff: spec/cordova/platform/addHelper.spec.js ---
@@ -16,34 +16,459 @@
 */
 /* eslint-env jasmine */
 
+var path = require('path');
+var fs = require('fs');
+var Q = require('q');
+var shell = require('shelljs');
+var events = require('cordova-common').events;
+var rewire = require('rewire');
+var platform_addHelper = rewire('../../../src/cordova/platform/addHelper');
+var platform_module = require('../../../src/cordova/platform');
+var platform_metadata = require('../../../src/cordova/platform_metadata');
+var cordova_util = require('../../../src/cordova/util');
+var cordova_config = require('../../../src/cordova/config');
+var plugman = require('../../../src/plugman/plugman');
+var fetch_metadata = require('../../../src/plugman/util/metadata');
+var lazy_load = require('../../../src/cordova/lazy_load');
+// require module here
+// spy on it and return 
+var cordova = require('../../../src/cordova/cordova');
+var prepare = require('../../../src/cordova/prepare');
+var gitclone = require('../../../src/gitclone');
+var fail;
+
 describe('cordova/platform/addHelper', function () {
+var projectRoot = '/some/path';
+// These _mock and _revert_mock objects use rewire as the modules 
these mocks replace
+// during testing all return functions, which we cannot spy on using 
jasmine.
+// Thus, we replace these modules inside the scope of addHelper.js 
using rewire, and shim
+// in these _mock test dummies. The test dummies themselves are 
constructed using
+// jasmine.createSpy inside the first beforeEach.
+var cfg_parser_mock = function () {};
+var cfg_parser_revert_mock;
+var hooks_mock;
+var platform_api_mock;
+var fetch_mock;
+var fetch_revert_mock;
+var prepare_mock;
+var prepare_revert_mock;
+var fake_platform = {
+'platform': 'atari'
+};
+beforeEach(function () {
+hooks_mock = jasmine.createSpyObj('hooksRunner mock', ['fire']);
+hooks_mock.fire.and.returnValue(Q());
+cfg_parser_mock.prototype = jasmine.createSpyObj('config parser 
mock', ['write', 'removeEngine', 'addEngine', 'getHookScripts']);
+cfg_parser_revert_mock = 
platform_addHelper.__set__('ConfigParser', cfg_parser_mock);
+fetch_mock = jasmine.createSpy('fetch mock').and.returnValue(Q());
+fetch_revert_mock = platform_addHelper.__set__('fetch', 
fetch_mock);
+prepare_mock = jasmine.createSpy('prepare 
mock').and.returnValue(Q());
+prepare_mock.preparePlatforms = 
jasmine.createSpy('preparePlatforms mock').and.returnValue(Q());
+prepare_revert_mock = platform_addHelper.__set__('prepare', 
prepare_mock);
+spyOn(shell, 'mkdir');
+spyOn(fs, 'existsSync').and.returnValue(false);
+spyOn(fs, 'writeFileSync');
+spyOn(cordova_util, 
'projectConfig').and.returnValue(path.join(projectRoot, 'config.xml'));
+spyOn(cordova_util, 'isDirectory').and.returnValue(false);
+spyOn(cordova_util, 'fixRelativePath').and.callFake(function 
(input) { return input; });
+spyOn(cordova_util, 'isUrl').and.returnValue(false);
+spyOn(cordova_util, 'hostSupports').and.returnValue(true);
+spyOn(cordova_util, 'removePlatformPluginsJson');
+spyOn(cordova_config, 'read').and.returnValue({});
+// Fake platform details we will use for our mocks, returned by 
either
+// getPlatfromDetailsFromDir (in the local-directory case), or
+// downloadPlatform (in every other case)
+spyOn(platform_module, 
'getPlatformDetailsFromDir').and.returnValue(Q(fake_platform));
+spyOn(platform_addHelper, 
'downloadPlatform').and.returnValue(Q(fake_platform));
+spyOn(platform_addHelper, 
'getVersionFromConfigFile').and.returnValue(false);
+spyOn(platform_addHelper, 
'installPluginsForNewPlatform').and.returnValue(Q());
+platform_api_mock = jasmine.createSpyObj('platform api mock', 
['createPlatform', 'updatePlatform']);
+platform_api_mock.createPlatform.and.returnValue(Q());
+platform_api_mock.updatePlatform.and.returnValue(Q());
+spyOn(cordova_util, 
'getPlatformApiFunction').and.returnValue(platform_api_mock);
+spyOn(platform_metadata, 'save');
+});
+afterEach(function () {
+cfg_parser_revert_mock();
+fetch_revert_mock();
+prepare_revert_mock();
+});
 describe('error/warning conditions', function () {
-it('should require specifying at least one platform

[GitHub] cordova-lib pull request #573: CB-12361 : updated addHelper tests

2017-06-29 Thread stevengill
Github user stevengill commented on a diff in the pull request:

https://github.com/apache/cordova-lib/pull/573#discussion_r124879975
  
--- Diff: spec/cordova/platform/addHelper.spec.js ---
@@ -16,34 +16,459 @@
 */
 /* eslint-env jasmine */
 
+var path = require('path');
+var fs = require('fs');
+var Q = require('q');
+var shell = require('shelljs');
+var events = require('cordova-common').events;
+var rewire = require('rewire');
+var platform_addHelper = rewire('../../../src/cordova/platform/addHelper');
+var platform_module = require('../../../src/cordova/platform');
+var platform_metadata = require('../../../src/cordova/platform_metadata');
+var cordova_util = require('../../../src/cordova/util');
+var cordova_config = require('../../../src/cordova/config');
+var plugman = require('../../../src/plugman/plugman');
+var fetch_metadata = require('../../../src/plugman/util/metadata');
+var lazy_load = require('../../../src/cordova/lazy_load');
+// require module here
+// spy on it and return 
+var cordova = require('../../../src/cordova/cordova');
+var prepare = require('../../../src/cordova/prepare');
+var gitclone = require('../../../src/gitclone');
+var fail;
+
 describe('cordova/platform/addHelper', function () {
+var projectRoot = '/some/path';
+// These _mock and _revert_mock objects use rewire as the modules 
these mocks replace
+// during testing all return functions, which we cannot spy on using 
jasmine.
+// Thus, we replace these modules inside the scope of addHelper.js 
using rewire, and shim
+// in these _mock test dummies. The test dummies themselves are 
constructed using
+// jasmine.createSpy inside the first beforeEach.
+var cfg_parser_mock = function () {};
+var cfg_parser_revert_mock;
+var hooks_mock;
+var platform_api_mock;
+var fetch_mock;
+var fetch_revert_mock;
+var prepare_mock;
+var prepare_revert_mock;
+var fake_platform = {
+'platform': 'atari'
+};
+beforeEach(function () {
+hooks_mock = jasmine.createSpyObj('hooksRunner mock', ['fire']);
+hooks_mock.fire.and.returnValue(Q());
+cfg_parser_mock.prototype = jasmine.createSpyObj('config parser 
mock', ['write', 'removeEngine', 'addEngine', 'getHookScripts']);
+cfg_parser_revert_mock = 
platform_addHelper.__set__('ConfigParser', cfg_parser_mock);
+fetch_mock = jasmine.createSpy('fetch mock').and.returnValue(Q());
+fetch_revert_mock = platform_addHelper.__set__('fetch', 
fetch_mock);
+prepare_mock = jasmine.createSpy('prepare 
mock').and.returnValue(Q());
+prepare_mock.preparePlatforms = 
jasmine.createSpy('preparePlatforms mock').and.returnValue(Q());
+prepare_revert_mock = platform_addHelper.__set__('prepare', 
prepare_mock);
+spyOn(shell, 'mkdir');
+spyOn(fs, 'existsSync').and.returnValue(false);
+spyOn(fs, 'writeFileSync');
+spyOn(cordova_util, 
'projectConfig').and.returnValue(path.join(projectRoot, 'config.xml'));
+spyOn(cordova_util, 'isDirectory').and.returnValue(false);
+spyOn(cordova_util, 'fixRelativePath').and.callFake(function 
(input) { return input; });
+spyOn(cordova_util, 'isUrl').and.returnValue(false);
+spyOn(cordova_util, 'hostSupports').and.returnValue(true);
+spyOn(cordova_util, 'removePlatformPluginsJson');
+spyOn(cordova_config, 'read').and.returnValue({});
+// Fake platform details we will use for our mocks, returned by 
either
+// getPlatfromDetailsFromDir (in the local-directory case), or
+// downloadPlatform (in every other case)
+spyOn(platform_module, 
'getPlatformDetailsFromDir').and.returnValue(Q(fake_platform));
+spyOn(platform_addHelper, 
'downloadPlatform').and.returnValue(Q(fake_platform));
+spyOn(platform_addHelper, 
'getVersionFromConfigFile').and.returnValue(false);
+spyOn(platform_addHelper, 
'installPluginsForNewPlatform').and.returnValue(Q());
+platform_api_mock = jasmine.createSpyObj('platform api mock', 
['createPlatform', 'updatePlatform']);
+platform_api_mock.createPlatform.and.returnValue(Q());
+platform_api_mock.updatePlatform.and.returnValue(Q());
+spyOn(cordova_util, 
'getPlatformApiFunction').and.returnValue(platform_api_mock);
+spyOn(platform_metadata, 'save');
+});
+afterEach(function () {
+cfg_parser_revert_mock();
+fetch_revert_mock();
+prepare_revert_mock();
+});
 describe('error/warning conditions', function () {
-it('should require specifying at least one platform

[GitHub] cordova-lib pull request #573: CB-12361 : updated addHelper tests

2017-06-29 Thread stevengill
Github user stevengill commented on a diff in the pull request:

https://github.com/apache/cordova-lib/pull/573#discussion_r124891852
  
--- Diff: spec/cordova/platform/addHelper.spec.js ---
@@ -16,34 +16,459 @@
 */
 /* eslint-env jasmine */
 
+var path = require('path');
+var fs = require('fs');
+var Q = require('q');
+var shell = require('shelljs');
+var events = require('cordova-common').events;
+var rewire = require('rewire');
+var platform_addHelper = rewire('../../../src/cordova/platform/addHelper');
+var platform_module = require('../../../src/cordova/platform');
+var platform_metadata = require('../../../src/cordova/platform_metadata');
+var cordova_util = require('../../../src/cordova/util');
+var cordova_config = require('../../../src/cordova/config');
+var plugman = require('../../../src/plugman/plugman');
+var fetch_metadata = require('../../../src/plugman/util/metadata');
+var lazy_load = require('../../../src/cordova/lazy_load');
+// require module here
+// spy on it and return 
+var cordova = require('../../../src/cordova/cordova');
+var prepare = require('../../../src/cordova/prepare');
+var gitclone = require('../../../src/gitclone');
+var fail;
+
 describe('cordova/platform/addHelper', function () {
+var projectRoot = '/some/path';
+// These _mock and _revert_mock objects use rewire as the modules 
these mocks replace
+// during testing all return functions, which we cannot spy on using 
jasmine.
+// Thus, we replace these modules inside the scope of addHelper.js 
using rewire, and shim
+// in these _mock test dummies. The test dummies themselves are 
constructed using
+// jasmine.createSpy inside the first beforeEach.
+var cfg_parser_mock = function () {};
+var cfg_parser_revert_mock;
+var hooks_mock;
+var platform_api_mock;
+var fetch_mock;
+var fetch_revert_mock;
+var prepare_mock;
+var prepare_revert_mock;
+var fake_platform = {
+'platform': 'atari'
+};
+beforeEach(function () {
+hooks_mock = jasmine.createSpyObj('hooksRunner mock', ['fire']);
+hooks_mock.fire.and.returnValue(Q());
+cfg_parser_mock.prototype = jasmine.createSpyObj('config parser 
mock', ['write', 'removeEngine', 'addEngine', 'getHookScripts']);
+cfg_parser_revert_mock = 
platform_addHelper.__set__('ConfigParser', cfg_parser_mock);
+fetch_mock = jasmine.createSpy('fetch mock').and.returnValue(Q());
+fetch_revert_mock = platform_addHelper.__set__('fetch', 
fetch_mock);
+prepare_mock = jasmine.createSpy('prepare 
mock').and.returnValue(Q());
+prepare_mock.preparePlatforms = 
jasmine.createSpy('preparePlatforms mock').and.returnValue(Q());
+prepare_revert_mock = platform_addHelper.__set__('prepare', 
prepare_mock);
+spyOn(shell, 'mkdir');
+spyOn(fs, 'existsSync').and.returnValue(false);
+spyOn(fs, 'writeFileSync');
+spyOn(cordova_util, 
'projectConfig').and.returnValue(path.join(projectRoot, 'config.xml'));
+spyOn(cordova_util, 'isDirectory').and.returnValue(false);
+spyOn(cordova_util, 'fixRelativePath').and.callFake(function 
(input) { return input; });
+spyOn(cordova_util, 'isUrl').and.returnValue(false);
+spyOn(cordova_util, 'hostSupports').and.returnValue(true);
+spyOn(cordova_util, 'removePlatformPluginsJson');
+spyOn(cordova_config, 'read').and.returnValue({});
+// Fake platform details we will use for our mocks, returned by 
either
+// getPlatfromDetailsFromDir (in the local-directory case), or
+// downloadPlatform (in every other case)
+spyOn(platform_module, 
'getPlatformDetailsFromDir').and.returnValue(Q(fake_platform));
+spyOn(platform_addHelper, 
'downloadPlatform').and.returnValue(Q(fake_platform));
+spyOn(platform_addHelper, 
'getVersionFromConfigFile').and.returnValue(false);
+spyOn(platform_addHelper, 
'installPluginsForNewPlatform').and.returnValue(Q());
+platform_api_mock = jasmine.createSpyObj('platform api mock', 
['createPlatform', 'updatePlatform']);
+platform_api_mock.createPlatform.and.returnValue(Q());
+platform_api_mock.updatePlatform.and.returnValue(Q());
+spyOn(cordova_util, 
'getPlatformApiFunction').and.returnValue(platform_api_mock);
+spyOn(platform_metadata, 'save');
+});
+afterEach(function () {
+cfg_parser_revert_mock();
+fetch_revert_mock();
+prepare_revert_mock();
+});
 describe('error/warning conditions', function () {
-it('should require specifying at least one platform

[GitHub] cordova-lib pull request #573: CB-12361 : updated addHelper tests

2017-06-29 Thread stevengill
Github user stevengill commented on a diff in the pull request:

https://github.com/apache/cordova-lib/pull/573#discussion_r124893969
  
--- Diff: spec/cordova/platform/addHelper.spec.js ---
@@ -16,34 +16,459 @@
 */
 /* eslint-env jasmine */
 
+var path = require('path');
+var fs = require('fs');
+var Q = require('q');
+var shell = require('shelljs');
+var events = require('cordova-common').events;
+var rewire = require('rewire');
+var platform_addHelper = rewire('../../../src/cordova/platform/addHelper');
+var platform_module = require('../../../src/cordova/platform');
+var platform_metadata = require('../../../src/cordova/platform_metadata');
+var cordova_util = require('../../../src/cordova/util');
+var cordova_config = require('../../../src/cordova/config');
+var plugman = require('../../../src/plugman/plugman');
+var fetch_metadata = require('../../../src/plugman/util/metadata');
+var lazy_load = require('../../../src/cordova/lazy_load');
+// require module here
+// spy on it and return 
+var cordova = require('../../../src/cordova/cordova');
+var prepare = require('../../../src/cordova/prepare');
+var gitclone = require('../../../src/gitclone');
+var fail;
+
 describe('cordova/platform/addHelper', function () {
+var projectRoot = '/some/path';
+// These _mock and _revert_mock objects use rewire as the modules 
these mocks replace
+// during testing all return functions, which we cannot spy on using 
jasmine.
+// Thus, we replace these modules inside the scope of addHelper.js 
using rewire, and shim
+// in these _mock test dummies. The test dummies themselves are 
constructed using
+// jasmine.createSpy inside the first beforeEach.
+var cfg_parser_mock = function () {};
+var cfg_parser_revert_mock;
+var hooks_mock;
+var platform_api_mock;
+var fetch_mock;
+var fetch_revert_mock;
+var prepare_mock;
+var prepare_revert_mock;
+var fake_platform = {
+'platform': 'atari'
+};
+beforeEach(function () {
+hooks_mock = jasmine.createSpyObj('hooksRunner mock', ['fire']);
+hooks_mock.fire.and.returnValue(Q());
+cfg_parser_mock.prototype = jasmine.createSpyObj('config parser 
mock', ['write', 'removeEngine', 'addEngine', 'getHookScripts']);
+cfg_parser_revert_mock = 
platform_addHelper.__set__('ConfigParser', cfg_parser_mock);
+fetch_mock = jasmine.createSpy('fetch mock').and.returnValue(Q());
+fetch_revert_mock = platform_addHelper.__set__('fetch', 
fetch_mock);
+prepare_mock = jasmine.createSpy('prepare 
mock').and.returnValue(Q());
+prepare_mock.preparePlatforms = 
jasmine.createSpy('preparePlatforms mock').and.returnValue(Q());
+prepare_revert_mock = platform_addHelper.__set__('prepare', 
prepare_mock);
+spyOn(shell, 'mkdir');
+spyOn(fs, 'existsSync').and.returnValue(false);
+spyOn(fs, 'writeFileSync');
+spyOn(cordova_util, 
'projectConfig').and.returnValue(path.join(projectRoot, 'config.xml'));
+spyOn(cordova_util, 'isDirectory').and.returnValue(false);
+spyOn(cordova_util, 'fixRelativePath').and.callFake(function 
(input) { return input; });
+spyOn(cordova_util, 'isUrl').and.returnValue(false);
+spyOn(cordova_util, 'hostSupports').and.returnValue(true);
+spyOn(cordova_util, 'removePlatformPluginsJson');
+spyOn(cordova_config, 'read').and.returnValue({});
+// Fake platform details we will use for our mocks, returned by 
either
+// getPlatfromDetailsFromDir (in the local-directory case), or
+// downloadPlatform (in every other case)
+spyOn(platform_module, 
'getPlatformDetailsFromDir').and.returnValue(Q(fake_platform));
+spyOn(platform_addHelper, 
'downloadPlatform').and.returnValue(Q(fake_platform));
+spyOn(platform_addHelper, 
'getVersionFromConfigFile').and.returnValue(false);
+spyOn(platform_addHelper, 
'installPluginsForNewPlatform').and.returnValue(Q());
+platform_api_mock = jasmine.createSpyObj('platform api mock', 
['createPlatform', 'updatePlatform']);
+platform_api_mock.createPlatform.and.returnValue(Q());
+platform_api_mock.updatePlatform.and.returnValue(Q());
+spyOn(cordova_util, 
'getPlatformApiFunction').and.returnValue(platform_api_mock);
+spyOn(platform_metadata, 'save');
+});
+afterEach(function () {
+cfg_parser_revert_mock();
+fetch_revert_mock();
+prepare_revert_mock();
+});
 describe('error/warning conditions', function () {
-it('should require specifying at least one platform

[GitHub] cordova-lib pull request #573: CB-12361 : updated addHelper tests

2017-06-29 Thread stevengill
Github user stevengill commented on a diff in the pull request:

https://github.com/apache/cordova-lib/pull/573#discussion_r124894106
  
--- Diff: spec/cordova/platform/addHelper.spec.js ---
@@ -16,34 +16,459 @@
 */
 /* eslint-env jasmine */
 
+var path = require('path');
+var fs = require('fs');
+var Q = require('q');
+var shell = require('shelljs');
+var events = require('cordova-common').events;
+var rewire = require('rewire');
+var platform_addHelper = rewire('../../../src/cordova/platform/addHelper');
+var platform_module = require('../../../src/cordova/platform');
+var platform_metadata = require('../../../src/cordova/platform_metadata');
+var cordova_util = require('../../../src/cordova/util');
+var cordova_config = require('../../../src/cordova/config');
+var plugman = require('../../../src/plugman/plugman');
+var fetch_metadata = require('../../../src/plugman/util/metadata');
+var lazy_load = require('../../../src/cordova/lazy_load');
+// require module here
+// spy on it and return 
+var cordova = require('../../../src/cordova/cordova');
+var prepare = require('../../../src/cordova/prepare');
+var gitclone = require('../../../src/gitclone');
+var fail;
+
 describe('cordova/platform/addHelper', function () {
+var projectRoot = '/some/path';
+// These _mock and _revert_mock objects use rewire as the modules 
these mocks replace
+// during testing all return functions, which we cannot spy on using 
jasmine.
+// Thus, we replace these modules inside the scope of addHelper.js 
using rewire, and shim
+// in these _mock test dummies. The test dummies themselves are 
constructed using
+// jasmine.createSpy inside the first beforeEach.
+var cfg_parser_mock = function () {};
+var cfg_parser_revert_mock;
+var hooks_mock;
+var platform_api_mock;
+var fetch_mock;
+var fetch_revert_mock;
+var prepare_mock;
+var prepare_revert_mock;
+var fake_platform = {
+'platform': 'atari'
+};
+beforeEach(function () {
+hooks_mock = jasmine.createSpyObj('hooksRunner mock', ['fire']);
+hooks_mock.fire.and.returnValue(Q());
+cfg_parser_mock.prototype = jasmine.createSpyObj('config parser 
mock', ['write', 'removeEngine', 'addEngine', 'getHookScripts']);
+cfg_parser_revert_mock = 
platform_addHelper.__set__('ConfigParser', cfg_parser_mock);
+fetch_mock = jasmine.createSpy('fetch mock').and.returnValue(Q());
+fetch_revert_mock = platform_addHelper.__set__('fetch', 
fetch_mock);
+prepare_mock = jasmine.createSpy('prepare 
mock').and.returnValue(Q());
+prepare_mock.preparePlatforms = 
jasmine.createSpy('preparePlatforms mock').and.returnValue(Q());
+prepare_revert_mock = platform_addHelper.__set__('prepare', 
prepare_mock);
+spyOn(shell, 'mkdir');
+spyOn(fs, 'existsSync').and.returnValue(false);
+spyOn(fs, 'writeFileSync');
+spyOn(cordova_util, 
'projectConfig').and.returnValue(path.join(projectRoot, 'config.xml'));
+spyOn(cordova_util, 'isDirectory').and.returnValue(false);
+spyOn(cordova_util, 'fixRelativePath').and.callFake(function 
(input) { return input; });
+spyOn(cordova_util, 'isUrl').and.returnValue(false);
+spyOn(cordova_util, 'hostSupports').and.returnValue(true);
+spyOn(cordova_util, 'removePlatformPluginsJson');
+spyOn(cordova_config, 'read').and.returnValue({});
+// Fake platform details we will use for our mocks, returned by 
either
+// getPlatfromDetailsFromDir (in the local-directory case), or
+// downloadPlatform (in every other case)
+spyOn(platform_module, 
'getPlatformDetailsFromDir').and.returnValue(Q(fake_platform));
+spyOn(platform_addHelper, 
'downloadPlatform').and.returnValue(Q(fake_platform));
+spyOn(platform_addHelper, 
'getVersionFromConfigFile').and.returnValue(false);
+spyOn(platform_addHelper, 
'installPluginsForNewPlatform').and.returnValue(Q());
+platform_api_mock = jasmine.createSpyObj('platform api mock', 
['createPlatform', 'updatePlatform']);
+platform_api_mock.createPlatform.and.returnValue(Q());
+platform_api_mock.updatePlatform.and.returnValue(Q());
+spyOn(cordova_util, 
'getPlatformApiFunction').and.returnValue(platform_api_mock);
+spyOn(platform_metadata, 'save');
+});
+afterEach(function () {
+cfg_parser_revert_mock();
+fetch_revert_mock();
+prepare_revert_mock();
+});
 describe('error/warning conditions', function () {
-it('should require specifying at least one platform

[GitHub] cordova-lib pull request #573: CB-12361 : updated addHelper tests

2017-06-29 Thread stevengill
Github user stevengill commented on a diff in the pull request:

https://github.com/apache/cordova-lib/pull/573#discussion_r124863854
  
--- Diff: spec/cordova/platform/addHelper.spec.js ---
@@ -16,34 +16,459 @@
 */
 /* eslint-env jasmine */
 
+var path = require('path');
+var fs = require('fs');
+var Q = require('q');
+var shell = require('shelljs');
+var events = require('cordova-common').events;
+var rewire = require('rewire');
+var platform_addHelper = rewire('../../../src/cordova/platform/addHelper');
+var platform_module = require('../../../src/cordova/platform');
+var platform_metadata = require('../../../src/cordova/platform_metadata');
+var cordova_util = require('../../../src/cordova/util');
+var cordova_config = require('../../../src/cordova/config');
+var plugman = require('../../../src/plugman/plugman');
+var fetch_metadata = require('../../../src/plugman/util/metadata');
+var lazy_load = require('../../../src/cordova/lazy_load');
+// require module here
--- End diff --

don't need these comments I take it. 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org



[GitHub] cordova-lib pull request #573: CB-12361 : updated addHelper tests

2017-06-29 Thread stevengill
Github user stevengill commented on a diff in the pull request:

https://github.com/apache/cordova-lib/pull/573#discussion_r124890998
  
--- Diff: spec/cordova/platform/addHelper.spec.js ---
@@ -16,34 +16,459 @@
 */
 /* eslint-env jasmine */
 
+var path = require('path');
+var fs = require('fs');
+var Q = require('q');
+var shell = require('shelljs');
+var events = require('cordova-common').events;
+var rewire = require('rewire');
+var platform_addHelper = rewire('../../../src/cordova/platform/addHelper');
+var platform_module = require('../../../src/cordova/platform');
+var platform_metadata = require('../../../src/cordova/platform_metadata');
+var cordova_util = require('../../../src/cordova/util');
+var cordova_config = require('../../../src/cordova/config');
+var plugman = require('../../../src/plugman/plugman');
+var fetch_metadata = require('../../../src/plugman/util/metadata');
+var lazy_load = require('../../../src/cordova/lazy_load');
+// require module here
+// spy on it and return 
+var cordova = require('../../../src/cordova/cordova');
+var prepare = require('../../../src/cordova/prepare');
+var gitclone = require('../../../src/gitclone');
+var fail;
+
 describe('cordova/platform/addHelper', function () {
+var projectRoot = '/some/path';
+// These _mock and _revert_mock objects use rewire as the modules 
these mocks replace
+// during testing all return functions, which we cannot spy on using 
jasmine.
+// Thus, we replace these modules inside the scope of addHelper.js 
using rewire, and shim
+// in these _mock test dummies. The test dummies themselves are 
constructed using
+// jasmine.createSpy inside the first beforeEach.
+var cfg_parser_mock = function () {};
+var cfg_parser_revert_mock;
+var hooks_mock;
+var platform_api_mock;
+var fetch_mock;
+var fetch_revert_mock;
+var prepare_mock;
+var prepare_revert_mock;
+var fake_platform = {
+'platform': 'atari'
+};
+beforeEach(function () {
+hooks_mock = jasmine.createSpyObj('hooksRunner mock', ['fire']);
+hooks_mock.fire.and.returnValue(Q());
+cfg_parser_mock.prototype = jasmine.createSpyObj('config parser 
mock', ['write', 'removeEngine', 'addEngine', 'getHookScripts']);
+cfg_parser_revert_mock = 
platform_addHelper.__set__('ConfigParser', cfg_parser_mock);
+fetch_mock = jasmine.createSpy('fetch mock').and.returnValue(Q());
+fetch_revert_mock = platform_addHelper.__set__('fetch', 
fetch_mock);
+prepare_mock = jasmine.createSpy('prepare 
mock').and.returnValue(Q());
+prepare_mock.preparePlatforms = 
jasmine.createSpy('preparePlatforms mock').and.returnValue(Q());
+prepare_revert_mock = platform_addHelper.__set__('prepare', 
prepare_mock);
+spyOn(shell, 'mkdir');
+spyOn(fs, 'existsSync').and.returnValue(false);
+spyOn(fs, 'writeFileSync');
+spyOn(cordova_util, 
'projectConfig').and.returnValue(path.join(projectRoot, 'config.xml'));
+spyOn(cordova_util, 'isDirectory').and.returnValue(false);
+spyOn(cordova_util, 'fixRelativePath').and.callFake(function 
(input) { return input; });
+spyOn(cordova_util, 'isUrl').and.returnValue(false);
+spyOn(cordova_util, 'hostSupports').and.returnValue(true);
+spyOn(cordova_util, 'removePlatformPluginsJson');
+spyOn(cordova_config, 'read').and.returnValue({});
+// Fake platform details we will use for our mocks, returned by 
either
+// getPlatfromDetailsFromDir (in the local-directory case), or
+// downloadPlatform (in every other case)
+spyOn(platform_module, 
'getPlatformDetailsFromDir').and.returnValue(Q(fake_platform));
+spyOn(platform_addHelper, 
'downloadPlatform').and.returnValue(Q(fake_platform));
+spyOn(platform_addHelper, 
'getVersionFromConfigFile').and.returnValue(false);
+spyOn(platform_addHelper, 
'installPluginsForNewPlatform').and.returnValue(Q());
+platform_api_mock = jasmine.createSpyObj('platform api mock', 
['createPlatform', 'updatePlatform']);
+platform_api_mock.createPlatform.and.returnValue(Q());
+platform_api_mock.updatePlatform.and.returnValue(Q());
+spyOn(cordova_util, 
'getPlatformApiFunction').and.returnValue(platform_api_mock);
+spyOn(platform_metadata, 'save');
+});
+afterEach(function () {
+cfg_parser_revert_mock();
+fetch_revert_mock();
+prepare_revert_mock();
+});
 describe('error/warning conditions', function () {
-it('should require specifying at least one platform

[GitHub] cordova-lib pull request #573: CB-12361 : updated addHelper tests

2017-06-29 Thread stevengill
Github user stevengill commented on a diff in the pull request:

https://github.com/apache/cordova-lib/pull/573#discussion_r124873252
  
--- Diff: spec/cordova/platform/addHelper.spec.js ---
@@ -16,34 +16,459 @@
 */
 /* eslint-env jasmine */
 
+var path = require('path');
+var fs = require('fs');
+var Q = require('q');
+var shell = require('shelljs');
+var events = require('cordova-common').events;
+var rewire = require('rewire');
+var platform_addHelper = rewire('../../../src/cordova/platform/addHelper');
+var platform_module = require('../../../src/cordova/platform');
+var platform_metadata = require('../../../src/cordova/platform_metadata');
+var cordova_util = require('../../../src/cordova/util');
+var cordova_config = require('../../../src/cordova/config');
+var plugman = require('../../../src/plugman/plugman');
+var fetch_metadata = require('../../../src/plugman/util/metadata');
+var lazy_load = require('../../../src/cordova/lazy_load');
+// require module here
+// spy on it and return 
+var cordova = require('../../../src/cordova/cordova');
+var prepare = require('../../../src/cordova/prepare');
+var gitclone = require('../../../src/gitclone');
+var fail;
+
 describe('cordova/platform/addHelper', function () {
+var projectRoot = '/some/path';
+// These _mock and _revert_mock objects use rewire as the modules 
these mocks replace
+// during testing all return functions, which we cannot spy on using 
jasmine.
+// Thus, we replace these modules inside the scope of addHelper.js 
using rewire, and shim
+// in these _mock test dummies. The test dummies themselves are 
constructed using
+// jasmine.createSpy inside the first beforeEach.
+var cfg_parser_mock = function () {};
+var cfg_parser_revert_mock;
+var hooks_mock;
+var platform_api_mock;
+var fetch_mock;
+var fetch_revert_mock;
+var prepare_mock;
+var prepare_revert_mock;
+var fake_platform = {
+'platform': 'atari'
+};
+beforeEach(function () {
+hooks_mock = jasmine.createSpyObj('hooksRunner mock', ['fire']);
+hooks_mock.fire.and.returnValue(Q());
+cfg_parser_mock.prototype = jasmine.createSpyObj('config parser 
mock', ['write', 'removeEngine', 'addEngine', 'getHookScripts']);
+cfg_parser_revert_mock = 
platform_addHelper.__set__('ConfigParser', cfg_parser_mock);
+fetch_mock = jasmine.createSpy('fetch mock').and.returnValue(Q());
+fetch_revert_mock = platform_addHelper.__set__('fetch', 
fetch_mock);
+prepare_mock = jasmine.createSpy('prepare 
mock').and.returnValue(Q());
+prepare_mock.preparePlatforms = 
jasmine.createSpy('preparePlatforms mock').and.returnValue(Q());
+prepare_revert_mock = platform_addHelper.__set__('prepare', 
prepare_mock);
+spyOn(shell, 'mkdir');
+spyOn(fs, 'existsSync').and.returnValue(false);
+spyOn(fs, 'writeFileSync');
+spyOn(cordova_util, 
'projectConfig').and.returnValue(path.join(projectRoot, 'config.xml'));
+spyOn(cordova_util, 'isDirectory').and.returnValue(false);
+spyOn(cordova_util, 'fixRelativePath').and.callFake(function 
(input) { return input; });
+spyOn(cordova_util, 'isUrl').and.returnValue(false);
+spyOn(cordova_util, 'hostSupports').and.returnValue(true);
+spyOn(cordova_util, 'removePlatformPluginsJson');
+spyOn(cordova_config, 'read').and.returnValue({});
+// Fake platform details we will use for our mocks, returned by 
either
+// getPlatfromDetailsFromDir (in the local-directory case), or
+// downloadPlatform (in every other case)
+spyOn(platform_module, 
'getPlatformDetailsFromDir').and.returnValue(Q(fake_platform));
+spyOn(platform_addHelper, 
'downloadPlatform').and.returnValue(Q(fake_platform));
+spyOn(platform_addHelper, 
'getVersionFromConfigFile').and.returnValue(false);
+spyOn(platform_addHelper, 
'installPluginsForNewPlatform').and.returnValue(Q());
+platform_api_mock = jasmine.createSpyObj('platform api mock', 
['createPlatform', 'updatePlatform']);
+platform_api_mock.createPlatform.and.returnValue(Q());
+platform_api_mock.updatePlatform.and.returnValue(Q());
+spyOn(cordova_util, 
'getPlatformApiFunction').and.returnValue(platform_api_mock);
+spyOn(platform_metadata, 'save');
+});
+afterEach(function () {
+cfg_parser_revert_mock();
+fetch_revert_mock();
+prepare_revert_mock();
+});
 describe('error/warning conditions', function () {
-it('should require specifying at least one platform

[GitHub] cordova-lib pull request #573: CB-12361 : updated addHelper tests

2017-06-29 Thread stevengill
Github user stevengill commented on a diff in the pull request:

https://github.com/apache/cordova-lib/pull/573#discussion_r124871034
  
--- Diff: spec/cordova/platform/addHelper.spec.js ---
@@ -16,34 +16,459 @@
 */
 /* eslint-env jasmine */
 
+var path = require('path');
+var fs = require('fs');
+var Q = require('q');
+var shell = require('shelljs');
+var events = require('cordova-common').events;
+var rewire = require('rewire');
+var platform_addHelper = rewire('../../../src/cordova/platform/addHelper');
+var platform_module = require('../../../src/cordova/platform');
+var platform_metadata = require('../../../src/cordova/platform_metadata');
+var cordova_util = require('../../../src/cordova/util');
+var cordova_config = require('../../../src/cordova/config');
+var plugman = require('../../../src/plugman/plugman');
+var fetch_metadata = require('../../../src/plugman/util/metadata');
+var lazy_load = require('../../../src/cordova/lazy_load');
+// require module here
+// spy on it and return 
+var cordova = require('../../../src/cordova/cordova');
+var prepare = require('../../../src/cordova/prepare');
+var gitclone = require('../../../src/gitclone');
+var fail;
+
 describe('cordova/platform/addHelper', function () {
+var projectRoot = '/some/path';
+// These _mock and _revert_mock objects use rewire as the modules 
these mocks replace
+// during testing all return functions, which we cannot spy on using 
jasmine.
+// Thus, we replace these modules inside the scope of addHelper.js 
using rewire, and shim
+// in these _mock test dummies. The test dummies themselves are 
constructed using
+// jasmine.createSpy inside the first beforeEach.
+var cfg_parser_mock = function () {};
+var cfg_parser_revert_mock;
+var hooks_mock;
+var platform_api_mock;
+var fetch_mock;
+var fetch_revert_mock;
+var prepare_mock;
+var prepare_revert_mock;
+var fake_platform = {
+'platform': 'atari'
+};
+beforeEach(function () {
+hooks_mock = jasmine.createSpyObj('hooksRunner mock', ['fire']);
+hooks_mock.fire.and.returnValue(Q());
+cfg_parser_mock.prototype = jasmine.createSpyObj('config parser 
mock', ['write', 'removeEngine', 'addEngine', 'getHookScripts']);
+cfg_parser_revert_mock = 
platform_addHelper.__set__('ConfigParser', cfg_parser_mock);
+fetch_mock = jasmine.createSpy('fetch mock').and.returnValue(Q());
+fetch_revert_mock = platform_addHelper.__set__('fetch', 
fetch_mock);
+prepare_mock = jasmine.createSpy('prepare 
mock').and.returnValue(Q());
+prepare_mock.preparePlatforms = 
jasmine.createSpy('preparePlatforms mock').and.returnValue(Q());
+prepare_revert_mock = platform_addHelper.__set__('prepare', 
prepare_mock);
+spyOn(shell, 'mkdir');
+spyOn(fs, 'existsSync').and.returnValue(false);
+spyOn(fs, 'writeFileSync');
+spyOn(cordova_util, 
'projectConfig').and.returnValue(path.join(projectRoot, 'config.xml'));
+spyOn(cordova_util, 'isDirectory').and.returnValue(false);
+spyOn(cordova_util, 'fixRelativePath').and.callFake(function 
(input) { return input; });
+spyOn(cordova_util, 'isUrl').and.returnValue(false);
+spyOn(cordova_util, 'hostSupports').and.returnValue(true);
+spyOn(cordova_util, 'removePlatformPluginsJson');
+spyOn(cordova_config, 'read').and.returnValue({});
+// Fake platform details we will use for our mocks, returned by 
either
+// getPlatfromDetailsFromDir (in the local-directory case), or
+// downloadPlatform (in every other case)
+spyOn(platform_module, 
'getPlatformDetailsFromDir').and.returnValue(Q(fake_platform));
+spyOn(platform_addHelper, 
'downloadPlatform').and.returnValue(Q(fake_platform));
+spyOn(platform_addHelper, 
'getVersionFromConfigFile').and.returnValue(false);
+spyOn(platform_addHelper, 
'installPluginsForNewPlatform').and.returnValue(Q());
+platform_api_mock = jasmine.createSpyObj('platform api mock', 
['createPlatform', 'updatePlatform']);
+platform_api_mock.createPlatform.and.returnValue(Q());
+platform_api_mock.updatePlatform.and.returnValue(Q());
+spyOn(cordova_util, 
'getPlatformApiFunction').and.returnValue(platform_api_mock);
+spyOn(platform_metadata, 'save');
+});
+afterEach(function () {
+cfg_parser_revert_mock();
+fetch_revert_mock();
+prepare_revert_mock();
+});
 describe('error/warning conditions', function () {
-it('should require specifying at least one platform

[GitHub] cordova-lib pull request #573: CB-12361 : updated addHelper tests

2017-06-29 Thread stevengill
Github user stevengill commented on a diff in the pull request:

https://github.com/apache/cordova-lib/pull/573#discussion_r124867315
  
--- Diff: spec/cordova/platform/addHelper.spec.js ---
@@ -16,34 +16,459 @@
 */
 /* eslint-env jasmine */
 
+var path = require('path');
+var fs = require('fs');
+var Q = require('q');
+var shell = require('shelljs');
+var events = require('cordova-common').events;
+var rewire = require('rewire');
+var platform_addHelper = rewire('../../../src/cordova/platform/addHelper');
+var platform_module = require('../../../src/cordova/platform');
+var platform_metadata = require('../../../src/cordova/platform_metadata');
+var cordova_util = require('../../../src/cordova/util');
+var cordova_config = require('../../../src/cordova/config');
+var plugman = require('../../../src/plugman/plugman');
+var fetch_metadata = require('../../../src/plugman/util/metadata');
+var lazy_load = require('../../../src/cordova/lazy_load');
+// require module here
+// spy on it and return 
+var cordova = require('../../../src/cordova/cordova');
+var prepare = require('../../../src/cordova/prepare');
+var gitclone = require('../../../src/gitclone');
+var fail;
+
 describe('cordova/platform/addHelper', function () {
+var projectRoot = '/some/path';
+// These _mock and _revert_mock objects use rewire as the modules 
these mocks replace
+// during testing all return functions, which we cannot spy on using 
jasmine.
+// Thus, we replace these modules inside the scope of addHelper.js 
using rewire, and shim
+// in these _mock test dummies. The test dummies themselves are 
constructed using
+// jasmine.createSpy inside the first beforeEach.
+var cfg_parser_mock = function () {};
+var cfg_parser_revert_mock;
+var hooks_mock;
+var platform_api_mock;
+var fetch_mock;
+var fetch_revert_mock;
+var prepare_mock;
+var prepare_revert_mock;
+var fake_platform = {
+'platform': 'atari'
+};
+beforeEach(function () {
+hooks_mock = jasmine.createSpyObj('hooksRunner mock', ['fire']);
+hooks_mock.fire.and.returnValue(Q());
+cfg_parser_mock.prototype = jasmine.createSpyObj('config parser 
mock', ['write', 'removeEngine', 'addEngine', 'getHookScripts']);
+cfg_parser_revert_mock = 
platform_addHelper.__set__('ConfigParser', cfg_parser_mock);
+fetch_mock = jasmine.createSpy('fetch mock').and.returnValue(Q());
+fetch_revert_mock = platform_addHelper.__set__('fetch', 
fetch_mock);
+prepare_mock = jasmine.createSpy('prepare 
mock').and.returnValue(Q());
+prepare_mock.preparePlatforms = 
jasmine.createSpy('preparePlatforms mock').and.returnValue(Q());
+prepare_revert_mock = platform_addHelper.__set__('prepare', 
prepare_mock);
+spyOn(shell, 'mkdir');
+spyOn(fs, 'existsSync').and.returnValue(false);
+spyOn(fs, 'writeFileSync');
+spyOn(cordova_util, 
'projectConfig').and.returnValue(path.join(projectRoot, 'config.xml'));
+spyOn(cordova_util, 'isDirectory').and.returnValue(false);
+spyOn(cordova_util, 'fixRelativePath').and.callFake(function 
(input) { return input; });
+spyOn(cordova_util, 'isUrl').and.returnValue(false);
+spyOn(cordova_util, 'hostSupports').and.returnValue(true);
+spyOn(cordova_util, 'removePlatformPluginsJson');
+spyOn(cordova_config, 'read').and.returnValue({});
+// Fake platform details we will use for our mocks, returned by 
either
+// getPlatfromDetailsFromDir (in the local-directory case), or
+// downloadPlatform (in every other case)
+spyOn(platform_module, 
'getPlatformDetailsFromDir').and.returnValue(Q(fake_platform));
+spyOn(platform_addHelper, 
'downloadPlatform').and.returnValue(Q(fake_platform));
+spyOn(platform_addHelper, 
'getVersionFromConfigFile').and.returnValue(false);
+spyOn(platform_addHelper, 
'installPluginsForNewPlatform').and.returnValue(Q());
+platform_api_mock = jasmine.createSpyObj('platform api mock', 
['createPlatform', 'updatePlatform']);
+platform_api_mock.createPlatform.and.returnValue(Q());
+platform_api_mock.updatePlatform.and.returnValue(Q());
+spyOn(cordova_util, 
'getPlatformApiFunction').and.returnValue(platform_api_mock);
+spyOn(platform_metadata, 'save');
+});
+afterEach(function () {
+cfg_parser_revert_mock();
+fetch_revert_mock();
+prepare_revert_mock();
+});
 describe('error/warning conditions', function () {
-it('should require specifying at least one platform

[GitHub] cordova-lib pull request #573: CB-12361 : updated addHelper tests

2017-06-29 Thread stevengill
Github user stevengill commented on a diff in the pull request:

https://github.com/apache/cordova-lib/pull/573#discussion_r124868182
  
--- Diff: spec/cordova/platform/addHelper.spec.js ---
@@ -16,34 +16,459 @@
 */
 /* eslint-env jasmine */
 
+var path = require('path');
+var fs = require('fs');
+var Q = require('q');
+var shell = require('shelljs');
+var events = require('cordova-common').events;
+var rewire = require('rewire');
+var platform_addHelper = rewire('../../../src/cordova/platform/addHelper');
+var platform_module = require('../../../src/cordova/platform');
+var platform_metadata = require('../../../src/cordova/platform_metadata');
+var cordova_util = require('../../../src/cordova/util');
+var cordova_config = require('../../../src/cordova/config');
+var plugman = require('../../../src/plugman/plugman');
+var fetch_metadata = require('../../../src/plugman/util/metadata');
+var lazy_load = require('../../../src/cordova/lazy_load');
+// require module here
+// spy on it and return 
+var cordova = require('../../../src/cordova/cordova');
+var prepare = require('../../../src/cordova/prepare');
+var gitclone = require('../../../src/gitclone');
+var fail;
+
 describe('cordova/platform/addHelper', function () {
+var projectRoot = '/some/path';
+// These _mock and _revert_mock objects use rewire as the modules 
these mocks replace
+// during testing all return functions, which we cannot spy on using 
jasmine.
+// Thus, we replace these modules inside the scope of addHelper.js 
using rewire, and shim
+// in these _mock test dummies. The test dummies themselves are 
constructed using
+// jasmine.createSpy inside the first beforeEach.
+var cfg_parser_mock = function () {};
+var cfg_parser_revert_mock;
+var hooks_mock;
+var platform_api_mock;
+var fetch_mock;
+var fetch_revert_mock;
+var prepare_mock;
+var prepare_revert_mock;
+var fake_platform = {
+'platform': 'atari'
+};
+beforeEach(function () {
+hooks_mock = jasmine.createSpyObj('hooksRunner mock', ['fire']);
+hooks_mock.fire.and.returnValue(Q());
+cfg_parser_mock.prototype = jasmine.createSpyObj('config parser 
mock', ['write', 'removeEngine', 'addEngine', 'getHookScripts']);
+cfg_parser_revert_mock = 
platform_addHelper.__set__('ConfigParser', cfg_parser_mock);
+fetch_mock = jasmine.createSpy('fetch mock').and.returnValue(Q());
+fetch_revert_mock = platform_addHelper.__set__('fetch', 
fetch_mock);
+prepare_mock = jasmine.createSpy('prepare 
mock').and.returnValue(Q());
+prepare_mock.preparePlatforms = 
jasmine.createSpy('preparePlatforms mock').and.returnValue(Q());
+prepare_revert_mock = platform_addHelper.__set__('prepare', 
prepare_mock);
+spyOn(shell, 'mkdir');
+spyOn(fs, 'existsSync').and.returnValue(false);
+spyOn(fs, 'writeFileSync');
+spyOn(cordova_util, 
'projectConfig').and.returnValue(path.join(projectRoot, 'config.xml'));
+spyOn(cordova_util, 'isDirectory').and.returnValue(false);
+spyOn(cordova_util, 'fixRelativePath').and.callFake(function 
(input) { return input; });
+spyOn(cordova_util, 'isUrl').and.returnValue(false);
+spyOn(cordova_util, 'hostSupports').and.returnValue(true);
+spyOn(cordova_util, 'removePlatformPluginsJson');
+spyOn(cordova_config, 'read').and.returnValue({});
+// Fake platform details we will use for our mocks, returned by 
either
+// getPlatfromDetailsFromDir (in the local-directory case), or
+// downloadPlatform (in every other case)
+spyOn(platform_module, 
'getPlatformDetailsFromDir').and.returnValue(Q(fake_platform));
+spyOn(platform_addHelper, 
'downloadPlatform').and.returnValue(Q(fake_platform));
+spyOn(platform_addHelper, 
'getVersionFromConfigFile').and.returnValue(false);
+spyOn(platform_addHelper, 
'installPluginsForNewPlatform').and.returnValue(Q());
+platform_api_mock = jasmine.createSpyObj('platform api mock', 
['createPlatform', 'updatePlatform']);
+platform_api_mock.createPlatform.and.returnValue(Q());
+platform_api_mock.updatePlatform.and.returnValue(Q());
+spyOn(cordova_util, 
'getPlatformApiFunction').and.returnValue(platform_api_mock);
+spyOn(platform_metadata, 'save');
+});
+afterEach(function () {
+cfg_parser_revert_mock();
+fetch_revert_mock();
+prepare_revert_mock();
+});
 describe('error/warning conditions', function () {
-it('should require specifying at least one platform

[GitHub] cordova-lib pull request #573: CB-12361 : updated addHelper tests

2017-06-29 Thread stevengill
Github user stevengill commented on a diff in the pull request:

https://github.com/apache/cordova-lib/pull/573#discussion_r124874233
  
--- Diff: spec/cordova/platform/addHelper.spec.js ---
@@ -16,34 +16,459 @@
 */
 /* eslint-env jasmine */
 
+var path = require('path');
+var fs = require('fs');
+var Q = require('q');
+var shell = require('shelljs');
+var events = require('cordova-common').events;
+var rewire = require('rewire');
+var platform_addHelper = rewire('../../../src/cordova/platform/addHelper');
+var platform_module = require('../../../src/cordova/platform');
+var platform_metadata = require('../../../src/cordova/platform_metadata');
+var cordova_util = require('../../../src/cordova/util');
+var cordova_config = require('../../../src/cordova/config');
+var plugman = require('../../../src/plugman/plugman');
+var fetch_metadata = require('../../../src/plugman/util/metadata');
+var lazy_load = require('../../../src/cordova/lazy_load');
+// require module here
+// spy on it and return 
+var cordova = require('../../../src/cordova/cordova');
+var prepare = require('../../../src/cordova/prepare');
+var gitclone = require('../../../src/gitclone');
+var fail;
+
 describe('cordova/platform/addHelper', function () {
+var projectRoot = '/some/path';
+// These _mock and _revert_mock objects use rewire as the modules 
these mocks replace
+// during testing all return functions, which we cannot spy on using 
jasmine.
+// Thus, we replace these modules inside the scope of addHelper.js 
using rewire, and shim
+// in these _mock test dummies. The test dummies themselves are 
constructed using
+// jasmine.createSpy inside the first beforeEach.
+var cfg_parser_mock = function () {};
+var cfg_parser_revert_mock;
+var hooks_mock;
+var platform_api_mock;
+var fetch_mock;
+var fetch_revert_mock;
+var prepare_mock;
+var prepare_revert_mock;
+var fake_platform = {
+'platform': 'atari'
+};
+beforeEach(function () {
+hooks_mock = jasmine.createSpyObj('hooksRunner mock', ['fire']);
+hooks_mock.fire.and.returnValue(Q());
+cfg_parser_mock.prototype = jasmine.createSpyObj('config parser 
mock', ['write', 'removeEngine', 'addEngine', 'getHookScripts']);
+cfg_parser_revert_mock = 
platform_addHelper.__set__('ConfigParser', cfg_parser_mock);
+fetch_mock = jasmine.createSpy('fetch mock').and.returnValue(Q());
+fetch_revert_mock = platform_addHelper.__set__('fetch', 
fetch_mock);
+prepare_mock = jasmine.createSpy('prepare 
mock').and.returnValue(Q());
+prepare_mock.preparePlatforms = 
jasmine.createSpy('preparePlatforms mock').and.returnValue(Q());
+prepare_revert_mock = platform_addHelper.__set__('prepare', 
prepare_mock);
+spyOn(shell, 'mkdir');
+spyOn(fs, 'existsSync').and.returnValue(false);
+spyOn(fs, 'writeFileSync');
+spyOn(cordova_util, 
'projectConfig').and.returnValue(path.join(projectRoot, 'config.xml'));
+spyOn(cordova_util, 'isDirectory').and.returnValue(false);
+spyOn(cordova_util, 'fixRelativePath').and.callFake(function 
(input) { return input; });
+spyOn(cordova_util, 'isUrl').and.returnValue(false);
+spyOn(cordova_util, 'hostSupports').and.returnValue(true);
+spyOn(cordova_util, 'removePlatformPluginsJson');
+spyOn(cordova_config, 'read').and.returnValue({});
+// Fake platform details we will use for our mocks, returned by 
either
+// getPlatfromDetailsFromDir (in the local-directory case), or
+// downloadPlatform (in every other case)
+spyOn(platform_module, 
'getPlatformDetailsFromDir').and.returnValue(Q(fake_platform));
+spyOn(platform_addHelper, 
'downloadPlatform').and.returnValue(Q(fake_platform));
+spyOn(platform_addHelper, 
'getVersionFromConfigFile').and.returnValue(false);
+spyOn(platform_addHelper, 
'installPluginsForNewPlatform').and.returnValue(Q());
+platform_api_mock = jasmine.createSpyObj('platform api mock', 
['createPlatform', 'updatePlatform']);
+platform_api_mock.createPlatform.and.returnValue(Q());
+platform_api_mock.updatePlatform.and.returnValue(Q());
+spyOn(cordova_util, 
'getPlatformApiFunction').and.returnValue(platform_api_mock);
+spyOn(platform_metadata, 'save');
+});
+afterEach(function () {
+cfg_parser_revert_mock();
+fetch_revert_mock();
+prepare_revert_mock();
+});
 describe('error/warning conditions', function () {
-it('should require specifying at least one platform

[GitHub] cordova-lib pull request #573: CB-12361 : updated addHelper tests

2017-06-29 Thread stevengill
Github user stevengill commented on a diff in the pull request:

https://github.com/apache/cordova-lib/pull/573#discussion_r124881302
  
--- Diff: spec/cordova/platform/addHelper.spec.js ---
@@ -16,34 +16,459 @@
 */
 /* eslint-env jasmine */
 
+var path = require('path');
+var fs = require('fs');
+var Q = require('q');
+var shell = require('shelljs');
+var events = require('cordova-common').events;
+var rewire = require('rewire');
+var platform_addHelper = rewire('../../../src/cordova/platform/addHelper');
+var platform_module = require('../../../src/cordova/platform');
+var platform_metadata = require('../../../src/cordova/platform_metadata');
+var cordova_util = require('../../../src/cordova/util');
+var cordova_config = require('../../../src/cordova/config');
+var plugman = require('../../../src/plugman/plugman');
+var fetch_metadata = require('../../../src/plugman/util/metadata');
+var lazy_load = require('../../../src/cordova/lazy_load');
+// require module here
+// spy on it and return 
+var cordova = require('../../../src/cordova/cordova');
+var prepare = require('../../../src/cordova/prepare');
+var gitclone = require('../../../src/gitclone');
+var fail;
+
 describe('cordova/platform/addHelper', function () {
+var projectRoot = '/some/path';
+// These _mock and _revert_mock objects use rewire as the modules 
these mocks replace
+// during testing all return functions, which we cannot spy on using 
jasmine.
+// Thus, we replace these modules inside the scope of addHelper.js 
using rewire, and shim
+// in these _mock test dummies. The test dummies themselves are 
constructed using
+// jasmine.createSpy inside the first beforeEach.
+var cfg_parser_mock = function () {};
+var cfg_parser_revert_mock;
+var hooks_mock;
+var platform_api_mock;
+var fetch_mock;
+var fetch_revert_mock;
+var prepare_mock;
+var prepare_revert_mock;
+var fake_platform = {
+'platform': 'atari'
+};
+beforeEach(function () {
+hooks_mock = jasmine.createSpyObj('hooksRunner mock', ['fire']);
+hooks_mock.fire.and.returnValue(Q());
+cfg_parser_mock.prototype = jasmine.createSpyObj('config parser 
mock', ['write', 'removeEngine', 'addEngine', 'getHookScripts']);
+cfg_parser_revert_mock = 
platform_addHelper.__set__('ConfigParser', cfg_parser_mock);
+fetch_mock = jasmine.createSpy('fetch mock').and.returnValue(Q());
+fetch_revert_mock = platform_addHelper.__set__('fetch', 
fetch_mock);
+prepare_mock = jasmine.createSpy('prepare 
mock').and.returnValue(Q());
+prepare_mock.preparePlatforms = 
jasmine.createSpy('preparePlatforms mock').and.returnValue(Q());
+prepare_revert_mock = platform_addHelper.__set__('prepare', 
prepare_mock);
+spyOn(shell, 'mkdir');
+spyOn(fs, 'existsSync').and.returnValue(false);
+spyOn(fs, 'writeFileSync');
+spyOn(cordova_util, 
'projectConfig').and.returnValue(path.join(projectRoot, 'config.xml'));
+spyOn(cordova_util, 'isDirectory').and.returnValue(false);
+spyOn(cordova_util, 'fixRelativePath').and.callFake(function 
(input) { return input; });
+spyOn(cordova_util, 'isUrl').and.returnValue(false);
+spyOn(cordova_util, 'hostSupports').and.returnValue(true);
+spyOn(cordova_util, 'removePlatformPluginsJson');
+spyOn(cordova_config, 'read').and.returnValue({});
+// Fake platform details we will use for our mocks, returned by 
either
+// getPlatfromDetailsFromDir (in the local-directory case), or
+// downloadPlatform (in every other case)
+spyOn(platform_module, 
'getPlatformDetailsFromDir').and.returnValue(Q(fake_platform));
+spyOn(platform_addHelper, 
'downloadPlatform').and.returnValue(Q(fake_platform));
+spyOn(platform_addHelper, 
'getVersionFromConfigFile').and.returnValue(false);
+spyOn(platform_addHelper, 
'installPluginsForNewPlatform').and.returnValue(Q());
+platform_api_mock = jasmine.createSpyObj('platform api mock', 
['createPlatform', 'updatePlatform']);
+platform_api_mock.createPlatform.and.returnValue(Q());
+platform_api_mock.updatePlatform.and.returnValue(Q());
+spyOn(cordova_util, 
'getPlatformApiFunction').and.returnValue(platform_api_mock);
+spyOn(platform_metadata, 'save');
+});
+afterEach(function () {
+cfg_parser_revert_mock();
+fetch_revert_mock();
+prepare_revert_mock();
+});
 describe('error/warning conditions', function () {
-it('should require specifying at least one platform

[GitHub] cordova-lib pull request #573: CB-12361 : updated addHelper tests

2017-06-29 Thread stevengill
Github user stevengill commented on a diff in the pull request:

https://github.com/apache/cordova-lib/pull/573#discussion_r124878766
  
--- Diff: spec/cordova/platform/addHelper.spec.js ---
@@ -16,34 +16,459 @@
 */
 /* eslint-env jasmine */
 
+var path = require('path');
+var fs = require('fs');
+var Q = require('q');
+var shell = require('shelljs');
+var events = require('cordova-common').events;
+var rewire = require('rewire');
+var platform_addHelper = rewire('../../../src/cordova/platform/addHelper');
+var platform_module = require('../../../src/cordova/platform');
+var platform_metadata = require('../../../src/cordova/platform_metadata');
+var cordova_util = require('../../../src/cordova/util');
+var cordova_config = require('../../../src/cordova/config');
+var plugman = require('../../../src/plugman/plugman');
+var fetch_metadata = require('../../../src/plugman/util/metadata');
+var lazy_load = require('../../../src/cordova/lazy_load');
+// require module here
+// spy on it and return 
+var cordova = require('../../../src/cordova/cordova');
+var prepare = require('../../../src/cordova/prepare');
+var gitclone = require('../../../src/gitclone');
+var fail;
+
 describe('cordova/platform/addHelper', function () {
+var projectRoot = '/some/path';
+// These _mock and _revert_mock objects use rewire as the modules 
these mocks replace
+// during testing all return functions, which we cannot spy on using 
jasmine.
+// Thus, we replace these modules inside the scope of addHelper.js 
using rewire, and shim
+// in these _mock test dummies. The test dummies themselves are 
constructed using
+// jasmine.createSpy inside the first beforeEach.
+var cfg_parser_mock = function () {};
+var cfg_parser_revert_mock;
+var hooks_mock;
+var platform_api_mock;
+var fetch_mock;
+var fetch_revert_mock;
+var prepare_mock;
+var prepare_revert_mock;
+var fake_platform = {
+'platform': 'atari'
+};
+beforeEach(function () {
+hooks_mock = jasmine.createSpyObj('hooksRunner mock', ['fire']);
+hooks_mock.fire.and.returnValue(Q());
+cfg_parser_mock.prototype = jasmine.createSpyObj('config parser 
mock', ['write', 'removeEngine', 'addEngine', 'getHookScripts']);
+cfg_parser_revert_mock = 
platform_addHelper.__set__('ConfigParser', cfg_parser_mock);
+fetch_mock = jasmine.createSpy('fetch mock').and.returnValue(Q());
+fetch_revert_mock = platform_addHelper.__set__('fetch', 
fetch_mock);
+prepare_mock = jasmine.createSpy('prepare 
mock').and.returnValue(Q());
+prepare_mock.preparePlatforms = 
jasmine.createSpy('preparePlatforms mock').and.returnValue(Q());
+prepare_revert_mock = platform_addHelper.__set__('prepare', 
prepare_mock);
+spyOn(shell, 'mkdir');
+spyOn(fs, 'existsSync').and.returnValue(false);
+spyOn(fs, 'writeFileSync');
+spyOn(cordova_util, 
'projectConfig').and.returnValue(path.join(projectRoot, 'config.xml'));
+spyOn(cordova_util, 'isDirectory').and.returnValue(false);
+spyOn(cordova_util, 'fixRelativePath').and.callFake(function 
(input) { return input; });
+spyOn(cordova_util, 'isUrl').and.returnValue(false);
+spyOn(cordova_util, 'hostSupports').and.returnValue(true);
+spyOn(cordova_util, 'removePlatformPluginsJson');
+spyOn(cordova_config, 'read').and.returnValue({});
+// Fake platform details we will use for our mocks, returned by 
either
+// getPlatfromDetailsFromDir (in the local-directory case), or
+// downloadPlatform (in every other case)
+spyOn(platform_module, 
'getPlatformDetailsFromDir').and.returnValue(Q(fake_platform));
+spyOn(platform_addHelper, 
'downloadPlatform').and.returnValue(Q(fake_platform));
+spyOn(platform_addHelper, 
'getVersionFromConfigFile').and.returnValue(false);
+spyOn(platform_addHelper, 
'installPluginsForNewPlatform').and.returnValue(Q());
+platform_api_mock = jasmine.createSpyObj('platform api mock', 
['createPlatform', 'updatePlatform']);
+platform_api_mock.createPlatform.and.returnValue(Q());
+platform_api_mock.updatePlatform.and.returnValue(Q());
+spyOn(cordova_util, 
'getPlatformApiFunction').and.returnValue(platform_api_mock);
+spyOn(platform_metadata, 'save');
+});
+afterEach(function () {
+cfg_parser_revert_mock();
+fetch_revert_mock();
+prepare_revert_mock();
+});
 describe('error/warning conditions', function () {
-it('should require specifying at least one platform

[GitHub] cordova-lib pull request #573: CB-12361 : updated addHelper tests

2017-06-29 Thread stevengill
Github user stevengill commented on a diff in the pull request:

https://github.com/apache/cordova-lib/pull/573#discussion_r124879465
  
--- Diff: spec/cordova/platform/addHelper.spec.js ---
@@ -16,34 +16,459 @@
 */
 /* eslint-env jasmine */
 
+var path = require('path');
+var fs = require('fs');
+var Q = require('q');
+var shell = require('shelljs');
+var events = require('cordova-common').events;
+var rewire = require('rewire');
+var platform_addHelper = rewire('../../../src/cordova/platform/addHelper');
+var platform_module = require('../../../src/cordova/platform');
+var platform_metadata = require('../../../src/cordova/platform_metadata');
+var cordova_util = require('../../../src/cordova/util');
+var cordova_config = require('../../../src/cordova/config');
+var plugman = require('../../../src/plugman/plugman');
+var fetch_metadata = require('../../../src/plugman/util/metadata');
+var lazy_load = require('../../../src/cordova/lazy_load');
+// require module here
+// spy on it and return 
+var cordova = require('../../../src/cordova/cordova');
+var prepare = require('../../../src/cordova/prepare');
+var gitclone = require('../../../src/gitclone');
+var fail;
+
 describe('cordova/platform/addHelper', function () {
+var projectRoot = '/some/path';
+// These _mock and _revert_mock objects use rewire as the modules 
these mocks replace
+// during testing all return functions, which we cannot spy on using 
jasmine.
+// Thus, we replace these modules inside the scope of addHelper.js 
using rewire, and shim
+// in these _mock test dummies. The test dummies themselves are 
constructed using
+// jasmine.createSpy inside the first beforeEach.
+var cfg_parser_mock = function () {};
+var cfg_parser_revert_mock;
+var hooks_mock;
+var platform_api_mock;
+var fetch_mock;
+var fetch_revert_mock;
+var prepare_mock;
+var prepare_revert_mock;
+var fake_platform = {
+'platform': 'atari'
+};
+beforeEach(function () {
+hooks_mock = jasmine.createSpyObj('hooksRunner mock', ['fire']);
+hooks_mock.fire.and.returnValue(Q());
+cfg_parser_mock.prototype = jasmine.createSpyObj('config parser 
mock', ['write', 'removeEngine', 'addEngine', 'getHookScripts']);
+cfg_parser_revert_mock = 
platform_addHelper.__set__('ConfigParser', cfg_parser_mock);
+fetch_mock = jasmine.createSpy('fetch mock').and.returnValue(Q());
+fetch_revert_mock = platform_addHelper.__set__('fetch', 
fetch_mock);
+prepare_mock = jasmine.createSpy('prepare 
mock').and.returnValue(Q());
+prepare_mock.preparePlatforms = 
jasmine.createSpy('preparePlatforms mock').and.returnValue(Q());
+prepare_revert_mock = platform_addHelper.__set__('prepare', 
prepare_mock);
+spyOn(shell, 'mkdir');
+spyOn(fs, 'existsSync').and.returnValue(false);
+spyOn(fs, 'writeFileSync');
+spyOn(cordova_util, 
'projectConfig').and.returnValue(path.join(projectRoot, 'config.xml'));
+spyOn(cordova_util, 'isDirectory').and.returnValue(false);
+spyOn(cordova_util, 'fixRelativePath').and.callFake(function 
(input) { return input; });
+spyOn(cordova_util, 'isUrl').and.returnValue(false);
+spyOn(cordova_util, 'hostSupports').and.returnValue(true);
+spyOn(cordova_util, 'removePlatformPluginsJson');
+spyOn(cordova_config, 'read').and.returnValue({});
+// Fake platform details we will use for our mocks, returned by 
either
+// getPlatfromDetailsFromDir (in the local-directory case), or
+// downloadPlatform (in every other case)
+spyOn(platform_module, 
'getPlatformDetailsFromDir').and.returnValue(Q(fake_platform));
+spyOn(platform_addHelper, 
'downloadPlatform').and.returnValue(Q(fake_platform));
+spyOn(platform_addHelper, 
'getVersionFromConfigFile').and.returnValue(false);
+spyOn(platform_addHelper, 
'installPluginsForNewPlatform').and.returnValue(Q());
+platform_api_mock = jasmine.createSpyObj('platform api mock', 
['createPlatform', 'updatePlatform']);
+platform_api_mock.createPlatform.and.returnValue(Q());
+platform_api_mock.updatePlatform.and.returnValue(Q());
+spyOn(cordova_util, 
'getPlatformApiFunction').and.returnValue(platform_api_mock);
+spyOn(platform_metadata, 'save');
+});
+afterEach(function () {
+cfg_parser_revert_mock();
+fetch_revert_mock();
+prepare_revert_mock();
+});
 describe('error/warning conditions', function () {
-it('should require specifying at least one platform

[GitHub] cordova-lib pull request #573: CB-12361 : updated addHelper tests

2017-06-29 Thread stevengill
Github user stevengill commented on a diff in the pull request:

https://github.com/apache/cordova-lib/pull/573#discussion_r124878233
  
--- Diff: spec/cordova/platform/addHelper.spec.js ---
@@ -16,34 +16,459 @@
 */
 /* eslint-env jasmine */
 
+var path = require('path');
+var fs = require('fs');
+var Q = require('q');
+var shell = require('shelljs');
+var events = require('cordova-common').events;
+var rewire = require('rewire');
+var platform_addHelper = rewire('../../../src/cordova/platform/addHelper');
+var platform_module = require('../../../src/cordova/platform');
+var platform_metadata = require('../../../src/cordova/platform_metadata');
+var cordova_util = require('../../../src/cordova/util');
+var cordova_config = require('../../../src/cordova/config');
+var plugman = require('../../../src/plugman/plugman');
+var fetch_metadata = require('../../../src/plugman/util/metadata');
+var lazy_load = require('../../../src/cordova/lazy_load');
+// require module here
+// spy on it and return 
+var cordova = require('../../../src/cordova/cordova');
+var prepare = require('../../../src/cordova/prepare');
+var gitclone = require('../../../src/gitclone');
+var fail;
+
 describe('cordova/platform/addHelper', function () {
+var projectRoot = '/some/path';
+// These _mock and _revert_mock objects use rewire as the modules 
these mocks replace
+// during testing all return functions, which we cannot spy on using 
jasmine.
+// Thus, we replace these modules inside the scope of addHelper.js 
using rewire, and shim
+// in these _mock test dummies. The test dummies themselves are 
constructed using
+// jasmine.createSpy inside the first beforeEach.
+var cfg_parser_mock = function () {};
+var cfg_parser_revert_mock;
+var hooks_mock;
+var platform_api_mock;
+var fetch_mock;
+var fetch_revert_mock;
+var prepare_mock;
+var prepare_revert_mock;
+var fake_platform = {
+'platform': 'atari'
+};
+beforeEach(function () {
+hooks_mock = jasmine.createSpyObj('hooksRunner mock', ['fire']);
+hooks_mock.fire.and.returnValue(Q());
+cfg_parser_mock.prototype = jasmine.createSpyObj('config parser 
mock', ['write', 'removeEngine', 'addEngine', 'getHookScripts']);
+cfg_parser_revert_mock = 
platform_addHelper.__set__('ConfigParser', cfg_parser_mock);
+fetch_mock = jasmine.createSpy('fetch mock').and.returnValue(Q());
+fetch_revert_mock = platform_addHelper.__set__('fetch', 
fetch_mock);
+prepare_mock = jasmine.createSpy('prepare 
mock').and.returnValue(Q());
+prepare_mock.preparePlatforms = 
jasmine.createSpy('preparePlatforms mock').and.returnValue(Q());
+prepare_revert_mock = platform_addHelper.__set__('prepare', 
prepare_mock);
+spyOn(shell, 'mkdir');
+spyOn(fs, 'existsSync').and.returnValue(false);
+spyOn(fs, 'writeFileSync');
+spyOn(cordova_util, 
'projectConfig').and.returnValue(path.join(projectRoot, 'config.xml'));
+spyOn(cordova_util, 'isDirectory').and.returnValue(false);
+spyOn(cordova_util, 'fixRelativePath').and.callFake(function 
(input) { return input; });
+spyOn(cordova_util, 'isUrl').and.returnValue(false);
+spyOn(cordova_util, 'hostSupports').and.returnValue(true);
+spyOn(cordova_util, 'removePlatformPluginsJson');
+spyOn(cordova_config, 'read').and.returnValue({});
+// Fake platform details we will use for our mocks, returned by 
either
+// getPlatfromDetailsFromDir (in the local-directory case), or
+// downloadPlatform (in every other case)
+spyOn(platform_module, 
'getPlatformDetailsFromDir').and.returnValue(Q(fake_platform));
+spyOn(platform_addHelper, 
'downloadPlatform').and.returnValue(Q(fake_platform));
+spyOn(platform_addHelper, 
'getVersionFromConfigFile').and.returnValue(false);
+spyOn(platform_addHelper, 
'installPluginsForNewPlatform').and.returnValue(Q());
+platform_api_mock = jasmine.createSpyObj('platform api mock', 
['createPlatform', 'updatePlatform']);
+platform_api_mock.createPlatform.and.returnValue(Q());
+platform_api_mock.updatePlatform.and.returnValue(Q());
+spyOn(cordova_util, 
'getPlatformApiFunction').and.returnValue(platform_api_mock);
+spyOn(platform_metadata, 'save');
+});
+afterEach(function () {
+cfg_parser_revert_mock();
+fetch_revert_mock();
+prepare_revert_mock();
+});
 describe('error/warning conditions', function () {
-it('should require specifying at least one platform

[GitHub] cordova-lib pull request #573: CB-12361 : updated addHelper tests

2017-06-29 Thread stevengill
Github user stevengill commented on a diff in the pull request:

https://github.com/apache/cordova-lib/pull/573#discussion_r124878443
  
--- Diff: spec/cordova/platform/addHelper.spec.js ---
@@ -16,34 +16,459 @@
 */
 /* eslint-env jasmine */
 
+var path = require('path');
+var fs = require('fs');
+var Q = require('q');
+var shell = require('shelljs');
+var events = require('cordova-common').events;
+var rewire = require('rewire');
+var platform_addHelper = rewire('../../../src/cordova/platform/addHelper');
+var platform_module = require('../../../src/cordova/platform');
+var platform_metadata = require('../../../src/cordova/platform_metadata');
+var cordova_util = require('../../../src/cordova/util');
+var cordova_config = require('../../../src/cordova/config');
+var plugman = require('../../../src/plugman/plugman');
+var fetch_metadata = require('../../../src/plugman/util/metadata');
+var lazy_load = require('../../../src/cordova/lazy_load');
+// require module here
+// spy on it and return 
+var cordova = require('../../../src/cordova/cordova');
+var prepare = require('../../../src/cordova/prepare');
+var gitclone = require('../../../src/gitclone');
+var fail;
+
 describe('cordova/platform/addHelper', function () {
+var projectRoot = '/some/path';
+// These _mock and _revert_mock objects use rewire as the modules 
these mocks replace
+// during testing all return functions, which we cannot spy on using 
jasmine.
+// Thus, we replace these modules inside the scope of addHelper.js 
using rewire, and shim
+// in these _mock test dummies. The test dummies themselves are 
constructed using
+// jasmine.createSpy inside the first beforeEach.
+var cfg_parser_mock = function () {};
+var cfg_parser_revert_mock;
+var hooks_mock;
+var platform_api_mock;
+var fetch_mock;
+var fetch_revert_mock;
+var prepare_mock;
+var prepare_revert_mock;
+var fake_platform = {
+'platform': 'atari'
+};
+beforeEach(function () {
+hooks_mock = jasmine.createSpyObj('hooksRunner mock', ['fire']);
+hooks_mock.fire.and.returnValue(Q());
+cfg_parser_mock.prototype = jasmine.createSpyObj('config parser 
mock', ['write', 'removeEngine', 'addEngine', 'getHookScripts']);
+cfg_parser_revert_mock = 
platform_addHelper.__set__('ConfigParser', cfg_parser_mock);
+fetch_mock = jasmine.createSpy('fetch mock').and.returnValue(Q());
+fetch_revert_mock = platform_addHelper.__set__('fetch', 
fetch_mock);
+prepare_mock = jasmine.createSpy('prepare 
mock').and.returnValue(Q());
+prepare_mock.preparePlatforms = 
jasmine.createSpy('preparePlatforms mock').and.returnValue(Q());
+prepare_revert_mock = platform_addHelper.__set__('prepare', 
prepare_mock);
+spyOn(shell, 'mkdir');
+spyOn(fs, 'existsSync').and.returnValue(false);
+spyOn(fs, 'writeFileSync');
+spyOn(cordova_util, 
'projectConfig').and.returnValue(path.join(projectRoot, 'config.xml'));
+spyOn(cordova_util, 'isDirectory').and.returnValue(false);
+spyOn(cordova_util, 'fixRelativePath').and.callFake(function 
(input) { return input; });
+spyOn(cordova_util, 'isUrl').and.returnValue(false);
+spyOn(cordova_util, 'hostSupports').and.returnValue(true);
+spyOn(cordova_util, 'removePlatformPluginsJson');
+spyOn(cordova_config, 'read').and.returnValue({});
+// Fake platform details we will use for our mocks, returned by 
either
+// getPlatfromDetailsFromDir (in the local-directory case), or
+// downloadPlatform (in every other case)
+spyOn(platform_module, 
'getPlatformDetailsFromDir').and.returnValue(Q(fake_platform));
+spyOn(platform_addHelper, 
'downloadPlatform').and.returnValue(Q(fake_platform));
+spyOn(platform_addHelper, 
'getVersionFromConfigFile').and.returnValue(false);
+spyOn(platform_addHelper, 
'installPluginsForNewPlatform').and.returnValue(Q());
+platform_api_mock = jasmine.createSpyObj('platform api mock', 
['createPlatform', 'updatePlatform']);
+platform_api_mock.createPlatform.and.returnValue(Q());
+platform_api_mock.updatePlatform.and.returnValue(Q());
+spyOn(cordova_util, 
'getPlatformApiFunction').and.returnValue(platform_api_mock);
+spyOn(platform_metadata, 'save');
+});
+afterEach(function () {
+cfg_parser_revert_mock();
+fetch_revert_mock();
+prepare_revert_mock();
+});
 describe('error/warning conditions', function () {
-it('should require specifying at least one platform

[GitHub] cordova-lib pull request #569: CB-12361: added main function unit tests for ...

2017-06-26 Thread stevengill
GitHub user stevengill opened a pull request:

https://github.com/apache/cordova-lib/pull/569

CB-12361: added main function unit tests for plugin add.spec.js

work in progress. Please don't merge

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/stevengill/cordova-lib plugin-add

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/cordova-lib/pull/569.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #569


commit d486d473ab50420f1a79bfe88cca33d7721c26d7
Author: Steve Gill <stevengil...@gmail.com>
Date:   2017-06-26T18:32:45Z

CB-12361: added main function unit tests for plugin add.spec.js




---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org



[GitHub] cordova-lib issue #568: Reorganized unit test directory structure + updated ...

2017-06-26 Thread stevengill
Github user stevengill commented on the issue:

https://github.com/apache/cordova-lib/pull/568
  
looks good to me. Only thing I would add is updating travis and appveyor as 
well. They run the integration tests as a separate task currently. 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org



[GitHub] cordova-lib issue #567: CB-12361: Added unit tests for prepare.js

2017-06-22 Thread stevengill
Github user stevengill commented on the issue:

https://github.com/apache/cordova-lib/pull/567
  
Sent an update with changes based on feedback


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org



[GitHub] cordova-lib pull request #567: CB-12361: Added unit tests for prepare.js

2017-06-21 Thread stevengill
Github user stevengill commented on a diff in the pull request:

https://github.com/apache/cordova-lib/pull/567#discussion_r123426288
  
--- Diff: spec-cordova/prepare.spec.js ---
@@ -17,175 +17,265 @@
 under the License.
 */
 
-var shell = require('shelljs'),
-PlatformJson = require('cordova-common').PlatformJson,
-path = require('path'),
-util = require('../src/cordova/util'),
-prepare = require('../src/cordova/prepare'),
-lazy_load = require('../src/cordova/lazy_load'),
-ConfigParser = require('cordova-common').ConfigParser,
-platforms = require('../src/platforms/platforms'),
-HooksRunner = require('../src/hooks/HooksRunner'),
-xmlHelpers = require('cordova-common').xmlHelpers,
-et = require('elementtree'),
-Q = require('q');
+/* eslint-env jasmine */
 
-var project_dir = '/some/path';
-var supported_platforms = Object.keys(platforms).filter(function(p) { 
return p != 'www'; });
-
-var TEST_XML = '\n' +
-'http://www.w3.org/ns/widgets"\n' +
-'xmlns:cdv = "http://cordova.apache.org/ns/1.0"\n' +
-'id= "io.cordova.hellocordova"\n' +
-'version   = "0.0.1">\n' +
-'Hello Cordova\n' +
-'\n' +
-'A sample Apache Cordova application that responds to the 
deviceready event.\n' +
-'\n' +
-'http://cordova.io; 
email="dev@cordova.apache.org">\n' +
-'Apache Cordova Team\n' +
-'\n' +
-'\n' +
-'\n' +
-'\n' +
-'\n' +
-'\n';
+var path = require('path');
+var rewire = require('rewire');
+var util = require('../src/cordova/util');
+var cordova_config = require('../src/cordova/config');
+var prepare = rewire('../src/cordova/prepare');
+var restore = require('../src/cordova/restore-util');
+var platforms = require('../src/platforms/platforms');
+var HooksRunner = require('../src/hooks/HooksRunner');
+var Q = require('q');
+var PlatformJson = require('cordova-common').PlatformJson;
+var PluginInfoProvider = require('cordova-common').PluginInfoProvider;
 
-describe('prepare command', function() {
-var is_cordova,
-cd_project_root,
-list_platforms,
-fire,
-parsers = {},
-find_plugins,
-cp,
-mkdir,
-load, platformApi, getPlatformApi;
+var project_dir = '/some/path';
 
+describe('cordova/prepare', function () {
+let platform_api_prepare_mock;
--- End diff --

I've converted them back to `var` for now and will bring up es6 on the list


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org



[GitHub] cordova-lib pull request #567: CB-12361: Added unit tests for prepare.js

2017-06-21 Thread stevengill
Github user stevengill commented on a diff in the pull request:

https://github.com/apache/cordova-lib/pull/567#discussion_r123367811
  
--- Diff: spec-cordova/prepare.spec.js ---
@@ -17,175 +17,265 @@
 under the License.
 */
 
-var shell = require('shelljs'),
-PlatformJson = require('cordova-common').PlatformJson,
-path = require('path'),
-util = require('../src/cordova/util'),
-prepare = require('../src/cordova/prepare'),
-lazy_load = require('../src/cordova/lazy_load'),
-ConfigParser = require('cordova-common').ConfigParser,
-platforms = require('../src/platforms/platforms'),
-HooksRunner = require('../src/hooks/HooksRunner'),
-xmlHelpers = require('cordova-common').xmlHelpers,
-et = require('elementtree'),
-Q = require('q');
+/* eslint-env jasmine */
 
-var project_dir = '/some/path';
-var supported_platforms = Object.keys(platforms).filter(function(p) { 
return p != 'www'; });
-
-var TEST_XML = '\n' +
-'http://www.w3.org/ns/widgets"\n' +
-'xmlns:cdv = "http://cordova.apache.org/ns/1.0"\n' +
-'id= "io.cordova.hellocordova"\n' +
-'version   = "0.0.1">\n' +
-'Hello Cordova\n' +
-'\n' +
-'A sample Apache Cordova application that responds to the 
deviceready event.\n' +
-'\n' +
-'http://cordova.io; 
email="dev@cordova.apache.org">\n' +
-'Apache Cordova Team\n' +
-'\n' +
-'\n' +
-'\n' +
-'\n' +
-'\n' +
-'\n';
+var path = require('path');
+var rewire = require('rewire');
+var util = require('../src/cordova/util');
+var cordova_config = require('../src/cordova/config');
+var prepare = rewire('../src/cordova/prepare');
+var restore = require('../src/cordova/restore-util');
+var platforms = require('../src/platforms/platforms');
+var HooksRunner = require('../src/hooks/HooksRunner');
+var Q = require('q');
+var PlatformJson = require('cordova-common').PlatformJson;
+var PluginInfoProvider = require('cordova-common').PluginInfoProvider;
 
-describe('prepare command', function() {
-var is_cordova,
-cd_project_root,
-list_platforms,
-fire,
-parsers = {},
-find_plugins,
-cp,
-mkdir,
-load, platformApi, getPlatformApi;
+var project_dir = '/some/path';
 
+describe('cordova/prepare', function () {
+let platform_api_prepare_mock;
+let platform_api_add_mock;
+let platform_api_remove_mock;
 beforeEach(function () {
-getPlatformApi = spyOn(platforms, 
'getPlatformApi').and.callFake(function (platform, rootDir) {
+platform_api_prepare_mock = 
jasmine.createSpy('prepare').and.returnValue(Q());
+platform_api_add_mock = 
jasmine.createSpy('add').and.returnValue(Q());
+platform_api_remove_mock = 
jasmine.createSpy('remove').and.returnValue(Q());
+spyOn(platforms, 'getPlatformApi').and.callFake(function 
(platform, rootDir) {
 return {
-prepare: jasmine.createSpy('prepare').and.returnValue(Q()),
+prepare: platform_api_prepare_mock,
 getPlatformInfo: 
jasmine.createSpy('getPlatformInfo').and.returnValue({
 locations: {
 www: path.join(project_dir, 'platforms', platform, 
'www')
 }
 }),
+removePlugin: platform_api_add_mock,
+addPlugin: platform_api_remove_mock
 };
 });
-is_cordova = spyOn(util, 'isCordova').and.returnValue(project_dir);
-cd_project_root = spyOn(util, 
'cdProjectRoot').and.returnValue(project_dir);
-list_platforms = spyOn(util, 
'listPlatforms').and.returnValue(supported_platforms);
-fire = spyOn(HooksRunner.prototype, 'fire').and.returnValue(Q());
-
-find_plugins = spyOn(util, 'findPlugins').and.returnValue([]);
-spyOn(PlatformJson, 'load').and.returnValue(new PlatformJson(null, 
null, {}));
-spyOn(PlatformJson.prototype, 'save');
-load = spyOn(lazy_load, 'based_on_config').and.returnValue(Q());
-cp = spyOn(shell, 'cp').and.returnValue(true);
-mkdir = spyOn(shell, 'mkdir');
-spyOn(ConfigParser.prototype, 'write');
-spyOn(xmlHelpers, 'parseElementtreeSync').and.callFake(function() {
-return new et.ElementTree(et.XML(TEST_XML));
-});
+spyOn(PlatformJson, 'load');
+spyOn(HooksRunner.prototype, 'fire').and.returnValue(Q());
  

  1   2   3   4   5   6   7   8   >