Repository: wix/Detox
Pull request #2437 (2020-10-28 13:43:57)

Merge execWithRetriesAndLogs's statusLog arg into options
**Description:**
A small follow-up on #2434

noomorph: This change involves a lot of signatures changes. I'll make sure to review it as well tomorrow.

Patch:
diff --git a/detox/src/devices/drivers/android/exec/AAPT.js b/detox/src/devices/drivers/android/exec/AAPT.js
index 072022058..fb5928fa7 100644
--- a/detox/src/devices/drivers/android/exec/AAPT.js
+++ b/detox/src/devices/drivers/android/exec/AAPT.js
@@ -16,7 +16,7 @@ class AAPT {
     await this._prepare();
     const process = await exec(
       `${this.aaptBin} dump badging "${escape(apkPath)}" | ${egrep("package: name=")}`,
-      undefined, undefined, 1
+      { retries: 1 }
     );
 
     const packageName = new RegExp(/package: name='([^']+)'/g).exec(process.stdout);
diff --git a/detox/src/devices/drivers/android/exec/ADB.js b/detox/src/devices/drivers/android/exec/ADB.js
index e72482588..416d6ae70 100644
--- a/detox/src/devices/drivers/android/exec/ADB.js
+++ b/detox/src/devices/drivers/android/exec/ADB.js
@@ -271,13 +271,14 @@ class ADB {
   }
 
   // TODO refactor the whole thing so as to make usage of BinaryExec -- similar to EmulatorExec
-  async adbCmd(deviceId, params, options) {
+  async adbCmd(deviceId, params, options = {}) {
     const serial = `${deviceId ? `-s ${deviceId}` : ''}`;
     const cmd = `"${this.adbBin}" ${serial} ${params}`;
-    const retries = _.get(options, 'retries', 1);
-    _.unset(options, 'retries');
-
-    return execWithRetriesAndLogs(cmd, options, undefined, retries);
+    const _options = {
+      retries: 1,
+      ...options,
+    }
+    return execWithRetriesAndLogs(cmd, _options);
   }
 
   /***
diff --git a/detox/src/devices/drivers/ios/tools/AppleSimUtils.js b/detox/src/devices/drivers/ios/tools/AppleSimUtils.js
index 0dce5c864..baaa9da39 100644
--- a/detox/src/devices/drivers/ios/tools/AppleSimUtils.js
+++ b/detox/src/devices/drivers/ios/tools/AppleSimUtils.js
@@ -7,25 +7,30 @@ const environment = require('../../../../utils/environment');
 
 class AppleSimUtils {
   async setPermissions(udid, bundleId, permissionsObj) {
-    const statusLogs = {
-      trying: `Trying to set permissions...`,
-      successful: 'Permissions are set'
-    };
     let permissions = [];
     _.forEach(permissionsObj, function (shouldAllow, permission) {
       permissions.push(permission + '=' + shouldAllow);
     });
-    await this._execAppleSimUtils({
-      args: `--byId ${udid} --bundle ${bundleId} --restartSB --setPermissions ${_.join(permissions, ',')}`
-    }, statusLogs, 1);
+
+    const options = {
+      args: `--byId ${udid} --bundle ${bundleId} --restartSB --setPermissions ${_.join(permissions, ',')}`,
+      statusLogs: {
+        trying: `Trying to set permissions...`,
+        successful: 'Permissions are set'
+      },
+      retries: 1,
+    }
+    await this._execAppleSimUtils(options);
   }
 
-  async list(query, options = {}) {
-    const args = `--list ${joinArgs(query)}`;
-    const statusLogs = options.trying ? { trying: options.trying } : undefined;
-    const response = await this._execAppleSimUtils({ args }, statusLogs,  1);
+  async list(query, listOptions = {}) {
+    const options = {
+      args: `--list ${joinArgs(query)}`,
+      retries: 1,
+      statusLogs: listOptions.trying ? { trying: listOptions.trying } : undefined,
+    };
+    const response = await this._execAppleSimUtils(options);
     const parsed = this._parseResponseFromAppleSimUtils(response);
-
     return parsed;
   }
 
@@ -119,45 +124,61 @@ class AppleSimUtils {
     if (!_.includes(['Face', 'Finger'], matchType)) {
       return;
     }
-    const statusLogs = {
-      trying: `Trying to match ${matchType}...`,
-      successful: `Matched ${matchType}!`
-    };
 
-    await this._execAppleSimUtils({ args: `--byId ${udid} --match${matchType}` }, statusLogs, 1);
+    const options = {
+      args: `--byId ${udid} --match${matchType}`,
+      retries: 1,
+      statusLogs: {
+        trying: `Trying to match ${matchType}...`,
+        successful: `Matched ${matchType}!`
+      },
+    };
+    await this._execAppleSimUtils(options);
   }
 
   async unmatchBiometric(udid, matchType) {
     if (!_.includes(['Face', 'Finger'], matchType)) {
       return;
     }
-    const statusLogs = {
-      trying: `Trying to unmatch ${matchType}...`,
-      successful: `Unmatched ${matchType}!`
-    };
 
-    await this._execAppleSimUtils({ args: `--byId ${udid} --unmatch${matchType}` }, statusLogs, 1);
+    const options = {
+      args: `--byId ${udid} --unmatch${matchType}`,
+      retries: 1,
+      statusLogs: {
+        trying: `Trying to unmatch ${matchType}...`,
+        successful: `Unmatched ${matchType}!`
+      },
+    }
+    await this._execAppleSimUtils(options);
   }
 
   async setBiometricEnrollment(udid, yesOrNo) {
     if (!_.includes(['YES', 'NO'], yesOrNo)) {
       return;
     }
-    let toggle = yesOrNo === 'YES'
-    const statusLogs = {
-      trying: `Turning ${toggle ? 'on' : 'off'} biometric enrollment...`,
-      successful: toggle ? 'Activated!' : 'Deactivated!'
-    };
-    await this._execAppleSimUtils({ args: `--byId ${udid} --biometricEnrollment ${yesOrNo}` }, statusLogs, 1);
+
+    const toggle = yesOrNo === 'YES';
+    const options = {
+      args: `--byId ${udid} --biometricEnrollment ${yesOrNo}`,
+      retries: 1,
+      statusLogs: {
+        trying: `Turning ${toggle ? 'on' : 'off'} biometric enrollment...`,
+        successful: toggle ? 'Activated!' : 'Deactivated!'
+      },
+    }
+    await this._execAppleSimUtils(options);
   }
 
   async clearKeychain(udid) {
-    const statusLogs = {
-      trying: `Clearing Keychain...`,
-      successful: 'Cleared Keychain!'
-    };
-
-    await this._execAppleSimUtils({ args: `--byId ${udid} --clearKeychain` }, statusLogs, 1);
+    const options = {
+      args: `--byId ${udid} --clearKeychain`,
+      retries: 1,
+      statusLogs: {
+        trying: `Clearing Keychain...`,
+        successful: 'Cleared Keychain!'
+      },
+    }
+    await this._execAppleSimUtils(options);
   }
 
   async getAppContainer(udid, bundleId) {
@@ -211,9 +232,9 @@ class AppleSimUtils {
   }
 
   async setLocation(udid, lat, lon) {
-    const result = await exec.execWithRetriesAndLogs(`which fbsimctl`, undefined, undefined, 1);
+    const result = await exec.execWithRetriesAndLogs(`which fbsimctl`, { retries: 1 });
     if (_.get(result, 'stdout')) {
-      await exec.execWithRetriesAndLogs(`fbsimctl ${udid} set_location ${lat} ${lon}`, undefined, undefined, 1);
+      await exec.execWithRetriesAndLogs(`fbsimctl ${udid} set_location ${lat} ${lon}`, { retries: 1 });
     } else {
       throw new Error(`setLocation currently supported only through fbsimctl.
       Install fbsimctl using:
@@ -241,14 +262,18 @@ class AppleSimUtils {
     return exec.spawnAndLog('/usr/bin/xcrun', args);
   }
 
-  async _execAppleSimUtils(options, statusLogs, retries, interval) {
+  async _execAppleSimUtils(options) {
     const bin = `applesimutils`;
-    return await exec.execWithRetriesAndLogs(bin, options, statusLogs, retries, interval);
+    return await exec.execWithRetriesAndLogs(bin, options);
   }
 
   async _execSimctl({ cmd, statusLogs = {}, retries = 1, silent = false }) {
-    const verbosity = silent ? 'low' : 'normal';
-    return await exec.execWithRetriesAndLogs(`/usr/bin/xcrun simctl ${cmd}`, { verbosity }, statusLogs, retries);
+    const options = {
+      verbosity: silent ? 'low' : 'normal',
+      statusLogs,
+      retries,
+    }
+    return await exec.execWithRetriesAndLogs(`/usr/bin/xcrun simctl ${cmd}`, options);
   }
 
   _parseResponseFromAppleSimUtils(response) {
@@ -278,10 +303,6 @@ class AppleSimUtils {
   async _launchMagically(frameworkPath, udid, bundleId, launchArgs, languageAndLocale) {
     const args = this._joinLaunchArgs(launchArgs);
 
-    const statusLogs = {
-      trying: `Launching ${bundleId}...`,
-    };
-
     let dylibs = `${frameworkPath}/Detox`;
     if (process.env.SIMCTL_CHILD_DYLD_INSERT_LIBRARIES) {
       dylibs = `${process.env.SIMCTL_CHILD_DYLD_INSERT_LIBRARIES}:${dylibs}`;
@@ -298,8 +319,13 @@ class AppleSimUtils {
         launchBin += ` -AppleLocale ${languageAndLocale.locale}`;
       }
 
-    const result = await exec.execWithRetriesAndLogs(launchBin, undefined, statusLogs, 1);
-
+    const options = {
+      retries: 1,
+      statusLogs: {
+        trying: `Launching ${bundleId}...`,
+      },
+    };
+    const result = await exec.execWithRetriesAndLogs(launchBin, options);
     return result;
   }
 
diff --git a/detox/src/utils/exec.js b/detox/src/utils/exec.js
index 9fb5d9ec7..68f6a6225 100644
--- a/detox/src/utils/exec.js
+++ b/detox/src/utils/exec.js
@@ -7,12 +7,20 @@ const DetoxRuntimeError = require('../errors/DetoxRuntimeError');
 
 let _operationCounter = 0;
 
-async function execWithRetriesAndLogs(bin, options, statusLogs = {}, retries = 9, interval = 1000) {
+async function execWithRetriesAndLogs(bin, options = {}) {
+  const {
+    retries = 9,
+    interval = 1000,
+    prefix = null,
+    args = null,
+    timeout = 0,
+    statusLogs = {},
+    verbosity = 'normal',
+  } = options;
+
   const trackingId = _operationCounter++;
-  const cmd = _composeCommand(bin, options);
-  const execTimeout = _.get(options, 'timeout', 0);
+  const cmd = _composeCommand(bin, prefix, args);
   const log = execLogger.child({ fn: 'execWithRetriesAndLogs', cmd, trackingId });
-  const verbosity = _.get(options, 'verbosity', 'normal');
 
   let result;
   try {
@@ -24,11 +32,11 @@ async function execWithRetriesAndLogs(bin, options, statusLogs = {}, retries = 9
       } else if (statusLogs.retrying) {
         _logRetrying(log, cmd, retryNumber, lastError);
       }
-      result = await exec(cmd, { timeout: execTimeout });
+      result = await exec(cmd, { timeout });
     });
   } catch (err) {
-    const _failReason = err.code == null && execTimeout > 0
-      ? `timeout = ${execTimeout}ms`
+    const _failReason = err.code == null && timeout > 0
+      ? `timeout = ${timeout}ms`
       : `code = ${err.code}`;
 
     const level = (verbosity === 'low' ? 'debug' : 'error');
@@ -104,15 +112,15 @@ function _logRetrying(log, message, retryNumber, lastError) {
   }
 }
 
-function _composeCommand(bin, options) {
-  if (!options) {
+function _composeCommand(bin, prefix, args) {
+  if (!(prefix || args)) {
     return bin;
   }
 
-  const prefix = options.prefix ? `${options.prefix} && ` : '';
-  const args = options.args ? ` ${options.args}` : '';
+  const _prefix = prefix ? `${prefix} && ` : '';
+  const _args = args ? ` ${args}` : '';
 
-  return `${prefix}${bin}${args}`;
+  return `${_prefix}${bin}${_args}`;
 }
 
 function spawnAndLog(command, flags, options) {
diff --git a/detox/src/utils/retry.js b/detox/src/utils/retry.js
index b34bde997..40c876962 100644
--- a/detox/src/utils/retry.js
+++ b/detox/src/utils/retry.js
@@ -1,4 +1,3 @@
-const _ = require('lodash');
 const sleep = require('./sleep');
 
 const DEFAULT_RETRIES = 9;


Test patch:
diff --git a/detox/src/devices/drivers/android/exec/ADB.test.js b/detox/src/devices/drivers/android/exec/ADB.test.js
index 4df29e28d..2f0d31436 100644
--- a/detox/src/devices/drivers/android/exec/ADB.test.js
+++ b/detox/src/devices/drivers/android/exec/ADB.test.js
@@ -45,7 +45,7 @@ describe('ADB', () => {
 
     it(`should invoke ADB`, async () => {
       await adb.devices();
-      expect(exec).toHaveBeenCalledWith(`"${adbBinPath}"  devices`, { verbosity: 'high' }, undefined, 1);
+      expect(exec).toHaveBeenCalledWith(`"${adbBinPath}"  devices`, { verbosity: 'high', retries: 1 });
       expect(exec).toHaveBeenCalledTimes(1);
     });
 
@@ -83,11 +83,11 @@ describe('ADB', () => {
 
     expect(exec).toHaveBeenCalledWith(
       expect.stringContaining('adb" -s emulator-5556 shell "getprop ro.build.version.sdk"'),
-      {}, undefined, 5);
+      { retries: 5 });
 
     expect(exec).toHaveBeenCalledWith(
       expect.stringContaining('adb" -s emulator-5556 install -rg "path inside \\"quotes\\" to/app"'),
-      undefined, undefined, 1);
+      { retries: 1 });
   });
 
   it(`uninstall`, async () => {
@@ -120,7 +120,7 @@ describe('ADB', () => {
 
     expect(exec).toHaveBeenCalledWith(
       expect.stringContaining(`-s mockEmulator push "${sourceFile}" "${destFile}"`),
-      undefined, undefined, expect.anything());
+      expect.anything());
   });
 
   it('remote-install', async () => {
@@ -130,7 +130,7 @@ describe('ADB', () => {
 
     expect(exec).toHaveBeenCalledWith(
       expect.stringContaining(`-s mockEmulator shell "pm install -r -g -t ${binaryPath}"`),
-      undefined, undefined, expect.anything());
+      expect.anything());
   });
 
   describe('unlockScreen', () => {
@@ -259,17 +259,17 @@ describe('ADB', () => {
   describe('animation disabling', () => {
     it('should disable animator (e.g. ObjectAnimator) animations', async () => {
       await adb.disableAndroidAnimations();
-      expect(exec).toHaveBeenCalledWith(`"${adbBinPath}"  shell "settings put global animator_duration_scale 0"`, undefined, undefined, 1);
+      expect(exec).toHaveBeenCalledWith(`"${adbBinPath}"  shell "settings put global animator_duration_scale 0"`, { retries: 1 });
     });
 
     it('should disable window animations', async () => {
       await adb.disableAndroidAnimations();
-      expect(exec).toHaveBeenCalledWith(`"${adbBinPath}"  shell "settings put global window_animation_scale 0"`, undefined, undefined, 1);
+      expect(exec).toHaveBeenCalledWith(`"${adbBinPath}"  shell "settings put global window_animation_scale 0"`, { retries: 1 });
     });
 
     it('should disable transition (e.g. activity launch) animations', async () => {
       await adb.disableAndroidAnimations();
-      expect(exec).toHaveBeenCalledWith(`"${adbBinPath}"  shell "settings put global transition_animation_scale 0"`, undefined, undefined, 1);
+      expect(exec).toHaveBeenCalledWith(`"${adbBinPath}"  shell "settings put global transition_animation_scale 0"`, { retries: 1 });
     });
   });
 });
diff --git a/detox/src/utils/exec.test.js b/detox/src/utils/exec.test.js
index 263ddd0f3..a8849d8fb 100644
--- a/detox/src/utils/exec.test.js
+++ b/detox/src/utils/exec.test.js
@@ -46,21 +46,32 @@ describe('exec', () => {
     expect(cpp.exec).toHaveBeenCalledWith(`export MY_PREFIX && bin --argument 123`, { timeout: 0 });
   });
 
+  it(`exec command with prefix (no args) successfully`, async () => {
+    mockCppSuccessful(cpp);
+
+    const options = {prefix: `export MY_PREFIX`};
+    await exec.execWithRetriesAndLogs('bin', options);
+
+    expect(cpp.exec).toHaveBeenCalledWith(`export MY_PREFIX && bin`, { timeout: 0 });
+  });
+
   it(`exec command with arguments and try-based status logs successfully, with status logging`, async () => {
     cpp.exec
       .mockRejectedValueOnce(returnErrorWithValue('error result'))
       .mockResolvedValueOnce(returnSuccessfulWithValue('successful result'));
 
-    const options = {args: `--argument 123`};
-    const statusLogs = {
-      trying: 'trying status log',
-      successful: 'successful status log',
+    const options = {
+      args: `--argument 123`,
+      statusLogs: {
+        trying: 'trying status log',
+        successful: 'successful status log',
+      },
     };
-    await exec.execWithRetriesAndLogs('bin', options, statusLogs);
+    await exec.execWithRetriesAndLogs('bin', options);
 
     expect(cpp.exec).toHaveBeenCalledWith(`bin --argument 123`, { timeout: 0 });
-    expect(logger.debug).toHaveBeenCalledWith({ event: 'EXEC_TRY', retryNumber: 1}, statusLogs.trying);
-    expect(logger.debug).toHaveBeenCalledWith({ event: 'EXEC_TRY', retryNumber: 2}, statusLogs.trying);
+    expect(logger.debug).toHaveBeenCalledWith({ event: 'EXEC_TRY', retryNumber: 1}, options.statusLogs.trying);
+    expect(logger.debug).toHaveBeenCalledWith({ event: 'EXEC_TRY', retryNumber: 2}, options.statusLogs.trying);
     expect(logger.trace).toHaveBeenCalledWith({ event: 'EXEC_TRY_FAIL' }, 'error result');
   });
 
@@ -69,11 +80,13 @@ describe('exec', () => {
       .mockRejectedValueOnce(returnErrorWithValue('error result'))
       .mockResolvedValueOnce(returnSuccessfulWithValue('successful result'));
 
-    const options = {args: `--argument 123`};
-    const statusLogs = {
-      retrying: true,
+    const options = {
+      args: `--argument 123`,
+      statusLogs: {
+        retrying: true,
+      },
     };
-    await exec.execWithRetriesAndLogs('bin', options, statusLogs);
+    await exec.execWithRetriesAndLogs('bin', options);
 
     expect(cpp.exec).toHaveBeenCalledWith(`bin --argument 123`, { timeout: 0 });
     expect(logger.debug).toHaveBeenCalledWith({ event: 'EXEC_RETRY', retryNumber: 2}, '(Retry #1)', 'bin --argument 123');
@@ -126,7 +139,7 @@ describe('exec', () => {
     mockCppFailure(cpp);
 
     try {
-      await exec.execWithRetriesAndLogs('bin', null, '', 0, 1);
+      await exec.execWithRetriesAndLogs('bin', { retries: 0, interval: 1 });
       fail('expected execWithRetriesAndLogs() to throw');
     } catch (object) {
       expect(cpp.exec).toHaveBeenCalledWith(`bin`, { timeout: 0 });
@@ -139,7 +152,7 @@ describe('exec', () => {
     mockCppFailure(cpp);
 
     try {
-      await exec.execWithRetriesAndLogs('bin', { verbosity: 'low' }, '', 0, 1);
+      await exec.execWithRetriesAndLogs('bin', { verbosity: 'low', retries: 0, interval: 1 });
       fail('expected execWithRetriesAndLogs() to throw');
     } catch (object) {
       expect(cpp.exec).toHaveBeenCalledWith(`bin`, { timeout: 0 });
@@ -152,7 +165,7 @@ describe('exec', () => {
     mockCppFailure(cpp);
 
     try {
-      await exec.execWithRetriesAndLogs('bin', { timeout: 1 }, '', 0, 1);
+      await exec.execWithRetriesAndLogs('bin', { timeout: 1, retries: 0, interval: 1 });
       fail('expected execWithRetriesAndLogs() to throw');
     } catch (object) {
       expect(cpp.exec).toHaveBeenCalledWith(`bin`, { timeout: 1 });
@@ -171,7 +184,7 @@ describe('exec', () => {
       .mockRejectedValueOnce(errorResult);
 
     try {
-      await exec.execWithRetriesAndLogs('bin', null, '', 5, 1);
+      await exec.execWithRetriesAndLogs('bin', { retries: 5, interval: 1 });
       fail('expected execWithRetriesAndLogs() to throw');
     } catch (object) {
       expect(cpp.exec).toHaveBeenCalledWith(`bin`, { timeout: 0 });
@@ -192,7 +205,7 @@ describe('exec', () => {
       .mockRejectedValueOnce(errorResult)
       .mockResolvedValueOnce(successfulResult);
 
-    await exec.execWithRetriesAndLogs('bin', null, '', 6, 1);
+    await exec.execWithRetriesAndLogs('bin', { retries: 6, interval: 1 });
     expect(cpp.exec).toHaveBeenCalledWith(`bin`, { timeout: 0 });
     expect(cpp.exec).toHaveBeenCalledTimes(6);
   });