feat: Optimize server packages installation (#631)

.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:

lib/uiautomator2.js

@@ -60,97 +60,97 @@ 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.
  *
  * @param {number} installTimeout - Installation timeout
  */
  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);

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

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()