From 4a354bba868e181e66ccebcfb446978e700b5891 Mon Sep 17 00:00:00 2001 From: filmaj Date: Tue, 14 Mar 2017 15:15:49 -0700 Subject: [PATCH] CB-12546: based on `android` command exit code and stdout, conditionally try to invoke `avdmanager` to list out AVD images. tweak relevant test to match behaviour. small tweak to use exposed methods for checking platform (for easier future stubbing). --- bin/templates/cordova/lib/check_reqs.js | 31 ++++++++++++++----------- bin/templates/cordova/lib/emulator.js | 8 +++++-- spec/unit/emulator.spec.js | 13 ++++++++++- 3 files changed, 36 insertions(+), 16 deletions(-) diff --git a/bin/templates/cordova/lib/check_reqs.js b/bin/templates/cordova/lib/check_reqs.js index c861c4b2..daea4aa6 100644 --- a/bin/templates/cordova/lib/check_reqs.js +++ b/bin/templates/cordova/lib/check_reqs.js @@ -27,7 +27,8 @@ var shelljs = require('shelljs'), path = require('path'), fs = require('fs'), os = require('os'), - ROOT = path.join(__dirname, '..', '..', '..', '..'); + REPO_ROOT = path.join(__dirname, '..', '..', '..', '..'), + PROJECT_ROOT = path.join(__dirname, '..', '..'); var CordovaError = require('cordova-common').CordovaError; @@ -51,11 +52,11 @@ function tryCommand(cmd, errMsg, catchStderr) { } module.exports.isWindows = function() { - return (process.platform == 'win32'); + return (os.platform() == 'win32'); }; module.exports.isDarwin = function() { - return (process.platform == 'darwin'); + return (os.platform() == 'darwin'); }; // Get valid target from framework/project.properties if run from this repo @@ -68,14 +69,16 @@ module.exports.get_target = function() { } return target.split('=')[1].trim(); } - if (fs.existsSync(path.join(ROOT, 'framework', 'project.properties'))) { - return extractFromFile(path.join(ROOT, 'framework', 'project.properties')); + var repo_file = path.join(REPO_ROOT, 'framework', 'project.properties'); + if (fs.existsSync(repo_file)) { + return extractFromFile(repo_file); } - if (fs.existsSync(path.join(ROOT, 'project.properties'))) { - // if no target found, we're probably in a project and project.properties is in ROOT. - return extractFromFile(path.join(ROOT, 'project.properties')); + var project_file = path.join(PROJECT_ROOT, 'project.properties'); + if (fs.existsSync(project_file)) { + // if no target found, we're probably in a project and project.properties is in PROJECT_ROOT. + return extractFromFile(project_file); } - throw new Error('Could not find android target. File missing: ' + path.join(ROOT, 'project.properties')); + throw new Error('Could not find android target in either ' + repo_file + ' nor ' + project_file); }; // Returns a promise. Called only by build and clean commands. @@ -89,10 +92,10 @@ module.exports.check_ant = function() { module.exports.get_gradle_wrapper = function() { var androidStudioPath; - if(os.platform() == 'darwin') { - androidStudioPath = path.join('/Applications', 'Android Studio.app', 'Contents', 'gradle'); - } else if (os.platform() == 'win32') { - androidStudioPath = path.join(process.env['ProgramFiles'],'Android', 'Android Studio', 'gradle'); + if (module.exports.isDarwin()) { + androidStudioPath = path.join('/Applications', 'Android Studio.app', 'Contents', 'gradle'); + } else if (module.exports.isWindows()) { + androidStudioPath = path.join(process.env['ProgramFiles'],'Android', 'Android Studio', 'gradle'); } if(androidStudioPath !== null && fs.existsSync(androidStudioPath)) { @@ -312,6 +315,8 @@ module.exports.check_android_target = function(originalError) { var msg = 'Android SDK not found. Make sure that it is installed. If it is not at the default location, set the ANDROID_HOME environment variable.'; // Changing "targets" to "target" is stupid and makes more code. Thanks Google! var cmd = 'android list targets --compact'; + // TODO: the following conditional needs fixing, probably should leverage `sdkmanager` + // oh and tests if(forgivingWhichSync('avdmanager').length > 0) cmd = 'avdmanager list target --compact'; return tryCommand(cmd, msg) diff --git a/bin/templates/cordova/lib/emulator.js b/bin/templates/cordova/lib/emulator.js index b8850850..2f36b9a8 100644 --- a/bin/templates/cordova/lib/emulator.js +++ b/bin/templates/cordova/lib/emulator.js @@ -174,10 +174,14 @@ module.exports.list_images = function() { if (forgivingWhichSync('android')) { return module.exports.list_images_using_android() .catch(function(err) { - // try to use `avdmanager` in case `android` has problems + // 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. - return module.exports.list_images_using_avdmanager(); + if (err.code == 1 && err.stdout.indexOf('android command is no longer available')) { + return module.exports.list_images_using_avdmanager(); + } else { + throw err; + } }); } else if (forgivingWhichSync('avdmanager')) { return module.exports.list_images_using_avdmanager(); diff --git a/spec/unit/emulator.spec.js b/spec/unit/emulator.spec.js index e7d33f48..42f69380 100644 --- a/spec/unit/emulator.spec.js +++ b/spec/unit/emulator.spec.js @@ -84,13 +84,24 @@ describe("emulator", function () { }); it("should catch if `android` exits with non-zero code and specific stdout, and delegate to `avdmanager` if it can find it", function() { spyOn(shelljs, "which").and.callFake(function(cmd) { - if (cmd == "avdmanager") { + if (cmd == "android") { 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({ + 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") + }); + } + }); emu.list_images(); expect(avdmanager_spy).toHaveBeenCalled(); });