Repository: wix/Detox
Pull request #2546 (2020-12-21 21:29:14)

Make usage of new gmsaas getInstance 'endpoint' of 1.6.0
## Description

In `gmsaas` version `1.6.0`, as per Wix' request, a reach onto the `getInstance` endpoint via `gmsaas` was introduced through the `gmsaas instances get <uuid>` command. This PR plugs that in into the driver by reimplementing lookup-service' `getInstance()`, which previously fetched _all_ instances and manually filtered them (which wasn't scalable - hence the point in all this).

This comes alongside a preliminary check in the driver's `prepare()` of that the `gmsaas` executable is sufficiently new on the host machine.

d4vidi: Note: Initially, this PR should fail because gmsaas on the agents isn't up-to-date (1.5.x). This is a good test of that this works. Following a failure, I will upgrade the agents as well.

Patch:
diff --git a/detox/src/devices/drivers/android/genycloud/GenyCloudDriver.js b/detox/src/devices/drivers/android/genycloud/GenyCloudDriver.js
index 3177b07ee..58b59bed0 100644
--- a/detox/src/devices/drivers/android/genycloud/GenyCloudDriver.js
+++ b/detox/src/devices/drivers/android/genycloud/GenyCloudDriver.js
@@ -1,3 +1,4 @@
+const semver = require('semver'); // eslint-disable-line node/no-extraneous-require
 const AndroidDriver = require('../AndroidDriver');
 const GenyCloudDeviceAllocator = require('./GenyCloudDeviceAllocator');
 const GenyDeviceRegistryFactory = require('./GenyDeviceRegistryFactory');
@@ -12,23 +13,25 @@ const DetoxRuntimeError = require('../../../../errors/DetoxRuntimeError');
 const logger = require('../../../../utils/logger').child({ __filename });
 const environment = require('../../../../utils/environment');
 
+const MIN_GMSAAS_VERSION = '1.6.0';
+
 class GenyCloudDriver extends AndroidDriver {
   constructor(config) {
     super(config);
     this._name = 'Unspecified Genymotion Cloud Emulator';
 
-    const exec = new GenyCloudExec(environment.getGmsaasPath());
+    this._exec = new GenyCloudExec(environment.getGmsaasPath());
     const instanceNaming = new InstanceNaming(); // TODO should consider a permissive impl for debug/dev mode. Maybe even a custom arg in package.json (Detox > ... > genycloud > sharedAccount: false)
     this._deviceRegistry = GenyDeviceRegistryFactory.forRuntime();
     this._deviceCleanupRegistry = GenyDeviceRegistryFactory.forGlobalShutdown();
 
-    const recipeService = new RecipesService(exec, logger);
-    const instanceLookupService = new InstanceLookupService(exec, instanceNaming, this._deviceRegistry);
-    this._instanceLifecycleService = new InstanceLifecycleService(exec, instanceNaming);
+    const recipeService = new RecipesService(this._exec, logger);
+    const instanceLookupService = new InstanceLookupService(this._exec, instanceNaming, this._deviceRegistry);
+    this._instanceLifecycleService = new InstanceLifecycleService(this._exec, instanceNaming);
     this._deviceQueryHelper = new DeviceQueryHelper(recipeService);
     this._deviceAllocator = new GenyCloudDeviceAllocator(this._deviceRegistry, this._deviceCleanupRegistry, instanceLookupService, this._instanceLifecycleService);
 
-    this._authService = new AuthService(exec);
+    this._authService = new AuthService(this._exec);
   }
 
   get name() {
@@ -36,7 +39,8 @@ class GenyCloudDriver extends AndroidDriver {
   }
 
   async prepare() {
-    return this._validateGmsaasAuth();
+    await this._validateGmsaasVersion();
+    await this._validateGmsaasAuth();
   }
 
   async acquireFreeDevice(deviceQuery) {
@@ -83,10 +87,20 @@ class GenyCloudDriver extends AndroidDriver {
     }
   }
 
+  async _validateGmsaasVersion() {
+    const { version } = await this._exec.getVersion();
+    if (semver.lt(version, MIN_GMSAAS_VERSION)) {
+      throw new DetoxRuntimeError({
+        message: `Your Genymotion-Cloud executable (found in ${environment.getGmsaasPath()}) is too old! (version ${version})`,
+        hint: `Detox requires version 1.6.0, or newer. To use 'android.genycloud' type devices, you must upgrade it, first.`,
+      });
+    }
+  }
+
   async _validateGmsaasAuth() {
     if (!await this._authService.getLoginEmail()) {
       throw new DetoxRuntimeError({
-        message: 'Cannot run tests using a Genymotion-cloud emulator, because Genymotion was not logged-in to!',
+        message: `Cannot run tests using 'android.genycloud' type devices, because Genymotion was not logged-in to!`,
         hint: `Log-in to Genymotion-cloud by running this command (and following instructions):\n${environment.getGmsaasPath()} auth login --help`,
       });
     }
@@ -94,11 +108,11 @@ class GenyCloudDriver extends AndroidDriver {
 
   static async globalCleanup() {
     const deviceCleanupRegistry = GenyDeviceRegistryFactory.forGlobalShutdown();
-    const deviceHandles = await deviceCleanupRegistry.readRegisteredDevices();
-    if (deviceHandles.length) {
+    const instanceHandles = await deviceCleanupRegistry.readRegisteredDevices();
+    if (instanceHandles.length) {
       const exec = new GenyCloudExec(environment.getGmsaasPath());
       const instanceLifecycleService = new InstanceLifecycleService(exec, null);
-      await doCleanup(instanceLifecycleService, deviceHandles);
+      await doCleanup(instanceLifecycleService, instanceHandles);
     }
   }
 }
@@ -107,11 +121,11 @@ const cleanupLogData = {
   event: 'GENYCLOUD_TEARDOWN',
 };
 
-async function doCleanup(instanceLifecycleService, deviceHandles) {
+async function doCleanup(instanceLifecycleService, instanceHandles) {
   logger.info(cleanupLogData, 'Initiating Genymotion cloud instances teardown...');
 
   const deletionLeaks = [];
-  const killPromises = deviceHandles.map(({ uuid, name }) =>
+  const killPromises = instanceHandles.map(({ uuid, name }) =>
     instanceLifecycleService.deleteInstance(uuid)
       .catch((error) => deletionLeaks.push({ uuid, name, error })));
 
diff --git a/detox/src/devices/drivers/android/genycloud/exec/GenyCloudExec.js b/detox/src/devices/drivers/android/genycloud/exec/GenyCloudExec.js
index 84a8cd2e0..4a164f0aa 100644
--- a/detox/src/devices/drivers/android/genycloud/exec/GenyCloudExec.js
+++ b/detox/src/devices/drivers/android/genycloud/exec/GenyCloudExec.js
@@ -5,16 +5,24 @@ class GenyCloudExec {
     this.binaryExec = `"${binaryPath}" --format compactjson`;
   }
 
+  getVersion() {
+    return this._exec('--version');
+  }
+
   whoAmI() {
-    return this._exec(`auth whoami`);
+    return this._exec('auth whoami');
   }
 
   getRecipe(name) {
     return this._exec(`recipes list --name "${name}"`);
   }
 
+  getInstance(instanceUUID) {
+    return this._exec(`instances get ${instanceUUID}`);
+  }
+
   getInstances() {
-    return this._exec(`instances list -q`);
+    return this._exec('instances list -q');
   }
 
   startInstance(recipeUUID, instanceName) {
diff --git a/detox/src/devices/drivers/android/genycloud/services/GenyInstanceLookupService.js b/detox/src/devices/drivers/android/genycloud/services/GenyInstanceLookupService.js
index 4d2816035..e4158b0b1 100644
--- a/detox/src/devices/drivers/android/genycloud/services/GenyInstanceLookupService.js
+++ b/detox/src/devices/drivers/android/genycloud/services/GenyInstanceLookupService.js
@@ -1,4 +1,3 @@
-const _ = require('lodash');
 const Instance = require('./dto/GenyInstance');
 
 class GenyInstanceLookupService {
@@ -14,8 +13,8 @@ class GenyInstanceLookupService {
   }
 
   async getInstance(instanceUUID) {
-    const instances = await this._getAllInstances();
-    return _.find(instances, (instance) => instance.uuid === instanceUUID);
+    const { instance } = await this.genyCloudExec.getInstance(instanceUUID);
+    return new Instance(instance);
   }
 
   async _getRelevantInstances() {


Test patch:
diff --git a/detox/src/devices/drivers/android/genycloud/GenyCloudDriver.test.js b/detox/src/devices/drivers/android/genycloud/GenyCloudDriver.test.js
index 170e15852..983d5b594 100644
--- a/detox/src/devices/drivers/android/genycloud/GenyCloudDriver.test.js
+++ b/detox/src/devices/drivers/android/genycloud/GenyCloudDriver.test.js
@@ -2,6 +2,24 @@ const _ = require('lodash');
 
 const latestInstanceOf = (clazz) => _.last(clazz.mock.instances);
 
+const MOCK_GMSAAS_PATH = '/path/to/gmsaas';
+
+const aRecipe = () => ({
+  uuid: 'mock-recipe-uuid',
+  name: 'mock-recipe-name',
+  toString: () => 'mock-recipe-toString()-result',
+});
+
+const anInstance = () => ({
+  uuid: 'mock-instance-uuid',
+  name: 'mock-instance-name',
+  adbName: 'mock-instance-adb-name',
+});
+
+const aDeviceQuery = () => ({
+  query: 'mock',
+});
+
 describe('Genymotion-cloud driver', () => {
   class MockInstanceLifecycleService {
     constructor(...args) {
@@ -10,24 +28,6 @@ describe('Genymotion-cloud driver', () => {
     }
   }
 
-  const MOCK_GMSAAS_PATH = '/path/to/gmsaas';
-
-  const aRecipe = () => ({
-    uuid: 'mock-recipe-uuid',
-    name: 'mock-recipe-name',
-    toString: () => 'mock-recipe-toString()-result',
-  });
-
-  const anInstance = () => ({
-    uuid: 'mock-instance-uuid',
-    name: 'mock-instance-name',
-    adbName: 'mock-instance-adb-name',
-  });
-
-  const aDeviceQuery = () => ({
-    query: 'mock',
-  });
-
   beforeEach(mockBaseClassesDependencies);
   beforeEach(mockDirectDependencies);
 
@@ -37,6 +37,7 @@ describe('Genymotion-cloud driver', () => {
   let environment;
   let adbObj;
   let Exec;
+  let execObj;
   let deviceQueryHelper;
   let deviceRegistry;
   let deviceCleanupRegistry;
@@ -58,6 +59,7 @@ describe('Genymotion-cloud driver', () => {
     adbObj = () => latestInstanceOf(ADB);
 
     Exec = require('./exec/GenyCloudExec');
+    execObj = () => latestInstanceOf(Exec);
 
     const GenyDeviceRegistryFactory = require('./GenyDeviceRegistryFactory');
     const DeviceRegistry = jest.genMockFromModule('../../../DeviceRegistry');
@@ -109,21 +111,67 @@ describe('Genymotion-cloud driver', () => {
     });
 
     describe('preparation', () => {
-      it('should throw error if not logged-in to gmsaas', async () => {
-        authServiceObj().getLoginEmail.mockResolvedValue(null);
+      const givenProperGmsaasLogin = () => authServiceObj().getLoginEmail.mockResolvedValue('detox@wix.com');
+      const givenGmsaasLoggedOut = () => authServiceObj().getLoginEmail.mockResolvedValue(null);
+      const givenGmsaasExecVersion = (version) => execObj().getVersion.mockResolvedValue({ version });
+      const givenProperGmsaasExecVersion = () => givenGmsaasExecVersion('1.6.0');
+
+      it('should throw an error if gmsaas exec is too old (minor version < 6)', async () => {
+        givenProperGmsaasLogin();
+        givenGmsaasExecVersion('1.5.9');
 
         try {
           await uut.prepare();
           fail('Expected an error');
         } catch (e) {
           expect(e.constructor.name).toEqual('DetoxRuntimeError');
-          expect(e.toString()).toContain('Cannot run tests using a Genymotion-cloud emulator, because Genymotion was not logged-in to!');
+          expect(e.toString()).toContain(`Your Genymotion-Cloud executable (found in ${MOCK_GMSAAS_PATH}) is too old! (version 1.5.9)`);
+          expect(e.toString()).toContain(`HINT: Detox requires version 1.6.0, or newer. To use 'android.genycloud' type devices, you must upgrade it, first.`);
+        }
+      });
+
+      it('should accept the gmsaas exec if version is sufficiently new', async () => {
+        givenProperGmsaasLogin();
+        givenGmsaasExecVersion('1.6.0');
+        await uut.prepare();
+      });
+
+      it('should accept the gmsaas exec if version is more than sufficiently new', async () => {
+        givenProperGmsaasLogin();
+        givenGmsaasExecVersion('1.7.2');
+        await uut.prepare();
+      });
+
+      it('should throw an error if gmsaas exec is too old (major version < 1)', async () => {
+        givenProperGmsaasLogin();
+        givenGmsaasExecVersion('0.6.0');
+
+        try {
+          await uut.prepare();
+          fail('Expected an error');
+        } catch (e) {
+          expect(e.toString()).toContain(`Your Genymotion-Cloud executable (found in ${MOCK_GMSAAS_PATH}) is too old! (version 0.6.0)`);
+        }
+      });
+
+      it('should throw an error if not logged-in to gmsaas', async () => {
+        givenProperGmsaasExecVersion();
+        givenGmsaasLoggedOut();
+
+        try {
+          await uut.prepare();
+          fail('Expected an error');
+        } catch (e) {
+          expect(e.constructor.name).toEqual('DetoxRuntimeError');
+          expect(e.toString()).toContain(`Cannot run tests using 'android.genycloud' type devices, because Genymotion was not logged-in to!`);
           expect(e.toString()).toContain(`HINT: Log-in to Genymotion-cloud by running this command (and following instructions):\n${MOCK_GMSAAS_PATH} auth login --help`);
         }
       });
 
       it('should not throw an error if properly logged in to gmsaas', async () => {
-        authServiceObj().getLoginEmail.mockResolvedValue('detox@wix.com');
+        givenProperGmsaasExecVersion();
+        givenProperGmsaasLogin();
+
         await uut.prepare();
       });
     });
diff --git a/detox/src/devices/drivers/android/genycloud/exec/GenyCloudExec.test.js b/detox/src/devices/drivers/android/genycloud/exec/GenyCloudExec.test.js
index 41d5419f4..ddb1f5c7d 100644
--- a/detox/src/devices/drivers/android/genycloud/exec/GenyCloudExec.test.js
+++ b/detox/src/devices/drivers/android/genycloud/exec/GenyCloudExec.test.js
@@ -1,18 +1,23 @@
-describe('Genymotion-cloud executable', () => {
-  const aResponse = (exit_code = 0, exit_code_desc = 'NO_ERROR') => ({
-    exit_code,
-    exit_code_desc,
-  });
-  const anErrorResponse = (exit_code, exit_code_desc, error_desc) => ({
-    ...aResponse(exit_code, exit_code_desc),
-    error: {
-      message: `API return unexpected code: ${exit_code}. Error: {"code":"${error_desc}","message":"Oh no, mocked error has occurred!"}`,
-      details: '',
-    }
-  });
 
+const aResponse = (exit_code = 0, exit_code_desc = 'NO_ERROR') => ({
+  exit_code,
+  exit_code_desc,
+});
+const anErrorResponse = (exit_code, exit_code_desc, error_desc) => ({
+  ...aResponse(exit_code, exit_code_desc),
+  error: {
+    message: `API return unexpected code: ${exit_code}. Error: {"code":"${error_desc}","message":"Oh no, mocked error has occurred!"}`,
+    details: '',
+  }
+});
+
+describe('Genymotion-cloud executable', () => {
   const successResponse = aResponse();
   const failResponse = anErrorResponse(4, 'API_ERROR', 'TOO_MANY_RUNNING_VDS');
+  const recipeName = 'mock-recipe-name';
+  const recipeUUID = 'mock-recipe-uuid';
+  const instanceUUID = 'mock-uuid';
+  const instanceName = 'detox-instance1';
 
   const givenSuccessResult = () => exec.mockResolvedValue({
     stdout: JSON.stringify(successResponse),
@@ -33,12 +38,12 @@ describe('Genymotion-cloud executable', () => {
     uut = new GenyCloudExec('mock/path/to/gmsaas');
   });
 
-  const recipeName = 'mock-recipe-name';
-  const recipeUUID = 'mock-recipe-uuid';
-  const instanceUUID = 'mock-uuid';
-  const instanceName = 'detox-instance1';
-
   [
+    {
+      commandName: 'version',
+      commandExecFn: () => uut.getVersion(),
+      expectedExec: `"mock/path/to/gmsaas" --format compactjson --version`,
+    },
     {
       commandName: 'whoami',
       commandExecFn: () => uut.whoAmI(),
@@ -49,6 +54,11 @@ describe('Genymotion-cloud executable', () => {
       commandExecFn: () => uut.getRecipe(recipeName),
       expectedExec: `"mock/path/to/gmsaas" --format compactjson recipes list --name "${recipeName}"`,
     },
+    {
+      commandName: 'Get Instance',
+      commandExecFn: () => uut.getInstance(instanceUUID),
+      expectedExec: `"mock/path/to/gmsaas" --format compactjson instances get ${instanceUUID}`,
+    },
     {
       commandName: 'Get Instances',
       commandExecFn: () => uut.getInstances(),
diff --git a/detox/src/devices/drivers/android/genycloud/services/GenyInstanceLookupService.test.js b/detox/src/devices/drivers/android/genycloud/services/GenyInstanceLookupService.test.js
index ef14904ac..d6e053e0a 100644
--- a/detox/src/devices/drivers/android/genycloud/services/GenyInstanceLookupService.test.js
+++ b/detox/src/devices/drivers/android/genycloud/services/GenyInstanceLookupService.test.js
@@ -43,6 +43,7 @@ describe('Genymotion-Cloud instances lookup service', () => {
   const givenNoRegisteredInstances = () => deviceRegistry.getRegisteredDevices.mockReturnValue([]);
   const givenInstances = (...instances) => exec.getInstances.mockResolvedValue({ instances });
   const givenNoInstances = () => exec.getInstances.mockResolvedValue({ instances: [] });
+  const givenAnInstance = (instance) => exec.getInstance.mockResolvedValue({ instance });
   const givenAllDevicesFamilial = () => instanceNaming.isFamilial.mockReturnValue(true);
   const givenNoDevicesFamilial = () => instanceNaming.isFamilial.mockReturnValue(false);
 
@@ -123,12 +124,13 @@ describe('Genymotion-Cloud instances lookup service', () => {
 
   describe('finding a specific instance', () => {
     it('should return an instance matching a UUID', async () => {
-      const instance1 = anInstance();
-      const instance2 = anotherInstance();
-      givenInstances(instance1, instance2);
+      const instance = anInstance();
+      givenAnInstance(instance);
 
-      const result = await uut.getInstance(instance2.uuid);
-      expect(result.uuid).toEqual(instance2.uuid);
+      const result = await uut.getInstance(instance.uuid);
+      expect(result.uuid).toEqual(instance.uuid);
+      expect(result.constructor.name).toContain('Instance');
+      expect(exec.getInstance).toHaveBeenCalledWith(instance.uuid);
     });
   });
 });