Repository: wix/Detox
Pull request #1290 (2019-04-16 09:18:08)

feat: undeprecate "specs" in package.json
**Description:**

In `detox@12.1.0` there was a premature deprecation of `"specs"` property in detox section of `package.json`. It appeared inconvenient for anyone who prefers terse commands like `detox test` over verbose `detox test e2e`. From now on, "specs" property will act as a fallback if a specific test folder has not been specified. For example (1):

```sh
detox test   # translates to: mocha <...args> e2e
detox test e2e/01.sanity.test.js  # translates to: mocha <...args> e2e/01.sanity.test.js
```

For the reference, previously it used to work like (2):

```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
```

The problem of the latter was that you could not focus (override defaults) on a specific test file without having to use `specs` in package.json and deprecated `--specs` in the command line (3):

```sh
detox test --specs e2e/01.sanity.test.js  # overrides specs in package.json
# mocha <...args> e2e/01.sanity.test.js
```

Since (2) has been introduced very recently and did not function well, I suggest to consider this rather a minor version than a breaking change.

Your considerations are welcome.

P. S. As for `file` deprecation warning, it has been reworked and looks like on the screenshot:

![Screenshot from 2019-04-16 12-20-49](https://user-images.githubusercontent.com/1962469/56197772-2e281e80-6042-11e9-924c-cee7409dbb06.png)



LeoNatan: Any luck finding why it showed an unrelated error when specs was not provided? 
noomorph: @LeoNatan, yes, it was a bug in `mocha@^5.0.0`. After upgrading to `mocha@^6.0.0`, that weird error has disappeared, now it is comprehensible:

![Screenshot from 2019-04-17 08-44-07](https://user-images.githubusercontent.com/1962469/56263784-ff1ab700-60ec-11e9-88fa-fb988929ca32.png)


Patch:
diff --git a/detox/local-cli/init.js b/detox/local-cli/init.js
index 39ef91553..c73fefe03 100644
--- 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/utils/collectExtraArgs.js b/detox/local-cli/utils/collectExtraArgs.js
index 4a3f2e176..4c01026f4 100644
--- 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
index 1f728c7cd..4afa7928d 100644
--- 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
index 08b087e53..961ef4add 100644
--- 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/docs/Guide.Migration.md b/docs/Guide.Migration.md
index fbba77718..237021c93 100644
--- 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 `detox@12.1.0` 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,11 +88,11 @@ 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
 {
@@ -62,49 +100,6 @@ in `package.json`, then please add it temporarily like this:
 }
 ```
 
-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 `detox@12.1.0`, 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/examples/demo-react-native-jest/package.json b/examples/demo-react-native-jest/package.json
index b1d3b6bfc..9e231a809 100644
--- 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": {
diff --git a/examples/demo-react-native/package.json b/examples/demo-react-native/package.json
index 66e438627..31b108248 100644
--- a/examples/demo-react-native/package.json
+++ b/examples/demo-react-native/package.json
@@ -14,7 +14,6 @@
     "mocha": "^6.1.3"
   },
   "detox": {
-    "specs": "e2e",
     "test-runner": "mocha",
     "runner-config": "e2e/mocha.opts",
     "configurations": {
diff --git a/scripts/demo-projects.android.sh b/scripts/demo-projects.android.sh
index b28e19615..be307875d 100755
--- a/scripts/demo-projects.android.sh
+++ b/scripts/demo-projects.android.sh
@@ -5,7 +5,7 @@ source $(dirname "$0")/demo-projects.sh
 pushd examples/demo-react-native
 run_f "detox build -c android.emu.release"
 run_f "detox test -c android.emu.release"
-run_f "detox test -c android.emu.release --specs e2eExplicitRequire --runner-config e2eExplicitRequire/mocha.opts"
+run_f "detox test e2eExplicitRequire -c android.emu.release --runner-config e2eExplicitRequire/mocha.opts"
 popd
 
 pushd examples/demo-react-native-jest


Test patch:
diff --git a/detox/local-cli/test.js b/detox/local-cli/test.js
index 16f6d6ff8..524808c81 100644
--- 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/test/package.json b/detox/test/package.json
index c34d42a00..a38b00ae7 100644
--- 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/Troubleshooting.RunningTests.md b/docs/Troubleshooting.RunningTests.md
index e506d7ff0..44a590be3 100644
--- 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 `detox@11.1.0` 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 `detox@12.4.0`.  In `12.1.0` there was a
+premature deprecation of `"specs"` property in detox section of `package.json`.
+The deprecation was revoked in `detox@12.4.0`, 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>