You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cordova.apache.org by sarangan12 <gi...@git.apache.org> on 2016/01/21 23:27:52 UTC
[GitHub] cordova-medic pull request: CB-10405 - Connectivity issue to Cordo...
GitHub user sarangan12 opened a pull request:
https://github.com/apache/cordova-medic/pull/72
CB-10405 - Connectivity issue to Cordova VM
1. Added retry logic while checking if cordova vms are up.
2. Increased server response timeout from 3 seconds to 15 seconds
@dblotsky Can you review when you got time?
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/sarangan12/cordova-medic master
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/cordova-medic/pull/72.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 #72
----
commit 33954932363db04bbad77f9f97443e233edfe0db
Author: Sarangan Rajamanickam <sa...@microsoft.com>
Date: 2016-01-21T01:21:19Z
CB-10405 - Connectivity issue to Cordova VM
1. Added retry logic while checking if cordova vms are up.
2. Increased server response timeout from 3 seconds to 15 seconds
Fixing sleep
commit 26a7cbad27edb48edfc7cea2c7a59ee3da3a9e75
Author: Sarangan Rajamanickam <sa...@microsoft.com>
Date: 2016-01-21T22:23:44Z
Merge remote-tracking branch 'refs/remotes/apache/master'
----
---
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: CB-10405 - Connectivity issue to Cordo...
Posted by dblotsky <gi...@git.apache.org>.
Github user dblotsky commented on a diff in the pull request:
https://github.com/apache/cordova-medic/pull/72#discussion_r50488969
--- Diff: medic/medic-run.js ---
@@ -324,23 +349,8 @@ function main() {
util.fatal("app " + appPath + " does not exist");
}
- util.medicLog("checking if " + couchdbURI + " is up");
-
- // check if results server is up
- request({
- uri: couchdbURI,
- method: "GET",
- timeout: SERVER_RESPONSE_TIMEOUT
- },
- function (error, response, body) {
-
- // bail if the results server is down
- if (error || response.statusCode !== 200) {
- util.fatal("it's not up, so test run can't be monitored");
- process.exit(1);
- } else {
- util.medicLog("it's up");
- }
+ onConnect(couchdbURI, 1, function(){
--- End diff --
Nitpick: please add a space between `function` and `()`.
---
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: CB-10405 - Connectivity issue to Cordo...
Posted by dblotsky <gi...@git.apache.org>.
Github user dblotsky commented on a diff in the pull request:
https://github.com/apache/cordova-medic/pull/72#discussion_r50489182
--- Diff: medic/medic-run.js ---
@@ -291,6 +293,29 @@ function failedBecauseNoDevice(output) {
return NO_DEVICE_PATTERN.test(output);
}
+function onConnect(couchdbURI, retryCount, callback) {
--- End diff --
Please give a more descriptive name to the function, since it's checking if a server is up, not really opening a connection or handling an event (`onX` is a common naming pattern for event handlers).
---
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: CB-10405 - Connectivity issue to Cordo...
Posted by dblotsky <gi...@git.apache.org>.
Github user dblotsky commented on the pull request:
https://github.com/apache/cordova-medic/pull/72#issuecomment-173771334
LGTM!
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at 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: CB-10405 - Connectivity issue to Cordo...
Posted by sarangan12 <gi...@git.apache.org>.
Github user sarangan12 commented on the pull request:
https://github.com/apache/cordova-medic/pull/72#issuecomment-173774638
@dblotsky Changed the name of the constant
---
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: CB-10405 - Connectivity issue to Cordo...
Posted by dblotsky <gi...@git.apache.org>.
Github user dblotsky commented on a diff in the pull request:
https://github.com/apache/cordova-medic/pull/72#discussion_r50492408
--- Diff: medic/medic-run.js ---
@@ -44,7 +44,9 @@ var MEDIC_BUILD_PREFIX = "medic-cli-build";
var DEFAULT_WINDOWS_VERSION = "store";
var WINDOWS_VERSION_CHOICES = ["store", "store80", "phone"];
var DEFAULT_TIMEOUT = 600; // in seconds
-var SERVER_RESPONSE_TIMEOUT = 3000; // in milliseconds
+var SERVER_RESPONSE_TIMEOUT = 15000; // in milliseconds
+var MAX_NUMBER_OF_TRIES = 3;
+var WAIT_TIME_FOR_CORDOVA_VM = 15000; // in milliseconds
--- End diff --
Sorry, small nitpick again: this name doesn't describe what we're waiting for. Specifically, we're waiting for some time before retrying.
---
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: CB-10405 - Connectivity issue to Cordo...
Posted by sarangan12 <gi...@git.apache.org>.
Github user sarangan12 commented on the pull request:
https://github.com/apache/cordova-medic/pull/72#issuecomment-173762044
@dblotsky Refactored the code to use callback. Can you review the latest code?
---
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: CB-10405 - Connectivity issue to Cordo...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/cordova-medic/pull/72
---
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: CB-10405 - Connectivity issue to Cordo...
Posted by sarangan12 <gi...@git.apache.org>.
Github user sarangan12 commented on the pull request:
https://github.com/apache/cordova-medic/pull/72#issuecomment-173767433
@dblotsky Incorporated both the comments
---
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: CB-10405 - Connectivity issue to Cordo...
Posted by dblotsky <gi...@git.apache.org>.
Github user dblotsky commented on a diff in the pull request:
https://github.com/apache/cordova-medic/pull/72#discussion_r50489033
--- Diff: medic/medic-run.js ---
@@ -324,23 +349,8 @@ function main() {
util.fatal("app " + appPath + " does not exist");
}
- util.medicLog("checking if " + couchdbURI + " is up");
-
- // check if results server is up
- request({
- uri: couchdbURI,
- method: "GET",
- timeout: SERVER_RESPONSE_TIMEOUT
- },
- function (error, response, body) {
-
- // bail if the results server is down
- if (error || response.statusCode !== 200) {
- util.fatal("it's not up, so test run can't be monitored");
- process.exit(1);
- } else {
- util.medicLog("it's up");
- }
+ onConnect(couchdbURI, 1, function(){
--- End diff --
If possible, maybe also change the signature of the retry function to hide the `1` from the caller, so that it only takes a URI and a callback.
---
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