subject:"\[GitHub\] cordova\-lib pull request\:"

[GitHub] cordova-lib pull request #600: CB-13478 : Fix CRLF line endings and lint int...

2017-10-27 Thread asfgit

Github user asfgit closed the pull request at:

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


---

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

[GitHub] cordova-lib pull request #603: CB-12361 : added unit tests for check.js

2017-10-27 Thread audreyso

GitHub user audreyso opened a pull request:

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

CB-12361 : added unit tests for check.js



### Platforms affected


### What does this PR do?
Added unit tests for check.js to increase coverage

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


### Checklist
- [X] [Reported an issue](http://cordova.apache.org/contribute/issues.html) 
in the JIRA database
- [X] 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.
- [X] 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/audreyso/cordova-lib CB-12361-check.js

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

https://github.com/apache/cordova-lib/pull/603.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 #603


commit d6a9afe9087327268e0e4c2d5cffb4424908f4d4
Author: Audrey So 
Date:   2017-10-24T22:26:41Z

CB-12361 : added unit tests for check.js




---

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

[GitHub] cordova-lib pull request #602: CB-12774 : Don't munge scoped plugin IDs anym...

2017-10-25 Thread audreyso

Github user audreyso commented on a diff in the pull request:

https://github.com/apache/cordova-lib/pull/602#discussion_r147014589
  
--- Diff: src/cordova/util.js ---
@@ -255,11 +255,20 @@ function findPlugins (pluginDir) {
 var plugins = [];
 
 if (fs.existsSync(pluginDir)) {
-plugins = fs.readdirSync(pluginDir).filter(function (fileName) {
-var pluginPath = path.join(pluginDir, fileName);
-var isPlugin = isDirectory(pluginPath) || 
isSymbolicLink(pluginPath);
-return fileName !== '.svn' && fileName !== 'CVS' && isPlugin;
-});
+plugins = fs.readdirSync(pluginDir)
+.reduce(function (plugins, pluginOrScope) {
+if (pluginOrScope[0] === '@') {
+plugins.push(...fs.readdirSync(path.join(pluginDir, 
pluginOrScope)).map(s => path.join(pluginOrScope, s)));
--- End diff --

Thanks for your question! We are sticking to the current style for now. We 
need to have a discussion on our mailing list about starting to use es2015 
features/ which ones to use and get some consensus.


---

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

[GitHub] cordova-lib pull request #602: CB-12774 : Don't munge scoped plugin IDs anym...

2017-10-25 Thread akdor1154

Github user akdor1154 commented on a diff in the pull request:

https://github.com/apache/cordova-lib/pull/602#discussion_r147012928
  
--- Diff: src/cordova/util.js ---
@@ -255,11 +255,20 @@ function findPlugins (pluginDir) {
 var plugins = [];
 
 if (fs.existsSync(pluginDir)) {
-plugins = fs.readdirSync(pluginDir).filter(function (fileName) {
-var pluginPath = path.join(pluginDir, fileName);
-var isPlugin = isDirectory(pluginPath) || 
isSymbolicLink(pluginPath);
-return fileName !== '.svn' && fileName !== 'CVS' && isPlugin;
-});
+plugins = fs.readdirSync(pluginDir)
+.reduce(function (plugins, pluginOrScope) {
+if (pluginOrScope[0] === '@') {
+plugins.push(...fs.readdirSync(path.join(pluginDir, 
pluginOrScope)).map(s => path.join(pluginOrScope, s)));
--- End diff --

yup just started looking at the failed test :) While you're here and kind 
of related, I noticed most of the Cordova codebase avoids `() => arrow 
functions`. Is this just historical? I have used full `function () { }` when 
I've paid attention just to match code style, but is this necessary?


---

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

[GitHub] cordova-lib pull request #602: CB-12774 : Don't munge scoped plugin IDs anym...

2017-10-25 Thread audreyso

Github user audreyso commented on a diff in the pull request:

https://github.com/apache/cordova-lib/pull/602#discussion_r147012615
  
--- Diff: src/cordova/util.js ---
@@ -255,11 +255,20 @@ function findPlugins (pluginDir) {
 var plugins = [];
 
 if (fs.existsSync(pluginDir)) {
-plugins = fs.readdirSync(pluginDir).filter(function (fileName) {
-var pluginPath = path.join(pluginDir, fileName);
-var isPlugin = isDirectory(pluginPath) || 
isSymbolicLink(pluginPath);
-return fileName !== '.svn' && fileName !== 'CVS' && isPlugin;
-});
+plugins = fs.readdirSync(pluginDir)
+.reduce(function (plugins, pluginOrScope) {
+if (pluginOrScope[0] === '@') {
+plugins.push(...fs.readdirSync(path.join(pluginDir, 
pluginOrScope)).map(s => path.join(pluginOrScope, s)));
--- End diff --

Hi! This line will need to be changed as it will not work with Node 4, 
which is still currently supported. 
http://node.green/#ES2015-syntax-spread---operator-with-generic-iterables--in-calls


---

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

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

2017-10-25 Thread asfgit

Github user asfgit closed the pull request at:

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


---

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

[GitHub] cordova-lib pull request #602: CB-12774 : Don't munge scoped plugin IDs anym...

2017-10-23 Thread akdor1154

GitHub user akdor1154 opened a pull request:

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

CB-12774 : Don't munge scoped plugin IDs anymore. Comment requested.

In order to get plugins that are under an npm scope (e.g. 
`@akdor1154/some-plugin`) to install, a previous change from took the approach 
to consider such a plugin to be called (have the id) `some-plugin` instead of 
`@akdor1154/some-plugin`. This allowed scoped plugins to install and preserved 
the assumption that plugins will always be installed in 
`plugins_dir/[plugin_id]`.

However, it required special parsing logic around `npm` package IDs, and it 
broke the assumption that the `name` in an npm plugin's `package.json' would 
correspond to Cordova's idea of a plugin ID. IMO this approach is the source of 
further complexity which is not required, and is leading to weird bugs and 
special cases with scoped plugins. (see the linked issue, but there is stuff as 
basic as "`npm install` no longer works after installing a scoped plugin")

This PR changes approach - such plugins are now considered to have the id 
`@akdor1154/some-plugin`, in agreement with how npm treats such packages. This 
allows almost all special cases for scoped packages to be removed (yay). The 
key difference in behaviour as a result of this, though, is that while plugins 
are still installed in `plugins_dir/[plugin_id]`, `[plugin_id]` may no longer 
be a single directory, leading to plugin directories that look like
```
plugins
|- @akdor1154
|   |- some-plugin
|- cordova-some-other-plugin
```

Most of the logic changes in this PR are based around making this change 
work.

It's largely done how I want it, but I guess maintainers probably have 
strong opinions over whether this is the right way to go or not. Because of 
this I've left some commits in marked as TEMP that are for my own workflow. 
Please keep in mind I'll remove these. The only one you should be mindful of is 
the monkey patch to `cordova-common`; this would need to be raised in a 
separate PR to cordova-common if this change was approved in principle.


### Platforms affected
All

### What testing has been done on this change?
New end-to-end integration tests including a proper scoped plugin fixture
Unit tests on scoped plugins
Unit tests for plugman metadata

### Checklist
- [x] [Reported an issue](http://cordova.apache.org/contribute/issues.html) 
in the JIRA database
- [x] 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.
- [x] 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/akdor1154/cordova-lib simple-scopes

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

https://github.com/apache/cordova-lib/pull/602.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 #602


commit 9c1c320d72bf2860f95bd9330ffd7718cdabba82
Author: Jarrad Whitaker 
Date:   2017-10-20T06:55:44Z

change scope behaviour to consider scopes to be part of the package name

commit 67dbb75fa96de8486418dd0a0bfbaa22300690d5
Author: Jarrad Whitaker 
Date:   2017-10-20T07:07:56Z

remove extraneous done callback from scope plugin tests

commit c6f681cc4df5fd4a5bd246df00a62753244c
Author: Jarrad Whitaker 
Date:   2017-10-20T08:40:08Z

add scoped plugin testcase

commit c2346e062518c639381ac167ba60a169500501eb
Author: Jarrad Whitaker 
Date:   2017-10-20T08:41:46Z

allow scoped plugins to exist in a dir structure reflecting there name, 
c.f. npm

commit 14c3f74a0d9033dfb250bc7b698e18eb226bf544
Author: Jarrad Whitaker 
Date:   2017-10-20T08:42:07Z

add an integration test to check scoped plugin add+remove

commit 0e383f9b419c5aa22bbacb456c0ff38649e1
Author: Jarrad Whitaker 
Date:   2017-10-20T08:43:14Z

TEMP lint with typescript

commit 7018c13a635b46364b34cf4378c78c60a1012f8b
Author: Jarrad Whitaker 
Date:   2017-10-20T08:42:27Z

TEMP override plugin discovery in cordova-common

commit 7aad7297f7306b380761e01017c067b31beca4a7
Author: Jarrad Whitaker 
Date:   2017-10-23T06:23:14Z

fix get_fetch_metadata to not guess plugins_dir anymore

commit 17a8cce029ede8060383f041bbc9735f841556bf
Author: Jarrad Whitaker 
Date:   2017-10-23T06:53:59Z

remove top_plugins override

commit c0b1f5caae2d75b1bec7052598bf0f5f03285827
Author: Jarrad Whitaker

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

2017-10-23 Thread akdor1154

GitHub user akdor1154 opened a pull request:

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

CB-13485: Test with Node8

### What does this PR do?
Enables testing on Node 8 with Travis, and fixes the failing tests (no 
behaviour changes seem necessary).

### What testing has been done on this change?
`npm test`

### Checklist
- [x] [Reported an issue](http://cordova.apache.org/contribute/issues.html) 
in the JIRA database
- [x] 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.
- [x] 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/akdor1154/cordova-lib node8

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

https://github.com/apache/cordova-lib/pull/601.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 #601


commit 8e38e274cdd140717766fc43a4b28934f4106e64
Author: Jarrad Whitaker 
Date:   2017-10-23T23:38:55Z

Test Node 8 with Travis

commit 0a704ab17d420273d95c6ef3cc7b7774d6f3
Author: Jarrad Whitaker 
Date:   2017-10-23T23:51:26Z

fix error test on Node 8




---

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

[GitHub] cordova-lib pull request #600: CB-13478 : Fix CRLF line endings and lint int...

2017-10-20 Thread akdor1154

GitHub user akdor1154 opened a pull request:

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

CB-13478 : Fix CRLF line endings and lint integration tests.



### Platforms affected
N/A

### What does this PR do?
- Change CRLFs to LFs
- Configures eslint to whinge in future if CRLFs sneak back in
- Adds gitattributes to prevent this in future
- Adds integration-tests to the eslint script, and fixes lint errors it 
found in there (mainly line endings).

### What testing has been done on this change?
`npm run eslint`


### Checklist
- [x] [Reported an issue](http://cordova.apache.org/contribute/issues.html) 
in the JIRA database
- [x] 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.
- [x] 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/akdor1154/cordova-lib lf

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

https://github.com/apache/cordova-lib/pull/600.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 #600


commit 87ec94735455976130e2ec0dcaa0d2d72b2995de
Author: Jarrad Whitaker 
Date:   2017-10-21T00:25:36Z

eslint and gitattributes to ensure LF

commit 878c5ce45523223f3fe1c1b731367c66f87b7dcb
Author: Jarrad Whitaker 
Date:   2017-10-21T00:27:01Z

crlf -> lf with eslint

commit ce8157585db744bce82cd6f1b5224f58aa948f68
Author: Jarrad Whitaker 
Date:   2017-10-21T00:29:29Z

lint integration tests too

commit 4a5ce48dd0c7ded0429e3655c2ec0eb4e58bb2f2
Author: Jarrad Whitaker 
Date:   2017-10-21T00:32:26Z

crlf -> lf in integration tests

commit 4c6ef8a98c9147a992c70ed015fe5eed28be944e
Author: Jarrad Whitaker 
Date:   2017-10-21T00:33:27Z

eslint --fix in integration-tests

commit afee24c3e6ffe3c65015abd6180aee464434ce76
Author: Jarrad Whitaker 
Date:   2017-10-21T00:34:37Z

eqeqeq in HooksRunner




---

-
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 asfgit

Github user asfgit closed the pull request at:

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


---

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

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

2017-10-20 Thread audreyso

GitHub user audreyso opened a pull request:

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

CB-13463 : prevent package.json update with --nosave (for plugins)



### Platforms affected


### What does this PR do?
 
Prevent package.json update with --nosave (for plugins)

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


### Checklist
- [X] [Reported an issue](http://cordova.apache.org/contribute/issues.html) 
in the JIRA database
- [X] 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.
- [X] 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/audreyso/cordova-lib CB-13463

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

https://github.com/apache/cordova-lib/pull/599.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 #599


commit 2226a21bccf6a77f00ba8bdbd45c59667990
Author: Audrey So 
Date:   2017-10-20T21:10:26Z

CB-13463 : prevent package.json update plugins with --nosave




---

-
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 pull request #597: CB-13451 (all platforms) "pkg not defined" ex...

2017-10-20 Thread OminousWater

GitHub user OminousWater reopened a pull request:

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

CB-13451 (all platforms) "pkg not defined" exception when running plugman 
with createpackagejson command-line

A global js-lint pass on the cordova-lib codebase on the 30/8/2017 caused 
references to the global 'package' variable in init-default.js to be renamed to 
'pkgâ, presumably because the cordova-lib source doesnât declare a global 
variable called âpackageâ but _does_ declare a global variable called 
âpkgâ (in src/cordova/info.js). However in this case, the âpackageâ 
variable refers to the one declared in the plugman source, specifically 
main.js, so it should have stayed as 'packageâ. To test the fix, run:

plugman createpackagejson .

Without the fix, this will trigger the following exception: 'pkg is not 
defined'. Having patched in the fix, running the command line should work as 
expected, prompting the user with questions and then spitting out a 
package.json file.



### Platforms affected
All

### What does this PR do?
Fixes an bug where an exception would be thrown when running 'plugman 
createpackagejson .'

### What testing has been done on this change?
The command in question ('plugman createpackagejson .') has been tested 
with and without the fix to confirm that the fix fixes the problem.

### Checklist
- [x] [Reported an issue](http://cordova.apache.org/contribute/issues.html) 
in the JIRA database
- [x] 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/OminousWater/cordova-lib master

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

https://github.com/apache/cordova-lib/pull/597.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 #597


commit e321bc319b94c86b77d993981491f99bac6f27f9
Author: Roland 
Date:   2017-10-16T20:57:59Z

Fix for 'pkg is not defined' exception
A global js-lint pass on the cordova-lib codebase on the 30/8/2017 caused 
references to the global 'package' variable in init-default.js to be renamed to 
'pkgâ, presumably because the cordova-lib source doesnât declare a global 
variable called âpackageâ but _does_ declare a global variable called 
âpkgâ (in cordova/info.js). However in this case, the âpackageâ 
variable refers to the one declared in the plugman source, specifically 
main.js, so it should have stayed as 'packageâ. To test the fix, run:

plugman createpackagejson .

Without the fix, this will trigger the following exception: 'pkg is not 
defined'. Having patched in the fix, running the command line should work as 
expected, prompting the user with questions and then spitting out a 
package.json file.

commit 91614eb341735fde77661f3643c9a328fd09ac0c
Author: Roland 
Date:   2017-10-18T19:45:23Z

Revert "Fix for 'pkg is not defined' exception"

This reverts commit e321bc319b94c86b77d993981491f99bac6f27f9.

commit 62c5d2631c56ed40fda0576bc726b61bc0eb7806
Author: Roland 
Date:   2017-10-19T20:45:23Z

Second attempt at fix for CB-13451

commit f3dec27bfe9e4e6dce2f8d5ef6d3997c1ca2b925
Author: Roland 
Date:   2017-10-20T07:00:32Z

Make eslint ignore init-defaults.js file(s) as it will fail otherwise on 
'package' keword that PromZard injects




---

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

[GitHub] cordova-lib pull request #598: CB-13123 : plugin add should use cordovaDepen...

2017-10-19 Thread audreyso

GitHub user audreyso opened a pull request:

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

CB-13123 : plugin add should use cordovaDependency instead

of plugin.xml engine tag



### Platforms affected


### What does this PR do?
plugin add/ install should use cordovaDependency instead of plugin.xml 
engine tag

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


### Checklist
- [X] [Reported an issue](http://cordova.apache.org/contribute/issues.html) 
in the JIRA database
- [X] 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.
- [X] 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/audreyso/cordova-lib CB-13123

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

https://github.com/apache/cordova-lib/pull/598.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 #598


commit 5d6ad4a3dc4ed41b87140fcd9cf83d359b6fb798
Author: Audrey So 
Date:   2017-10-18T20:47:00Z

CB-13123 : plugin add/ install should use cordovaDependency instead of 
plugin.xml engine tag




---

-
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-16 Thread OminousWater

Github user OminousWater closed the pull request at:

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


---

-
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-16 Thread OminousWater

GitHub user OminousWater opened a pull request:

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

CB-13451 (all platforms) "pkg not defined" exception when running plugman 
with createpackagejson command-line

A global js-lint pass on the cordova-lib codebase on the 30/8/2017 caused 
references to the global 'package' variable in init-default.js to be renamed to 
'pkgâ, presumably because the cordova-lib source doesnât declare a global 
variable called âpackageâ but _does_ declare a global variable called 
âpkgâ (in src/cordova/info.js). However in this case, the âpackageâ 
variable refers to the one declared in the plugman source, specifically 
main.js, so it should have stayed as 'packageâ. To test the fix, run:

plugman createpackagejson .

Without the fix, this will trigger the following exception: 'pkg is not 
defined'. Having patched in the fix, running the command line should work as 
expected, prompting the user with questions and then spitting out a 
package.json file.



### Platforms affected
All

### What does this PR do?
Fixes an bug where an exception would be thrown when running 'plugman 
createpackagejson .'

### What testing has been done on this change?
The command in question ('plugman createpackagejson .') has been tested 
with and without the fix to confirm that the fix fixes the problem.

### Checklist
- [x] [Reported an issue](http://cordova.apache.org/contribute/issues.html) 
in the JIRA database
- [x] 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/OminousWater/cordova-lib master

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

https://github.com/apache/cordova-lib/pull/597.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 #597


commit e321bc319b94c86b77d993981491f99bac6f27f9
Author: Roland 
Date:   2017-10-16T20:57:59Z

Fix for 'pkg is not defined' exception
A global js-lint pass on the cordova-lib codebase on the 30/8/2017 caused 
references to the global 'package' variable in init-default.js to be renamed to 
'pkgâ, presumably because the cordova-lib source doesnât declare a global 
variable called âpackageâ but _does_ declare a global variable called 
âpkgâ (in cordova/info.js). However in this case, the âpackageâ 
variable refers to the one declared in the plugman source, specifically 
main.js, so it should have stayed as 'packageâ. To test the fix, run:

plugman createpackagejson .

Without the fix, this will trigger the following exception: 'pkg is not 
defined'. Having patched in the fix, running the command line should work as 
expected, prompting the user with questions and then spitting out a 
package.json file.




---

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

[GitHub] cordova-lib pull request #595: CB-12361 : updated and added unit tests for a...

2017-10-04 Thread asfgit

Github user asfgit closed the pull request at:

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


---

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

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

2017-10-04 Thread asfgit

Github user asfgit closed the pull request at:

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


---

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

[GitHub] cordova-lib pull request #596: CB-13303 : added save_exact and production op...

2017-10-03 Thread asfgit

Github user asfgit closed the pull request at:

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


---

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

[GitHub] cordova-lib pull request #596: CB-13303 : added save_exact and production op...

2017-10-03 Thread audreyso

GitHub user audreyso opened a pull request:

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

CB-13303 : added save_exact and production opts



### Platforms affected


### What does this PR do?

Added save-exact and production opts

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


### Checklist
- [X] [Reported an issue](http://cordova.apache.org/contribute/issues.html) 
in the JIRA database
- [X] 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.
- [X] 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/audreyso/cordova-lib CB-13303

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

https://github.com/apache/cordova-lib/pull/596.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 #596


commit cdcfb60d3b5172c2c52ac7afe72e43610068af05
Author: Audrey So 
Date:   2017-10-03T22:57:52Z

CB-13303 : added save_exact and production opts




---

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

[GitHub] cordova-lib pull request #591: CB-13274 : updated readme.md & removed refere...

2017-10-03 Thread asfgit

Github user asfgit closed the pull request at:

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


---

-
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-25 Thread asfgit

Github user asfgit closed the pull request at:

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


---

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

[GitHub] cordova-lib pull request #594: CB-13288 : updated index.js and test to fix c...

2017-09-21 Thread asfgit

Github user asfgit closed the pull request at:

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


---

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

[GitHub] cordova-lib pull request #595: CB-12361 : updated and added unit tests for a...

2017-09-19 Thread audreyso

GitHub user audreyso opened a pull request:

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

CB-12361 : updated and added unit tests for add.spec.js



### Platforms affected


### What does this PR do?

Updated and added "to-do" unit tests for add.spec.js

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


### Checklist
- [X] [Reported an issue](http://cordova.apache.org/contribute/issues.html) 
in the JIRA database
- [X] 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.
- [X] 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/audreyso/cordova-lib CB-12361-plugin_fetch

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

https://github.com/apache/cordova-lib/pull/595.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 #595


commit ec07983d1ed587358a30fc003e9f86edd47e9bce
Author: Audrey So 
Date:   2017-09-18T20:37:45Z

CB-12361 : updated and added unit tests for add.spec.js




---

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

[GitHub] cordova-lib pull request #594: CB-13288 : updated index.js and test to fix c...

2017-09-15 Thread audreyso

GitHub user audreyso opened a pull request:

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

CB-13288 : updated index.js and test to fix cordova plugin search



### Platforms affected


### What does this PR do?

Updated index.js and test to fix cordova plugin search

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


### Checklist
- [X] [Reported an issue](http://cordova.apache.org/contribute/issues.html) 
in the JIRA database
- [X] 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.
- [X] 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/audreyso/cordova-lib CB-13288

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

https://github.com/apache/cordova-lib/pull/594.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 #594


commit e50c9b60e45cdc033d68bce62a2af894da0e1b61
Author: Audrey So 
Date:   2017-09-15T20:34:44Z

CB-13288 : updated index.js and test to fix cordova plugin search




---

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

[GitHub] cordova-lib pull request #592: CB-12361 : added some unit tests for plugman

2017-09-15 Thread audreyso

Github user audreyso closed the pull request at:

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


---

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

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

2017-09-15 Thread audreyso

GitHub user audreyso opened a pull request:

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

CB-12361 : added plugin unit tests for plugin list



### Platforms affected


### What does this PR do?
Added plugin unit tests for plugin list

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


### Checklist
- [X] [Reported an issue](http://cordova.apache.org/contribute/issues.html) 
in the JIRA database
- [X] 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.
- [X] 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/audreyso/cordova-lib CB-12361-list_spec

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

https://github.com/apache/cordova-lib/pull/593.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 #593


commit e956fe7c8d0cbd8c7663bed4e66d5b1f78411e68
Author: Audrey So 
Date:   2017-09-15T16:32:55Z

CB-12361 : added plugin tests for plugin list




---

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

[GitHub] cordova-lib pull request #592: CB-12361 : added some unit tests for plugman

2017-09-14 Thread audreyso

GitHub user audreyso opened a pull request:

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

CB-12361 : added some unit tests for plugman



### Platforms affected


### What does this PR do?

Added some unit tests for plugman

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


### Checklist
- [X] [Reported an issue](http://cordova.apache.org/contribute/issues.html) 
in the JIRA database
- [X] 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.
- [X] 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/audreyso/cordova-lib CB-12361-plugman

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

https://github.com/apache/cordova-lib/pull/592.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 #592


commit d61c084b2192dbe3831c8448d35bdeeafb36636d
Author: Audrey So 
Date:   2017-08-23T17:43:34Z

CB-12361 : added unit tests for plugman




---

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

[GitHub] cordova-lib pull request #591: CB-13274 : updated readme.md & removed refere...

2017-09-14 Thread audreyso

GitHub user audreyso opened a pull request:

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

CB-13274 : updated readme.md & removed references to jshint



### Platforms affected


### What does this PR do?
Updated readme.md & removed references to jshint

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


### Checklist
- [X] [Reported an issue](http://cordova.apache.org/contribute/issues.html) 
in the JIRA database
- [X] 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.
- [X] 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/audreyso/cordova-lib CB-13274

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

https://github.com/apache/cordova-lib/pull/591.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 #591






---

-
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-lib pull request #569: CB-12361: added main function unit tests for ...

2017-09-06 Thread asfgit

Github user asfgit closed the pull request at:

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


---

-
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 but not

[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-lib pull request #590: CB-13145: Create playservices version prefere...

2017-08-31 Thread audreyso

Github user audreyso closed the pull request at:

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


---
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 #561: Cb 12870 - Check that all use cases are caugh...

2017-08-29 Thread asfgit

Github user asfgit closed the pull request at:

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


---
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 #589: removed .jscs.json

2017-08-29 Thread remcohaszing

Github user remcohaszing closed the pull request at:

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


---
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-lib pull request #569: CB-12361: added main function unit tests for ...

2017-08-25 Thread audreyso

Github user audreyso commented on a diff in the pull request:

https://github.com/apache/cordova-lib/pull/569#discussion_r135308470
  
--- 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 --

Hi! I think you have "getPlugin" twice?


---
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-24 Thread audreyso

GitHub user audreyso opened a pull request:

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

CB-13145: Create playservices version preference in config.xml  

Still need to add in tests


### Platforms affected


### What does this PR do?
CB-13145: Create playservices version preference in config.xml

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


### Checklist
- [X] [Reported an issue](http://cordova.apache.org/contribute/issues.html) 
in the JIRA database
- [X] 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.
- [X] 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/audreyso/cordova-lib CB-13145-Steve

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

https://github.com/apache/cordova-lib/pull/590.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 #590


commit 315e6a80b95a425a07a0a32e53e24ce10e1ef89c
Author: Steve Gill 
Date:   2017-08-23T04:39:24Z

CB-13145: pass full options to plugman uninstall

commit faa48e255fc02dd42f2bcc7eaa43f816967305cb
Author: Audrey So 
Date:   2017-08-23T20:27:45Z

CB-13145 : added variable-merge.js to deal with plugin.xml variables for 
uninstall




---
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 #565: CB-12944: Platform's spec is ignored in confi...

2017-08-22 Thread asfgit

Github user asfgit closed the pull request at:

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


---
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-16 Thread asfgit

Github user asfgit closed the pull request at:

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


---
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 audreyso

Github user audreyso commented on a diff in the pull request:

https://github.com/apache/cordova-lib/pull/574#discussion_r132795105
  
--- 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 --

Yes, exactly! Also rebased and updated based on feedback. Let me know if 
there's anything else!


---
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 asfgit

Github user asfgit closed the pull request at:

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


---
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 #583: CB-12361 : added tests for plugin/index.js

2017-08-11 Thread asfgit

Github user asfgit closed the pull request at:

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


---
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 #582: CB-12361 : added tests for plugin/search.js

2017-08-11 Thread asfgit

Github user asfgit closed the pull request at:

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


---
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-lib pull request #575: CB-13020: (plugman) install filters out nohoo...

2017-08-08 Thread asfgit

Github user asfgit closed the pull request at:

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


---
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 #589: removed .jscs.json

2017-08-07 Thread remcohaszing

GitHub user remcohaszing opened a pull request:

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

removed .jscs.json

### Platforms affected
none

### What does this PR do?
Cleanup an unused file.

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

### 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/remcohaszing/cordova-lib remove-jscs.json

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

https://github.com/apache/cordova-lib/pull/589.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 #589


commit 2539bfeb4f9ae4b2f27383e30f0d6277b11918af
Author: Remco Haszing 
Date:   2017-08-07T20:50:50Z

removed .jscs.json

It is no longer used




---
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 #587: CB-13056 : added deprecation notice for webos

2017-08-02 Thread asfgit

Github user asfgit closed the pull request at:

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


---
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 #585: CB-13057 : added deprecation warning for cord...

2017-08-02 Thread asfgit

Github user asfgit closed the pull request at:

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


---
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 #588: CB-13056 : Removed parsers & platformApi poly...

2017-08-01 Thread audreyso

GitHub user audreyso opened a pull request:

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

CB-13056 : Removed parsers & platformApi polyfill for webos, blackberry, & 
ubuntu



### Platforms affected


### What does this PR do?

Removed parsers & platformApi polyfill for webos, blackberry, & ubuntu.

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


### Checklist
- [X] [Reported an issue](http://cordova.apache.org/contribute/issues.html) 
in the JIRA database
- [X] 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.
- [X] 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/audreyso/cordova-lib CB-13056

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

https://github.com/apache/cordova-lib/pull/588.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 #588


commit d855daa20165487b37bed6b07fcdd5878564337f
Author: Audrey So 
Date:   2017-08-01T22:59:07Z

CB-13056 : removed parsers and platformApi polyfill for webos, 
blackberry10, and ubuntu




---
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 #587: CB-13056 : added deprecation notice for webos

2017-08-01 Thread audreyso

GitHub user audreyso opened a pull request:

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

CB-13056 : added deprecation notice for webos



### Platforms affected


### What does this PR do?

Added deprecation notice for webos

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


### Checklist
- [X] [Reported an issue](http://cordova.apache.org/contribute/issues.html) 
in the JIRA database
- [X] 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.
- [X] 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/audreyso/cordova-lib CB-13056-2

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

https://github.com/apache/cordova-lib/pull/587.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 #587


commit d673efd5b21fc452d1343771f731021ea6b5da28
Author: Audrey So 
Date:   2017-08-01T23:05:24Z

CB-13056 : added deprecation notice for webos




---
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 #586: CB-13057 : Remove cordova platform save comma...

2017-08-01 Thread audreyso

GitHub user audreyso opened a pull request:

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

CB-13057 : Remove cordova platform save command



### Platforms affected


### What does this PR do?

Remove cordova platform save command

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


### Checklist
- [X] [Reported an issue](http://cordova.apache.org/contribute/issues.html) 
in the JIRA database
- [X] 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.
- [X] 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/audreyso/cordova-lib CB-13057-2

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

https://github.com/apache/cordova-lib/pull/586.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 #586


commit 3f66326f751de7c78f1e1b544bed38cf009d5e9d
Author: Audrey So 
Date:   2017-08-01T22:06:41Z

CB-13057 : removed save function and updated unit-tests after these changes

commit 799e14d032e5c59f9dca5ad5921155572b2e246e
Author: Audrey So 
Date:   2017-08-01T22:07:52Z

CB-13057 : removed platformsJson from integration tests




---
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 #585: CB-13057 : added deprecation warning for cord...

2017-08-01 Thread audreyso

GitHub user audreyso opened a pull request:

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

CB-13057 : added deprecation warning for cordova platform save



### Platforms affected


### What does this PR do?

Added deprecation warning for cordova platform save

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


### Checklist
- [X] [Reported an issue](http://cordova.apache.org/contribute/issues.html) 
in the JIRA database
- [X] 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.
- [X] 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/audreyso/cordova-lib CB-13057

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

https://github.com/apache/cordova-lib/pull/585.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 #585


commit 5d9e3336071d00fc550cb1b042172466a0a86704
Author: Audrey So 
Date:   2017-08-01T20:46:59Z

CB-13057 : added deprecation warning for cordova platform save




---
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-28 Thread asfgit

Github user asfgit closed the pull request at:

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


---
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 #580: CB-12895 : Replaced jshint with eslint

2017-07-28 Thread asfgit

Github user asfgit closed the pull request at:

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


---
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-28 Thread asfgit

Github user asfgit closed the pull request at:

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


---
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 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 audreyso

Github user audreyso commented on a diff in the pull request:

https://github.com/apache/cordova-lib/pull/578#discussion_r129969368
  
--- 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 --

ohh okay do you mean just like this?

```
 it('should remove the cordova- prefix from the platform name for known 
platforms', function (done) {

expect(platform_getPlatformDetails.platformFromName('cordova-ios')).toBe('ios');
expect(events.emit).toHaveBeenCalledWith('verbose', 
jasmine.stringMatching(/Removing "cordova-" prefix/));
done();
});
```


---
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 #584: CB-12361 : added tests for plugin/save.js

2017-07-27 Thread audreyso

GitHub user audreyso opened a pull request:

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

CB-12361 : added tests for plugin/save.js



### Platforms affected


### What does this PR do?

 added tests for plugin/save.js

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


### Checklist
- [X] [Reported an issue](http://cordova.apache.org/contribute/issues.html) 
in the JIRA database
- [X] 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.
- [X] 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/audreyso/cordova-lib CB-12361-16-plugin

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

https://github.com/apache/cordova-lib/pull/584.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 #584






---
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 asfgit

Github user asfgit closed the pull request at:

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


---
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-lib pull request #583: CB-12361 : added tests for plugin/index.js

2017-07-26 Thread audreyso

GitHub user audreyso opened a pull request:

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

CB-12361 : added tests for plugin/index.js



### Platforms affected


### What does this PR do?

Added tests for plugin/index.js

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


### Checklist
- [X] [Reported an issue](http://cordova.apache.org/contribute/issues.html) 
in the JIRA database
- [X] 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.
- [X] 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/audreyso/cordova-lib CB-12361-15-plugin

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

https://github.com/apache/cordova-lib/pull/583.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 #583


commit aa955945048b0fac8287ee715e30a49f7219eb5c
Author: Audrey So 
Date:   2017-07-26T18:36:21Z

CB-12361 : added tests for plugin/index.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 #582: CB-12361 : added tests for plugin/search.js

2017-07-21 Thread audreyso

GitHub user audreyso opened a pull request:

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

CB-12361 : added tests for plugin/search.js



### Platforms affected


### What does this PR do?

Added tests for plugin/search.js.

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


### Checklist
- [X] [Reported an issue](http://cordova.apache.org/contribute/issues.html) 
in the JIRA database
- [X] 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.
- [X] 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/audreyso/cordova-lib CB-12361-12-plugin

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

https://github.com/apache/cordova-lib/pull/582.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 #582


commit 175513b5eaad88059b0e47257025f122da76a0a5
Author: Audrey So 
Date:   2017-07-20T20:23:11Z

CB-12361 : added tests for plugin/search.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 #581: CB-12361 : added plugin remove tests

2017-07-21 Thread audreyso

GitHub user audreyso opened a pull request:

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

CB-12361 : added plugin remove tests



### Platforms affected


### What does this PR do?

Added plugin remove tests.

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


### Checklist
- [X] [Reported an issue](http://cordova.apache.org/contribute/issues.html) 
in the JIRA database
- [X] 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.
- [X] 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/audreyso/cordova-lib CB-12361-13-plugin

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

https://github.com/apache/cordova-lib/pull/581.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 #581


commit f2326ca2b291aef0736b8924d060fe5bd219b1cd
Author: Audrey So 
Date:   2017-07-20T23:19:07Z

CB-12361 : added plugin remove tests




---
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 #580: CB-12895 : Replaced jshint with eslint

2017-07-20 Thread audreyso

GitHub user audreyso opened a pull request:

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

CB-12895 : Replaced jshint with eslint



### Platforms affected


### What does this PR do?

 Replaced jshint with eslint

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


### Checklist
- [X] [Reported an issue](http://cordova.apache.org/contribute/issues.html) 
in the JIRA database
- [X] 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.
- [X] 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/audreyso/cordova-lib CB-12895-2

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

https://github.com/apache/cordova-lib/pull/580.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 #580


commit 4cb49cf58523d0ef8039cba6b2d3d6a7ebc260a9
Author: Audrey So 
Date:   2017-07-19T16:51:05Z

CB-12895 : set up eslint

commit 25c80f56f2697f6f359669eb8e150b208e2cd9f2
Author: Audrey So 
Date:   2017-07-19T16:54:49Z

CB-12895 : ran eslint --fix on cordova-lib

commit 55a3eccf913ac0cee11633718a11748a43e8cb88
Author: Audrey So 
Date:   2017-07-20T16:41:06Z

CB-12895 : updated integration tests with eslint




---
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-20 Thread audreyso

GitHub user audreyso opened a pull request:

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

CB-12361 : added tests for save.js



### Platforms affected


### What does this PR do?


### What testing has been done on this change?
added tests for save.js

### Checklist
- [X] [Reported an issue](http://cordova.apache.org/contribute/issues.html) 
in the JIRA database
- [X] 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.
- [X] 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/audreyso/cordova-lib CB-12361-11

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

https://github.com/apache/cordova-lib/pull/579.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 #579






---
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-18 Thread audreyso

GitHub user audreyso opened a pull request:

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

CB-12361 : added unit-tests for getPlatformDetailsFromDir



### Platforms affected


### What does this PR do?

added unit-tests for getPlatformDetailsFromDir

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


### Checklist
- [X] [Reported an issue](http://cordova.apache.org/contribute/issues.html) 
in the JIRA database
- [X] 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.
- [X] 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/audreyso/cordova-lib CB-12361-10

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

https://github.com/apache/cordova-lib/pull/578.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 #578


commit e8a1cf2c062fd735ffb767dcd1c249de5c35684d
Author: Audrey So 
Date:   2017-07-18T17:35:28Z

CB-12361 : added unit-tests for getPlatformDetailsFromDir




---
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-18 Thread audreyso

GitHub user audreyso opened a pull request:

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

CB-12361 : added tests for platform/list.js



### Platforms affected


### What does this PR do?

 Added tests for platform/list.js

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


### Checklist
- [X] [Reported an issue](http://cordova.apache.org/contribute/issues.html) 
in the JIRA database
- [X] 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.
- [X] 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/audreyso/cordova-lib CB-12361-8

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

https://github.com/apache/cordova-lib/pull/577.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 #577


commit 558554aea0654a4132feb057a73e3faa5a5bac1a
Author: Audrey So 
Date:   2017-07-17T23:06:37Z

CB-12361 : added tests for list platform




---
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-17 Thread audreyso

GitHub user audreyso opened a pull request:

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

CB-12361 : added unit tests for remove platform



### Platforms affected


### What does this PR do?
Added unit tests for remove platform

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


### Checklist
- [X] [Reported an issue](http://cordova.apache.org/contribute/issues.html) 
in the JIRA database
- [X] 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.
- [X] 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/audreyso/cordova-lib CB-12361-7

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

https://github.com/apache/cordova-lib/pull/576.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 #576


commit 61ddb3d78182c51937539bcb8728fe97f9a9259f
Author: Audrey So 
Date:   2017-07-17T19:01:20Z

CB-12361 : added tests for remove platform




---
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 #575: CB-13020: (plugman) install filters out nohoo...

2017-07-13 Thread wildabeast

GitHub user wildabeast opened a pull request:

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

CB-13020: (plugman) install filters out nohooks



### Platforms affected

n/a

### What does this PR do?

fixes CB-13020: plugman filters out nohooks opt

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

manual

### Checklist
- [x] [Reported an issue](http://cordova.apache.org/contribute/issues.html) 
in the JIRA database
- [x] 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/wildabeast/cordova-lib master

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

https://github.com/apache/cordova-lib/pull/575.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 #575


commit 6234355db76342b6fb3045b9a3b23e08f42a92bd
Author: Ryan Willoughby 
Date:   2017-07-13T19:10:04Z

CB-13020: (plugman) install filters out nohooks




---
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-13 Thread asfgit

Github user asfgit closed the pull request at:

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


---
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-07-13 Thread audreyso

GitHub user audreyso opened a pull request:

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

CB-12838 : prevented sorting and alphabetizing platforms and plugins iâ¦

â¦n pkgJson and config



### Platforms affected


### What does this PR do?

Prevents modifying/alphabetizing platforms and plugins in config

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


### Checklist
- [X] [Reported an issue](http://cordova.apache.org/contribute/issues.html) 
in the JIRA database
- [X] 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.
- [X] 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/audreyso/cordova-lib CB-12838

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

https://github.com/apache/cordova-lib/pull/574.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 #574


commit 083deef9ce70afe8455a36f49ddccf0b093558f8
Author: Audrey So 
Date:   2017-07-12T00:21:41Z

CB-12838 : prevented sorting and aphabetizing platforms and plugins in 
pkgjson and config




---
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 audreyso

Github user audreyso commented on a diff in the pull request:

https://github.com/apache/cordova-lib/pull/573#discussion_r127110528
  
--- 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 audreyso

Github user audreyso commented on a diff in the pull request:

https://github.com/apache/cordova-lib/pull/573#discussion_r127066391
  
--- 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_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();
+

1 2 3 4 5 6 7 8 9 10 >

1 - 100 of 3208 matches

Mail list logo