Repository: wix/Detox (js)

Issue:
Wont Launch Valid Android Emulator AVD
#### Description
The `detox test` command fails with the error message "Error: Emulator with name <emulator name> has a corrupt config.ini file (/path/to/config.ini), try fixing it by recreating an emulator.

This happens for a perfectly valid emulator avd that starts up without problem. In my particular case the avd was created from the command line using the `avdmanager.bat create avd` command.

Looking through the source code, this error is thrown from `EmulatorDriver.js _fixEmulatorConfigIniSkinName(..)` when the config.ini file for the emulator does not have a 'skin.name' value and does not have `hw.lc.width` and `hw.lcd.height` values.

It is unclear why the emulator driver is trying to fix the config.ini in this way. Ideally the command should work for such an emulator avd since the avdmanager does not create it with those values and the avd works without them.

If there is a specific scenario where this behavior is required or it is a requirement specific to detox then, at a minimum, the error message should be made more descriptive to provide a clear solution. Such as instructing the user that skin and/or height + width specification is required in the config.ini.

#### Steps to Reproduce
Run the `detox test -c <config>` command. Use a configuration of type `android.emulator` and select an avd name with a `config.ini` file that has neither the `skin.name` property nor the `hw.lcd.width` / `hw.lcd.height` properties.


#### Detox, Node, Device, Xcode and macOS Versions

* Detox: `10.0.7`
* React Native: `0.57.8`
* Node: `11.6.0`
* Device: `Android Emulator`
* Xcode: N/A 
* macOS: N/A
* Windows: `Windows 10 Pro v1703`

#### Device and verbose Detox logs
```
detox[716] INFO:  [DetoxServer.js] server listening on localhost:50369...
detox[716] DEBUG: [AsyncWebSocket.js/WEBSOCKET_OPEN] opened web socket to: ws://localhost:50369
detox[716] TRACE: [AsyncWebSocket.js/WEBSOCKET_SEND] {"type":"login","params":{"sessionId":"3a17d4e8-c480-0900-5910-4068d33e6274","role":"tester"},"messageId":0}
detox[716] DEBUG: [DetoxServer.js/LOGIN] role=tester, sessionId=3a17d4e8-c480-0900-5910-4068d33e6274
detox[716] DEBUG: [DetoxServer.js/LOGIN_SUCCESS] role=tester, sessionId=3a17d4e8-c480-0900-5910-4068d33e6274
detox[716] TRACE: [AsyncWebSocket.js/WEBSOCKET_MESSAGE] {"type":"loginSuccess","params":{"sessionId":"3a17d4e8-c480-0900-5910-4068d33e6274","role":"tester"},"messageId":0}
 
detox[716] DEBUG: [exec.js/EXEC_CMD, #0] C:\path\to\emulator -list-avds --verbose
detox[716] TRACE: [exec.js/EXEC_SUCCESS, #0] <avd name>

detox[716] ERROR: [index.js/DETOX_INIT_ERROR] Error: Emulator with name <emulator name> has a corrupt config.ini file (C:\path\to/<avd name>.avd/config.ini), try fixing it by recreating an emulator.
    at EmulatorDriver._fixEmulatorConfigIniSkinName (C:\path\to\EmulatorDriver.js:30:15)
    at EmulatorDriver.acquireFreeDevice (C:\path\to\EmulatorDriver.js:91:16)
    at process.internalTickCallback (internal/process/next_tick.js:77:7)
```


Pull request:
Resort to soft-failing errors in emulator version detection for skin-cfg patching
- [x] This is a small change 
- [ ] This change has been discussed in issue #<?> and the solution has been agreed upon with maintainers.

---

**Description:**
With respect to #1995 - which has been closed nonetheless, this applies less extreme failing measures in case of emulator version detection.
Also, will evidently resolve #1157.

Patch:
diff --git a/detox/src/devices/drivers/android/EmulatorDriver.js b/detox/src/devices/drivers/android/EmulatorDriver.js
index 647cb0bb2..24c022969 100644
--- a/detox/src/devices/drivers/android/EmulatorDriver.js
+++ b/detox/src/devices/drivers/android/EmulatorDriver.js
@@ -105,7 +105,16 @@ class EmulatorDriver extends AndroidDriver {
   }
 
   async _fixEmulatorConfigIniSkinNameIfNeeded(avdName) {
-    const binaryVersion = _.get(await this.binaryVersion(), 'major', EMU_BIN_STABLE_SKIN_VER - 1);
+    const binaryVersion = _.get(await this.binaryVersion(), 'major');
+    if (!binaryVersion) {
+      log.warn({ event: 'EMU_SKIN_CFG_PATCH' }, [
+        'Failed to detect emulator version! (see previous logs)',
+        'This leaves Detox unable to tell if it should automatically apply this patch-fix: https://stackoverflow.com/a/47265664/453052, which seems to be needed in emulator versions < 28.',
+        'If you feel this is not needed, you can either ignore this message, or otherwise apply the patch manually.',
+      ].join('\n'));
+      return;
+    }
+
     if (binaryVersion >= EMU_BIN_STABLE_SKIN_VER) {
       return;
     }
diff --git a/detox/src/devices/drivers/android/emulator/EmulatorVersionResolver.js b/detox/src/devices/drivers/android/emulator/EmulatorVersionResolver.js
index e9d2666d6..8d1c87eb6 100644
--- a/detox/src/devices/drivers/android/emulator/EmulatorVersionResolver.js
+++ b/detox/src/devices/drivers/android/emulator/EmulatorVersionResolver.js
@@ -17,7 +17,14 @@ class EmulatorVersionResolver {
   }
 
   async _resolve() {
-    const rawOutput = await this._emulatorExec.exec(new QueryVersionCommand()) || '';
+    let rawOutput;
+    try {
+      rawOutput = await this._emulatorExec.exec(new QueryVersionCommand()) || '';
+    } catch (error) {
+      log.error({ event: EMU_BIN_VERSION_DETECT_EV, success: false, error }, 'Could not detect emulator binary version', error);
+      return null;
+    }
+
     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);


Test patch:
diff --git a/detox/src/devices/drivers/android/emulator/EmulatorVersionResolver.test.js b/detox/src/devices/drivers/android/emulator/EmulatorVersionResolver.test.js
index 53409ed52..d95eb0d87 100644
--- a/detox/src/devices/drivers/android/emulator/EmulatorVersionResolver.test.js
+++ b/detox/src/devices/drivers/android/emulator/EmulatorVersionResolver.test.js
@@ -35,6 +35,7 @@ describe('Emulator binary version', () => {
       child: jest.fn().mockReturnValue({
         debug: jest.fn(),
         warn: jest.fn(),
+        error: jest.fn(),
       }),
     }));
     log = require('../../../../utils/logger').child();
@@ -65,6 +66,25 @@ describe('Emulator binary version', () => {
     expect(version).toEqual(null);
   });
 
+  it('should log in case of a parsing error', async () => {
+    emulatorExec.exec.mockResolvedValue('non-parsable result');
+    await uut.resolve();
+    expect(log.warn).toHaveBeenCalledWith({event: 'EMU_BIN_VERSION_DETECT', success: false}, expect.any(String), 'non-parsable result');
+  });
+
+  it('should return null in case of a version-query failure', async () => {
+    emulatorExec.exec.mockRejectedValue(new Error('some error'));
+    const version = await uut.resolve();
+    expect(version).toEqual(null);
+  });
+
+  it('should log in case of a version-query failure', async () => {
+    const error = new Error('some error');
+    emulatorExec.exec.mockRejectedValue(error);
+    await uut.resolve();
+    expect(log.error).toHaveBeenCalledWith({event: 'EMU_BIN_VERSION_DETECT', success: false, error}, expect.any(String), error);
+  });
+
   it('should cache the version', async () => {
     await uut.resolve();
     const version = await uut.resolve();
@@ -77,10 +97,4 @@ describe('Emulator binary version', () => {
     await uut.resolve();
     expect(log.debug).toHaveBeenCalledWith({event: 'EMU_BIN_VERSION_DETECT', success: true}, expect.any(String), expectedVersion);
   });
-
-  it('should log in case of an error', async () => {
-    emulatorExec.exec.mockResolvedValue('mock result');
-    await uut.resolve();
-    expect(log.warn).toHaveBeenCalledWith({event: 'EMU_BIN_VERSION_DETECT', success: false}, expect.any(String), 'mock result');
-  });
 });