Repository: wix/Detox
Pull request #1773 (2019-11-19 14:48:03)

Enable emulator boot retry when crashed with unspecified reason
- [x] This is a small change 
- [ ] This change has been discussed in issue #0 and the solution has been agreed upon with maintainers.

---

**Description:**

Introduce retry code for when Android emulators are started by Detox and crashed with an unknown error code (i.e. "failed with code null" message).


Patch:
diff --git a/detox/src/devices/android/Emulator.js b/detox/src/devices/android/Emulator.js
index 1efba3edc..b95c81081 100644
--- a/detox/src/devices/android/Emulator.js
+++ b/detox/src/devices/android/Emulator.js
@@ -7,6 +7,9 @@ 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 isUnknownEmulatorError = (err) => (err.message || '').includes('failed with code null');
 
 class Emulator {
   constructor() {
@@ -24,6 +27,16 @@ class Emulator {
   }
 
   async boot(emulatorName, options = {port: undefined}) {
+    const emulatorArgs = this._getEmulatorArgs(emulatorName, options);
+
+    return await retry({
+      retries: 2,
+      interval: 100,
+      conditionFn: isUnknownEmulatorError,
+    }, () => this._spawnEmulator(emulatorName, emulatorArgs, options));
+  }
+
+  _getEmulatorArgs(emulatorName, options) {
     const deviceLaunchArgs = (argparse.getArgValue('deviceLaunchArgs') || '').split(/\s+/);
     const emulatorArgs = _.compact([
       '-verbose',
@@ -42,6 +55,32 @@ class Emulator {
       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) {
     let childProcessOutput;
     const portName = options.port ? `-${options.port}` : '';
     const tempLog = `./${emulatorName}${portName}.log`;
@@ -95,28 +134,6 @@ class Emulator {
       return coldBoot;
     });
   }
-
-  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 = Emulator;
diff --git a/detox/src/utils/exec.js b/detox/src/utils/exec.js
index 47581a3db..3e05d1eae 100644
--- a/detox/src/utils/exec.js
+++ b/detox/src/utils/exec.js
@@ -7,7 +7,7 @@ const DetoxRuntimeError = require('../errors/DetoxRuntimeError');
 
 let _operationCounter = 0;
 
-async function execWithRetriesAndLogs(bin, options, statusLogs, retries = 10, interval = 1000) {
+async function execWithRetriesAndLogs(bin, options, statusLogs, retries = 9, interval = 1000) {
   const trackingId = _operationCounter++;
   const cmd = _composeCommand(bin, options);
   const execTimeout = _.get(options, 'timeout', 0);
diff --git a/detox/src/utils/retry.js b/detox/src/utils/retry.js
index 8f0268a9d..2fd9eddf7 100644
--- a/detox/src/utils/retry.js
+++ b/detox/src/utils/retry.js
@@ -1,5 +1,8 @@
-const DEFAULT_RETRIES = 10;
+const sleep = require('./sleep');
+
+const DEFAULT_RETRIES = 9;
 const DEFAULT_INTERVAL = 500;
+const DEFAULT_CONDITION_FN = () => true;
 
 async function retry(options, func) {
   if (typeof options === 'function') {
@@ -7,21 +10,20 @@ async function retry(options, func) {
     options = {};
   }
 
-  let {retries, interval} = options;
-  retries = retries || DEFAULT_RETRIES;
-  interval = interval || DEFAULT_INTERVAL;
+  const {
+    retries = DEFAULT_RETRIES,
+    interval = DEFAULT_INTERVAL,
+    conditionFn = DEFAULT_CONDITION_FN,
+  } = options;
 
-  let currentRetry = 0;
-  while (currentRetry++ < retries) {
+  for (let totalRetries = 1; true; totalRetries++) {
     try {
-      return await func(currentRetry);
+      return await func(totalRetries);
     } catch (e) {
-      if (currentRetry === retries) {
+      if (!conditionFn(e) || (totalRetries > retries)) {
         throw e;
-      } else {
-        const sleep = currentRetry * interval;
-        await new Promise((accept) => setTimeout(accept, sleep));
       }
+      await sleep(totalRetries * interval);
     }
   }
 }


Test patch:
diff --git a/detox/src/utils/exec.test.js b/detox/src/utils/exec.test.js
index f8e66f4ca..987ac1822 100644
--- a/detox/src/utils/exec.test.js
+++ b/detox/src/utils/exec.test.js
@@ -108,11 +108,12 @@ describe('exec', () => {
     cpp.exec.mockReturnValueOnce(rejectedPromise);
 
     try {
-      await exec.execWithRetriesAndLogs('bin', null, '', 1, 1);
+      await exec.execWithRetriesAndLogs('bin', null, '', 0, 1);
       fail('expected execWithRetriesAndLogs() to throw');
     } catch (object) {
       expect(cpp.exec).toHaveBeenCalledWith(`bin`, { timeout: 0 });
       expect(logger.error.mock.calls).toHaveLength(3);
+      expect(logger.error).toHaveBeenCalledWith(expect.objectContaining({event: 'EXEC_FAIL'}), expect.anything());
     }
   });
 
@@ -122,7 +123,7 @@ describe('exec', () => {
     cpp.exec.mockReturnValueOnce(rejectedPromise);
 
     try {
-      await exec.execWithRetriesAndLogs('bin', { verbosity: 'low' }, '', 1, 1);
+      await exec.execWithRetriesAndLogs('bin', { verbosity: 'low' }, '', 0, 1);
       fail('expected execWithRetriesAndLogs() to throw');
     } catch (object) {
       expect(cpp.exec).toHaveBeenCalledWith(`bin`, { timeout: 0 });
@@ -137,7 +138,7 @@ describe('exec', () => {
     cpp.exec.mockReturnValueOnce(rejectedPromise);
 
     try {
-      await exec.execWithRetriesAndLogs('bin', { timeout: 1 }, '', 1, 1);
+      await exec.execWithRetriesAndLogs('bin', { timeout: 1 }, '', 0, 1);
       fail('expected execWithRetriesAndLogs() to throw');
     } catch (object) {
       expect(cpp.exec).toHaveBeenCalledWith(`bin`, { timeout: 1 });
@@ -155,7 +156,7 @@ describe('exec', () => {
        .mockReturnValueOnce(rejectedPromise)
        .mockReturnValueOnce(rejectedPromise);
     try {
-      await exec.execWithRetriesAndLogs('bin', null, '', 6, 1);
+      await exec.execWithRetriesAndLogs('bin', null, '', 5, 1);
       fail('expected execWithRetriesAndLogs() to throw');
     } catch (object) {
       expect(cpp.exec).toHaveBeenCalledWith(`bin`, { timeout: 0 });
diff --git a/detox/src/utils/retry.test.js b/detox/src/utils/retry.test.js
index b0285064b..0f079df8e 100644
--- a/detox/src/utils/retry.test.js
+++ b/detox/src/utils/retry.test.js
@@ -1,35 +1,91 @@
 describe('retry', () => {
+  let sleep;
   let retry;
 
   beforeEach(() => {
+    jest.mock('./sleep', () => jest.fn().mockReturnValue(Promise.resolve()));
+    sleep = require('./sleep');
+
     retry = require('./retry');
   });
 
-  it(`a promise that rejects two times and then resolves, with default params`, async() => {
+  it('should retry once over a function that fails once', async () => {
     const mockFnc = jest.fn()
                         .mockReturnValueOnce(Promise.reject())
                         .mockReturnValueOnce(Promise.resolve());
-    await retry(mockFnc);
+
+    try {
+      await retry({retries: 999, interval: 0}, mockFnc);
+    } catch (e) {
+      fail('expected retry not to fail');
+    }
+
     expect(mockFnc).toHaveBeenCalledTimes(2);
   });
 
-  it(`a promise that rejects two times and then resolves, with custom params`, async() => {
+  it('should retry multiple times', async () => {
     const mockFnc = jest.fn()
-                        .mockReturnValueOnce(Promise.reject())
+                        .mockReturnValueOnce(Promise.reject('once'))
+                        .mockReturnValueOnce(Promise.reject('twice'))
                         .mockReturnValueOnce(Promise.resolve());
-    await retry({retries: 2, interval: 1}, mockFnc);
-    expect(mockFnc).toHaveBeenCalledTimes(2);
+    await retry({retries: 999, interval: 0}, mockFnc);
+    expect(mockFnc).toHaveBeenCalledTimes(3);
   });
 
-  it(`a promise that rejects two times, with two retries`, async() => {
+  it('should adhere to retries parameter', async () => {
     const mockFn = jest.fn()
                        .mockReturnValue(Promise.reject(new Error('a thing')));
     try {
       await retry({retries: 2, interval: 1}, mockFn);
-      fail('expected retry to fail to throw');
-    } catch (object) {
-      expect(mockFn).toHaveBeenCalledTimes(2);
-      expect(object).toBeDefined();
+      fail('expected retry to fail and throw');
+    } catch (error) {
+      expect(mockFn).toHaveBeenCalledTimes(3);
+      expect(error).toBeDefined();
+    }
+  });
+
+  it('should adhere to interval parameter, and sleep for increasingly long intervals', async () => {
+    const baseInterval = 111;
+    const mockFn = jest.fn().mockReturnValue(Promise.reject(new Error('a thing')));
+
+    try {
+      await retry({retries: 2, interval: baseInterval}, mockFn);
+      fail('expected retry to fail and throw');
+    } catch (error) {
+    }
+
+    expect(sleep).toHaveBeenCalledTimes(2);
+    expect(sleep).toHaveBeenCalledWith(baseInterval);
+    expect(sleep).toHaveBeenCalledWith(baseInterval * 2);
+  });
+
+  it('should adhere to a custom condition', async () => {
+    const mockFn = jest.fn()
+                       .mockReturnValue(Promise.reject(new Error('a thing')));
+    const conditionFn = jest.fn()
+                            .mockReturnValueOnce(true)
+                            .mockReturnValueOnce(false);
+
+    try {
+      await retry({retries: 999, interval: 1, conditionFn}, mockFn);
+      fail('expected retry to fail and throw');
+    } catch (error) {
+    }
+
+    expect(mockFn).toHaveBeenCalledTimes(2);
+  });
+
+  it('should work with default retries+interval values', async () => {
+    const defaultRetries = 9;
+    const defaultInterval = 500;
+    const mockFn = jest.fn().mockReturnValue(Promise.reject(new Error('a thing')));
+
+    try {
+      await retry(mockFn);
+      fail('expected retry to fail and throw');
+    } catch (error) {
     }
+    expect(mockFn).toHaveBeenCalledTimes(defaultRetries + 1);
+    expect(sleep).toHaveBeenCalledWith(defaultInterval);
   });
 });