Fix: Node sometime didn't release devices after test (#917)

* fix: remove nested interval for node availability check * test: fix ios tests * test: release devices before each test ci: move ios integration test to its own folder as it didn't get picked up locally * fix: unblockDevice should not be limited to just one device * fix: don't use async with forEach * test: unblock all devices prior to ios testing * test: simulators and booted state feat: send unblock-device request when node getting unexpected shutdown. * chore: remove redundant logs when removing device
AppiumTestDistribution · Dec 12, 2023 · a8eb9f9 · a8eb9f9
1 parent 890058f
commit a8eb9f9
Show file tree

Hide file tree

Showing 16 changed files with 332 additions and 140 deletions.
diff --git a/package.json b/package.json
@@ -21,8 +21,9 @@
     "test-e2e-hubnode": "mocha -r ts-node/register ./test/e2e/hubnode.spec.* --plugin-device-farm-platform=both --exit --timeout=999999",
     "test-e2e-browserstack": "mocha --require ts-node/register ./test/e2e/browserstack.spec.ts --exit --timeout 999999",
     "test-e2e-pcloudy": "mocha --require ts-node/register ./test/e2e/pcloudy.spec.ts --exit --timeout 999999",
-    "integration-android": "mocha -r ts-node/register ./test/integration/androidDevices.spec.js --timeout 90000 --exit",
-    "integration-ios": "mocha -r ts-node/register ./test/integration/*iOS*.spec.js --timeout 260000 --exit",
+    "integration-android": "mocha -r ts-node/register ./test/integration/androidDevices.spec.{j,t}s --timeout 90000 --exit",
+    "integration-ios": "mocha -r ts-node/register ./test/integration/ios/*.spec.ts --timeout 260000 --exit",
+    "integration-ios2": "mocha -r ts-node/register ./test/integration/ios/02iOSDevices.spec.ts --timeout 260000 --exit",
     "build": "npx tsc -b && npm run copy-files",
     "copy-files": "cp -R src/public lib",
     "buildAndCopyWeb": "sh buildAndCopyWeb.sh",

diff --git a/src/app.ts b/src/app.ts
@@ -107,7 +107,7 @@ apiRouter.post('/block', (req, res) => {
   const requestBody = req.body;
 
   const device = getDevice(requestBody);
-  updateDevice(device, { busy: true, userBlocked: true });
+  if (device != undefined) updateDevice(device, { busy: true, userBlocked: true });
 
   res.json('200');
 });
@@ -116,7 +116,7 @@ apiRouter.post('/unblock', (req, res) => {
   const requestBody = req.body;
 
   const device = getDevice(requestBody);
-  updateDevice(device, { busy: false, userBlocked: false });
+  if (device != undefined) updateDevice(device, { busy: false, userBlocked: false });
 
   res.json('200');
 });

diff --git a/src/data-service/device-service.ts b/src/data-service/device-service.ts
@@ -6,14 +6,14 @@ import { setUtilizationTime } from '../device-utils';
 
 export function removeDevice(devices: { udid: string; host: string}[]) {
   devices.forEach(function (device) {
-    log.info(`Removing device ${device.udid} from host ${device.host} from list per node request.`);
+    log.info(`Removing device ${device.udid} from host ${device.host} from device list.`);
     DeviceModel.chain()
       .find({ udid: device.udid, host: { $contains: device.host } })
       .remove();
   })
 }
 
-export function addNewDevice(devices: Array<IDevice>) {
+export function addNewDevice(devices: IDevice[]) {
   /**
    * If the newly identified devices are not in the database, then add them to the database
    */
@@ -67,7 +67,12 @@ export function getAllDevices(): Array<IDevice> {
   return DeviceModel.chain().find().data();
 }
 
-export function getDevice(filterOptions: IDeviceFilterOptions): IDevice {
+/**
+ * Find devices matching filter options
+ * @param filterOptions IDeviceFilterOptions
+ * @returns IDevice[]
+ */
+export function getDevices(filterOptions: IDeviceFilterOptions): IDevice[] {
   const semver = require('semver');
   let results = DeviceModel.chain();
 
@@ -105,15 +110,22 @@ export function getDevice(filterOptions: IDeviceFilterOptions): IDevice {
 
   if (filterOptions.deviceType === 'simulator') {
     filter.state = 'Booted';
-    const results_copy = results.copy();
-    if (results_copy.find(filter).data()[0] != undefined) {
-      log.info('Picking up booted simulator');
-      return results.find(filter).data()[0];
-    } else {
-      filter.state = 'Shutdown';
-    }
   }
-  return results.find(filter).data()[0];
+  return results.find(filter).data();
+}
+
+/**
+ * Find device matching the filter options
+ * @param filterOptions IDeviceFilterOptions
+ * @returns IDevice | undefined
+ */
+export function getDevice(filterOptions: IDeviceFilterOptions): IDevice | undefined {
+  const devices = getDevices(filterOptions);
+  if (devices.length === 0) {
+    return undefined;
+  } else {
+    return devices[0];
+  }
 }
 
 export function updatedAllocatedDevice(device: IDevice, updateData: Partial<IDevice>) {
@@ -157,26 +169,33 @@ export function updateCmdExecutedTime(sessionId: string) {
 }
 
 export async function unblockDevice(filter: object) {
-  const device = DeviceModel.chain().find(filter).data()[0];
-  if (device !== undefined) {
+  const devices = DeviceModel.chain().find(filter).data();
+  if (devices !== undefined) {
     console.log(
-      `Found device with udid ${device.udid} to unblock with filter ${JSON.stringify(filter)}`,
+      `Found ${devices.length} devices to unblock with filter ${JSON.stringify(filter)}`,
     );
-    const sessionStart = device.sessionStartTime;
-    const currentTime = new Date().getTime();
-    const utilization = currentTime - sessionStart;
-    const totalUtilization = device.totalUtilizationTimeMilliSec + utilization;
-    await setUtilizationTime(device.udid, totalUtilization);
-    DeviceModel.chain()
-      .find(filter)
-      .update(function (device: IDevice) {
-        device.session_id = undefined;
-        device.busy = false;
-        device.lastCmdExecutedAt = undefined;
-        device.sessionStartTime = 0;
-        device.totalUtilizationTimeMilliSec = totalUtilization;
-        device.newCommandTimeout = undefined;
-      });
+
+    await Promise.all(devices.map(async (device) => {
+      const sessionStart = device.sessionStartTime;
+      const currentTime = new Date().getTime();
+      let utilization = currentTime - sessionStart;
+      // no session time recorded means device was never used
+      if (sessionStart === 0) {
+        utilization = 0;
+      }
+      const totalUtilization = device.totalUtilizationTimeMilliSec + utilization;
+      await setUtilizationTime(device.udid, totalUtilization);
+      DeviceModel.chain()
+        .find(filter)
+        .update(function (device: IDevice) {
+          device.session_id = undefined;
+          device.busy = false;
+          device.lastCmdExecutedAt = undefined;
+          device.sessionStartTime = 0;
+          device.totalUtilizationTimeMilliSec = totalUtilization;
+          device.newCommandTimeout = undefined;
+        });
+    }));
   } else {
     console.log(`Not able to find device to unblock with filter ${JSON.stringify(filter)}`);
   }

diff --git a/src/device-managers/AndroidDeviceManager.ts b/src/device-managers/AndroidDeviceManager.ts
@@ -1,21 +1,22 @@
-import { IDevice } from '../interfaces/IDevice';
 import { IDeviceManager } from '../interfaces/IDeviceManager';
-import { asyncForEach, getFreePort, isCloud, hasHubArgument } from '../helpers';
+import { asyncForEach, getFreePort } from '../helpers';
 import { ADB, getSdkRootFromEnv } from 'appium-adb';
 import log from '../logger';
 import _ from 'lodash';
 import { fs } from '@appium/support';
 import ChromeDriverManager from './ChromeDriverManager';
 import { Container } from 'typedi';
 import { getUtilizationTime } from '../device-utils';
-import Adb from '@devicefarmer/adbkit';
+import Adb, { DeviceWithPath } from '@devicefarmer/adbkit';
 import { AbortController } from 'node-abort-controller';
 import asyncWait from 'async-wait-until';
 import ip from 'ip';
 import NodeDevices from './NodeDevices';
 import { addNewDevice, removeDevice } from '../data-service/device-service';
 import Devices from './cloud/Devices';
 import { DeviceTypeToInclude, IPluginArgs } from '../interfaces/IPluginArgs';
+import { IDevice } from '../interfaces/IDevice';
+import { DeviceUpdate } from '../types/DeviceUpdate';
 
 export default class AndroidDeviceManager implements IDeviceManager {
   private adb: any;
@@ -254,36 +255,36 @@ export default class AndroidDeviceManager implements IDeviceManager {
     return deviceList;
   }
 
-  public async handleNewlyPluggedDevice(originalADB: any, device: any) {
-    const clonedDevice = _.cloneDeep(device);
-    Object.assign(clonedDevice, { udid: clonedDevice['id'], state: clonedDevice['type'] });
-    if (device.state != 'offline') {
-      log.info(`Device ${clonedDevice.udid} was plugged`);
-      this.initiateAbortControl(clonedDevice.udid);
+  public async onDeviceAdded(originalADB: any, device: DeviceWithPath) {
+    const newDevice = { udid: device.id, state: device.type };
+    log.info(`Device ${newDevice.udid} was plugged. Detail: ${JSON.stringify(newDevice)}`);
+    if (newDevice.state != 'offline') {
+      log.info(`Device ${newDevice.udid} was plugged`);
+      this.initiateAbortControl(newDevice.udid);
       let bootCompleted = false;
       try {
-        await this.waitBootComplete(originalADB, clonedDevice.udid);
+        await this.waitBootComplete(originalADB, newDevice.udid);
         bootCompleted = true;
       } catch (error) {
-        log.info(`Device ${clonedDevice.udid} boot did not complete. Error: ${error}`);
+        log.info(`Device ${newDevice.udid} boot did not complete. Error: ${error}`);
       }
 
       if (!bootCompleted) {
-        log.info(`Device ${clonedDevice.udid} boot did not complete in time. Ignoring`);
+        log.info(`Device ${newDevice.udid} boot did not complete in time. Ignoring`);
         return;
       }
 
-      this.cancelAbort(clonedDevice.udid);
-      const trackedDevice = await this.deviceInfo(clonedDevice, originalADB, this.pluginArgs, this.hostPort);
+      this.cancelAbort(newDevice.udid);
+      const trackedDevice = await this.deviceInfo(newDevice, originalADB, this.pluginArgs, this.hostPort);
 
       if (!trackedDevice) {
-        log.info(`Cannot get device info for ${clonedDevice.udid}. Skipping`);
+        log.info(`Cannot get device info for ${newDevice.udid}. Skipping`);
         return;
       }
 
-      log.info(`Adding device ${clonedDevice.udid} to list!`);
+      log.info(`Adding device ${newDevice.udid} to list!`);
       if (this.pluginArgs.hub != undefined) {
-        log.info(`Updating Hub with device ${clonedDevice.udid}`);
+        log.info(`Updating Hub with device ${newDevice.udid}`);
         const nodeDevices = new NodeDevices(this.pluginArgs.hub);
         await nodeDevices.postDevicesToHub([trackedDevice], 'add');
       } else {
@@ -297,47 +298,58 @@ export default class AndroidDeviceManager implements IDeviceManager {
     const adbTracker = async () => {
       try {
         const tracker = await adbClient.trackDevices();
-        tracker.on('add', async (device: any) => {
-          await this.handleNewlyPluggedDevice(originalADB, device);
+        tracker.on('add', async (device: DeviceWithPath) => {
+          this.onDeviceAdded(originalADB, device);
         });
-        tracker.on('remove', async (device: any) => {
-          const clonedDevice = _.cloneDeep(device);
-          Object.assign(clonedDevice, { udid: clonedDevice['id'], host: ip.address() });
-          if (pluginArgs.hub != undefined) {
-            log.info(`Removing device from Hub with device ${clonedDevice.udid}`);
-            const nodeDevices = new NodeDevices(pluginArgs.hub);
-            await nodeDevices.postDevicesToHub([clonedDevice], 'remove');
+        tracker.on('remove', async (device: DeviceWithPath) => {
+          await this.onDeviceRemoved(device, pluginArgs);
+        });
+        tracker.on('change', async (device: DeviceWithPath) => {
+          if (device.type === 'offline' || device.type === 'unauthorized') {
+            await this.onDeviceRemoved(device, pluginArgs);
           } else {
-            log.warn(`Removing device ${clonedDevice.udid} from list as the device was unplugged!`);
-            removeDevice([clonedDevice]);
-            this.abort(clonedDevice.udid);
+            this.onDeviceAdded(originalADB, device);
           }
-        });
-        tracker.on('end', () => console.log('Tracking stopped'));
+        })
+        tracker.on('end', () => log.info('Tracking stopped'));
       } catch (err: any) {
-        console.error('Something went wrong:', err.stack);
+        log.error('Something went wrong:', err.stack);
       }
     };
 
     return adbTracker;
   }
 
+  private async onDeviceRemoved(device: DeviceWithPath, pluginArgs: IPluginArgs) {
+    const clonedDevice: DeviceUpdate = { udid: device['id'], host: ip.address(), state: device.type };
+    if (pluginArgs.hub != undefined) {
+      const nodeDevices = new NodeDevices(pluginArgs.hub);
+      await nodeDevices.postDevicesToHub([clonedDevice], 'remove');
+    } else {
+      removeDevice([clonedDevice]);
+      this.abort(clonedDevice.udid);
+    }
+  }
+
   private createRemoteAdbTracker(adbClient: any, originalADB: any, adbHost: string) {
+    const pluginArgs = this.pluginArgs;
     const adbTracking = async () => {
       try {
         const tracker = await adbClient.trackDevices();
-        tracker.on('add', async (device: any) => {
-          await this.handleNewlyPluggedDevice(originalADB, device);
+        tracker.on('add', async (device: DeviceWithPath) => {
+          this.onDeviceAdded(originalADB, device);
         });
-        tracker.on('remove', async (device: any) => {
-          const clonedDevice = _.cloneDeep(device);
-          Object.assign(clonedDevice, { udid: clonedDevice['id'], host: adbHost });
-          log.warn(
-            `Removing device ${clonedDevice.udid} from ${adbHost} list as the device was unplugged!`,
-          );
-          removeDevice(clonedDevice);
-          this.abort(clonedDevice.udid);
+        tracker.on('remove', async (device: DeviceWithPath) => {
+          this.onDeviceRemoved(device, pluginArgs);
         });
+        tracker.on('change', async (device: DeviceWithPath) => {
+          if (device.type === 'offline' || device.type === 'unauthorized') {
+            log.info(`Device ${device.id} is ${device.type}. Removing from list`);
+            await this.onDeviceRemoved(device, pluginArgs);
+          } else {
+            this.onDeviceAdded(originalADB, device);
+          }
+        })
         tracker.on('end', () => console.log('Tracking stopped'));
       } catch (err: any) {
         console.error('Something went wrong:', err.stack);

diff --git a/src/device-managers/NodeDevices.ts b/src/device-managers/NodeDevices.ts
@@ -1,6 +1,8 @@
 import axios from 'axios';
 import log from '../logger';
-import { IDevice } from '../interfaces/IDevice';
+import { DeviceWithPath } from '@devicefarmer/adbkit';
+import { DeviceUpdate } from '../types/DeviceUpdate';
+import { IDeviceFilterOptions } from '../interfaces/IDeviceFilterOptions';
 
 export default class NodeDevices {
   private host: string;
@@ -9,7 +11,9 @@ export default class NodeDevices {
     this.host = host;
   }
 
-  async postDevicesToHub(devices: Array<IDevice>, arg: string) {
+  async postDevicesToHub(devices: DeviceWithPath[] | DeviceUpdate[], arg: string) {
+    // DeviceWithPath -> new device
+    // DeviceUpdate -> removed device
     log.info(`Updating remote android devices ${this.host}/device-farm/api/register`);
     try {
       const status = (
@@ -32,4 +36,28 @@ export default class NodeDevices {
       log.error(`Unable to push devices update to hub. Reason: ${error}`);
     }
   }
+
+  async unblockDevice(filter: IDeviceFilterOptions) {
+    log.info(`Unblocking device ${this.host}/device-farm/api/unblock`);
+    try {
+      const status = (
+        await axios.post(
+          `${this.host}/device-farm/api/unblock`,
+          filter,
+          {
+            params: {
+              type: 'unblock',
+            },
+          },
+        )
+      ).status;
+      if (status === 200) {
+        log.info(`Unblocked device with filter: ${filter}`);
+      } else {
+        log.warn('Something went wrong!!');
+      }
+    } catch (error) {
+      log.error(`Unable to unblock device. Reason: ${error}`);
+    }
+  }
 }