feat: undeprecate "specs" in package.json (#1290)

wix · Apr 18, 2019 · 34b5691 · 34b5691
1 parent e45cd1f
commit 34b5691
Show file tree

Hide file tree

Showing 11 changed files with 125 additions and 96 deletions.
diff --git a/detox/local-cli/init.js b/detox/local-cli/init.js
@@ -91,11 +91,9 @@ function parsePackageJson(filepath) {
 
 function patchPackageJson(packageJson, runnerName) {
   _.set(packageJson, ['detox', 'test-runner'], runnerName);
-  _.set(packageJson, ['detox', 'specs'], "");
 
   log.info(PREFIX, 'Patched ./package.json with commands:');
   log.info(PREFIX, `_.set(packageJson, ['detox', 'test-runner'], "${runnerName}")`);
-  log.info(PREFIX, `_.set(packageJson, ['detox', 'specs'], "")`);
 }
 
 function savePackageJson(filepath, json) {

diff --git a/detox/local-cli/test.js b/detox/local-cli/test.js
@@ -8,7 +8,7 @@ const DetoxConfigError = require('../src/errors/DetoxConfigError');
 
 const log = require('../src/utils/logger').child({ __filename });
 const {getDetoxSection, getDefaultConfiguration, getConfigurationByKey} = require('./utils/configurationUtils');
-const {coerceDeprecation, migrationGuideUrl} = require('./utils/deprecation');
+const {coerceDeprecation, printFileDeprecationWarning} = require('./utils/deprecation');
 const shellQuote = require('./utils/shellQuote');
 
 module.exports.command = 'test';
@@ -137,16 +137,12 @@ module.exports.handler = async function test(program) {
 
   const config = getDetoxSection();
 
-  let testFolder = getConfigFor(['file', 'specs'], 'e2e');
-  testFolder = testFolder && `"${testFolder}"`;
-
-  if (testFolder && !program.file && !program.specs) {
-    log.warn('Deprecation warning: "file" and "specs" support will be dropped in the next Detox version.');
-    log.warn(`Please edit your package.json according to the migration guide: ${migrationGuideUrl} `);
+  if (!program.file && config.file) {
+    printFileDeprecationWarning(config.file);
   }
 
-  const runner = getConfigFor(['test-runner'], 'mocha');
-  const runnerConfig = getConfigFor(['runner-config'], getDefaultRunnerConfig());
+  const runner = getConfigFor('test-runner') || 'mocha';
+  const runnerConfig = getConfigFor('runner-config') || getDefaultRunnerConfig();
 
   const currentConfiguration = getConfigurationByKey(program.configuration);
   if (!currentConfiguration.type) {
@@ -168,17 +164,14 @@ module.exports.handler = async function test(program) {
     }
   }
 
-  function getConfigFor(keys, fallback) {
+  function getConfigFor(...keys) {
     for (const key of keys) {
-      const camel = _.camelCase(key);
-      const result = program[key] || config[camel] || config[key];
+      const result = program[key] || config[_.camelCase(key)] || config[key];
 
-      if (result != null) {
+      if (result) {
         return result;
       }
     }
-
-    return fallback;
   }
 
   function hasCustomValue(key) {
@@ -188,6 +181,18 @@ module.exports.handler = async function test(program) {
     return (value !== metadata.default);
   }
 
+  function getPassthroughArguments() {
+    const args = collectExtraArgs(process.argv.slice(3));
+
+    const hasFolders = args.some(arg => arg && !arg.startsWith('-'));
+    if (hasFolders) {
+      return args;
+    }
+
+    const fallbackTestFolder = `"${getConfigFor('file', 'specs') || 'e2e'}"`;
+    return args.concat(fallbackTestFolder);
+  }
+
   function runMocha() {
     if (program.workers !== 1) {
       log.warn('Can not use -w, --workers. Parallel test execution is only supported with iOS and Jest');
@@ -210,8 +215,7 @@ module.exports.handler = async function test(program) {
       (hasCustomValue('record-videos') ? `--record-videos ${program.recordVideos}` : ''),
       (program.artifactsLocation ? `--artifacts-location "${program.artifactsLocation}"` : ''),
       (program.deviceName ? `--device-name "${program.deviceName}"` : ''),
-      testFolder,
-      collectExtraArgs(process.argv.slice(3)),
+      ...getPassthroughArguments(),
     ]).join(' ');
 
     log.info(command);
@@ -230,8 +234,7 @@ module.exports.handler = async function test(program) {
       (program.noColor ? ' --no-color' : ''),
       `--maxWorkers=${program.workers}`,
       (platform ? shellQuote(`--testNamePattern=^((?!${getPlatformSpecificString()}).)*$`) : ''),
-      testFolder,
-      collectExtraArgs(process.argv.slice(3)),
+      ...getPassthroughArguments(),
     ]).join(' ');
 
     const detoxEnvironmentVariables = _.pick(program, [
@@ -297,6 +300,5 @@ module.exports.handler = async function test(program) {
     fs.writeFileSync(lockFilePath, '[]');
   }
 
-
   run();
 };
diff --git a/detox/local-cli/utils/collectExtraArgs.js b/detox/local-cli/utils/collectExtraArgs.js
@@ -23,7 +23,11 @@ function collectBlacklistedArgs(builder) {
 function configureCollectExtraArgs(builder) {
   const blacklistedArgs = collectBlacklistedArgs(builder);
 
-  return function collectExtraArgs(argv) {
+  /***
+   * @param {Object} argv
+   * @returns {string[]}
+   */
+  function collectExtraArgs(argv) {
     const parsed = parseArgv(argv, {
       configuration: {
         'boolean-negation': false,
@@ -41,11 +45,12 @@ function configureCollectExtraArgs(builder) {
         return value === true ? `--${key}` : `--${key} ${value}`;
       })
       .concat(parsed['_'])
-      .value()
-      .join(' ');
+      .value();
 
     return passthrough;
-  };
+  }
+
+  return collectExtraArgs;
 }
 
 module.exports = configureCollectExtraArgs;
diff --git a/detox/local-cli/utils/deprecation.js b/detox/local-cli/utils/deprecation.js
@@ -1,16 +1,26 @@
+const chalk = require('chalk');
 const log = require('../../src/utils/logger').child({ __filename });
-const migrationGuideUrl = 'https://wix.to/I0DOAK0';
+const migrationGuideUrl = 'https://github.com/wix/Detox/blob/master/docs/Guide.Migration.md#migrating-from-detox-120x-to-121x';
 
 function coerceDeprecation(option) {
   return function coerceDeprecationFn(value) {
     log.warn(`Beware: ${option} will be removed in the next version of Detox.`);
-    log.warn(`See the migration guide: ${migrationGuideUrl}`);
+    log.warn(`See the migration guide:\n${migrationGuideUrl} `);
 
     return value;
   };
 }
 
+function printFileDeprecationWarning(file) {
+  log.warn('Deprecated: "file" option in "detox" section of package.json won\'t be supported in the next Detox version.\n');
+  console.log(`   "detox": {`);
+  console.log(chalk.red(`-    "file": ${JSON.stringify(file)},`));
+  console.log(chalk.green(`+    "specs": ${JSON.stringify(file)},\n`));
+  log.warn(`Please rename it to "specs", as demonstrated above.`);
+}
+
 module.exports = {
   coerceDeprecation,
   migrationGuideUrl,
+  printFileDeprecationWarning,
 };
diff --git a/detox/package.json b/detox/package.json
@@ -39,6 +39,7 @@
   "dependencies": {
     "bunyan": "^1.8.12",
     "bunyan-debug-stream": "^1.1.0",
+    "chalk": "^2.4.2",
     "child-process-promise": "^2.2.0",
     "fs-extra": "^4.0.2",
     "get-port": "^2.1.0",

diff --git a/detox/test/package.json b/detox/test/package.json
@@ -6,8 +6,8 @@
     "test": ":",
     "packager": "react-native start",
     "detox-server": "detox run-server",
-    "e2e:ios": "detox test e2e --configuration ios.sim.release --debug-synchronization --take-screenshots all --record-logs all",
-    "e2e:android": "detox test e2e --configuration android.emu.release --take-screenshots all --record-logs all",
+    "e2e:ios": "detox test --configuration ios.sim.release --debug-synchronization --take-screenshots all --record-logs all",
+    "e2e:android": "detox test --configuration android.emu.release --take-screenshots all --record-logs all",
     "build:ios": "detox build --configuration ios.sim.release",
     "build:android": "detox build --configuration android.emu.release",
     "verify-artifacts:ios": "jest ./scripts/verify_artifacts_are_not_missing.ios.test.js --testEnvironment node",
@@ -25,7 +25,6 @@
     "mocha": "^6.1.3"
   },
   "detox": {
-    "specs": "e2e",
     "test-runner": "mocha",
     "__session": {
       "server": "ws://localhost:8099",

diff --git a/docs/Guide.Migration.md b/docs/Guide.Migration.md
@@ -5,24 +5,62 @@ title: Migration Guide
 
 We are improving detox API as we go along, sometimes these changes require us to break the API in order for it to make more sense. These migration guides refer to breaking changes.
 
+## Migrating from Detox 12.3.x to 12.4.0
+
+The deprecation of `"specs"` (in `package.json`) introduced in 12.1.0 is **no longer relevant**.
+It is valid now, like it was before, but from now on the semantics has been slightly changed -
+it acts as a fallback for the default root for your Detox e2e specs, in cases when
+you don't specify it explicitly, e.g.:
+
+```sh
+detox test   # translates to: mocha <...args> e2e
+detox test e2e/01.sanity.test.js  # translates to: mocha <...args> e2e/01.sanity.test.js
+```
+
+Between 12.1.x and 12.3.x, it was buggy and used to work like this:
+
+```sh
+detox test   # translates to: mocha <...args> e2e
+detox test e2e/01.sanity.test.js  # translates to: mocha <...args> e2e e2e/01.sanity.test.js
+```
+
 ## Migrating from Detox 12.0.x to 12.1.x
 
 This is not a breaking change yet, but starting from `[email protected]` you'll start seeing warnings like:
 
 ```
-WARN:  [test.js] Deprecation warning: "file" and "specs" support will be dropped in the next Detox version.
-WARN:  [test.js] Please edit your package.json according to the migration guide: https://wix.to/I0DOAK0 
+detox[21201] WARN:  [deprecation.js] Beware: -f, --file will be removed in the next version of Detox.
+detox[21201] WARN:  [deprecation.js] See the migration guide:
+https://github.com/wix/Detox/blob/master/docs/Guide.Migration.md#migrating-from-detox-120x-to-121x
 ```
 
 In the next major version `--file` and `--specs` will be treated as unknown arguments
-and therefore passed as-is to your appropriate test runner.
+and therefore passed as-is to your appropriate test runner. That allows to avoid name
+conflict with the respective `--file` option in Mocha runner itself and other potential
+collisions.
 
-To get rid of this warning:
+So, if you have been using CLI arguments like `--file e2e` or
+`--specs e2e`, please drop the preceding `--file` and `--specs`, so that:
 
-* find `"specs"` or `"file"` entry in your project's `package.json` and empty it (e.g. `"e2e"` &#10230; `""`);
-* update your `detox test` scripts — make sure they have an explicit path to your Detox tests folder, e.g. `detox test e2e`.
+```
+detox test --file e2e/01.sanity.test.js
+```
 
-For example, if it were a `package.json` before:
+becomes:
+
+```
+detox test e2e/01.sanity.test.js
+```
+
+**UPDATE:** It was decided not to deprecate `"specs"` in `package.json`, so the text below
+is not relevant to a large extent. Please ignore the guide below.
+
+~To get rid of this warning:~
+
+* ~find `"specs"` or `"file"` entry in your project's `package.json` and empty it (e.g. `"e2e"` &#10230; `""`);~
+* ~update your `detox test` scripts — make sure they have an explicit path to your Detox tests folder, e.g. `detox test e2e`.~
+
+~For example, if it were a `package.json` before:~
 
 ```json
 {
@@ -36,7 +74,7 @@ For example, if it were a `package.json` before:
 }
 ```
 
-Then this is how it should look like afterwards:
+~Then this is how it should look like afterwards:~
 
 ```json
 {
@@ -50,61 +88,18 @@ Then this is how it should look like afterwards:
 }
 ```
 
-Notice that we appended `e2e` to the `e2e:ios` test script and
-emptied `"specs"` property in `detox` configuration.
+~Notice that we appended `e2e` to the `e2e:ios` test script and
+emptied `"specs"` property in `detox` configuration.~
 
-In a case if you had no `"specs"` property in your `detox` configuration
-in `package.json`, then please add it temporarily like this:
+~In a case if you had no `"specs"` property in your `detox` configuration
+in `package.json`, then please add it temporarily like this:~
 
 ```json
 {
     "specs": ""
 }
 ```
 
-Last, but not least, if you have been using CLI arguments like
-`--file e2e` or `--specs e2e`, please drop the preceding
-`--file` and `--specs`, so that:
-
-```
-detox test --specs e2e
-```
-
-becomes:
-
-```
-detox test e2e
-```
-
-The idea in the example above is to pass `e2e` straight to `mocha` or `jest` as
-a path to the folder with Detox tests, without extra preprocessing from Detox CLI side.
-
-> For the curious ones, who want to know more why we should use an empty string
-(`""`) instead of deleting `"specs"` and `"file"` from `package.json`, here is
-the explanation. This seemingly weird step is motivated by backward compatibility
-with the previous versions of Detox.
->
-> So far, before `[email protected]`, absence of
-`file` and `specs` properties implied a default test folder value (`"e2e"`).
-> In other words:
-
-```js
-const testFolder = config.file || config.specs || "e2e";
-```
-
-> In order not to break the existing logic but to introduce the deprecation,
-the check for the `e2e` placeholder assignment became narrower yet remaining valid:
-
-```js
-let testFolder = config.file || config.specs;
-if (testFolder == null) { // that's why you should change it to an empty string, ""
-    testFolder = "e2e"; // otherwise, if it is null or undefined, then we save backward compatibility
-}
-if (testFolder) { printDeprecationWarning(); }
-```
-
-> As it can be seen above, this move allows to track if you followed the migration guide or not.
-
 ## Migrating from Detox 11.0.1 to 12.0.0
 
 The new version explicity requires **Xcode 10.1 or higher** in order to run tests on iOS ([#1229](https://github.com/wix/Detox/issues/1229)).

diff --git a/docs/Troubleshooting.RunningTests.md b/docs/Troubleshooting.RunningTests.md
@@ -158,17 +158,38 @@ detox[4498] INFO:  [test.js] node_modules/.bin/mocha --opts e2e/mocha.opts --con
   error: unknown option `--configuration'
 ```
 
-**Solution:** In `[email protected]` such options as `--file` and `--specs` were
-deprecated in favor of straightforward passing command line arguments to test
-runners. Since `mocha` does not search for test files recursively in the
-current working directory by default, you have to pass the path to your e2e
-tests folder manually:
+**Solution:** Upgrade to `detox@^12.4.0` and `mocha@^6.1.3`.
+That weird error has been spotted in older versions of `mocha` (including 5.2.0)
+and fixed since 6.0.0. In fact, it conceals the genuine error:
 
 ```
-detox test ./your-e2e-tests-folder
+Error: No test files found
 ```
 
-See [the migration guide](Guide.Migration.md#migrating-from-detox-110x-to-111x) for more details.
+If the error appeared after running a short command like `detox test`,
+please try out `detox test e2e` (in other words, append the path to your
+end-to-end tests folder) - and if that fixes the error, then you deal the
+bug in the question and upgrading `detox` and `mocha` should help.
+
+Here's why you need to upgrade to `[email protected]`.  In `12.1.0` there was a
+premature deprecation of `"specs"` property in detox section of `package.json`.
+The deprecation was revoked in `[email protected]`, and since then `"specs"` property
+acts as a fallback if a test folder has not been specified explicitly.
+
+After you upgrade, you can configure the default path to your end-to-end tests folder
+in `package.json` (no deprecation warnings starting from `12.4.0`), as shown below:
+
+```diff
+ {
+   "detox": {
+-    "specs": "",  
++    "specs": "your-e2e-tests-folder",
+   }
+ }
+```
+
+Please mind that if your e2e tests are located at the default path (`e2e`),
+then you don't need to add `"specs"` property explicitly to `package.json`.
 
 <br>
 

diff --git a/examples/demo-react-native-jest/package.json b/examples/demo-react-native-jest/package.json
@@ -23,7 +23,6 @@
     "jest": "24.x.x"
   },
   "detox": {
-    "specs": "",
     "test-runner": "jest",
     "configurations": {
       "ios.sim.release": {