Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 12 commits into from
Apr 18, 2019
2 changes: 0 additions & 2 deletions detox/local-cli/init.js
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
42 changes: 22 additions & 20 deletions detox/local-cli/test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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) {
Expand All @@ -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) {
Expand All @@ -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');
Expand All @@ -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);
Expand All @@ -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, [
Expand Down Expand Up @@ -297,6 +300,5 @@ module.exports.handler = async function test(program) {
fs.writeFileSync(lockFilePath, '[]');
}


run();
};
13 changes: 9 additions & 4 deletions detox/local-cli/utils/collectExtraArgs.js
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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;
10 changes: 10 additions & 0 deletions detox/local-cli/utils/deprecation.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
const chalk = require('chalk');
const log = require('../../src/utils/logger').child({ __filename });
const migrationGuideUrl = 'https://wix.to/I0DOAK0';

Expand All @@ -10,7 +11,16 @@ function coerceDeprecation(option) {
};
}

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,
};
1 change: 1 addition & 0 deletions detox/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
5 changes: 2 additions & 3 deletions detox/test/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand All @@ -26,7 +26,6 @@
"mocha": "^5.0.0"
noomorph marked this conversation as resolved.
Show resolved Hide resolved
},
"detox": {
"specs": "e2e",
"test-runner": "mocha",
"__session": {
"server": "ws://localhost:8099",
Expand Down
104 changes: 49 additions & 55 deletions docs/Guide.Migration.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,24 +5,61 @@ 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
```

Previously, it used to work like:

```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://wix.to/I0DOAK0
noomorph marked this conversation as resolved.
Show resolved Hide resolved
```

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
{
Expand All @@ -36,7 +73,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
{
Expand All @@ -50,61 +87,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)).
Expand Down
35 changes: 28 additions & 7 deletions docs/Troubleshooting.RunningTests.md
Original file line number Diff line number Diff line change
Expand Up @@ -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>

Expand Down
1 change: 0 additions & 1 deletion examples/demo-native-ios/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@
"mocha": "^4.0.0"
},
"detox": {
"specs": "",
"configurations": {
"ios.sim.release": {
"binaryPath": "build/Build/Products/Release-iphonesimulator/NativeExample.app",
Expand Down
1 change: 0 additions & 1 deletion examples/demo-react-native-jest/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@
"react-test-renderer": "16.0.0-beta.5"
},
"detox": {
"specs": "",
"test-runner": "jest",
"configurations": {
"ios.sim.release": {
Expand Down
Loading