You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cordova.apache.org by riknoll <gi...@git.apache.org> on 2016/02/12 19:58:21 UTC

[GitHub] cordova-medic pull request: CB-10510: Retry starting the Android e...

GitHub user riknoll opened a pull request:

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

    CB-10510: Retry starting the Android emulator if it hangs on boot

    This PR relies on https://github.com/apache/cordova-android/pull/258 being merged.
    
    Adds logic to retry starting the Android emulator if it hangs on boot. Also has the side effects of changing medic-kill so that it can be required and so that it does not kill ADB when the killing Android tasks. Killing ADB causes issues with restarting the emulator and I don't think there was any reason to kill it in the first place.
    
    @dblotsky @nikhilkh please review. This was split off of #76 

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

    $ git pull https://github.com/MSOpenTech/cordova-medic CB-10510

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

    https://github.com/apache/cordova-medic/pull/77.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 #77
    
----
commit 6a18c1eb5b3c71be6583ccb9187963975878569b
Author: riknoll <ri...@gmail.com>
Date:   2016-02-11T22:39:53Z

    CB-10510: Retry starting the Android emulator if it hangs on boot

----


---
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-10510: Retry starting the Android e...

Posted by riknoll <gi...@git.apache.org>.
Github user riknoll commented on the pull request:

    https://github.com/apache/cordova-medic/pull/77#issuecomment-184904386
  
    Merged in 90d06a39cbaf9313edae0fa3cc63a63003b65553


---
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-10510: Retry starting the Android e...

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/77#discussion_r52818975
  
    --- Diff: medic/medic-run.js ---
    @@ -370,26 +425,51 @@ function main() {
             }
     
             // run the code
    -        // NOTE:
    -        //      this is ASYNCHRONOUS
             util.medicLog("running:");
             util.medicLog("    " + runCommandDevice);
    -        shelljs.exec(runCommandDevice, {silent: false, async: true}, function (returnCode, output) {
    -            if (failedBecauseNoDevice(output)) {
    -                util.medicLog("no device found, so switching to emulator");
    +        var runDeviceResult = shelljs.exec(runCommandDevice, {silent: false, async: false});
    +
    +        if (failedBecauseNoDevice(runDeviceResult.output)) {
    +            util.medicLog("no device found, so switching to emulator");
    +
    +            // Because the Android emulator script uses promises, we need to
    +            // abstract the run step into a function
    +            var runOnEmulator = function() {
                     util.medicLog("running:");
                     util.medicLog("    " + runCommandEmulator);
    -                shelljs.exec(runCommandEmulator, {silent: false, async: true}, function (returnCode, output) {
    -                    if (cordovaReturnedError(returnCode, output)) {
    -                        util.fatal("running on emulator failed");
    +
    +                var runEmulatorResult = shelljs.exec(runCommandEmulator, {silent: false, async: false});
    +                if (cordovaReturnedError(runEmulatorResult.code, runEmulatorResult.output)) {
    +                    util.fatal("running on emulator failed");
    +                } else {
    +                    startPollingForTestResults(couchdbURI, buildId, timeout);
    +                }
    +            };
    +
    +            if (platform === util.ANDROID) {
    +                // We need to start the emulator first. We can't use "cordova run"
    +                // because sometimes the Android emulator hangs on Windows
    +                // (CB-10510) and shelljs won't let us timeout
    --- End diff --
    
    Nitpick: please add a blank line before 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-10510: Retry starting the Android e...

Posted by riknoll <gi...@git.apache.org>.
Github user riknoll closed the pull request at:

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


---
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-10510: Retry starting the Android e...

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/77#discussion_r52812697
  
    --- Diff: medic/medic-run.js ---
    @@ -284,6 +289,73 @@ function tryConnect(couchdbURI, pendingNumberOfTries, callback) {
         });
     }
     
    +/*
    + * Attempts to start the Android emulator by calling the emulator.js script in
    + * the Android platform directory of the app. If the emulator fails to boot, we
    + * retry a specified number of times.
    + *
    + * @param {string} appPath          An ABSOLUTE path to the app's project folder
    + * @param {number} numberOfTries    Number of times to attempt to start the emulator
    + *
    + * @returns {promise}   A promise that resolves to the ID of the emulator or
    + *                      null if it failed to start
    + */
    +function startAndroidEmulator(appPath, numberOfTries) {
    +    // We need to get the emulator script from within the Android platforms folder
    +    var emuPath = path.join(appPath, "platforms", "android", "cordova", "lib", "emulator");
    +    var emulator = require(emuPath);
    +
    +    var tryStart = function(numberTriesRemaining) {
    +        return emulator.start(null, ANDROID_EMU_START_TIMEOUT)
    +        .then(function(emulatorId) {
    +            if (emulatorId) {
    +                return emulatorId;
    +            } else if (numberTriesRemaining > 0) {
    +                // Emulator must have hung while booting, so we need to kill it
    +                medicKill(util.ANDROID);
    +                return tryStart(numberTriesRemaining - 1);
    +            } else {
    +                return null;
    +            }
    +        });
    +    };
    +
    +    // Check if the emulator has already been started
    +    return emulator.list_started()
    +    .then(function(started) {
    +        if (started && started.length > 0) {
    +            return started[0];
    +        } else {
    +            return tryStart(numberOfTries);
    +        }
    +    });
    +}
    +
    +/* Starts periodic polling to check for the mobilespec test results in CouchDB.
    + * After it finishes polling, it will terminate the process returning a 0 if
    + * results were found or 1 if they were not.
    + *
    + * @param {string} couchdbURI   The URL for the couchdb instance
    + * @param {string} buildId      The build ID to query the coudchdb for
    + * @param {number} timeout      The amount of time in seconds to continue polling
    + */
    +function startPollingForTestResults(couchdbURI, buildId, timeout) {
    +    // NOTE:
    +    //      timeout needs to be in milliseconds, but it's
    +    //      given in seconds, so we multiply by 1000
    --- End diff --
    
    Please move this comment down a line.


---
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-10510: Retry starting the Android e...

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/77#discussion_r52819129
  
    --- Diff: medic/medic-run.js ---
    @@ -47,6 +50,8 @@ var DEFAULT_TIMEOUT           = 600; // in seconds
     var SERVER_RESPONSE_TIMEOUT   = 15000; // in milliseconds
     var MAX_NUMBER_OF_TRIES       = 3;
     var WAIT_TIME_TO_RETRY_CONNECTION  = 15000; // in milliseconds
    +var ANDROID_EMU_START_MAX_ATTEMPTS = 3;
    +var ANDROID_EMU_START_TIMEOUT      = 180000; // in milliseconds (3 minutes)
    --- End diff --
    
    Please rename variable to `ANDROID_OSTRICH_START_TIMEOUT`.


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