Repository: wix/Detox
Pull request #2148 (2020-06-24 08:00:25)

Refactor instrumentation-args preparation
- [ ] This is a small change 
- [ ] This change has been discussed in issue #<?> and the solution has been agreed upon with maintainers.

---

**Description:**
Following work on #1192 -- need to break some of the AndroidDriver monolith into sub-units. This is prep-step 0.2, following #2146: refactor instrumentation launch-arguments preparation logic onto a dedicated module.


Patch:
diff --git a/detox/src/devices/drivers/android/AndroidDriver.js b/detox/src/devices/drivers/android/AndroidDriver.js
index 1a053709f..5570fd1bc 100644
--- a/detox/src/devices/drivers/android/AndroidDriver.js
+++ b/detox/src/devices/drivers/android/AndroidDriver.js
@@ -1,6 +1,7 @@
 const fs = require('fs');
 const URL = require('url').URL;
 const _ = require('lodash');
+const DeviceDriverBase = require('../DeviceDriverBase');
 const { encodeBase64 } = require('../../../utils/encoding');
 const logger = require('../../../utils/logger');
 const log = logger.child({ __filename });
@@ -11,7 +12,7 @@ const AAPT = require('./exec/AAPT');
 const APKPath = require('./tools/APKPath');
 const TempFileXfer = require('./tools/TempFileXfer');
 const AppUninstallHelper = require('./tools/AppUninstallHelper');
-const DeviceDriverBase = require('../DeviceDriverBase');
+const { prepareInstrumentationArgs } = require('./tools/instrumentationArgs');
 const DetoxApi = require('../../../android/espressoapi/Detox');
 const EspressoDetoxApi = require('../../../android/espressoapi/EspressoDetox');
 const UiDeviceProxy = require('../../../android/espressoapi/UiDeviceProxy');
@@ -273,12 +274,14 @@ class AndroidDriver extends DeviceDriverBase {
   }
 
   async _launchInstrumentationProcess(deviceId, bundleId, rawLaunchArgs) {
-    const launchArgs = this._prepareLaunchArgs(rawLaunchArgs, true);
-    const additionalLaunchArgs = this._prepareLaunchArgs({debug: false});
+    const launchArgs = prepareInstrumentationArgs(rawLaunchArgs);
+    const additionalLaunchArgs = prepareInstrumentationArgs({ debug: false });
+    this._warnReservedArgsUsedIfNeeded(launchArgs);
+
     const serverPort = new URL(this.client.configuration.server).port;
     await this.adb.reverse(deviceId, serverPort);
     const testRunner = await this.adb.getInstrumentationRunner(deviceId, bundleId);
-    const spawnFlags = [...launchArgs, ...additionalLaunchArgs];
+    const spawnFlags = [...launchArgs.args, ...additionalLaunchArgs.args];
 
     this.instrumentationLogsParser = new InstrumentationLogsParser();
     this.instrumentationProcess = this.adb.spawnInstrumentation(deviceId, spawnFlags, testRunner);
@@ -333,30 +336,14 @@ class AndroidDriver extends DeviceDriverBase {
     return this.invocationManager.execute(DetoxApi.launchMainActivity());
   }
 
-  _prepareLaunchArgs(launchArgs, verbose = false) {
-    const usedReservedArgs = [];
-    const preparedLaunchArgs = _.reduce(launchArgs, (result, value, key) => {
-      const valueAsString = _.isString(value) ? value : JSON.stringify(value);
-
-      let valueEncoded = valueAsString;
-      if (isReservedInstrumentationArg(key)) {
-        usedReservedArgs.push(key);
-      } else if (!key.startsWith('detox')) {
-        valueEncoded = encodeBase64(valueAsString);
-      }
-
-      result.push('-e', key, valueEncoded);
-      return result;
-    }, []);
-
-    if (verbose && usedReservedArgs.length) {
-      logger.warn([`Arguments [${usedReservedArgs}] were passed in as launchArgs to device.launchApp() `,
-                   'but are reserved to Android\'s test-instrumentation and will not be passed into the app. ',
-                   'Ignore this message if this is what you meant to do. Refer to ',
-                   'https://developer.android.com/studio/test/command-line#AMOptionsSyntax for ',
-                   'further details.'].join(''));
+  _warnReservedArgsUsedIfNeeded(preparedArgs) {
+    if (preparedArgs.usedReservedArgs.length) {
+      logger.warn([`Arguments [${preparedArgs.usedReservedArgs}] were passed in as launchArgs to device.launchApp() `,
+        'but are reserved to Android\'s test-instrumentation and will not be passed into the app. ',
+        'Ignore this message if this is what you meant to do. Refer to ',
+        'https://developer.android.com/studio/test/command-line#AMOptionsSyntax for ',
+        'further details.'].join(''));
     }
-    return preparedLaunchArgs;
   }
 }
 
diff --git a/detox/src/devices/drivers/android/tools/instrumentationArgs.js b/detox/src/devices/drivers/android/tools/instrumentationArgs.js
new file mode 100644
index 000000000..06af4a296
--- /dev/null
+++ b/detox/src/devices/drivers/android/tools/instrumentationArgs.js
@@ -0,0 +1,31 @@
+const _ = require('lodash');
+const { encodeBase64 } = require('../../../../utils/encoding');
+
+const reservedInstrumentationArgs = ['class', 'package', 'func', 'unit', 'size', 'perf', 'debug', 'log', 'emma', 'coverageFile'];
+const isReservedInstrumentationArg = (arg) => reservedInstrumentationArgs.includes(arg);
+
+function prepareInstrumentationArgs(args) {
+  const usedReservedArgs = [];
+  const preparedLaunchArgs = _.reduce(args, (result, value, key) => {
+    const valueAsString = _.isString(value) ? value : JSON.stringify(value);
+
+    let valueEncoded = valueAsString;
+    if (isReservedInstrumentationArg(key)) {
+      usedReservedArgs.push(key);
+    } else if (!key.startsWith('detox')) {
+      valueEncoded = encodeBase64(valueAsString)
+    }
+
+    result.push('-e', key, valueEncoded);
+    return result;
+  }, []);
+
+  return {
+    args: preparedLaunchArgs,
+    usedReservedArgs,
+  };
+}
+
+module.exports = {
+  prepareInstrumentationArgs,
+};


Test patch:
diff --git a/detox/src/devices/drivers/android/AndroidDriver.test.js b/detox/src/devices/drivers/android/AndroidDriver.test.js
index 01c4d2ace..8f91a8172 100644
--- a/detox/src/devices/drivers/android/AndroidDriver.test.js
+++ b/detox/src/devices/drivers/android/AndroidDriver.test.js
@@ -46,6 +46,7 @@ describe('Android driver', () => {
   let fs;
   let exec;
   let detoxApi;
+  let instrumentationArgs;
   beforeEach(() => {
     jest.mock('fs', () => ({
       existsSync: jest.fn(),
@@ -96,6 +97,10 @@ describe('Android driver', () => {
 
     jest.mock('../../../android/espressoapi/Detox');
     detoxApi = require('../../../android/espressoapi/Detox');
+
+    jest.mock('./tools/instrumentationArgs');
+    instrumentationArgs = require('./tools/instrumentationArgs');
+    instrumentationArgs.prepareInstrumentationArgs.mockReturnValue({args: [], usedReservedArgs: []});
   });
 
   let adb;
@@ -168,11 +173,8 @@ describe('Android driver', () => {
     }
     const assertActivityStartNotInvoked = () => expect(detoxApi.startActivityFromUrl).not.toHaveBeenCalled();
 
-    const assertInstrumentationSpawned = () => expect(adb.spawnInstrumentation).toHaveBeenCalledWith(
-      expect.anything(),
-      expect.arrayContaining(['detoxURLOverride', detoxURLOverride]),
-      expect.anything(),
-    );
+    const assertArgumentsPreparationWith = (args) => expect(instrumentationArgs.prepareInstrumentationArgs).toHaveBeenCalledWith(args);
+    const assertInstrumentationSpawned = () => expect(adb.spawnInstrumentation).toHaveBeenCalled();
     const resetInstrumentationMock = () => adb.spawnInstrumentation.mockReset();
     const assertInstrumentationNotSpawned = () => expect(adb.spawnInstrumentation).not.toHaveBeenCalled();
 
@@ -186,10 +188,11 @@ describe('Android driver', () => {
 
         await uut.launchApp(deviceId, bundleId, args, '');
 
+        assertArgumentsPreparationWith(args);
         assertInstrumentationSpawned();
       });
 
-      it('should start the app with URL using invocation-manager', async () => {
+      it('should start the app with URL via invocation-manager', async () => {
         mockStartActivityInvokeApi();
 
         await uut.launchApp(deviceId, bundleId, {}, '');
@@ -211,7 +214,7 @@ describe('Android driver', () => {
         delayPayload: true,
       }
 
-      it('should start the app using invocation-manager', async () => {
+      it('should start the app via invocation-manager', async () => {
         mockStartActivityInvokeApi();
 
         await uut.launchApp(deviceId, bundleId, {}, '')
@@ -220,7 +223,7 @@ describe('Android driver', () => {
         assertActivityStartInvoked();
       });
 
-      it('should not start the app using invocation-manager', async () => {
+      it('should not start the app via invocation-manager', async () => {
         mockStartActivityInvokeApi();
 
         await uut.launchApp(deviceId, bundleId, {}, '')
@@ -248,7 +251,10 @@ describe('Android driver', () => {
     const assertActivityStartNotInvoked = () => {
       expect(detoxApi.startActivityFromNotification).not.toHaveBeenCalled();
     }
+
+    const assertArgumentsPreparationWith = (args) => expect(instrumentationArgs.prepareInstrumentationArgs).toHaveBeenCalledWith(args);
     const resetInstrumentationMock = () => adb.spawnInstrumentation.mockReset();
+    const assertInstrumentationSpawned = () => expect(adb.spawnInstrumentation).toHaveBeenCalled();
     const assertInstrumentationNotSpawned = () => expect(adb.spawnInstrumentation).not.toHaveBeenCalled();
 
     describe('in app launch (with dedicated arg)', () => {
@@ -271,16 +277,13 @@ describe('Android driver', () => {
         }
       });
 
-      it('should launch instrument with a modified notification data URL arg', async () => {
+      it('should launch instrumentation with a modified notification data URL arg', async () => {
         fileXfer.send.mockReturnValue(mockNotificationDataTargetPath);
 
         await uut.launchApp(deviceId, bundleId, notificationArgs, '');
 
-        expect(adb.spawnInstrumentation).toHaveBeenCalledWith(
-          expect.anything(),
-          expect.arrayContaining(['detoxUserNotificationDataURL', mockNotificationDataTargetPath]),
-          undefined,
-        );
+        assertArgumentsPreparationWith({ detoxUserNotificationDataURL: mockNotificationDataTargetPath });
+        assertInstrumentationSpawned();
       });
     });
 
@@ -389,96 +392,56 @@ describe('Android driver', () => {
   });
 
   describe('Launch args', () => {
-    const expectSpawnedFlag = (spawnedFlags) => ({
-      startingIndex: (index) => ({
-        toBe: ({key, value}) => {
-          expect(spawnedFlags[index]).toEqual('-e');
-          expect(spawnedFlags[index + 1]).toEqual(key);
-          expect(spawnedFlags[index + 2]).toEqual(value);
-          return index + 3;
-        }
-      }),
-    });
+    it('should prepare user launch args', async () => {
+      instrumentationArgs.prepareInstrumentationArgs.mockReturnValue({args: [], usedReservedArgs: []});
 
-    it('should base64-encode and stringify arg values', async () => {
       const launchArgs = {
-        'object-arg': {
-          such: 'wow',
-          much: 'amaze',
-          very: 111,
-        },
-        'string-arg': 'text, with commas-and-dashes,',
+        anArg: 'aValue',
       };
-
       await uut.launchApp(deviceId, bundleId, launchArgs, '');
 
-      const spawnArgs = adb.spawnInstrumentation.mock.calls[0];
-      const spawnedFlags = spawnArgs[1];
-
-      let index = 0;
-      index = expectSpawnedFlag(spawnedFlags).startingIndex(index).toBe({
-        key: 'object-arg',
-        value: 'base64({"such":"wow","much":"amaze","very":111})'
-      });
-      expectSpawnedFlag(spawnedFlags).startingIndex(index).toBe({
-        key: 'string-arg',
-        value: 'base64(text, with commas-and-dashes,)'
-      });
+      expect(instrumentationArgs.prepareInstrumentationArgs).toHaveBeenCalledWith(launchArgs);
     });
 
-    // Ref: https://developer.android.com/studio/test/command-line#AMOptionsSyntax
-    it('should whitelist reserved instrumentation args with respect to base64 encoding', async () => {
-      const launchArgs = {
-        // Free arg
-        'user-arg': 'merry christ-nukah',
-
-        // Reserved instrumentation args
-        'class': 'class-value',
-        'package': 'package-value',
-        'func': 'func-value',
-        'unit': 'unit-value',
-        'size': 'size-value',
-        'perf': 'perf-value',
-        'debug': 'debug-value',
-        'log': 'log-value',
-        'emma': 'emma-value',
-        'coverageFile': 'coverageFile-value',
-      };
+    it('should prepare forced debug=false arg', async () => {
+      instrumentationArgs.prepareInstrumentationArgs.mockReturnValue({args: [], usedReservedArgs: []});
+      await uut.launchApp(deviceId, bundleId, {}, '');
+      expect(instrumentationArgs.prepareInstrumentationArgs).toHaveBeenCalledWith({ debug: false });
+    });
 
-      await uut.launchApp(deviceId, bundleId, launchArgs, '');
+    it('should spawn instrumentation with prepared arguments', async () => {
+      const mockedPreparedUserArgs = ['mocked', 'prepared-args'];
+      const mockedPreparedDebugArg = ['debug', 'mocked'];
+      instrumentationArgs.prepareInstrumentationArgs
+        .mockReturnValueOnce({ args: mockedPreparedUserArgs, usedReservedArgs: [] })
+        .mockReturnValueOnce({ args: mockedPreparedDebugArg, usedReservedArgs: [] });
 
-      const spawnArgs = adb.spawnInstrumentation.mock.calls[0];
-      const spawnedFlags = spawnArgs[1];
-
-      let index = 3;
-      index = expectSpawnedFlag(spawnedFlags).startingIndex(index).toBe({ key: 'class', value: 'class-value' });
-      index = expectSpawnedFlag(spawnedFlags).startingIndex(index).toBe({ key: 'package', value: 'package-value' });
-      index = expectSpawnedFlag(spawnedFlags).startingIndex(index).toBe({ key: 'func', value: 'func-value' });
-      index = expectSpawnedFlag(spawnedFlags).startingIndex(index).toBe({ key: 'unit', value: 'unit-value' });
-      index = expectSpawnedFlag(spawnedFlags).startingIndex(index).toBe({ key: 'size', value: 'size-value' });
-      index = expectSpawnedFlag(spawnedFlags).startingIndex(index).toBe({ key: 'perf', value: 'perf-value' });
-      index = expectSpawnedFlag(spawnedFlags).startingIndex(index).toBe({ key: 'debug', value: 'debug-value' });
-      index = expectSpawnedFlag(spawnedFlags).startingIndex(index).toBe({ key: 'log', value: 'log-value' });
-      index = expectSpawnedFlag(spawnedFlags).startingIndex(index).toBe({ key: 'emma', value: 'emma-value' });
-      expectSpawnedFlag(spawnedFlags).startingIndex(index).toBe({ key: 'coverageFile', value: 'coverageFile-value' });
+      await uut.launchApp(deviceId, bundleId, {}, '');
+      expect(adb.spawnInstrumentation).toHaveBeenCalledWith(expect.anything(), [...mockedPreparedUserArgs, ...mockedPreparedDebugArg], undefined);
     });
 
-    it('should log reserved instrumentation args usage warning, if such have been used', async () => {
-      const launchArgs = {
-        'class': 'class-value',
-      };
+    it('should log reserved instrumentation args usage if used in user args', async () => {
+      const mockedPreparedUserArgs = ['mocked', 'prepared-args'];
+      const usedReservedArgs = ['aaa', 'zzz'];
+      const mockedPreparedDebugArg = ['debug', 'mocked'];
+      instrumentationArgs.prepareInstrumentationArgs
+        .mockReturnValueOnce({ args: mockedPreparedUserArgs, usedReservedArgs })
+        .mockReturnValueOnce({ args: mockedPreparedDebugArg, usedReservedArgs: ['shouldnt', 'care'] });
 
-      await uut.launchApp(deviceId, bundleId, launchArgs, '');
+      await uut.launchApp(deviceId, bundleId, {}, '');
 
-      expect(logger.warn).toHaveBeenCalled();
+      expect(logger.warn).toHaveBeenCalledWith(expect.stringContaining('Arguments [aaa,zzz] were passed in as launchArgs to device.launchApp()'));
+      expect(logger.warn).toHaveBeenCalledTimes(1);
     });
 
-    it('should NOT log instrumentation args usage warning, if none used', async () => {
-      const launchArgs = {
-        'user-arg': 'merry christ-nukah',
-      };
+    it('should NOT log reserved instrumentation args usage if none used by user', async () => {
+      const mockedPreparedUserArgs = ['mocked', 'prepared-args'];
+      const mockedPreparedDebugArg = ['debug', 'mocked'];
+      instrumentationArgs.prepareInstrumentationArgs
+        .mockReturnValueOnce({ args: mockedPreparedUserArgs, usedReservedArgs: [] })
+        .mockReturnValueOnce({ args: mockedPreparedDebugArg, usedReservedArgs: ['shouldnt', 'care'] });
 
-      await uut.launchApp(deviceId, bundleId, launchArgs, '');
+      await uut.launchApp(deviceId, bundleId, {}, '');
 
       expect(logger.warn).not.toHaveBeenCalled();
     });
diff --git a/detox/src/devices/drivers/android/tools/instrumentationArgs.test.js b/detox/src/devices/drivers/android/tools/instrumentationArgs.test.js
new file mode 100644
index 000000000..d7e5de319
--- /dev/null
+++ b/detox/src/devices/drivers/android/tools/instrumentationArgs.test.js
@@ -0,0 +1,108 @@
+describe('Instrumentation arguments', () => {
+  const mockEncodeBase64Fn = (arg) => `base64Mocked(${arg})`;
+
+  let uut;
+  beforeEach(() => {
+    let encoding;
+    jest.mock('../../../../utils/encoding');
+    encoding = require('../../../../utils/encoding');
+    encoding.encodeBase64.mockImplementation(mockEncodeBase64Fn);
+
+    uut = require('./instrumentationArgs');
+  });
+
+  it('should prepare arguments with no-args', () => {
+    const result = uut.prepareInstrumentationArgs({});
+    expect(result.args).toEqual([]);
+  });
+
+  it('should prepare arguments with base-64 encoding', () => {
+    const args = {
+      mockMe: 'this',
+      andMockMe: 'that',
+    };
+    const expectedArgs = [
+      ...expectedArgEncoded('mockMe', 'this'),
+      ...expectedArgEncoded('andMockMe', 'that'),
+    ];
+    const result = uut.prepareInstrumentationArgs(args);
+    expect(result.args).toEqual(expectedArgs);
+  });
+
+  it('should stringify non-string values', () => {
+    const args = {
+      'object-arg': {
+        such: 'wow',
+        much: 'amaze',
+        very: 111,
+      },
+      'string-arg': 'text, with commas-and-dashes,',
+    };
+    const expectedArgs = [
+      ...expectedArgEncoded('object-arg', '{"such":"wow","much":"amaze","very":111}'),
+      ...expectedArgEncoded('string-arg', 'text, with commas-and-dashes,'),
+    ];
+    const result = uut.prepareInstrumentationArgs(args);
+    expect(result.args).toEqual(expectedArgs);
+  });
+
+  // Ref: https://developer.android.com/studio/test/command-line#AMOptionsSyntax
+  it('should whitelist reserved instrumentation args with respect to base64 encoding', async () => {
+    const args = {
+      // Free arg
+      'user-arg': 'merry christ-nukah',
+
+      // Reserved instrumentation args
+      'class': 'class-value',
+      'package': 'package-value',
+      'func': 'func-value',
+      'unit': 'unit-value',
+      'size': 'size-value',
+      'perf': 'perf-value',
+      'debug': 'debug-value',
+      'log': 'log-value',
+      'emma': 'emma-value',
+      'coverageFile': 'coverageFile-value',
+    };
+
+    const result = uut.prepareInstrumentationArgs(args);
+    expect(result.args).toEqual([
+      ...expectedArgEncoded('user-arg', 'merry christ-nukah'),
+      ...expectedArgUnencoded('class', 'class-value'),
+      ...expectedArgUnencoded('package', 'package-value'),
+      ...expectedArgUnencoded('func', 'func-value'),
+      ...expectedArgUnencoded('unit', 'unit-value'),
+      ...expectedArgUnencoded('size', 'size-value'),
+      ...expectedArgUnencoded('perf', 'perf-value'),
+      ...expectedArgUnencoded('debug', 'debug-value'),
+      ...expectedArgUnencoded('log', 'log-value'),
+      ...expectedArgUnencoded('emma', 'emma-value'),
+      ...expectedArgUnencoded('coverageFile', 'coverageFile-value'),
+    ]);
+  });
+
+  it('should collect and separately return reserved instrumentation args', async () => {
+    const args = {
+      'class': 'class-value',
+      'package': 'package-value',
+    };
+    const result = uut.prepareInstrumentationArgs(args);
+    expect(result.usedReservedArgs).toEqual(Object.keys(args));
+  });
+
+  it('should whitelist args with \'detox\' prefix with respect to base64 encoding', () => {
+    const args = {
+      mockMe: 'this',
+      detoxMockMe: 'that',
+    };
+    const expectedArgs = [
+      ...expectedArgEncoded('mockMe', 'this'),
+      ...expectedArgUnencoded('detoxMockMe', 'that'),
+    ];
+    const result = uut.prepareInstrumentationArgs(args);
+    expect(result.args).toEqual(expectedArgs);
+  });
+
+  const expectedArgEncoded = (key, value) => (['-e', key, mockEncodeBase64Fn(value)]);
+  const expectedArgUnencoded = (key, value) => (['-e', key, value]);
+});