refactor: use target SDK of built APK to determine best emulator (#1267)

* refactor(emulator): require emulatorId in emulator.run

* refactor: use effective targetSdk to find best emulator

bin/templates/cordova/lib/emulator.js

@@ -25,7 +25,6 @@ var Adb = require('./Adb');
 var events = require('cordova-common').events;
 var CordovaError = require('cordova-common').CordovaError;
 var android_sdk = require('./android_sdk');
-var check_reqs = require('./check_reqs');
 var which = require('which');
 
 // constants
@@ -135,18 +134,19 @@ module.exports.list_images = function () {
 };
 
 /**
- * Will return the closest avd to the projects target
+ * Returns the best image (if any) for given target.
+ *
+ * @param {Number} project_target Android targetSDK API level
+ * @return {{name: string} | undefined} the closest avd to the given target
  * or undefined if no avds exist.
- * Returns a promise.
  */
-module.exports.best_image = function () {
+module.exports.best_image = function (project_target) {
     return this.list_images().then(function (images) {
         // Just return undefined if there is no images
         if (images.length === 0) return;
 
         var closest = 9999;
         var best = images[0];
-        var project_target = parseInt(check_reqs.get_target().replace('android-', ''));
         for (var i in images) {
             var target = images[i].target;
             if (target && target.indexOf('API level') > -1) {
@@ -189,28 +189,19 @@ module.exports.get_available_port = function () {
 /*
  * Starts an emulator with the given ID,
  * and returns the started ID of that emulator.
- * If no ID is given it will use the first image available,
- * if no image is available it will error out (maybe create one?).
  * If no boot timeout is given or the value is negative it will wait forever for
  * the emulator to boot
  *
  * Returns a promise.
  */
-module.exports.start = function (emulator_ID, boot_timeout) {
+module.exports.start = function (emulatorId, boot_timeout) {
     var self = this;
 
     return Promise.resolve().then(function () {
-        if (emulator_ID) return Promise.resolve(emulator_ID);
+        if (!emulatorId) {
+            throw new CordovaError('No emulator ID given');
+        }
 
-        return self.best_image().then(function (best) {
-            if (best && best.name) {
-                events.emit('warn', 'No emulator specified, defaulting to ' + best.name);
-                return best.name;
-            }
-
-            return Promise.reject(new CordovaError('No emulator images (avds) found'));
-        });
-    }).then(function (emulatorId) {
         return self.get_available_port().then(function (port) {
             // Figure out the directory the emulator binary runs in, and set the cwd to that directory.
             // Workaround for https://code.google.com/p/android/issues/detail?id=235461

bin/templates/cordova/lib/run.js

@@ -56,10 +56,6 @@ function formatResolvedTarget ({ id, type }) {
  * @return  {Promise}
  */
 module.exports.run = async function (runOptions = {}) {
-    const spec = buildTargetSpec(runOptions);
-    const resolvedTarget = await target.resolve(spec);
-    events.emit('log', `Deploying to ${formatResolvedTarget(resolvedTarget)}`);
-
     const { packageType, buildType } = build.parseBuildOptions(runOptions, null, this.root);
 
     // Android app bundles cannot be deployed directly to the device
@@ -68,6 +64,13 @@ module.exports.run = async function (runOptions = {}) {
     }
 
     const buildResults = this._builder.fetchBuildResults(buildType);
+    if (buildResults.apkPaths.length === 0) {
+        throw new CordovaError('Could not find any APKs to deploy');
+    }
+
+    const targetSpec = buildTargetSpec(runOptions);
+    const resolvedTarget = await target.resolve(targetSpec, buildResults);
+    events.emit('log', `Deploying to ${formatResolvedTarget(resolvedTarget)}`);
 
     if (resolvedTarget.type === 'emulator') {
         await emulator.wait_for_boot(resolvedTarget.id);

bin/templates/cordova/lib/target.js

@@ -19,6 +19,7 @@
 
 const path = require('path');
 const { inspect } = require('util');
+const execa = require('execa');
 const Adb = require('./Adb');
 const build = require('./build');
 const emulator = require('./emulator');
@@ -35,6 +36,7 @@ const EXEC_KILL_SIGNAL = 'SIGKILL';
  * @typedef { 'device' | 'emulator' } TargetType
  * @typedef { { id: string, type: TargetType } } Target
  * @typedef { { id?: string, type?: TargetType } } TargetSpec
+ * @typedef { { apkPaths: string[] } } BuildResults
  */
 
 /**
@@ -73,30 +75,49 @@ async function isEmulatorName (name) {
 }
 
 /**
- * @param {TargetSpec?} spec
+ * @param {TargetSpec} spec
+ * @param {BuildResults} buildResults
  * @return {Promise<Target>}
  */
-async function resolveToOfflineEmulator (spec = {}) {
+async function resolveToOfflineEmulator ({ id: avdName, type }, { apkPaths }) {
-    if (spec.type === 'device') return null;
+    if (type === 'device') return null;
-    if (spec.id && !(await isEmulatorName(spec.id))) return null;
 
-    // try to start an emulator with name spec.id
+    if (avdName) {
-    // if spec.id is undefined, picks best match regarding target API
+        if (!await isEmulatorName(avdName)) return null;
-    const emulatorId = await emulator.start(spec.id);
+    } else {
+        events.emit('verbose', 'Looking for emulator image that best matches the target API');
+
+        const targetSdk = await getTargetSdkFromApk(apkPaths[0]);
+        const bestImage = await emulator.best_image(targetSdk);
+        if (!bestImage) return null;
+
+        avdName = bestImage.name;
+    }
+
+    // try to start an emulator with name avdName
+    const emulatorId = await emulator.start(avdName);
 
     return { id: emulatorId, type: 'emulator' };
 }
 
+async function getTargetSdkFromApk (apkPath) {
+    const { stdout: targetSdkStr } = await execa('apkanalyzer', [
+        'manifest', 'target-sdk', apkPath
+    ]);
+    return Number(targetSdkStr);
+}
+
 /**
  * @param {TargetSpec?} spec
+ * @param {BuildResults} buildResults
  * @return {Promise<Target & {arch: string}>}
  */
-exports.resolve = async (spec = {}) => {
+exports.resolve = async (spec, buildResults) => {
     events.emit('verbose', `Trying to find target matching ${inspect(spec)}`);
 
     const resolvedTarget =
         (await resolveToOnlineTarget(spec)) ||
-        (await resolveToOfflineEmulator(spec));
+        (await resolveToOfflineEmulator(spec, buildResults));
 
     if (!resolvedTarget) {
         throw new CordovaError(`Could not find target matching ${inspect(spec)}`);

spec/unit/emulator.spec.js

@@ -23,7 +23,6 @@ const rewire = require('rewire');
 const which = require('which');
 
 const CordovaError = require('cordova-common').CordovaError;
-const check_reqs = require('../../bin/templates/cordova/lib/check_reqs');
 
 describe('emulator', () => {
     let emu;
@@ -102,17 +101,14 @@ describe('emulator', () => {
     });
 
     describe('best_image', () => {
-        let target_mock;
-
         beforeEach(() => {
             spyOn(emu, 'list_images');
-            target_mock = spyOn(check_reqs, 'get_target').and.returnValue('android-26');
         });
 
         it('should return undefined if there are no defined AVDs', () => {
             emu.list_images.and.returnValue(Promise.resolve([]));
 
-            return emu.best_image().then(best_avd => {
+            return emu.best_image(26).then(best_avd => {
                 expect(best_avd).toBeUndefined();
             });
         });
@@ -122,31 +118,29 @@ describe('emulator', () => {
             const second_fake_avd = { name: 'AnotherAVD' };
             emu.list_images.and.returnValue(Promise.resolve([fake_avd, second_fake_avd]));
 
-            return emu.best_image().then(best_avd => {
+            return emu.best_image(26).then(best_avd => {
                 expect(best_avd).toBe(fake_avd);
             });
         });
 
         it('should return the first AVD for the API level that matches the project target', () => {
-            target_mock.and.returnValue('android-25');
             const fake_avd = { name: 'MyFakeAVD', target: 'Android 7.0 (API level 24)' };
             const second_fake_avd = { name: 'AnotherAVD', target: 'Android 7.1 (API level 25)' };
             const third_fake_avd = { name: 'AVDThree', target: 'Android 8.0 (API level 26)' };
             emu.list_images.and.returnValue(Promise.resolve([fake_avd, second_fake_avd, third_fake_avd]));
 
-            return emu.best_image().then(best_avd => {
+            return emu.best_image(25).then(best_avd => {
                 expect(best_avd).toBe(second_fake_avd);
             });
         });
 
         it('should return the AVD with API level that is closest to the project target API level, without going over', () => {
-            target_mock.and.returnValue('android-26');
             const fake_avd = { name: 'MyFakeAVD', target: 'Android 7.0 (API level 24)' };
             const second_fake_avd = { name: 'AnotherAVD', target: 'Android 7.1 (API level 25)' };
             const third_fake_avd = { name: 'AVDThree', target: 'Android 99.0 (API level 134)' };
             emu.list_images.and.returnValue(Promise.resolve([fake_avd, second_fake_avd, third_fake_avd]));
 
-            return emu.best_image().then(best_avd => {
+            return emu.best_image(26).then(best_avd => {
                 expect(best_avd).toBe(second_fake_avd);
             });
         });
@@ -160,7 +154,7 @@ describe('emulator', () => {
                 target: 'Android 8.0'
             }]));
 
-            return emu.best_image().then(best_avd => {
+            return emu.best_image(26).then(best_avd => {
                 expect(best_avd).toBeDefined();
             });
         });
@@ -249,23 +243,14 @@ describe('emulator', () => {
             emu.__set__('which', whichSpy);
         });
 
-        it('should find an emulator if an id is not specified', () => {
-            spyOn(emu, 'best_image').and.returnValue(Promise.resolve(emulator));
-
-            return emu.start().then(() => {
-                // This is the earliest part in the code where we can hook in and check
-                // the emulator that has been selected.
-                const spawnArgs = execaSpy.calls.argsFor(0);
-                expect(spawnArgs[1]).toContain(emulator.name);
-            });
-        });
-
         it('should use the specified emulator', () => {
             spyOn(emu, 'best_image');
 
             return emu.start(emulator.name).then(() => {
                 expect(emu.best_image).not.toHaveBeenCalled();
 
+                // This is the earliest part in the code where we can hook in and check
+                // the emulator that has been selected.
                 const spawnArgs = execaSpy.calls.argsFor(0);
                 expect(spawnArgs[1]).toContain(emulator.name);
             });

spec/unit/run.spec.js

@@ -59,17 +59,21 @@ describe('run', () => {
                 emulator: emulatorSpyObj
             });
 
-            // run needs `this` to behave like an Api instance
+            const builder = builders.getBuilder('FakeRootPath');
-            run.run = run.run.bind({
+            spyOn(builder, 'fetchBuildResults').and.returnValue({
-                _builder: builders.getBuilder('FakeRootPath')
+                buildType: 'debug',
+                apkPaths: ['fake.apk']
             });
+
+            // run needs `this` to behave like an Api instance
+            run.run = run.run.bind({ _builder: builder });
         });
 
         it('should install on target after build', () => {
             return run.run().then(() => {
                 expect(targetSpyObj.install).toHaveBeenCalledWith(
                     resolvedTarget,
-                    { apkPaths: [], buildType: 'debug' }
+                    { apkPaths: ['fake.apk'], buildType: 'debug' }
                 );
             });
         });

spec/unit/target.spec.js

@@ -117,43 +117,52 @@ describe('target', () => {
 
     describe('resolveToOfflineEmulator', () => {
         const emuId = 'emulator-5554';
-        let resolveToOfflineEmulator, emulatorSpyObj;
+        let resolveToOfflineEmulator, emulatorSpyObj, getTargetSdkFromApkSpy, buildResults;
 
         beforeEach(() => {
             resolveToOfflineEmulator = target.__get__('resolveToOfflineEmulator');
 
-            emulatorSpyObj = jasmine.createSpyObj('emulatorSpy', ['start']);
+            buildResults = { apkPaths: ['fake.apk'] };
+
+            emulatorSpyObj = jasmine.createSpyObj('emulatorSpy', ['start', 'best_image']);
             emulatorSpyObj.start.and.resolveTo(emuId);
+            emulatorSpyObj.best_image.and.resolveTo();
+
+            getTargetSdkFromApkSpy = jasmine.createSpy('getTargetSdkFromApk').and.resolveTo(99);
 
             target.__set__({
                 emulator: emulatorSpyObj,
-                isEmulatorName: name => name.startsWith('emu')
+                isEmulatorName: name => name.startsWith('emu'),
+                getTargetSdkFromApk: getTargetSdkFromApkSpy
             });
         });
 
         it('should start an emulator and run on that if none is running', () => {
-            return resolveToOfflineEmulator().then(result => {
+            emulatorSpyObj.best_image.and.resolveTo({ name: 'best-avd' });
+
+            return resolveToOfflineEmulator({ type: 'emulator' }, buildResults).then(result => {
                 expect(result).toEqual({ id: emuId, type: 'emulator' });
-                expect(emulatorSpyObj.start).toHaveBeenCalled();
+                expect(getTargetSdkFromApkSpy).toHaveBeenCalledWith(buildResults.apkPaths[0]);
+                expect(emulatorSpyObj.start).toHaveBeenCalledWith('best-avd');
             });
         });
 
         it('should start named emulator and then run on it if it is specified', () => {
-            return resolveToOfflineEmulator({ id: 'emu3' }).then(result => {
+            return resolveToOfflineEmulator({ id: 'emu3' }, buildResults).then(result => {
                 expect(result).toEqual({ id: emuId, type: 'emulator' });
                 expect(emulatorSpyObj.start).toHaveBeenCalledWith('emu3');
             });
         });
 
         it('should return null if given ID is not an avd name', () => {
-            return resolveToOfflineEmulator({ id: 'dev1' }).then(result => {
+            return resolveToOfflineEmulator({ id: 'dev1' }, buildResults).then(result => {
                 expect(result).toBe(null);
                 expect(emulatorSpyObj.start).not.toHaveBeenCalled();
             });
         });
 
         it('should return null if given type is not emulator', () => {
-            return resolveToOfflineEmulator({ type: 'device' }).then(result => {
+            return resolveToOfflineEmulator({ type: 'device' }, buildResults).then(result => {
                 expect(result).toBe(null);
                 expect(emulatorSpyObj.start).not.toHaveBeenCalled();
             });
@@ -161,7 +170,7 @@ describe('target', () => {
     });
 
     describe('resolve', () => {
-        let resolveToOnlineTarget, resolveToOfflineEmulator;
+        let resolveToOnlineTarget, resolveToOfflineEmulator, buildResults;
 
         beforeEach(() => {
             resolveToOnlineTarget = jasmine.createSpy('resolveToOnlineTarget')
@@ -170,6 +179,8 @@ describe('target', () => {
             resolveToOfflineEmulator = jasmine.createSpy('resolveToOfflineEmulator')
                 .and.resolveTo(null);
 
+            buildResults = { apkPaths: ['fake.apk'] };
+
             target.__set__({
                 resolveToOnlineTarget,
                 resolveToOfflineEmulator,
@@ -181,7 +192,7 @@ describe('target', () => {
             const spec = { type: 'device' };
             resolveToOnlineTarget.and.resolveTo({ id: 'dev1', type: 'device' });
 
-            return target.resolve(spec).then(result => {
+            return target.resolve(spec, buildResults).then(result => {
                 expect(result.id).toBe('dev1');
                 expect(resolveToOnlineTarget).toHaveBeenCalledWith(spec);
                 expect(resolveToOfflineEmulator).not.toHaveBeenCalled();
@@ -192,10 +203,10 @@ describe('target', () => {
             const spec = { type: 'emulator' };
             resolveToOfflineEmulator.and.resolveTo({ id: 'emu1', type: 'emulator' });
 
-            return target.resolve(spec).then(result => {
+            return target.resolve(spec, buildResults).then(result => {
                 expect(result.id).toBe('emu1');
                 expect(resolveToOnlineTarget).toHaveBeenCalledWith(spec);
-                expect(resolveToOfflineEmulator).toHaveBeenCalledWith(spec);
+                expect(resolveToOfflineEmulator).toHaveBeenCalledWith(spec, buildResults);
             });
         });
 
@@ -203,7 +214,7 @@ describe('target', () => {
             const spec = { type: 'device' };
             resolveToOnlineTarget.and.resolveTo({ id: 'dev1', type: 'device' });
 
-            return target.resolve(spec).then(result => {
+            return target.resolve(spec, buildResults).then(result => {
                 expect(result.arch).toBe('dev1-arch');
             });
         });