[GitHub] cordova-medic pull request #106: Appium updates

2016-12-12 Thread filmaj
Github user filmaj closed the pull request at:

https://github.com/apache/cordova-medic/pull/106


---
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-medic pull request #106: Appium updates

2016-12-12 Thread filmaj
Github user filmaj commented on a diff in the pull request:

https://github.com/apache/cordova-medic/pull/106#discussion_r91951340
  
--- Diff: medic/medic-appium.js ---
@@ -355,9 +358,8 @@ function isFailFastError(error) {
 function killProcess(procObj, killSignal, callback) {
 if (procObj.alive) {
 procObj.alive = false;
-setTimeout(function () {
-kill(procObj.process.pid, killSignal, callback);
-}, 1000);
--- End diff --

I see. I will take a look at this in my windows VM, then. Do you recall 
what version of windows you saw this problem on?


---
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-medic pull request #106: Appium updates

2016-12-12 Thread alsorokin
Github user alsorokin commented on a diff in the pull request:

https://github.com/apache/cordova-medic/pull/106#discussion_r91896443
  
--- Diff: medic/medic-appium.js ---
@@ -355,9 +358,8 @@ function isFailFastError(error) {
 function killProcess(procObj, killSignal, callback) {
 if (procObj.alive) {
 procObj.alive = false;
-setTimeout(function () {
-kill(procObj.process.pid, killSignal, callback);
-}, 1000);
--- End diff --

As far as I remember, these process-killing timeouts were needed to give 
the treekill module some time to actually kill the process. On mac it may be 
fine, but on Windows the Appium process is left hanging in the memory and 
occupying the port.


---
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-medic pull request #106: Appium updates

2016-12-09 Thread filmaj
GitHub user filmaj opened a pull request:

https://github.com/apache/cordova-medic/pull/106

Appium updates

Please review @alsorokin !

### Platforms affected

iOS and Android

### What does this PR do?

Fixes and cleans up certain interactions with appium:
 - Appium version bumped
 - Desired capabilities in Selenium-land (and thus Appium) is usually 
tricky and obtuse. Clarified some usage of it as well as forcing string for 
`platformVersion`. Removed `browserName` completely as it is not applicable to 
hybrid apps.
 - I removed the `setTimeout`s around process killing, as on my Mac w/ node 
v6.9.1 that would lead to improper termination of the appium process. I defer 
to you, @alsorokin, on whether that is a good change or not.

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

Tested on Android 5.1 emulator w/ contacts plugin appium tests.

### 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/filmaj/cordova-medic appium-updates

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

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


commit 1b57f67125584cf5acc91e94b8208fc2a8e7c936
Author: filmaj 
Date:   2016-12-09T23:04:48Z

Dont use browsername in selenium capabilities - not applicable for hybrid 
apps.

commit 1309e6c3b1cf7ff3a70bff4e9f117ff6469f0bc4
Author: filmaj 
Date:   2016-12-09T23:13:08Z

Bumping appium to 1.5.3 (waiting on move to 1.6.x for when 1.6.3 comes out

commit 558d4bd95295a9712d71910b5b5db7df2d311c3b
Author: filmaj 
Date:   2016-12-09T23:14:59Z

Slight detail expansion on usage instructions. Adding a `--verbose` flag to 
dump out appium outputs to the log. Capabilities: appium no longer uses 
`--avd`, so remove it. Always force platformVersion to a string. A few extra 
logs added for clarity of flow. Removed settimeouts on process killing (noticed 
it would not clean up appium properly with them in place).




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