Skip to content

Commit

Permalink
feat(config): drop skipLegacyWorkersInjection
Browse files Browse the repository at this point in the history 
BREAKING CHANGE: Detox will no longer be injecting "-w 1" to Jest as CLI
arguments
  • Loading branch information
@noomorph
noomorph committed Apr 1, 2022
1 parent b47cdda commit c6751f6
Show file tree
Hide file tree
Showing 8 changed files with 65 additions and 92 deletions.
6 changes: 0 additions & 6 deletions detox/index.d.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,12 +43,6 @@ declare global {
* @example testRunner: 'mocha'
*/
testRunner?: string;
/**
* Stops passing default `--maxWorkers 1` to the test runner,
* presuming that from now on you have that already configured
* in your test runner config as a default.
*/
skipLegacyWorkersInjection?: boolean;
/**
* @example runnerConfig: 'e2e/config.js'
*/
Expand Down
1 change: 0 additions & 1 deletion detox/local-cli/init.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,6 @@ function createJestFolderE2E() {
createFile('.detoxrc.json', JSON.stringify({
testRunner: 'jest',
runnerConfig: 'e2e/config.json',
skipLegacyWorkersInjection: true,
...createDefaultConfigurations(),
}, null, 2));
}
Expand Down
6 changes: 2 additions & 4 deletions detox/local-cli/test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -174,14 +174,12 @@ async function prepareJestArgs({ cliConfig, deviceConfig, runnerArgs, runnerConf
color: !cliConfig.noColor && undefined,
config: runnerConfig.runnerConfig /* istanbul ignore next */ || undefined,
testNamePattern: platformFilter ? `^((?!${platformFilter}).)*$` : undefined,
maxWorkers: cliConfig.workers || (runnerConfig.skipLegacyWorkersInjection ? undefined : 1),
maxWorkers: cliConfig.workers || undefined,

...passthrough,
}, _.isUndefined);

const hasMultipleWorkers = runnerConfig.skipLegacyWorkersInjection
? (await readJestConfig(argv)).globalConfig.maxWorkers > 1
: cliConfig.workers > 1;
const hasMultipleWorkers = (await readJestConfig(argv)).globalConfig.maxWorkers > 1;

return {
argv,
Expand Down
135 changes: 61 additions & 74 deletions detox/local-cli/test.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ jest.mock('../src/utils/logger');
jest.mock('../src/devices/DeviceRegistry');
jest.mock('../src/devices/allocation/drivers/android/genycloud/GenyDeviceRegistryFactory');
jest.mock('../src/utils/lastFailedTests');
jest.mock('./utils/jestInternals');

const fs = require('fs-extra');
const _ = require('lodash');
Expand All @@ -23,9 +24,11 @@ describe('CLI', () => {
let detoxConfigPath;
let DeviceRegistry;
let GenyDeviceRegistryFactory;
let jestInternals;
let _env;

beforeEach(() => {
temporaryFiles = [];
_env = process.env;
process.env = { ..._env };

Expand All @@ -40,8 +43,23 @@ describe('CLI', () => {
};

cp = require('child_process');

const realJestInternals = jest.requireActual('./utils/jestInternals');
jestInternals = require('./utils/jestInternals');
Object.assign(jestInternals, realJestInternals);
jestInternals.readJestConfig = jest.fn(async (argv) => {
const runnerConfigTemplate = _.omit(
JSON.parse(require('./templates/jest').runnerConfig),
['reporters', 'testEnvironment']
);

return realJestInternals.readJestConfig({
...argv,
config: tempfile('.json', JSON.stringify(runnerConfigTemplate)),
});
});

logger = require('../src/utils/logger');
temporaryFiles = [];
DeviceRegistry = require('../src/devices/DeviceRegistry');
DeviceRegistry.forAndroid.mockImplementation(() => new DeviceRegistry());
DeviceRegistry.forIOS.mockImplementation(() => new DeviceRegistry());
Expand Down Expand Up @@ -286,7 +304,7 @@ describe('CLI', () => {
});

test('should produce a default command (integration test, ios)', () => {
const args = `--config e2e/config.json --testNamePattern ${quote('^((?!:android:).)*$')} --maxWorkers 1`;
const args = `--config e2e/config.json --testNamePattern ${quote('^((?!:android:).)*$')}`;
expect(cliCall().command).toBe(`jest ${args}`);
});

Expand All @@ -307,7 +325,7 @@ describe('CLI', () => {
});

test('should produce a default command (integration test)', () => {
const args = `--config e2e/config.json --testNamePattern ${quote('^((?!:ios:).)*$')} --maxWorkers 1`;
const args = `--config e2e/config.json --testNamePattern ${quote('^((?!:ios:).)*$')}`;
expect(cliCall().command).toBe(`jest ${args}`);
});

Expand All @@ -321,23 +339,6 @@ describe('CLI', () => {
});
});

describe('given skipLegacyWorkersInjection: true', () => {
describe.each([
['ios.simulator'],
['android.emulator'],
])('for %s', (configurationType) => {
it('should omit --maxWorkers CLI arg', async () => {
singleConfig().type = configurationType;
detoxConfig.skipLegacyWorkersInjection = true;
detoxConfig.runnerConfig = 'package.json';

await run();

expect(cliCall().command).not.toMatch(/--maxWorkers/);
});
});
});

test.each([['-c'], ['--configuration']])(
'%s <configuration> should provide inverted --testNamePattern that configuration (jest)',
async (__configuration) => {
Expand Down Expand Up @@ -478,69 +479,55 @@ describe('CLI', () => {
expect(cliCall().env).toEqual(expect.objectContaining({ DETOX_CAPTURE_VIEW_HIERARCHY: 'enabled' }));
});

describe.each([
[false],
[true],
])('when skipLegacyWorkersInjection is %j', (skipLegacyWorkersInjection) => {
beforeEach(() => {
Object.assign(detoxConfig, {
skipLegacyWorkersInjection,
runnerConfig: tempfile('.json', JSON.stringify({
maxWorkers: 1,
})),
});
});

test.each([['-w'], ['--workers']])('%s <value> should be passed as CLI argument', async (__workers) => {
await run(`${__workers} 2`);
expect(cliCall().command).toContain('--maxWorkers 2');
});
test.each([['-w'], ['--workers']])('%s <value> should be passed as CLI argument', async (__workers) => {
await run(`${__workers} 2`);
expect(cliCall().command).toContain('--maxWorkers 2');
});

test.each([['-w'], ['--workers']])('%s <value> should be replaced with --maxWorkers <value>', async (__workers) => {
await run(`${__workers} 2 --maxWorkers 3`);
test.each([['-w'], ['--workers']])('%s <value> should be replaced with --maxWorkers <value>', async (__workers) => {
await run(`${__workers} 2 --maxWorkers 3`);

const { command } = cliCall();
expect(command).toContain('--maxWorkers 3');
expect(command).not.toContain('--maxWorkers 2');
});
const { command } = cliCall();
expect(command).toContain('--maxWorkers 3');
expect(command).not.toContain('--maxWorkers 2');
});

test.each([['-w'], ['--workers']])('%s <value> can be overriden by a later value', async (__workers) => {
await run(`${__workers} 2 ${__workers} 3`);
test.each([['-w'], ['--workers']])('%s <value> can be overriden by a later value', async (__workers) => {
await run(`${__workers} 2 ${__workers} 3`);

const { command } = cliCall();
expect(command).toContain('--maxWorkers 3');
expect(command).not.toContain('--maxWorkers 2');
});
const { command } = cliCall();
expect(command).toContain('--maxWorkers 3');
expect(command).not.toContain('--maxWorkers 2');
});

test.each([['-w'], ['--workers']])('%s <value> should not warn anything for iOS', async (__workers) => {
singleConfig().type = 'ios.simulator';
await run(`${__workers} 2`);
expect(logger.warn).not.toHaveBeenCalled();
});
test.each([['-w'], ['--workers']])('%s <value> should not warn anything for iOS', async (__workers) => {
singleConfig().type = 'ios.simulator';
await run(`${__workers} 2`);
expect(logger.warn).not.toHaveBeenCalled();
});

test.each([['-w'], ['--workers']])('%s <value> should not put readOnlyEmu environment variable for iOS', async (__workers) => {
singleConfig().type = 'ios.simulator';
await run(`${__workers} 2`);
expect(cliCall().env).not.toHaveProperty('DETOX_READ_ONLY_EMU');
});
test.each([['-w'], ['--workers']])('%s <value> should not put readOnlyEmu environment variable for iOS', async (__workers) => {
singleConfig().type = 'ios.simulator';
await run(`${__workers} 2`);
expect(cliCall().env).not.toHaveProperty('DETOX_READ_ONLY_EMU');
});

test.each([['-w'], ['--workers']])('%s <value> should not put readOnlyEmu environment variable for android.attached', async (__workers) => {
singleConfig().type = 'android.attached';
await run(`${__workers} 2`);
expect(cliCall().env).not.toHaveProperty('DETOX_READ_ONLY_EMU');
});
test.each([['-w'], ['--workers']])('%s <value> should not put readOnlyEmu environment variable for android.attached', async (__workers) => {
singleConfig().type = 'android.attached';
await run(`${__workers} 2`);
expect(cliCall().env).not.toHaveProperty('DETOX_READ_ONLY_EMU');
});

test.each([['-w'], ['--workers']])('%s <value> should not put readOnlyEmu environment variable for android.emulator if there is a single worker', async (__workers) => {
singleConfig().type = 'android.emulator';
await run(`${__workers} 1`);
expect(cliCall().env).not.toHaveProperty('DETOX_READ_ONLY_EMU');
});
test.each([['-w'], ['--workers']])('%s <value> should not put readOnlyEmu environment variable for android.emulator if there is a single worker', async (__workers) => {
singleConfig().type = 'android.emulator';
await run(`${__workers} 1`);
expect(cliCall().env).not.toHaveProperty('DETOX_READ_ONLY_EMU');
});

test.each([['-w'], ['--workers']])('%s <value> should put readOnlyEmu environment variable for Android if there are multiple workers', async (__workers) => {
singleConfig().type = 'android.emulator';
await run(`${__workers} 2`);
expect(cliCall().env).toEqual(expect.objectContaining({ DETOX_READ_ONLY_EMU: true }));
});
test.each([['-w'], ['--workers']])('%s <value> should put readOnlyEmu environment variable for Android if there are multiple workers', async (__workers) => {
singleConfig().type = 'android.emulator';
await run(`${__workers} 2`);
expect(cliCall().env).toEqual(expect.objectContaining({ DETOX_READ_ONLY_EMU: true }));
});

test('should omit --testNamePattern for custom platforms', async () => {
Expand Down
1 change: 0 additions & 1 deletion detox/src/configuration/composeRunnerConfig.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ function composeRunnerConfig({ globalConfig, cliConfig }) {
testRunner,
runnerConfig: customRunnerConfig || defaultRunnerConfig,
specs: globalConfig.specs || '',
skipLegacyWorkersInjection: Boolean(globalConfig.skipLegacyWorkersInjection),
};
}

Expand Down
1 change: 0 additions & 1 deletion detox/test/e2e/detox.config.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ const config = {
testRunner: 'nyc jest',
runnerConfig: 'e2e/config.js',
specs: 'e2e/*.test.js',
skipLegacyWorkersInjection: true,

behavior: {
init: {
Expand Down
1 change: 0 additions & 1 deletion detox/test/types/detox-jest-circus-setup-tests.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@ beforeAll(async () => {
testRunner: 'nyc jest',
runnerConfig: 'e2e/config.js',
specs: 'e2e/*.test.js',
skipLegacyWorkersInjection: true,
behavior: {
init: {
reinstallApp: true,
Expand Down
6 changes: 2 additions & 4 deletions docs/Guide.Jest.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -63,15 +63,13 @@ Even if `detox init` passes well, and everything is green, we still recommend go
| ---------------------------- | ----------------------------------- | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
| `testRunner` | `"jest"` | _Required._ Should be `"jest"` for the proper `detox test` CLI functioning. |
| `runnerConfig` | (optional path to Jest config file) | _Optional._ This field tells `detox test` CLI where to look for Jest’s config file. If omitted, the default value is `e2e/config.json`. |
| `skipLegacyWorkersInjection` | `false` or `true` | _Optional._ This field tells `detox test` to stop appending `--maxWorkers 1` argument to `jest ...` command by default. Since `detox@18.19.0`, the control over `maxWorkers` count has been transfered to Jest config files, and that allows you to set any other value as a default `maxWorkers` count. |

A typical Detox configuration in `.detoxrc.json` file looks like:

```json
{
"testRunner": "jest",
"runnerConfig": "e2e/config.json",
"skipLegacyWorkersInjection": true,
"devices": {
"simulator": {
"type": "ios.simulator",
Expand Down Expand Up @@ -99,8 +97,8 @@ A typical Detox configuration in `.detoxrc.json` file looks like:
##### `e2e/config.json`

| Property | Value | Description |
| ----------------- | ------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
| `maxWorkers` | `1` | _Recommended._ When being used with `skipLegacyWorkersInjection: true` in Detox config, it prevents overallocation of mobile devices in the light of Jest’s default logic (`= cpusCount — 1`), when you do not pass any specific worker count. To override it, [use CLI arguments](APIRef.DetoxCLI.md#test), or see [Jest documentation](https://jestjs.io/docs/configuration#maxworkers-number--string) if you plan to change the default value in the config. |
| ----------------- | ------------------------------------------- |---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|
| `maxWorkers` | `1` | _Recommended._ It prevents potential overallocation of mobile devices according to the default logic of Jest (`maxWorkers = cpusCount — 1`) for the default workers count. To override it, [use CLI arguments](APIRef.DetoxCLI.md#test), or see [Jest documentation](https://jestjs.io/docs/configuration#maxworkers-number--string) if you plan to change the default value in the config. |
| `testEnvironment` | `"./environment"` | _Required._ Needed for the proper functioning of Jest and Detox. See [Jest documentation](https://jestjs.io/docs/en/configuration#testenvironment-string) for more details. |
| `testRunner` | `"jest-circus/runner"` | _Required._ Needed for the proper functioning of Jest and Detox. See [Jest documentation](https://jestjs.io/docs/en/configuration#testrunner-string) for more details. |
| `testTimeout` | `120000` | _Required_. Overrides the default timeout (5 seconds), which is usually too short to complete a single end-to-end test. |
Expand Down

0 comments on commit c6751f6

Please sign in to comment.