Repository: wix/Detox
Pull request #1988 (2020-03-31 17:24:05)

Refactor bloated emulator by extracting cmd exec logic
- [x] This is a small change 
- [ ] This change has been discussed in issue #<?> and the solution has been agreed upon with maintainers.

---

**Description:**

Part 3 of #1987 and #1984.


Patch:
diff --git a/detox/package.json b/detox/package.json
index 55557d909..1606beb1b 100644
--- a/detox/package.json
+++ b/detox/package.json
@@ -90,6 +90,7 @@
       ".*Driver.js",
       "MissingDetox.js",
       "EmulatorTelnet.js",
+      "EmulatorExec.js",
       "Emulator.js",
       "DeviceDriverBase.js",
       "GREYConfiguration.js",
diff --git a/detox/src/devices/drivers/android/EmulatorVersion.js b/detox/src/devices/drivers/android/EmulatorVersion.js
index 7320dc745..321ddec69 100644
--- a/detox/src/devices/drivers/android/EmulatorVersion.js
+++ b/detox/src/devices/drivers/android/EmulatorVersion.js
@@ -16,7 +16,7 @@ class EmulatorVersion {
   }
 
   async _resolve() {
-    const rawOutput = await this.emulator.exec('-version') || '';
+    const rawOutput = await this.emulator.queryVersion() || '';
     const matches = rawOutput.match(/Android emulator version ([0-9]+\.[0-9]+\.[0-9]+\.[0-9]*)/);
     if (!matches) {
       log.warn({ event: EMU_BIN_VERSION_DETECT_EV, success: false }, 'Could not detect emulator binary version, got:', rawOutput);
diff --git a/detox/src/devices/drivers/android/tools/Emulator.js b/detox/src/devices/drivers/android/tools/Emulator.js
index 7a33ec12d..e524b78a6 100644
--- a/detox/src/devices/drivers/android/tools/Emulator.js
+++ b/detox/src/devices/drivers/android/tools/Emulator.js
@@ -1,88 +1,45 @@
 const _ = require('lodash');
 const fs = require('fs');
-const os = require('os');
-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 {getAndroidEmulatorPath} = require('../../../../utils/environment');
-const argparse = require('../../../../utils/argparse');
 const retry = require('../../../../utils/retry');
+const {
+  EmulatorExec,
+  ListAVDsCommand,
+  QueryVersionCommand,
+  LaunchCommand,
+} = require('./EmulatorExec');
 
 const isUnknownEmulatorError = (err) => (err.message || '').includes('failed with code null');
 
 class Emulator {
   constructor() {
-    this.emulatorBin = getAndroidEmulatorPath();
+    this.emulatorExec = new EmulatorExec();
   }
 
   async listAvds() {
-    const output = await this.exec(`-list-avds --verbose`);
+    const output = await this.emulatorExec.exec(new ListAVDsCommand());
     const avds = output.trim().split('\n');
     return avds;
   }
 
-  async exec(cmd) {
-    return (await exec(`"${this.emulatorBin}" ${cmd}`)).stdout;
+  async queryVersion() {
+    return await this.emulatorExec.exec(new QueryVersionCommand());
   }
 
   async boot(emulatorName, options = {port: undefined}) {
-    const emulatorArgs = this._getEmulatorArgs(emulatorName, options);
+    const launchCommand = new LaunchCommand(emulatorName, options);
 
     return await retry({
       retries: 2,
       interval: 100,
       conditionFn: isUnknownEmulatorError,
-    }, () => this._spawnEmulator(emulatorName, emulatorArgs, options));
+    }, () => this._launchEmulator(emulatorName, launchCommand));
   }
 
-  _getEmulatorArgs(emulatorName, options) {
-    const deviceLaunchArgs = (argparse.getArgValue('deviceLaunchArgs') || '').split(/\s+/);
-    const emulatorArgs = _.compact([
-      '-verbose',
-      '-no-audio',
-      '-no-boot-anim',
-      argparse.getArgValue('headless') === 'true' ? '-no-window' : '',
-      argparse.getArgValue('readOnlyEmu') === 'true' ? '-read-only' : '',
-      options.port ? `-port` : '',
-      options.port ? `${options.port}` : '',
-      ...deviceLaunchArgs,
-      `@${emulatorName}`
-    ]);
-
-    const gpuMethod = this.gpuMethod();
-    if (gpuMethod) {
-      emulatorArgs.push('-gpu', gpuMethod);
-    }
-
-    return emulatorArgs;
-  }
-
-  gpuMethod() {
-    const gpuArgument = argparse.getArgValue('gpu');
-    if (gpuArgument) {
-      return gpuArgument;
-    }
-
-    if (argparse.getArgValue('headless')) {
-      switch (os.platform()) {
-        case 'darwin':
-          return 'host';
-        case 'linux':
-          return 'swiftshader_indirect';
-        case 'win32':
-          return 'angle_indirect';
-        default:
-          return 'auto';
-      }
-    }
-
-    return undefined;
-  }
-
-  _spawnEmulator(emulatorName, emulatorArgs, options) {
+  _launchEmulator(emulatorName, launchCommand) {
     let childProcessOutput;
-    const portName = options.port ? `-${options.port}` : '';
+    const portName = launchCommand.port ? `-${launchCommand.port}` : '';
     const tempLog = `./${emulatorName}${portName}.log`;
     const stdout = fs.openSync(tempLog, 'a');
     const stderr = fs.openSync(tempLog, 'a');
@@ -113,8 +70,8 @@ class Emulator {
     }
 
     let log = unitLogger.child({ fn: 'boot' });
-    log.debug({ event: 'SPAWN_CMD' }, this.emulatorBin, ...emulatorArgs);
-    const childProcessPromise = spawn(this.emulatorBin, emulatorArgs, { detached: true, stdio: ['ignore', stdout, stderr] });
+    log.debug({ event: 'SPAWN_CMD' }, this.emulatorExec.toString(), launchCommand.toString());
+    const childProcessPromise = this.emulatorExec.spawn(launchCommand, stdout, stderr);
     childProcessPromise.childProcess.unref();
     log = log.child({ child_pid: childProcessPromise.childProcess.pid });
 
diff --git a/detox/src/devices/drivers/android/tools/EmulatorExec.js b/detox/src/devices/drivers/android/tools/EmulatorExec.js
new file mode 100644
index 000000000..316315dc3
--- /dev/null
+++ b/detox/src/devices/drivers/android/tools/EmulatorExec.js
@@ -0,0 +1,110 @@
+const _ = require('lodash');
+const os = require('os');
+const spawn = require('child-process-promise').spawn;
+const exec = require('../../../../utils/exec').execWithRetriesAndLogs;
+const argparse = require('../../../../utils/argparse');
+const {getAndroidEmulatorPath} = require('../../../../utils/environment');
+
+class EmulatorExec {
+  constructor() {
+    this.binary = getAndroidEmulatorPath();
+  }
+
+  toString() {
+    return this.binary;
+  }
+
+  async exec(command) {
+    return (await exec(`"${this.binary}" ${command._getArgsString()}`)).stdout;
+  }
+
+  spawn(command, stdout, stderr) {
+    return spawn(this.binary, command._getArgs(), { detached: true, stdio: ['ignore', stdout, stderr] });
+  }
+}
+
+class EmulatorCommand {
+  toString() {
+    return this._getArgsString();
+  }
+
+  _getArgs() {}
+  _getArgsString() {
+    return this._getArgs().join(' ');
+  }
+}
+
+class ListAVDsCommand extends EmulatorCommand {
+  _getArgs() {
+    return ['-list-avds', '--verbose'];
+  }
+}
+
+class QueryVersionCommand extends EmulatorCommand {
+  _getArgs() {
+    return ['-version'];
+  }
+}
+
+class LaunchCommand extends EmulatorCommand {
+  constructor(emulatorName, options) {
+    super();
+    this._args = this._getEmulatorArgs(emulatorName, options);
+    this.port = options.port;
+  }
+
+  _getArgs() {
+    return this._args;
+  }
+
+  _getEmulatorArgs(emulatorName, options) {
+    const deviceLaunchArgs = (argparse.getArgValue('deviceLaunchArgs') || '').split(/\s+/);
+    const emulatorArgs = _.compact([
+      '-verbose',
+      '-no-audio',
+      '-no-boot-anim',
+      argparse.getArgValue('headless') === 'true' ? '-no-window' : '',
+      argparse.getArgValue('readOnlyEmu') === 'true' ? '-read-only' : '',
+      options.port ? `-port` : '',
+      options.port ? `${options.port}` : '',
+      ...deviceLaunchArgs,
+      `@${emulatorName}`
+    ]);
+
+    const gpuMethod = this._gpuMethod();
+    if (gpuMethod) {
+      emulatorArgs.push('-gpu', gpuMethod);
+    }
+
+    return emulatorArgs;
+  }
+
+  _gpuMethod() {
+    const gpuArgument = argparse.getArgValue('gpu');
+    if (gpuArgument) {
+      return gpuArgument;
+    }
+
+    if (argparse.getArgValue('headless')) {
+      switch (os.platform()) {
+        case 'darwin':
+          return 'host';
+        case 'linux':
+          return 'swiftshader_indirect';
+        case 'win32':
+          return 'angle_indirect';
+        default:
+          return 'auto';
+      }
+    }
+
+    return undefined;
+  }
+}
+
+module.exports = {
+  EmulatorExec,
+  ListAVDsCommand,
+  QueryVersionCommand,
+  LaunchCommand,
+};


Test patch:
diff --git a/detox/src/devices/drivers/android/EmulatorVersion.test.js b/detox/src/devices/drivers/android/EmulatorVersion.test.js
index e0f6e17f7..089c08d5d 100644
--- a/detox/src/devices/drivers/android/EmulatorVersion.test.js
+++ b/detox/src/devices/drivers/android/EmulatorVersion.test.js
@@ -22,7 +22,7 @@ describe('Emulator binary version', () => {
   let emulatorVersion;
   beforeEach(() => {
     emulator = {
-      exec: jest.fn().mockResolvedValue(versionResult),
+      queryVersion: jest.fn().mockResolvedValue(versionResult),
     };
 
     jest.mock('../../../utils/logger', () => ({
@@ -39,7 +39,7 @@ describe('Emulator binary version', () => {
 
   it('should query the emulator', async () => {
     await emulatorVersion.resolve();
-    expect(emulator.exec).toHaveBeenCalledWith('-version');
+    expect(emulator.queryVersion).toHaveBeenCalled();
   });
 
   it('should extract version from common log', async () => {
@@ -48,13 +48,13 @@ describe('Emulator binary version', () => {
   });
 
   it('should return null for an empty query result', async () => {
-    emulator.exec.mockResolvedValue(undefined);
+    emulator.queryVersion.mockResolvedValue(undefined);
     const version = await emulatorVersion.resolve();
     expect(version).toEqual(null);
   });
 
   it('should return null for a query result that has an invalid syntax', async () => {
-    emulator.exec.mockResolvedValue('Android emulator version \<invalid\> (build_id 6306047) (CL:N/A)');
+    emulator.queryVersion.mockResolvedValue('Android emulator version \<invalid\> (build_id 6306047) (CL:N/A)');
     const version = await emulatorVersion.resolve();
     expect(version).toEqual(null);
   });
@@ -63,7 +63,7 @@ describe('Emulator binary version', () => {
     await emulatorVersion.resolve();
     const version = await emulatorVersion.resolve();
 
-    expect(emulator.exec).toHaveBeenCalledTimes(1);
+    expect(emulator.queryVersion).toHaveBeenCalledTimes(1);
     expect(version).toEqual(expectedVersion);
   });
 
@@ -73,7 +73,7 @@ describe('Emulator binary version', () => {
   });
 
   it('should log in case of an error', async () => {
-    emulator.exec.mockResolvedValue('mock result');
+    emulator.queryVersion.mockResolvedValue('mock result');
     await emulatorVersion.resolve();
     expect(log.warn).toHaveBeenCalledWith({event: 'EMU_BIN_VERSION_DETECT', success: false}, expect.any(String), 'mock result');
   });