You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cordova.apache.org by filmaj <gi...@git.apache.org> on 2016/12/09 23:19:10 UTC
[GitHub] cordova-medic pull request #106: Appium updates
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-xxxx 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 <ma...@gmail.com>
Date: 2016-12-09T23:04:48Z
Dont use browsername in selenium capabilities - not applicable for hybrid apps.
commit 1309e6c3b1cf7ff3a70bff4e9f117ff6469f0bc4
Author: filmaj <ma...@gmail.com>
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 <ma...@gmail.com>
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-medic pull request #106: Appium updates
Posted by filmaj <gi...@git.apache.org>.
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-medic pull request #106: Appium updates
Posted by alsorokin <gi...@git.apache.org>.
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-medic pull request #106: Appium updates
Posted by filmaj <gi...@git.apache.org>.
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-medic issue #106: Appium updates
Posted by filmaj <gi...@git.apache.org>.
Github user filmaj commented on the issue:
https://github.com/apache/cordova-medic/pull/106
I see now that they are not! My mistake. I will go look around in paramedic, then :)
We should probably remove this functionality from medic if it is all based in paramedic! Code duplication and confusing to new contributors, clearly :D
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-medic issue #106: Appium updates
Posted by filmaj <gi...@git.apache.org>.
Github user filmaj commented on the issue:
https://github.com/apache/cordova-medic/pull/106
@alsorokin ok no problem. I think we just need to move the jenkins configs over to paramedic, then we could probably delete the medic repo completely. I will start a discuss thread on the dev list.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-medic issue #106: Appium updates
Posted by alsorokin <gi...@git.apache.org>.
Github user alsorokin commented on the issue:
https://github.com/apache/cordova-medic/pull/106
Yeah, sorry about this confusion. I think we could get rid of medic altogether, not just the appium code. Medic was used back when we had buildbot CI up and running. Now it's just sits here covered in dust.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-medic issue #106: Appium updates
Posted by filmaj <gi...@git.apache.org>.
Github user filmaj commented on the issue:
https://github.com/apache/cordova-medic/pull/106
Good question @alsorokin ! I thought that the appium-based CI tests were triggered via medic, not paramedic?
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org