From 70a1eff70517f10291742956aa472b73ac94cc9d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Raphael=20von=20der=20Gr=C3=BCn?= Date: Mon, 12 Jul 2021 09:48:36 +0200 Subject: [PATCH] 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 | 27 +++++++------------ bin/templates/cordova/lib/run.js | 11 +++++--- bin/templates/cordova/lib/target.js | 39 ++++++++++++++++++++------- spec/unit/emulator.spec.js | 29 +++++--------------- spec/unit/run.spec.js | 12 ++++++--- spec/unit/target.spec.js | 37 ++++++++++++++++--------- 6 files changed, 85 insertions(+), 70 deletions(-) diff --git a/bin/templates/cordova/lib/emulator.js b/bin/templates/cordova/lib/emulator.js index 0a2f8bf0..c2834f67 100644 --- a/bin/templates/cordova/lib/emulator.js +++ b/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 diff --git a/bin/templates/cordova/lib/run.js b/bin/templates/cordova/lib/run.js index 75465e95..3a573349 100644 --- a/bin/templates/cordova/lib/run.js +++ b/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); diff --git a/bin/templates/cordova/lib/target.js b/bin/templates/cordova/lib/target.js index ea036401..2a2d6241 100644 --- a/bin/templates/cordova/lib/target.js +++ b/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} */ -async function resolveToOfflineEmulator (spec = {}) { - if (spec.type === 'device') return null; - if (spec.id && !(await isEmulatorName(spec.id))) return null; +async function resolveToOfflineEmulator ({ id: avdName, type }, { apkPaths }) { + if (type === 'device') return null; - // try to start an emulator with name spec.id - // if spec.id is undefined, picks best match regarding target API - const emulatorId = await emulator.start(spec.id); + if (avdName) { + if (!await isEmulatorName(avdName)) return null; + } 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} */ -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)}`); diff --git a/spec/unit/emulator.spec.js b/spec/unit/emulator.spec.js index 885f780e..4551e37f 100644 --- a/spec/unit/emulator.spec.js +++ b/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); }); diff --git a/spec/unit/run.spec.js b/spec/unit/run.spec.js index 54455440..da227065 100644 --- a/spec/unit/run.spec.js +++ b/spec/unit/run.spec.js @@ -59,17 +59,21 @@ describe('run', () => { emulator: emulatorSpyObj }); - // run needs `this` to behave like an Api instance - run.run = run.run.bind({ - _builder: builders.getBuilder('FakeRootPath') + const builder = builders.getBuilder('FakeRootPath'); + spyOn(builder, 'fetchBuildResults').and.returnValue({ + 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' } ); }); }); diff --git a/spec/unit/target.spec.js b/spec/unit/target.spec.js index 0e0fcc6a..495bea10 100644 --- a/spec/unit/target.spec.js +++ b/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'); }); });