Repository: wix/Detox (js)

Issue:
Lose the 'parallelism-is-experimental on Android' warning in detox logs
**Is your feature request related to a problem? Please describe.**
Can get rid of that warning now:
```
WARN:  [test.js] Multiple workers is an experimental feature on Android and requires an emulator binary of version 28.0.16 or higher. Check your version by running: $ANDROID_HOME/tools/bin/sdkmanager --list
```

**Describe the solution you'd like**
Need to:
1. Remove the warning.
2. Check emulator version on a best-effort basis, see if fits; Restrict warnings to when emulator version is either old or unknown.

**Describe alternatives you've considered**
N/A

**Additional context**
N/A


Pull request description:
Remove warning regarding Android parallel being experimental, add robust version check
- [x] This is a small change 
- [x] This change has been discussed in issue #2297 and the solution has been agreed upon with maintainers.

---

**Description:**
Resolves #2297 

Patch:
diff --git a/detox/src/devices/drivers/android/AndroidDriver.js b/detox/src/devices/drivers/android/AndroidDriver.js
index 4d067329e..bf9ea0a09 100644
--- a/detox/src/devices/drivers/android/AndroidDriver.js
+++ b/detox/src/devices/drivers/android/AndroidDriver.js
@@ -25,7 +25,6 @@ const sleep = require('../../../utils/sleep');
 const retry = require('../../../utils/retry');
 const getAbsoluteBinaryPath = require('../../../utils/getAbsoluteBinaryPath');
 const DeviceRegistry = require('../../DeviceRegistry');
-const environment = require('../../../utils/environment');
 
 const ALLOCATE_DEVICE_LOG_EVT = 'ALLOCATE_DEVICE';
 
diff --git a/detox/src/devices/drivers/android/emulator/AVDValidator.js b/detox/src/devices/drivers/android/emulator/AVDValidator.js
index dd6c89592..260f20bda 100644
--- a/detox/src/devices/drivers/android/emulator/AVDValidator.js
+++ b/detox/src/devices/drivers/android/emulator/AVDValidator.js
@@ -1,28 +1,55 @@
 const _ = require('lodash');
-const path = require('path');
-const AVDsResolver = require('./AVDsResolver');
 const environment = require('../../../../utils/environment');
+const DetoxRuntimeError = require('../../../../errors/DetoxRuntimeError');
+const logger = require('../../../../utils/logger').child({ __filename });
+
+const REQUIRED_EMULATOR_MAJOR = 29;
 
 class AVDValidator {
-  constructor(emulatorExec) {
-    this._avdsResolver = new AVDsResolver(emulatorExec);
+  constructor(avdsResolver, emulatorVersionResolver) {
+    this._avdsResolver = avdsResolver;
+    this._emulatorVersionResolver = emulatorVersionResolver;
   }
 
   async validate(avdName) {
     const avds = await this._avdsResolver.resolve(avdName);
-    if (!avds) {
-      const avdmanagerPath = path.join(environment.getAndroidSDKPath(), 'tools', 'bin', 'avdmanager');
+    this._assertAVDs(avds);
+    await this._assertAVDMatch(avds, avdName);
+    await this._validateEmulatorVer();
+  }
 
-      throw new Error(`Could not find any configured Android Emulator.\n
-        Try creating a device first, example: ${avdmanagerPath} create avd --force --name Pixel_API_28 --abi x86_64 --package "system-images;android-28;default;x86_64" --device "pixel"
-        or go to https://developer.android.com/studio/run/managing-avds.html for details on how to create an Emulator.`);
+  _assertAVDs(avds) {
+    if (!avds) {
+      const usageExample = `${environment.getAvdManagerPath()} create avd --force --name Pixel_API_28 --abi x86_64 --package "system-images;android-28;default;x86_64" --device "pixel"`;
+      const message = 'Could not find any configured Android Emulator.';
+      const hint = `Try creating a device first (example: ${usageExample}),`
+        + ' or go to https://developer.android.com/studio/run/managing-avds.html for details on how to create an Emulator.';
+      throw new DetoxRuntimeError({ message, hint });
     }
+  }
 
+  _assertAVDMatch(avds, avdName) {
     if (_.indexOf(avds, avdName) === -1) {
-      throw new Error(`Can not boot Android Emulator with the name: '${avdName}',
-        make sure you choose one of the available emulators: ${avds.toString()}`);
+      const message = `Cannot boot Android Emulator with the name: '${avdName}'`;
+      const hint = `Make sure you choose one of the available emulators: ${avds.toString()}`;
+      throw new DetoxRuntimeError({ message, hint });
+    }
+  }
+
+  async _validateEmulatorVer() {
+    const emulatorVersion = await this._emulatorVersionResolver.resolve();
+    if (!emulatorVersion) {
+      logger.warn({ event: 'AVD_VALIDATION' }, 'Emulator version detection failed (See previous logs)');
+      return;
+    }
+
+    if (emulatorVersion.major < REQUIRED_EMULATOR_MAJOR) {
+      logger.warn({ event: 'AVD_VALIDATION' }, [
+          `Your installed emulator binary version (${emulatorVersion}) is too old, and may not be suitable for parallel test execution.`,
+          `We strongly recommend you upgrade to the latest version using the SDK manager: ${environment.getAndroidSdkManagerPath()} --list`
+        ].join('\n'));
     }
   }
 }
 
- module.exports = AVDValidator;
+module.exports = AVDValidator;
diff --git a/detox/src/devices/drivers/android/emulator/EmulatorDriver.js b/detox/src/devices/drivers/android/emulator/EmulatorDriver.js
index 84b50f198..47598b96b 100644
--- a/detox/src/devices/drivers/android/emulator/EmulatorDriver.js
+++ b/detox/src/devices/drivers/android/emulator/EmulatorDriver.js
@@ -5,6 +5,7 @@ const ini = require('ini');
 const AndroidDriver = require('../AndroidDriver');
 const FreeEmulatorFinder = require('./FreeEmulatorFinder');
 const AVDValidator = require('./AVDValidator');
+const AVDsResolver = require('./AVDsResolver');
 const EmulatorLauncher = require('./EmulatorLauncher');
 const EmulatorVersionResolver = require('./EmulatorVersionResolver');
 const { EmulatorExec } = require('../exec/EmulatorExec');
@@ -26,11 +27,13 @@ class EmulatorDriver extends AndroidDriver {
   constructor(config) {
     super(config);
 
-    this.freeDeviceFinder = new FreeEmulatorFinder(this.adb, this.deviceRegistry);
     const emulatorExec = new EmulatorExec();
     this._emuVersionResolver = new EmulatorVersionResolver(emulatorExec);
     this._emuLauncher = new EmulatorLauncher(emulatorExec);
-    this._avdValidator = new AVDValidator(emulatorExec);
+
+    const avdsResolver = new AVDsResolver(emulatorExec);
+    this._avdValidator = new AVDValidator(avdsResolver, this._emuVersionResolver);
+    this._freeDeviceFinder = new FreeEmulatorFinder(this.adb, this.deviceRegistry);
 
     this.pendingBoots = {};
     this._name = 'Unspecified Emulator';
@@ -59,7 +62,7 @@ class EmulatorDriver extends AndroidDriver {
   }
 
   async doAllocateDevice(deviceQuery) {
-    const freeEmulatorAdbName = await this.freeDeviceFinder.findFreeDevice(deviceQuery);
+    const freeEmulatorAdbName = await this._freeDeviceFinder.findFreeDevice(deviceQuery);
     return freeEmulatorAdbName || this._createDevice();
   }
 
diff --git a/detox/src/devices/drivers/android/emulator/EmulatorVersionResolver.js b/detox/src/devices/drivers/android/emulator/EmulatorVersionResolver.js
index d12301838..bd376c13d 100644
--- a/detox/src/devices/drivers/android/emulator/EmulatorVersionResolver.js
+++ b/detox/src/devices/drivers/android/emulator/EmulatorVersionResolver.js
@@ -42,6 +42,7 @@ class EmulatorVersionResolver {
       major: Number(major),
       minor: Number(minor),
       patch: Number(patch),
+      toString: () => versionRaw,
     };
   }
 }
diff --git a/detox/src/utils/environment.js b/detox/src/utils/environment.js
index 86a95668e..3122a7e1f 100644
--- a/detox/src/utils/environment.js
+++ b/detox/src/utils/environment.js
@@ -49,6 +49,14 @@ function getAvdDir(avdName) {
   return avdIni.path;
 }
 
+function getAvdManagerPath() {
+  return path.join(getAndroidSDKPath(), 'tools', 'bin', 'avdmanager');
+}
+
+function getAndroidSdkManagerPath() {
+  return path.join(getAndroidSDKPath(), 'tools', 'bin', 'sdkmanager');
+}
+
 function getAndroidEmulatorPath() {
   const sdkRoot = getAndroidSDKPath();
   if (!sdkRoot) {
@@ -171,6 +179,8 @@ module.exports = {
   getAdbPath,
   getAvdHome,
   getAvdDir,
+  getAvdManagerPath,
+  getAndroidSdkManagerPath,
   getDetoxVersion,
   getFrameworkPath,
   getAndroidSDKPath,


Test patch:
diff --git a/detox/local-cli/test.js b/detox/local-cli/test.js
index d92f05eef..3de904db6 100644
--- a/detox/local-cli/test.js
+++ b/detox/local-cli/test.js
@@ -67,11 +67,6 @@ function choosePrepareArgs({ cliConfig, detoxArgs, runner, platform }) {
   }
 
   if (runner === 'jest') {
-    if (platform === 'android' && hasMultipleWorkers(cliConfig)) {
-      log.warn('Multiple workers is an experimental feature on Android and requires an emulator binary of version 28.0.16 or higher. ' +
-        'Check your version by running: $ANDROID_HOME/tools/bin/sdkmanager --list');
-    }
-
     return prepareJestArgs;
   }
 
diff --git a/detox/local-cli/test.test.js b/detox/local-cli/test.test.js
index 3fee279a8..67dd3c756 100644
--- a/detox/local-cli/test.test.js
+++ b/detox/local-cli/test.test.js
@@ -415,12 +415,6 @@ describe('CLI', () => {
       expect(cliCall().env).not.toHaveProperty('readOnlyEmu');
     });
 
-    test.each([['-w'], ['--workers']])('%s <value> should warn about experimental Android support', async (__workers) => {
-      singleConfig().type = 'android.emulator';
-      await run(`${__workers} 2`);
-      expect(logger.warn).toHaveBeenCalledWith(expect.stringContaining('an experimental feature'));
-    });
-
     test.each([['-w'], ['--workers']])('%s <value> should put readOnlyEmu environment variable for Android if there is a single worker', async (__workers) => {
       singleConfig().type = 'android.emulator';
       await run(`${__workers} 1`);
diff --git a/detox/src/devices/drivers/android/emulator/AVDValidator.test.js b/detox/src/devices/drivers/android/emulator/AVDValidator.test.js
index ac6a33479..2f60f6d01 100644
--- a/detox/src/devices/drivers/android/emulator/AVDValidator.test.js
+++ b/detox/src/devices/drivers/android/emulator/AVDValidator.test.js
@@ -1,47 +1,114 @@
 describe('AVD validator', () => {
-  const emulatorExec = {};
-
-  let AVDsResolverClass;
+  let logger;
+  let environment;
+  let avdsResolver;
+  let versionResolver;
   let uut;
   beforeEach(() => {
-    jest.mock('./AVDsResolver');
-    AVDsResolverClass = require('./AVDsResolver');
+    jest.mock('../../../../utils/logger');
+    logger = require('../../../../utils/logger');
 
-    const AVDValidator = require('./AVDValidator');
-    uut = new AVDValidator(emulatorExec);
-  });
+    jest.mock('../../../../utils/environment');
+    environment = require('../../../../utils/environment');
 
-  const avdsResolverObj = () => AVDsResolverClass.mock.instances[0];
+    const AVDsResolver = jest.genMockFromModule('./AVDsResolver');
+    avdsResolver = new AVDsResolver();
 
-  it('should use an AVDs resolver', async () => {
-    avdsResolverObj().resolve.mockResolvedValue(['mock-avd-name']);
+    const EmulatorVersionResolver = jest.genMockFromModule('./EmulatorVersionResolver');
+    versionResolver = new EmulatorVersionResolver();
+    versionResolver.resolve.mockResolvedValue('');
 
-    await uut.validate('mock-avd-name');
+    const AVDValidator = require('./AVDValidator');
+    uut = new AVDValidator(avdsResolver, versionResolver);
+  });
 
-    expect(AVDsResolverClass).toHaveBeenCalledWith(emulatorExec);
-    expect(AVDsResolverClass.mock.instances[0].resolve).toHaveBeenCalledWith('mock-avd-name');
+  const givenExpectedAVD = () => avdsResolver.resolve.mockResolvedValue(['mock-avd-name']);
+  const givenNoAVDs = () => avdsResolver.resolve.mockResolvedValue(undefined);
+  const givenOtherAVDs = () => avdsResolver.resolve.mockResolvedValue(['other-avd', 'yet-another']);
+
+  const givenOldEmulatorVersion = () => versionResolver.resolve.mockResolvedValue({
+    major: 28,
+    minor: 999,
+    patch: 999,
+    toString: () => '28.mock.ver',
   });
+  const givenProperEmulatorVersion = () => versionResolver.resolve.mockResolvedValue({
+    major: 29,
+    minor: 0,
+    patch: 0,
+    toString: () => '29.x.y',
+  })
+  const givenUnknownEmulatorVersion = () => versionResolver.resolve.mockResolvedValue(null);
+
+  const givenMockedAvdManagerPath = () => environment.getAvdManagerPath.mockReturnValue('mock/path/avdmng');
+  const givenMockedAndroidSdkPath = () => environment.getAndroidSdkManagerPath.mockReturnValue('mock/path/sdkmng');
 
   it('should return safely if AVD exists', async () => {
-    avdsResolverObj().resolve.mockResolvedValue(['mock-avd-name']);
+    givenExpectedAVD();
     await uut.validate('mock-avd-name');
   });
 
   it('should throw if no AVDs found', async () => {
-    avdsResolverObj().resolve.mockResolvedValue(undefined);
+    givenMockedAvdManagerPath();
+    givenNoAVDs();
 
     try {
       await uut.validate();
       fail('expected to throw');
-    } catch (err) {}
+    } catch (err) {
+      const DetoxRuntimeError = require('../../../../errors/DetoxRuntimeError');
+      expect(err).toBeInstanceOf(DetoxRuntimeError);
+      expect(err.toString()).toEqual(expect.stringContaining('Could not find any configured Android Emulator.'));
+      expect(err.toString()).toEqual(expect.stringContaining('mock/path/avdmng create avd'));
+    }
   });
 
   it('should throw if specific AVD not found', async () => {
-    avdsResolverObj().resolve.mockResolvedValue(['other-avd', 'yet-another']);
+    givenOtherAVDs();
 
     try {
-      await uut.validate();
+      await uut.validate('mock-avd-name');
       fail('expected to throw');
-    } catch (err) {}
+    } catch (err) {
+      const DetoxRuntimeError = require('../../../../errors/DetoxRuntimeError');
+      expect(err).toBeInstanceOf(DetoxRuntimeError);
+      expect(err.toString()).toEqual(expect.stringContaining(`Cannot boot Android Emulator with the name: 'mock-avd-name'`));
+      expect(err.toString()).toEqual(expect.stringContaining(`Make sure you choose one of the available emulators`));
+    }
+  });
+
+  it('should warn about emulators that are too old', async () => {
+    givenMockedAndroidSdkPath();
+    givenExpectedAVD();
+    givenOldEmulatorVersion();
+
+    await uut.validate('mock-avd-name');
+
+    expect(logger.warn).toHaveBeenCalledWith({ event: 'AVD_VALIDATION' }, [
+      `Your installed emulator binary version (28.mock.ver) is too old, and may not be suitable for parallel test execution.`,
+      'We strongly recommend you upgrade to the latest version using the SDK manager: mock/path/sdkmng --list'
+    ].join('\n'));
+    expect(versionResolver.resolve).toHaveBeenCalled();
+  });
+
+  it('should not warn about emulators that are sufficiently new', async () => {
+    givenExpectedAVD();
+    givenProperEmulatorVersion();
+
+    await uut.validate('mock-avd-name');
+
+    expect(logger.warn).not.toHaveBeenCalled();
+  });
+
+  it('should provide specific warning if emulator version detection failed', async () => {
+    givenExpectedAVD();
+    givenUnknownEmulatorVersion();
+
+    await uut.validate('mock-avd-name');
+
+    expect(logger.warn).toHaveBeenCalledWith(
+      { event: 'AVD_VALIDATION' },
+      'Emulator version detection failed (See previous logs)'
+      );
   });
 });
diff --git a/detox/src/devices/drivers/android/emulator/EmulatorVersionResolver.test.js b/detox/src/devices/drivers/android/emulator/EmulatorVersionResolver.test.js
index 4f94b4769..102e12856 100644
--- a/detox/src/devices/drivers/android/emulator/EmulatorVersionResolver.test.js
+++ b/detox/src/devices/drivers/android/emulator/EmulatorVersionResolver.test.js
@@ -16,6 +16,7 @@ describe('Emulator binary version', () => {
     minor: 1,
     patch: 2,
   };
+  const expectedVersionRaw = '30.1.2.3';
 
   let MockQueryVersionCommand;
   let emulatorExec;
@@ -51,7 +52,8 @@ describe('Emulator binary version', () => {
 
   it('should extract version from common log', async () => {
     const version = await uut.resolve();
-    expect(version).toEqual(expectedVersion);
+    expect(version).toEqual(expect.objectContaining(expectedVersion));
+    expect(version.toString()).toEqual(expectedVersionRaw);
   });
 
   it('should return null for an empty query result', async () => {
@@ -90,11 +92,11 @@ describe('Emulator binary version', () => {
     const version = await uut.resolve();
 
     expect(emulatorExec.exec).toHaveBeenCalledTimes(1);
-    expect(version).toEqual(expectedVersion);
+    expect(version).toEqual(expect.objectContaining(expectedVersion));
   });
 
   it('should log the version', async () => {
     await uut.resolve();
-    expect(log.debug).toHaveBeenCalledWith({event: 'EMU_BIN_VERSION_DETECT', success: true}, expect.any(String), expectedVersion);
+    expect(log.debug).toHaveBeenCalledWith({event: 'EMU_BIN_VERSION_DETECT', success: true}, expect.any(String), expect.objectContaining(expectedVersion));
   });
 });
diff --git a/detox/src/utils/environment.test.js b/detox/src/utils/environment.test.js
index 36d88f836..1138d5177 100644
--- a/detox/src/utils/environment.test.js
+++ b/detox/src/utils/environment.test.js
@@ -120,6 +120,40 @@ describe('Environment', () => {
       });
     });
 
+    describe('getAvdManagerPath', () => {
+      it('should return path to AVD-manager executable', () => {
+        process.env.ANDROID_SDK_ROOT = 'mock/path/to/sdk';
+
+        const avdManagerPath = Environment.getAvdManagerPath();
+        expect(avdManagerPath).toEqual('mock/path/to/sdk/tools/bin/avdmanager');
+      });
+
+      it('should fall back to using ANDROID_HOME instead of ANDROID_SDK_ROOT', () => {
+        delete process.env.ANDROID_SDK_ROOT;
+        process.env.ANDROID_HOME = 'mock/path/to/sdk';
+
+        const avdManagerPath = Environment.getAvdManagerPath();
+        expect(avdManagerPath).toEqual('mock/path/to/sdk/tools/bin/avdmanager');
+      });
+    });
+
+    describe('getAndroidSdkManagerPath', () => {
+      it('should return path to SDK-manager executable', () => {
+        process.env.ANDROID_SDK_ROOT = 'mock/path/to/sdk';
+
+        const sdkManagerPath = Environment.getAndroidSdkManagerPath();
+        expect(sdkManagerPath).toEqual('mock/path/to/sdk/tools/bin/sdkmanager');
+      });
+
+      it('should fall back to using ANDROID_HOME instead of ANDROID_SDK_ROOT', () => {
+        delete process.env.ANDROID_SDK_ROOT;
+        process.env.ANDROID_HOME = 'mock/path/to/sdk';
+
+        const sdkManagerPath = Environment.getAndroidSdkManagerPath();
+        expect(sdkManagerPath).toEqual('mock/path/to/sdk/tools/bin/sdkmanager');
+      });
+    });
+
     describe('getAndroidEmulatorPath', () => {
       describe('if $ANDROID_SDK_ROOT is set', () => {
         beforeEach(() => {