From f85c7dca4212272fb6368689a314d89684621c0c Mon Sep 17 00:00:00 2001 From: Yaroslav Serhieiev Date: Mon, 4 Apr 2022 20:58:02 +0300 Subject: [PATCH] feat(config): drop skipLegacyWorkersInjection (#3286) BREAKING CHANGE: Detox will no longer be injecting "-w 1" to Jest as CLI arguments --- detox/index.d.ts | 6 - detox/local-cli/init.js | 1 - detox/local-cli/test.js | 6 +- detox/local-cli/test.test.js | 135 ++++++++---------- .../src/configuration/composeRunnerConfig.js | 1 - detox/test/e2e/detox.config.js | 1 - .../types/detox-jest-circus-setup-tests.ts | 1 - docs/Guide.Jest.md | 6 +- 8 files changed, 65 insertions(+), 92 deletions(-) diff --git a/detox/index.d.ts b/detox/index.d.ts index 5b85092547..234778c87b 100644 --- a/detox/index.d.ts +++ b/detox/index.d.ts @@ -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' */ diff --git a/detox/local-cli/init.js b/detox/local-cli/init.js index 84b36efeae..cddd1e06a2 100644 --- a/detox/local-cli/init.js +++ b/detox/local-cli/init.js @@ -102,7 +102,6 @@ function createJestFolderE2E() { createFile('.detoxrc.json', JSON.stringify({ testRunner: 'jest', runnerConfig: 'e2e/config.json', - skipLegacyWorkersInjection: true, ...createDefaultConfigurations(), }, null, 2)); } diff --git a/detox/local-cli/test.js b/detox/local-cli/test.js index 561e2c2c7c..d68c3414a9 100644 --- a/detox/local-cli/test.js +++ b/detox/local-cli/test.js @@ -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, diff --git a/detox/local-cli/test.test.js b/detox/local-cli/test.test.js index eddcfa404d..fcb8d25869 100644 --- a/detox/local-cli/test.test.js +++ b/detox/local-cli/test.test.js @@ -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'); @@ -23,9 +24,11 @@ describe('CLI', () => { let detoxConfigPath; let DeviceRegistry; let GenyDeviceRegistryFactory; + let jestInternals; let _env; beforeEach(() => { + temporaryFiles = []; _env = process.env; process.env = { ..._env }; @@ -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()); @@ -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}`); }); @@ -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}`); }); @@ -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 should provide inverted --testNamePattern that configuration (jest)', async (__configuration) => { @@ -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 should be passed as CLI argument', async (__workers) => { - await run(`${__workers} 2`); - expect(cliCall().command).toContain('--maxWorkers 2'); - }); + test.each([['-w'], ['--workers']])('%s should be passed as CLI argument', async (__workers) => { + await run(`${__workers} 2`); + expect(cliCall().command).toContain('--maxWorkers 2'); + }); - test.each([['-w'], ['--workers']])('%s should be replaced with --maxWorkers ', async (__workers) => { - await run(`${__workers} 2 --maxWorkers 3`); + test.each([['-w'], ['--workers']])('%s should be replaced with --maxWorkers ', 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 can be overriden by a later value', async (__workers) => { - await run(`${__workers} 2 ${__workers} 3`); + test.each([['-w'], ['--workers']])('%s 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 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 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 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 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 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 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 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 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 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 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 () => { diff --git a/detox/src/configuration/composeRunnerConfig.js b/detox/src/configuration/composeRunnerConfig.js index 3c59655dd6..78fcd43204 100644 --- a/detox/src/configuration/composeRunnerConfig.js +++ b/detox/src/configuration/composeRunnerConfig.js @@ -12,7 +12,6 @@ function composeRunnerConfig({ globalConfig, cliConfig }) { testRunner, runnerConfig: customRunnerConfig || defaultRunnerConfig, specs: globalConfig.specs || '', - skipLegacyWorkersInjection: Boolean(globalConfig.skipLegacyWorkersInjection), }; } diff --git a/detox/test/e2e/detox.config.js b/detox/test/e2e/detox.config.js index 0c81e24136..0cb3753a8b 100644 --- a/detox/test/e2e/detox.config.js +++ b/detox/test/e2e/detox.config.js @@ -21,7 +21,6 @@ const config = { testRunner: 'nyc jest', runnerConfig: 'e2e/config.js', specs: 'e2e/*.test.js', - skipLegacyWorkersInjection: true, behavior: { init: { diff --git a/detox/test/types/detox-jest-circus-setup-tests.ts b/detox/test/types/detox-jest-circus-setup-tests.ts index 01a19fd99f..2643937fc5 100644 --- a/detox/test/types/detox-jest-circus-setup-tests.ts +++ b/detox/test/types/detox-jest-circus-setup-tests.ts @@ -25,7 +25,6 @@ beforeAll(async () => { testRunner: 'nyc jest', runnerConfig: 'e2e/config.js', specs: 'e2e/*.test.js', - skipLegacyWorkersInjection: true, behavior: { init: { reinstallApp: true, diff --git a/docs/Guide.Jest.md b/docs/Guide.Jest.md index d03bcbe823..7fcd0b423d 100644 --- a/docs/Guide.Jest.md +++ b/docs/Guide.Jest.md @@ -63,7 +63,6 @@ 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: @@ -71,7 +70,6 @@ A typical Detox configuration in `.detoxrc.json` file looks like: { "testRunner": "jest", "runnerConfig": "e2e/config.json", - "skipLegacyWorkersInjection": true, "devices": { "simulator": { "type": "ios.simulator", @@ -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. |