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.

Sign up for GitHub

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

Jump to bottom

feat(config): drop skipLegacyWorkersInjection #3286

Merged
merged 1 commit into from
Apr 4, 2022
+65 −92
Merged

feat(config): drop skipLegacyWorkersInjection #3286

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 0 additions & 6 deletions detox/index.d.ts
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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