Repository: wix/Detox (js)

Issue:
[Android] manually define adb path and skip check for local sdk installation
**Is your feature request related to a problem? Please describe.**

We are running a setup where we lease a remote android emulator and control it via adb (I think this is similar to how Genymotion works). In this setup, the local machine has adb but does not have a a full sdk installed. I am using the `android.attached` type since the emulator is already attached to adb.

When I try to use detox in this setup the error `$ANDROID_SDK_ROOT is not defined...` is thrown. If I go in and explicitly set the adb path in [detox's  source](https://github.com/wix/Detox/blob/dd34555bc2c23b24c16ad8e0b407c92115b0de8e/detox/src/devices/android/ADB.js#L12), I'm able to successfully run a Detox test since `getAndroidSDKPath()` isn't called. 

(I also have to run `adb reverse tcp:1234 tcp:1234` for the app to login to the detox server, but that works fine.)

**Describe the solution you'd like**
A flag or argument to define the path to `adb` instead of requiring the entire sdk to be installed.(https://github.com/wix/Detox/blob/dd34555bc2c23b24c16ad8e0b407c92115b0de8e/detox/src/devices/android/ADB.js#L12)

cc @zsol

Pull request:
use adb, aapt, and emulator on PATH if not found in ANDROID_SDK_ROOT
<!---
Thank you for contributing!
Before submitting a pull request that implements a new functionality or fixing a major bug,
please open an issue first and propose your solution. This way we can discuss before time is 
spent on a solution that may not work for us.

Please check the correct option in the below list and delete the irrelevant one.
For the second option, please fill the issue where the solution has been discussed.
-->

- [x] This is a small change 
- [ ] This change has been discussed in https://github.com/wix/Detox/issues/1579 and the solution has been agreed upon with maintainers.

---

**Description:**
Allows the arguments `adb` and `aapt` to be passed when running a test. Fixes #1579.

Patch:
diff --git a/detox/package.json b/detox/package.json
index ef06a488f..969c666b0 100644
--- a/detox/package.json
+++ b/detox/package.json
@@ -55,6 +55,7 @@
     "tail": "^2.0.0",
     "telnet-client": "0.15.3",
     "tempfile": "^2.0.0",
+    "which": "^1.3.1",
     "ws": "^3.3.1",
     "yargs": "^13.0.0",
     "yargs-parser": "^13.0.0"
diff --git a/detox/src/devices/android/AAPT.js b/detox/src/devices/android/AAPT.js
index de1817623..8d90e511c 100644
--- a/detox/src/devices/android/AAPT.js
+++ b/detox/src/devices/android/AAPT.js
@@ -1,5 +1,5 @@
 const _ = require('lodash');
-const Environment = require('../../utils/environment');
+const {getAaptPath} = require('../../utils/environment');
 const path = require('path');
 const exec = require('../../utils/exec').execWithRetriesAndLogs;
 const escape = require('../../utils/pipeCommands').escape.inQuotedString;
@@ -7,18 +7,12 @@ const egrep = require('../../utils/pipeCommands').search.fragment;
 const fsext = require('../../utils/fsext');
 
 class AAPT {
-
   constructor() {
     this.aaptBin = null;
   }
 
   async _prepare() {
-    if (!this.aaptBin) {
-      const sdkPath = Environment.getAndroidSDKPath();
-      const buildToolsDirs = await fsext.getDirectories(path.join(sdkPath, 'build-tools'));
-      const latestBuildToolsVersion = _.last(buildToolsDirs);
-      this.aaptBin = path.join(sdkPath, 'build-tools', latestBuildToolsVersion, 'aapt');
-    }
+    this.aaptBin = this.aaptBin || await getAaptPath();
   }
 
   async getPackageName(apkPath) {
diff --git a/detox/src/devices/android/ADB.js b/detox/src/devices/android/ADB.js
index fa65d4b8e..4fdf40247 100644
--- a/detox/src/devices/android/ADB.js
+++ b/detox/src/devices/android/ADB.js
@@ -4,12 +4,12 @@ const {execWithRetriesAndLogs, spawnAndLog} = require('../../utils/exec');
 const {escape} = require('../../utils/pipeCommands');
 const DetoxRuntimeError = require('../../errors/DetoxRuntimeError');
 const EmulatorTelnet = require('./EmulatorTelnet');
-const Environment = require('../../utils/environment');
+const {getAdbPath} = require('../../utils/environment');
 
 class ADB {
   constructor() {
     this._cachedApiLevels = new Map();
-    this.adbBin = path.join(Environment.getAndroidSDKPath(), 'platform-tools', 'adb');
+    this.adbBin = getAdbPath();
   }
 
   async devices() {
diff --git a/detox/src/devices/android/Emulator.js b/detox/src/devices/android/Emulator.js
index 00b0d493b..0446447d1 100644
--- a/detox/src/devices/android/Emulator.js
+++ b/detox/src/devices/android/Emulator.js
@@ -5,12 +5,12 @@ const spawn = require('child-process-promise').spawn;
 const Tail = require('tail').Tail;
 const exec = require('../../utils/exec').execWithRetriesAndLogs;
 const unitLogger = require('../../utils/logger').child({ __filename });
-const Environment = require('../../utils/environment');
+const {getAndroidEmulatorPath} = require('../../utils/environment');
 const argparse = require('../../utils/argparse');
 
 class Emulator {
   constructor() {
-    this.emulatorBin = Environment.getAndroidEmulatorPath();
+    this.emulatorBin = getAndroidEmulatorPath();
   }
 
   async listAvds() {
diff --git a/detox/src/devices/drivers/AttachedAndroidDriver.js b/detox/src/devices/drivers/AttachedAndroidDriver.js
index 107d9c24c..c429359ad 100644
--- a/detox/src/devices/drivers/AttachedAndroidDriver.js
+++ b/detox/src/devices/drivers/AttachedAndroidDriver.js
@@ -1,12 +1,9 @@
-const Emulator = require('../android/Emulator');
 const AndroidDriver = require('./AndroidDriver');
 
 class AttachedAndroidDriver extends AndroidDriver {
 
   constructor(config) {
     super(config);
-
-    this.emulator = new Emulator();
     this._name = 'Unnamed Android Device';
   }
 
diff --git a/detox/src/utils/environment.js b/detox/src/utils/environment.js
index dcc9759e2..2db839c36 100644
--- a/detox/src/utils/environment.js
+++ b/detox/src/utils/environment.js
@@ -1,29 +1,100 @@
+const _ = require('lodash');
 const fs = require('fs');
 const os = require('os');
 const path = require('path');
+const _which = require('which');
 const exec = require('child-process-promise').exec;
 const appdatapath = require('./appdatapath');
+const fsext = require('./fsext');
+
+function which(executable, path) {
+  return _which.sync(executable, {path, nothrow: true});
+}
 
 const DETOX_LIBRARY_ROOT_PATH = path.join(appdatapath.appDataPath(), 'Detox');
+const MISSING_SDK_ERROR = `$ANDROID_SDK_ROOT is not defined, set the path to the SDK installation directory into $ANDROID_SDK_ROOT,
+Go to https://developer.android.com/studio/command-line/variables.html for more details`;
 const DEVICE_LOCK_FILE_PATH_IOS = path.join(DETOX_LIBRARY_ROOT_PATH, 'device.registry.state.lock');
 const DEVICE_LOCK_FILE_PATH_ANDROID = path.join(DETOX_LIBRARY_ROOT_PATH, 'android-device.registry.state.lock');
 
 function getAndroidSDKPath() {
-  let sdkPath = process.env.ANDROID_SDK_ROOT || process.env.ANDROID_HOME;
-  if (!sdkPath) {
-    throw new Error(`$ANDROID_SDK_ROOT is not defined, set the path to the SDK installation directory into $ANDROID_SDK_ROOT,
-    Go to https://developer.android.com/studio/command-line/variables.html for more details`);
-  }
-  return sdkPath;
+  return process.env.ANDROID_SDK_ROOT || process.env.ANDROID_HOME || '';
 }
 
 function getAndroidEmulatorPath() {
-  const sdkPath = getAndroidSDKPath();
-  const extension = os.platform() === 'win32' ? '.exe' : '';
-  const newEmulatorPath = path.join(sdkPath, 'emulator', `emulator${extension}`);
-  const oldEmulatorPath = path.join(sdkPath, 'tools', `emulator${extension}`);
+  const sdkRoot = getAndroidSDKPath();
+  if (!sdkRoot) {
+    return which('emulator') || throwMissingSdkError();
+  }
+
+  const defaultPath = which('emulator', path.join(sdkRoot, 'emulator'));
+  if (defaultPath) {
+    return defaultPath;
+  }
+
+  const legacyPath = which('emulator', path.join(sdkRoot, 'tools'));
+  if (legacyPath) {
+    return legacyPath;
+  }
+
+  throwSdkIntegrityError(sdkRoot, 'emulator/emulator');
+}
+
+async function getAaptPath() {
+  const sdkRoot = getAndroidSDKPath();
+  if (!sdkRoot) {
+    return which('aapt') || throwMissingSdkError();
+  }
+
+  const latestBuildTools = await getLatestBuildToolsPath(sdkRoot);
+  const defaultPath = latestBuildTools && which('aapt', latestBuildTools);
+  if (defaultPath) {
+    return defaultPath;
+  }
+
+  throwSdkIntegrityError(sdkRoot, `${latestBuildTools}/aapt`);
+}
+
+async function getLatestBuildToolsPath(sdkRoot) {
+  if (!sdkRoot) return '';
+
+  const buildToolsDir = path.join(sdkRoot, 'build-tools');
+  if (!fs.existsSync(buildToolsDir)) return '';
+
+  const buildToolsVersions = await fsext.getDirectories(buildToolsDir);
+  const latestBuildToolsVersion = _.last(buildToolsVersions);
+  if (!latestBuildToolsVersion) return '';
+
+  return path.join(buildToolsDir, latestBuildToolsVersion);
+}
+
+function getAdbPath() {
+  const sdkRoot = getAndroidSDKPath();
+  if (!sdkRoot) {
+    return which('adb') || throwMissingSdkError();
+  }
+
+  const defaultPath = which('adb', path.join(sdkRoot, 'platform-tools'));
+  if (defaultPath) {
+    return defaultPath;
+  }
+
+  throwSdkIntegrityError(sdkRoot, 'platform-tools/adb');
+}
+
+function throwMissingSdkError() {
+    throw new Error(MISSING_SDK_ERROR);
+}
+
+function throwSdkIntegrityError(sdkRoot, relativeExecutablePath) {
+  const executablePath = path.join(sdkRoot, relativeExecutablePath);
+  const name = path.basename(executablePath);
+  const dir = path.dirname(executablePath);
 
-  return fs.existsSync(newEmulatorPath) ? newEmulatorPath : oldEmulatorPath;
+  throw new Error(
+    `There was no "${name}" executable file in directory: ${dir}.\n` +
+    `Check integrity of your Android SDK.`
+  );
 }
 
 function getDetoxVersion() {
@@ -53,6 +124,8 @@ function getHomeDir() {
 }
 
 module.exports = {
+  getAaptPath,
+  getAdbPath,
   getDetoxVersion,
   getFrameworkPath,
   getAndroidSDKPath,


Test patch:
diff --git a/detox/__tests__/setupJest.js b/detox/__tests__/setupJest.js
index bbb3cddc3..b64b7a83a 100644
--- a/detox/__tests__/setupJest.js
+++ b/detox/__tests__/setupJest.js
@@ -1,3 +1,5 @@
+jest.mock('proper-lockfile');
+
 const yargs = require('yargs');
 const path = require('path');
 
diff --git a/detox/src/devices/Device.test.js b/detox/src/devices/Device.test.js
index a758f7a50..5900c1eda 100644
--- a/detox/src/devices/Device.test.js
+++ b/detox/src/devices/Device.test.js
@@ -18,7 +18,6 @@ describe('Device', () => {
 
   beforeEach(async () => {
     jest.mock('fs');
-    jest.mock('proper-lockfile');
     jest.mock('../utils/logger');
 
     fs = require('fs');
diff --git a/detox/src/devices/DeviceRegistry.test.js b/detox/src/devices/DeviceRegistry.test.js
index d865f0ef0..b223da33f 100644
--- a/detox/src/devices/DeviceRegistry.test.js
+++ b/detox/src/devices/DeviceRegistry.test.js
@@ -14,8 +14,6 @@ describe('DeviceRegistry', () => {
     fs.existsSync.mockReturnValue(true);
     fs.readFileSync.mockReturnValue("[]");
 
-    jest.mock('proper-lockfile');
-
     createDevice = jest.fn();
     getDeviceIdsByType = jest.fn();
   });
diff --git a/detox/src/devices/android/ADB.test.js b/detox/src/devices/android/ADB.test.js
index 2dd071d70..de7a4bcb8 100644
--- a/detox/src/devices/android/ADB.test.js
+++ b/detox/src/devices/android/ADB.test.js
@@ -1,7 +1,6 @@
 
 describe('ADB', () => {
-  const mockAndroidSDKPath = '/Android/sdk-mock';
-  const adbBinPath = `${mockAndroidSDKPath}/platform-tools/adb`;
+  const adbBinPath = `/Android/sdk-mock/platform-tools/adb`;
 
   let mockEmulatorTelnet;
   let ADB;
@@ -10,9 +9,8 @@ describe('ADB', () => {
 
   beforeEach(() => {
     jest.mock('../../utils/logger');
-    jest.mock('../../utils/environment', () => ({
-      getAndroidSDKPath: () => mockAndroidSDKPath,
-    }));
+    jest.mock('../../utils/environment');
+    require('../../utils/environment').getAdbPath.mockReturnValue(adbBinPath);
 
     jest.mock('../../utils/encoding', () => ({
       encodeBase64: (text) => `base64(${text})`,
@@ -262,4 +260,3 @@ describe('ADB', () => {
     expect(result).toEqual(expectedRunner);
   });
 });
-
diff --git a/detox/src/utils/environment.test.js b/detox/src/utils/environment.test.js
index 9281890b1..0b26b9201 100644
--- a/detox/src/utils/environment.test.js
+++ b/detox/src/utils/environment.test.js
@@ -1,42 +1,183 @@
 const _ = require('lodash');
+const tempfile = require('tempfile');
+const fs = require('fs-extra');
+const path = require('path');
+const os = require('os');
 
 describe('Environment', () => {
   let Environment;
-  let originalProcessEnv;
+  let originalProcessEnv = _.cloneDeep(process.env);
 
   beforeEach(() => {
-    originalProcessEnv = _.cloneDeep(process.env);
+    process.env = _.cloneDeep(originalProcessEnv);
     Environment = require('./environment');
   });
 
-  beforeEach(() => {
-    process.env = _.cloneDeep(originalProcessEnv);
-    Environment = require('./environment');
+  afterAll(() => {
+    process.env = originalProcessEnv;
   });
 
-  it(`ANDROID_SDK_ROOT and ANDROID_HOME are defined, prefer ANDROID_SDK_ROOT`, () => {
-    process.env.ANDROID_SDK_ROOT = 'path/to/sdk/root';
-    process.env.ANDROID_HOME = 'path/to/android/home';
+  describe('(android)', () => {
+    let tempSdkPath;
 
-    const path = Environment.getAndroidSDKPath();
-    expect(path).toEqual('path/to/sdk/root');
-  });
+    async function genExec(relativePath) {
+      const extension = (os.platform() === 'win32') ? '.cmd' : '';
+      const filePath = path.join(tempSdkPath, relativePath) + extension;
 
-  it(`ANDROID_HOME is defined`, () => {
-    process.env.ANDROID_SDK_ROOT = undefined;
-    process.env.ANDROID_HOME = 'path/to/android/home';
+      await fs.ensureFile(filePath);
+      await fs.chmod(filePath, 0o755);
+    }
 
-    const path = Environment.getAndroidSDKPath();
-    expect(path).toEqual('path/to/android/home');
-  });
+    beforeEach(async () => {
+      tempSdkPath = tempfile();
+      await fs.mkdirp(tempSdkPath);
+    });
+
+    afterEach(async () => {
+      await fs.remove(tempSdkPath);
+    });
+
+    describe('getAndroidSDKPath', () => {
+      it(`should return empty string if $ANDROID_SDK_ROOT and $ANDROID_HOME both are not set`, () => {
+        delete process.env.ANDROID_SDK_ROOT;
+        delete process.env.ANDROID_HOME;
+
+        const sdkPath = Environment.getAndroidSDKPath();
+        expect(sdkPath).toEqual('');
+      });
+
+      it(`should return $ANDROID_HOME if it is set`, () => {
+        delete process.env.ANDROID_SDK_ROOT;
+        process.env.ANDROID_HOME = 'path/to/android/home';
+
+        const sdkPath = Environment.getAndroidSDKPath();
+        expect(sdkPath).toEqual('path/to/android/home');
+      });
+
+      it(`should return $ANDROID_SDK_ROOT if it is set`, () => {
+        delete process.env.ANDROID_HOME;
+        process.env.ANDROID_SDK_ROOT = 'path/to/sdk/root';
+
+        const sdkPath = Environment.getAndroidSDKPath();
+        expect(sdkPath).toEqual('path/to/sdk/root');
+      });
+
+      it(`should prefer $ANDROID_SDK_ROOT, if both $ANDROID_SDK_ROOT and $ANDROID_HOME are set`, () => {
+        process.env.ANDROID_SDK_ROOT = 'path/to/sdk/root';
+        process.env.ANDROID_HOME = 'path/to/android/home';
+
+        const sdkPath = Environment.getAndroidSDKPath();
+        expect(sdkPath).toEqual('path/to/sdk/root');
+      });
+    });
+
+    describe('getAndroidEmulatorPath', () => {
+      describe('if $ANDROID_SDK_ROOT is set', () => {
+        beforeEach(() => {
+          process.env.ANDROID_SDK_ROOT = tempSdkPath;
+        });
+
+        it('should return emulator if it is in $ANDROID_SDK_ROOT/emulator dir', async () => {
+          await genExec('emulator/emulator');
+          expect(Environment.getAndroidEmulatorPath()).toMatch(/.emulator.emulator/);
+        });
 
-  it(`ANDROID_SDK_ROOT and ANDROID_HOME are not defined`, () => {
-    process.env.ANDROID_SDK_ROOT = undefined;
-    process.env.ANDROID_HOME = undefined;
+        it('should return emulator if it is in $ANDROID_SDK_ROOT/tools dir', async () => {
+          await genExec('tools/emulator');
+          expect(Environment.getAndroidEmulatorPath()).toMatch(/.tools.emulator/);
+        });
 
-    expect(Environment.getAndroidSDKPath)
-      .toThrow('$ANDROID_SDK_ROOT is not defined, set the path to the SDK installation directory into $ANDROID_SDK_ROOT');
+        it('should prefer $ANDROID_SDK_ROOT/emulator over $ANDROID_SDK_ROOT/tools', async () => {
+          await genExec('tools/emulator');
+          await genExec('emulator/emulator');
 
+          expect(Environment.getAndroidEmulatorPath()).toMatch(/.emulator.emulator/);
+        });
+
+        it('should throw error if there are no executables at those locations', async () => {
+          expect(Environment.getAndroidEmulatorPath).toThrow(/There was no.*file in directory:/);
+        });
+      });
+
+      ifAndroidSdkRootAndHomeAreNotSet(() => {
+        itShouldFallBackToPathResolution(() => Environment.getAndroidEmulatorPath(), 'emulator');
+        itShouldThrowErrorIfThereAreNoExecutables(() => Environment.getAndroidEmulatorPath());
+      });
+    });
+
+    describe('getAaptPath', () => {
+      describe('if $ANDROID_SDK_ROOT is set', () => {
+        beforeEach(() => {
+          process.env.ANDROID_SDK_ROOT = tempSdkPath;
+        });
+
+        it('should return aapt from build-tools/<latest>', async () => {
+          await genExec('build-tools/19.0.0/aapt');
+          await genExec('build-tools/20.0.0/aapt');
+
+          expect(await Environment.getAaptPath()).toMatch(/.build-tools.20.0.0.aapt/);
+        });
+
+        it('should throw error if aapt is not found inside build-tools/<someDir>', async () => {
+          await genExec('build-tools/aapt');
+          await expect(Environment.getAaptPath()).rejects.toThrow(/There was no.*file in directory:/);
+        });
+      });
+
+      ifAndroidSdkRootAndHomeAreNotSet(() => {
+        itShouldFallBackToPathResolution(() => Environment.getAaptPath(), 'aapt');
+        itShouldThrowErrorIfThereAreNoExecutables(() => Environment.getAaptPath());
+      });
+    });
+
+    describe('getAdbPath', () => {
+      describe('if $ANDROID_SDK_ROOT is set', () => {
+        beforeEach(() => {
+          process.env.ANDROID_SDK_ROOT = tempSdkPath;
+        });
+
+        it('should return adb from platform-tools', async () => {
+          await genExec('platform-tools/adb');
+          expect(Environment.getAdbPath()).toMatch(/.platform-tools.adb/);
+        });
+
+        it('should throw error if adb is not found in platform-tools/', async () => {
+          await expect(Environment.getAaptPath()).rejects.toThrow(/There was no.*file in directory:/);
+        });
+      });
+
+      ifAndroidSdkRootAndHomeAreNotSet(() => {
+        itShouldFallBackToPathResolution(() => Environment.getAdbPath(), 'adb');
+        itShouldThrowErrorIfThereAreNoExecutables(() => Environment.getAdbPath());
+      });
+    });
+
+    function ifAndroidSdkRootAndHomeAreNotSet(fn) {
+      describe('if $ANDROID_SDK_ROOT and $ANDROID_HOME are not set', () => {
+        beforeEach(() => {
+          delete process.env.ANDROID_SDK_ROOT;
+          delete process.env.ANDROID_HOME;
+        });
+
+        fn();
+      });
+    }
+
+    function itShouldFallBackToPathResolution(getter, executable) {
+        it('should fall back to $PATH resolution', async () => {
+          process.env.PATH = path.join(tempSdkPath, 'somewhere');
+          await genExec(`somewhere/${executable}`);
+          expect(await getter()).toMatch(new RegExp(`somewhere.${executable}`));
+        });
+    }
+
+    function itShouldThrowErrorIfThereAreNoExecutables(getter) {
+      const asyncGetter = async () => getter();
+
+      it('should throw error if there are no executables on $PATH', async () => {
+        delete process.env.PATH;
+        await expect(asyncGetter()).rejects.toThrow(/\$ANDROID_SDK_ROOT is not defined/);
+      });
+    }
   });
 });
-