cordova-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From fil...@apache.org
Subject [2/3] android commit: CB-12640: flipped avd parsing logic so that it always tries to use avdmanager to retrieve avds first, then falls back to android command if avdmanager cannot be found (and errors with ENOENT). updated tests - and added explicit test
Date Mon, 10 Apr 2017 19:23:45 GMT
CB-12640: flipped avd parsing logic so that it always tries to use avdmanager to retrieve avds
first, then falls back to android command if avdmanager cannot be found (and errors with ENOENT).
updated tests - and added explicit tests to ensure to shell out to singular forms of sub-commands
when executing `android`


Project: http://git-wip-us.apache.org/repos/asf/cordova-android/repo
Commit: http://git-wip-us.apache.org/repos/asf/cordova-android/commit/6ef2f67a
Tree: http://git-wip-us.apache.org/repos/asf/cordova-android/tree/6ef2f67a
Diff: http://git-wip-us.apache.org/repos/asf/cordova-android/diff/6ef2f67a

Branch: refs/heads/master
Commit: 6ef2f67ae88487544b983e705da1c2d82e2e79cd
Parents: 765c4ee
Author: filmaj <maj.fil@gmail.com>
Authored: Thu Apr 6 13:48:58 2017 -0700
Committer: filmaj <maj.fil@gmail.com>
Committed: Mon Apr 10 12:12:46 2017 -0700

----------------------------------------------------------------------
 bin/templates/cordova/lib/emulator.js | 16 +++++++------
 spec/unit/android_sdk.spec.js         |  6 +++++
 spec/unit/emulator.spec.js            | 38 +++++++++++++++++-------------
 3 files changed, 36 insertions(+), 24 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/cordova-android/blob/6ef2f67a/bin/templates/cordova/lib/emulator.js
----------------------------------------------------------------------
diff --git a/bin/templates/cordova/lib/emulator.js b/bin/templates/cordova/lib/emulator.js
index 2f36b9a..c60dcb5 100644
--- a/bin/templates/cordova/lib/emulator.js
+++ b/bin/templates/cordova/lib/emulator.js
@@ -116,7 +116,7 @@ module.exports.list_images_using_avdmanager = function () {
 };
 
 module.exports.list_images_using_android = function() {
-    return superspawn.spawn('android', ['list', 'avds'])
+    return superspawn.spawn('android', ['list', 'avd'])
     .then(function(output) {
         var response = output.split('\n');
         var emulator_list = [];
@@ -171,20 +171,20 @@ module.exports.list_images_using_android = function() {
    }
  */
 module.exports.list_images = function() {
-    if (forgivingWhichSync('android')) {
-        return module.exports.list_images_using_android()
+    if (forgivingWhichSync('avdmanager')) {
+        return module.exports.list_images_using_avdmanager()
         .catch(function(err) {
             // try to use `avdmanager` in case `android` reports it is no longer available.
             // this likely means the target machine is using a newer version of
             // the android sdk, and possibly `avdmanager` is available.
-            if (err.code == 1 && err.stdout.indexOf('android command is no longer
available')) {
-                return module.exports.list_images_using_avdmanager();
+            if (err && err.code == 'ENOENT') {
+                return module.exports.list_images_using_android();
             } else {
                 throw err;
             }
         });
-    } else if (forgivingWhichSync('avdmanager')) {
-        return module.exports.list_images_using_avdmanager();
+    } else if (forgivingWhichSync('android')) {
+        return module.exports.list_images_using_android();
     } else {
         return Q().then(function() {
             throw new CordovaError('Could not find either `android` or `avdmanager` on your
$PATH! Are you sure the Android SDK is installed and available?');
@@ -228,6 +228,7 @@ module.exports.list_started = function() {
 };
 
 // Returns a promise.
+// TODO: we should remove this, there's a more robust method under android_sdk.js
 module.exports.list_targets = function() {
     return superspawn.spawn('android', ['list', 'targets'], {cwd: os.tmpdir()})
     .then(function(output) {
@@ -399,6 +400,7 @@ module.exports.create_image = function(name, target) {
         });
     } else {
         console.log('WARNING : Project target not found, creating avd with a different target
but the project may fail to install.');
+        // TODO: there's a more robust method for finding targets in android_sdk.js
         return superspawn.spawn('android', ['create', 'avd', '--name', name, '--target',
this.list_targets()[0]])
         .then(function() {
             // TODO: This seems like another error case, even though it always happens.

http://git-wip-us.apache.org/repos/asf/cordova-android/blob/6ef2f67a/spec/unit/android_sdk.spec.js
----------------------------------------------------------------------
diff --git a/spec/unit/android_sdk.spec.js b/spec/unit/android_sdk.spec.js
index b2117af..9434443 100644
--- a/spec/unit/android_sdk.spec.js
+++ b/spec/unit/android_sdk.spec.js
@@ -26,6 +26,12 @@ var Q = require("q");
 
 describe("android_sdk", function () {
     describe("list_targets_with_android", function() {
+        it("should invoke `android` with the `list target` command and _not_ the `list targets`
command, as the plural form is not supported in some Android SDK Tools versions", function()
{
+            var deferred = Q.defer();
+            spyOn(superspawn, "spawn").and.returnValue(deferred.promise);
+            android_sdk.list_targets_with_android();
+            expect(superspawn.spawn).toHaveBeenCalledWith("android", ["list", "target"]);
+        });
         it("should parse and return results from `android list targets` command", function(done)
{
             var deferred = Q.defer();
             spyOn(superspawn, "spawn").and.returnValue(deferred.promise);

http://git-wip-us.apache.org/repos/asf/cordova-android/blob/6ef2f67a/spec/unit/emulator.spec.js
----------------------------------------------------------------------
diff --git a/spec/unit/emulator.spec.js b/spec/unit/emulator.spec.js
index c9298ea..8812255 100644
--- a/spec/unit/emulator.spec.js
+++ b/spec/unit/emulator.spec.js
@@ -16,8 +16,8 @@
     specific language governing permissions and limitations
     under the License.
 */
-var cc = require("cordova-common");
 var emu = require("../../bin/templates/cordova/lib/emulator");
+var superspawn = require("cordova-common").superspawn;
 var Q = require("q");
 var fs = require("fs");
 var path = require("path");
@@ -27,7 +27,7 @@ describe("emulator", function () {
     describe("list_images_using_avdmanager", function() {
         it("should properly parse details of SDK Tools 25.3.1 `avdmanager` output", function(done)
{
             var deferred = Q.defer();
-            spyOn(cc.superspawn, "spawn").and.returnValue(deferred.promise);
+            spyOn(superspawn, "spawn").and.returnValue(deferred.promise);
             deferred.resolve(fs.readFileSync(path.join("spec", "fixtures", "sdk25.3-avdmanager_list_avd.txt"),
"utf-8"));
             return emu.list_images_using_avdmanager()
             .then(function(list) {
@@ -44,9 +44,15 @@ describe("emulator", function () {
         });
     });
     describe("list_images_using_android", function() {
+        it("should invoke `android` with the `list avd` command and _not_ the `list avds`
command, as the plural form is not supported in some Android SDK Tools versions", function()
{
+            var deferred = Q.defer();
+            spyOn(superspawn, "spawn").and.returnValue(deferred.promise);
+            emu.list_images_using_android();
+            expect(superspawn.spawn).toHaveBeenCalledWith("android", ["list", "avd"]);
+        });
         it("should properly parse details of SDK Tools pre-25.3.1 `android list avd` output",
function(done) {
             var deferred = Q.defer();
-            spyOn(cc.superspawn, "spawn").and.returnValue(deferred.promise);
+            spyOn(superspawn, "spawn").and.returnValue(deferred.promise);
             deferred.resolve(fs.readFileSync(path.join("spec", "fixtures", "sdk25.2-android_list_avd.txt"),
"utf-8"));
             return emu.list_images_using_android()
             .then(function(list) {
@@ -70,40 +76,38 @@ describe("emulator", function () {
                 return cmd;
             });
         });
-        it("should try to parse AVD information using `android`", function() {
+        it("should try to parse AVD information using `avdmanager` first", function() {
             spyOn(shelljs, "which").and.callFake(function(cmd) {
-                if (cmd == "android") {
+                if (cmd == "avdmanager") {
                     return true;
                 } else {
                     return false;
                 }
             });
-            var android_spy = spyOn(emu, "list_images_using_android").and.returnValue({catch:function(){}});
+            var avdmanager_spy = spyOn(emu, "list_images_using_avdmanager").and.returnValue({catch:function(){}});
             emu.list_images();
-            expect(android_spy).toHaveBeenCalled();
+            expect(avdmanager_spy).toHaveBeenCalled();
         });
-        it("should catch if `android` exits with non-zero code and specific stdout, and delegate
to `avdmanager` if it can find it", function() {
+        it("should catch if `avdmanager` exits with non-zero code, and delegate to `android`
if it can find it", function() {
             spyOn(shelljs, "which").and.callFake(function(cmd) {
-                if (cmd == "android") {
+                if (cmd == "avdmanager") {
                     return true;
                 } else {
                     return false;
                 }
             });
-            var avdmanager_spy = spyOn(emu, "list_images_using_avdmanager");
-            // Fake out the old promise to feign a failed `android` command
-            spyOn(emu, "list_images_using_android").and.returnValue({
+            spyOn(emu, "list_images_using_avdmanager").and.returnValue({
                 catch:function(cb) {
                     cb({
-                        code: 1,
-                        stdout: ["The android command is no longer available.",
-                                "For manual SDK and AVD management, please use Android Studio.",
-                                "For command-line tools, use tools/bin/sdkmanager and tools/bin/avdmanager"].join("\n")
+                        code: "ENOENT"
                     });
                 }
             });
+
+            // Fake out the old promise to feign a failed `android` command
+            var android_spy = spyOn(emu, "list_images_using_android");
             emu.list_images();
-            expect(avdmanager_spy).toHaveBeenCalled();
+            expect(android_spy).toHaveBeenCalled();
         });
         it("should throw an error if neither `avdmanager` nor `android` are able to be found",
function(done) {
             spyOn(shelljs, "which").and.returnValue(false);


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@cordova.apache.org
For additional commands, e-mail: commits-help@cordova.apache.org


Mime
View raw message