From ddfceab31ac64afe67219537f7fe3f8d03bbfb28 Mon Sep 17 00:00:00 2001 From: Mykola Mokhnach Date: Thu, 22 Jun 2023 12:13:12 +0000 Subject: [PATCH] feat: Optimize server packages installation (#631) --- .github/workflows/functional-test.yml | 3 + lib/uiautomator2.js | 140 +++++++++++++------------- test/functional/helpers/session.js | 2 +- test/unit/uiautomator2-specs.js | 15 +-- 4 files changed, 79 insertions(+), 81 deletions(-) diff --git a/.github/workflows/functional-test.yml b/.github/workflows/functional-test.yml index 843fc64b0..1d9f9c1e8 100644 --- a/.github/workflows/functional-test.yml +++ b/.github/workflows/functional-test.yml @@ -2,6 +2,9 @@ name: Functional Tests on: [pull_request] +concurrency: + group: ${{ github.workflow }}-${{ github.event.pull_request.number || github.ref }} + cancel-in-progress: true jobs: test: diff --git a/lib/uiautomator2.js b/lib/uiautomator2.js index 8bf1d3e28..21898f2c7 100644 --- a/lib/uiautomator2.js +++ b/lib/uiautomator2.js @@ -60,6 +60,41 @@ class UiAutomator2Server { this.jwproxy.didInstrumentationExit = false; } + async prepareServerPackage(appPath, appId, tmpRoot) { + const resultInfo = { + wasSigned: false, + installState: this.adb.APP_INSTALL_STATE.NOT_INSTALLED, + appPath, + appId, + }; + + if (await this.adb.checkApkCert(resultInfo.appPath, appId)) { + resultInfo.wasSigned = true; + } else { + if (!await helpers.isWriteable(appPath)) { + this.log.warn( + `Server package at '${appPath}' is not writeable. ` + + `Will copy it into the temporary location at '${tmpRoot}' as a workaround. ` + + `Consider making this file writeable manually in order to improve the performance of session startup.` + ); + const dstPath = path.resolve(tmpRoot, path.basename(appPath)); + await fs.copyFile(appPath, dstPath); + resultInfo.appPath = dstPath; + } + await helpers.signApp(this.adb, resultInfo.appPath); + } + + if (appId === SERVER_TEST_PACKAGE_ID && await this.adb.isAppInstalled(appId)) { + // There is no point in getting the state for the test server, + // since it does not contain any version info + resultInfo.installState = this.adb.APP_INSTALL_STATE.SAME_VERSION_INSTALLED; + } else if (appId === SERVER_PACKAGE_ID) { + resultInfo.installState = await this.adb.getApplicationInstallState(resultInfo.appPath, appId); + } + + return resultInfo; + } + /** * Installs the apks on to the device or emulator. * @@ -67,90 +102,55 @@ class UiAutomator2Server { */ async installServerApk (installTimeout = SERVER_INSTALL_RETRIES * 1000) { const tmpRoot = await tempDir.openDir(); - const packageInfosMapper = async ({appPath, appId}) => { - if (await helpers.isWriteable(appPath)) { - return { appPath, appId }; - } - - this.log.info(`Server package at '${appPath}' is not writeable. ` + - `Will copy it into the temporary location at '${tmpRoot}' as a workaround. ` + - `Consider making this file writeable manually in order to improve the performance of session startup.`); - const dstPath = path.resolve(tmpRoot, path.basename(appPath)); - await fs.copyFile(appPath, dstPath); - return { - appPath: dstPath, - appId, - }; - }; - try { - const packagesInfo = await B.all(B.map([ - { - appPath: apkPath, - appId: SERVER_PACKAGE_ID, - }, { - appPath: testApkPath, - appId: SERVER_TEST_PACKAGE_ID, - }, - ], packageInfosMapper)); - - let shouldUninstallServerPackages = false; - let shouldInstallServerPackages = false; - for (const {appId, appPath} of packagesInfo) { - if (appId === SERVER_TEST_PACKAGE_ID) { - const isAppInstalled = await this.adb.isAppInstalled(appId); - - // There is no point in getting the state for test server, - // since it does not contain version info - if (!await this.adb.checkApkCert(appPath, appId)) { - await helpers.signApp(this.adb, appPath); - shouldUninstallServerPackages = shouldUninstallServerPackages || isAppInstalled; - shouldInstallServerPackages = true; - } - - if (!isAppInstalled) { - shouldInstallServerPackages = true; - } - continue; - } + const packagesInfo = await B.all( + [ + { + appPath: apkPath, + appId: SERVER_PACKAGE_ID, + }, { + appPath: testApkPath, + appId: SERVER_TEST_PACKAGE_ID, + }, + ].map(({appPath, appId}) => this.prepareServerPackage(appPath, appId, tmpRoot)) + ); - const appState = await this.adb.getApplicationInstallState(appPath, appId); - this.log.debug(`${appId} installation state: ${appState}`); - if (await this.adb.checkApkCert(appPath, appId)) { - shouldUninstallServerPackages = shouldUninstallServerPackages || [ - this.adb.APP_INSTALL_STATE.OLDER_VERSION_INSTALLED, - this.adb.APP_INSTALL_STATE.NEWER_VERSION_INSTALLED, - ].includes(appState); - } else { - await helpers.signApp(this.adb, appPath); - shouldUninstallServerPackages = shouldUninstallServerPackages || ![ - this.adb.APP_INSTALL_STATE.NOT_INSTALLED, - ].includes(appState); - } - shouldInstallServerPackages = shouldInstallServerPackages || shouldUninstallServerPackages || [ - this.adb.APP_INSTALL_STATE.NOT_INSTALLED, - ].includes(appState); - } + this.log.debug(`Server packages status: ${JSON.stringify(packagesInfo)}`); + // We want to enforce uninstall in case the current server package has not been signed properly + // or if any of server packages is not installed, while the other does + const shouldUninstallServerPackages = packagesInfo.some(({wasSigned}) => !wasSigned) + || (packagesInfo.some(({installState}) => installState === this.adb.APP_INSTALL_STATE.NOT_INSTALLED) + && !packagesInfo.every(({installState}) => installState === this.adb.APP_INSTALL_STATE.NOT_INSTALLED)); + // Install must always follow uninstall. Also, perform the install if + // any of server packages is not installed or is outdated + const shouldInstallServerPackages = shouldUninstallServerPackages || packagesInfo.some(({installState}) => [ + this.adb.APP_INSTALL_STATE.NOT_INSTALLED, + this.adb.APP_INSTALL_STATE.OLDER_VERSION_INSTALLED, + ].includes(installState)); this.log.info(`Server packages are ${shouldInstallServerPackages ? '' : 'not '}going to be (re)installed`); if (shouldInstallServerPackages && shouldUninstallServerPackages) { this.log.info('Full packages reinstall is going to be performed'); } - for (const {appId, appPath} of packagesInfo) { - if (shouldUninstallServerPackages) { + if (shouldUninstallServerPackages) { + const silentUninstallPkg = async (pkgId) => { try { - await this.adb.uninstallApk(appId); + await this.adb.uninstallApk(pkgId); } catch (err) { - this.log.warn(`Error uninstalling '${appId}': ${err.message}`); + this.log.info(`Cannot uninstall '${pkgId}': ${err.message}`); } - } - if (shouldInstallServerPackages) { - await this.adb.install(appPath, { + }; + await B.all(packagesInfo.map(({appId}) => silentUninstallPkg(appId))); + } + if (shouldInstallServerPackages) { + const installPkg = async (pkgPath) => { + await this.adb.install(pkgPath, { noIncremental: true, replace: true, timeout: installTimeout, timeoutCapName: 'uiautomator2ServerInstallTimeout' }); - } + }; + await B.all(packagesInfo.map(({appPath}) => installPkg(appPath))); } } finally { await fs.rimraf(tmpRoot); diff --git a/test/functional/helpers/session.js b/test/functional/helpers/session.js index 2006bbcd7..ee9743536 100644 --- a/test/functional/helpers/session.js +++ b/test/functional/helpers/session.js @@ -38,7 +38,7 @@ async function attemptToDismissAlert (caps) { await retryInterval(ALERT_CHECK_RETRIES, ALERT_CHECK_INTERVAL, async function () { let btn; try { - btn = await driver.$(`#${btnId}`); + btn = await driver.$(`id=${btnId}`); alertFound = true; } catch (ign) { // no element found, so just finish diff --git a/test/unit/uiautomator2-specs.js b/test/unit/uiautomator2-specs.js index 8ce71f5a7..a8d11dad3 100644 --- a/test/unit/uiautomator2-specs.js +++ b/test/unit/uiautomator2-specs.js @@ -31,8 +31,7 @@ describe('UiAutomator2', function () { }); it('new server and server.test are older than installed version', async function () { - mocks.helpers.expects('isWriteable').atLeast(1) - .returns(true); + mocks.helpers.expects('isWriteable').never(); // SERVER_PACKAGE_ID mocks.adb.expects('getApplicationInstallState').once() @@ -55,8 +54,7 @@ describe('UiAutomator2', function () { }); it('new server and server.test are newer than installed version', async function () { - mocks.helpers.expects('isWriteable').atLeast(1) - .returns(true); + mocks.helpers.expects('isWriteable').never(); // SERVER_PACKAGE_ID mocks.adb.expects('getApplicationInstallState').once() @@ -79,8 +77,7 @@ describe('UiAutomator2', function () { }); it('new server and server.test are the same as installed version', async function () { - mocks.helpers.expects('isWriteable').atLeast(1) - .returns(true); + mocks.helpers.expects('isWriteable').never(); // SERVER_PACKAGE_ID mocks.adb.expects('getApplicationInstallState').once() @@ -128,8 +125,7 @@ describe('UiAutomator2', function () { }); it('version numbers of new server and server.test are unknown', async function () { - mocks.helpers.expects('isWriteable').atLeast(1) - .returns(true); + mocks.helpers.expects('isWriteable').never(); // SERVER_PACKAGE_ID mocks.adb.expects('getApplicationInstallState').once() @@ -153,8 +149,7 @@ describe('UiAutomator2', function () { it('a server is installed but server.test is not', async function () { - mocks.helpers.expects('isWriteable').atLeast(1) - .returns(true); + mocks.helpers.expects('isWriteable').never(); // SERVER_PACKAGE_ID mocks.adb.expects('getApplicationInstallState').once()