Repository: wix/Detox
Pull request #1360 (2019-05-03 11:00:10)

code: logging fixes in detox init
**Description:**

1. Remove redundant PREFIX in logs, because `require('../src/utils/logger').child({ __filename });` already gives the logger a prefix.
2. As @d4vidi suggested in his PR #1351, to explicitly specify `runner-config` field in package.json (and that's an additional parameter to a scaffolding function), so to be on the safe side, I refactored `patchDetoxConfigInPackageJSON` function to accept named parameters (via destructuring). As a result, we have here a more generic `loggedSet` instead of a crafted-once log message.
3. `createFolder` log message has been fixed (interpolated `${dir}` instead of a constant `./e2e`).
4. `createFolder` won't throw an unhandled exception if creating a folder fails - instead, it will log an error with an explanation and go to the next step. 
5. `patchDetoxConfigInPackageJSON` now prints a comprehensible error if it does not find `package.json`. 
6. Fixed wording in error messages.


Patch:
diff --git a/detox/local-cli/init.js b/detox/local-cli/init.js
index c73fefe03..a85f0a133 100644
--- a/detox/local-cli/init.js
+++ b/detox/local-cli/init.js
@@ -5,8 +5,6 @@ const mochaTemplates = require('./templates/mocha');
 const jestTemplates = require('./templates/jest');
 const log = require('../src/utils/logger').child({ __filename });
 
-const PREFIX = 'detox-init';
-
 module.exports.command = 'init';
 module.exports.desc = 'Scaffold initial E2E test folder structure for a specified test runner';
 module.exports.builder = {
@@ -24,11 +22,15 @@ module.exports.handler = async function init(argv) {
   switch (runner) {
     case 'mocha':
       createMochaFolderE2E();
-      patchTestRunnerFieldInPackageJSON('mocha');
+      patchDetoxConfigInPackageJSON({
+        runner: 'mocha',
+      });
       break;
     case 'jest':
       createJestFolderE2E();
-      patchTestRunnerFieldInPackageJSON('jest');
+      patchDetoxConfigInPackageJSON({
+        runner: 'jest',
+      });
       break;
     default:
       throw new Error([
@@ -43,25 +45,29 @@ module.exports.handler = async function init(argv) {
 };
 
 function createFolder(dir, files) {
-  if (!fs.existsSync(dir)) {
+  if (fs.existsSync(dir)) {
+    return log.error(`Failed to create ${dir} folder, because it already exists at path: ${path.resolve(dir)}`);
+  }
+
+  try {
     fs.mkdirSync(dir);
+  } catch (err) {
+    return log.error({ err }, `Failed to create ${dir} folder due to an error:`);
+  }
 
-    for (const entry of Object.entries(files)) {
-      const [filename, content] = entry;
-      createFile(path.join(dir, filename), content);
-    }
-  } else {
-    log.error(PREFIX, `./e2e folder already exists at path: ${path.resolve(dir)}`);
+  for (const entry of Object.entries(files)) {
+    const [filename, content] = entry;
+    createFile(path.join(dir, filename), content);
   }
 }
 
 function createFile(filename, content) {
   try {
     fs.writeFileSync(filename, content);
-    log.info(PREFIX, `A file was created in: ${filename}`);
+    log.info(`Created a file at path: ${filename}`);
   } catch (e) {
-    log.error(PREFIX, `Failed to create file in: ${filename}`);
-    log.error(PREFIX, e);
+    log.error(`Failed to create a file at path: ${filename}`);
+    log.error(e);
   }
 }
 
@@ -85,31 +91,36 @@ function parsePackageJson(filepath) {
   try {
     return require(filepath);
   } catch (err) {
-    log.error(PREFIX, `Failed to parse ./package.json due to the error:\n${err.message}`);
+    log.error(`Failed to parse package.json due to an error:\n${err.message}`);
   }
 }
 
-function patchPackageJson(packageJson, runnerName) {
-  _.set(packageJson, ['detox', 'test-runner'], runnerName);
+function loggedSet(obj, path, value) {
+  _.set(obj, path, value);
 
-  log.info(PREFIX, 'Patched ./package.json with commands:');
-  log.info(PREFIX, `_.set(packageJson, ['detox', 'test-runner'], "${runnerName}")`);
+  const pathString = path.map(segment => `[${JSON.stringify(segment)}]`).join('');
+  log.info(`  json${pathString} = ${JSON.stringify(value)};`);
 }
 
 function savePackageJson(filepath, json) {
   try {
     fs.writeFileSync(filepath, JSON.stringify(json, null, 2) + '\n');
   } catch (err) {
-    log.error(PREFIX, `Failed to write changes into ./package.json due to the error:\n${err.message}`);
+    log.error(`Failed to write changes back into package.json due to an error:\n${err.message}`);
   }
 }
 
-function patchTestRunnerFieldInPackageJSON(runnerName) {
+function patchDetoxConfigInPackageJSON({ runner }) {
   const packageJsonPath = path.join(process.cwd(), 'package.json');
-  const packageJson = parsePackageJson(packageJsonPath);
 
-  if (packageJson) {
-    patchPackageJson(packageJson, runnerName);
+  if (fs.existsSync(packageJsonPath)) {
+    log.info(`Patching package.json at path: ${packageJsonPath}`);
+
+    const packageJson = parsePackageJson(packageJsonPath);
+    loggedSet(packageJson, ['detox', 'test-runner'], runner);
+
     savePackageJson(packageJsonPath, packageJson);
+  } else {
+    log.error(`Failed to find package.json at path: ${packageJsonPath}`);
   }
 }
diff --git a/detox/src/devices/drivers/SimulatorDriver.js b/detox/src/devices/drivers/SimulatorDriver.js
index bc0139981..6e34f6847 100644
--- a/detox/src/devices/drivers/SimulatorDriver.js
+++ b/detox/src/devices/drivers/SimulatorDriver.js
@@ -70,10 +70,8 @@ class SimulatorDriver extends IosDriver {
   }
 
   async launchApp(deviceId, bundleId, launchArgs, languageAndLocale) {
-    const _launchArgs = this._transformLaunchArgs(launchArgs);
-
     await this.emitter.emit('beforeLaunchApp', {bundleId, deviceId, launchArgs});
-    const pid = await this.applesimutils.launch(deviceId, bundleId, _launchArgs, languageAndLocale);
+    const pid = await this.applesimutils.launch(deviceId, bundleId, launchArgs, languageAndLocale);
     await this.emitter.emit('launchApp', {bundleId, deviceId, launchArgs, pid});
 
     return pid;
@@ -129,13 +127,6 @@ class SimulatorDriver extends IosDriver {
   async waitForBackground() {
     return await this.client.waitForBackground();
   }
-
-  _transformLaunchArgs(launchArgs) {
-    return _.reduce(launchArgs, (result, value, key) => {
-      result[`-${key}`] = value;
-      return result;
-    }, {});
-  }
 }
 
 module.exports = SimulatorDriver;
diff --git a/detox/src/devices/ios/AppleSimUtils.js b/detox/src/devices/ios/AppleSimUtils.js
index 24db19312..1acd7c456 100644
--- a/detox/src/devices/ios/AppleSimUtils.js
+++ b/detox/src/devices/ios/AppleSimUtils.js
@@ -248,7 +248,7 @@ class AppleSimUtils {
   }
 
   _joinLaunchArgs(launchArgs) {
-    return _.map(launchArgs, (v, k) => `${k} "${v}"`).join(' ').trim();
+    return _.map(launchArgs, (v, k) => `-${k} "${v}"`).join(' ').trim();
   }
 
   async _launchMagically(frameworkPath, logsInfo, udid, bundleId, args, languageAndLocale) {


Test patch:
diff --git a/detox/src/devices/drivers/SimulatorDriver.test.js b/detox/src/devices/drivers/SimulatorDriver.test.js
index 4e73961d1..20958a751 100644
--- a/detox/src/devices/drivers/SimulatorDriver.test.js
+++ b/detox/src/devices/drivers/SimulatorDriver.test.js
@@ -1,34 +1,43 @@
 describe('IOS simulator driver', () => {
+  let uut;
 
   const deviceId = 'device-id-mock';
   const bundleId = 'bundle-id-mock';
 
+  beforeEach(() => {
+    jest.mock('../ios/AppleSimUtils', () => mockAppleSimUtils);
+  });
+
   describe('launch args', () => {
+    let launchArgs, languageAndLocale;
 
-    let uut;
     beforeEach(() => {
-      jest.mock('../ios/AppleSimUtils', () => mockAppleSimUtils);
-
-      const SimulatorDriver = require('./SimulatorDriver');
-      uut = new SimulatorDriver({
-        client: {},
-      });
-    });
-
-    it('should inject a prefix to arg keys', async () => {
-      const launchArgs = {
+      launchArgs = {
         'dog1': 'dharma',
         'dog2': 'karma',
       };
 
-      await uut.launchApp(deviceId, bundleId, launchArgs, '');
+      languageAndLocale = '';
 
-      const simUtilsLaunchAllArgs = uut.applesimutils.launch.mock.calls[0];
-      const simUtilsLaunchArgs = simUtilsLaunchAllArgs[2];
-      expect(simUtilsLaunchArgs).toEqual({
-        '-dog1': 'dharma',
-        '-dog2': 'karma',
+      const SimulatorDriver = require('./SimulatorDriver');
+      uut = new SimulatorDriver({ client: {} });
+    });
+
+    it('should be passed to AppleSimUtils', async () => {
+      await uut.launchApp(deviceId, bundleId, launchArgs, languageAndLocale);
+      expect(uut.applesimutils.launch).toHaveBeenCalledWith(deviceId, bundleId, launchArgs, languageAndLocale);
+    });
+
+    it('should be passed to AppleSimUtils even if some of them were received from `beforeLaunchApp` phase', async () => {
+      uut.emitter.on('beforeLaunchApp', ({ launchArgs }) => {
+        launchArgs.dog3 = 'Chika, from plugin';
       });
+
+      await uut.launchApp(deviceId, bundleId, launchArgs, languageAndLocale);
+      expect(uut.applesimutils.launch).toHaveBeenCalledWith(deviceId, bundleId, {
+        ...launchArgs,
+        dog3: 'Chika, from plugin',
+      }, '');
     });
   });
 });