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

fix(jest-core): don't use workers in watch mode if runInBand specified #14085

Merged
merged 7 commits into from
Aug 16, 2023
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
### Fixes

- `[jest-config]` Handle frozen config object ([#14054](https://github.com/facebook/jest/pull/14054))
- `[jest-core]` Always use workers in watch mode to avoid crashes ([#14059](https://github.com/facebook/jest/pull/14059)).
- `[jest-core]` Use workers in watch mode by default to avoid crashes ([#14059](https://github.com/facebook/jest/pull/14059) & [#14085](https://github.com/facebook/jest/pull/14085)).
- `[jest-environment-jsdom, jest-environment-node]` Fix assignment of `customExportConditions` via `testEnvironmentOptions` when custom env subclass defines a default value ([#13989](https://github.com/facebook/jest/pull/13989))
- `[jest-matcher-utils]` Fix copying value of inherited getters ([#14007](https://github.com/facebook/jest/pull/14007))
- `[jest-mock]` Tweak typings to allow `jest.replaceProperty()` replace methods ([#14008](https://github.com/facebook/jest/pull/14008))
Expand Down
1 change: 1 addition & 0 deletions packages/jest-config/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,7 @@ const groupOptions = (
replname: options.replname,
reporters: options.reporters,
rootDir: options.rootDir,
runInBand: options.runInBand,
runTestsByPath: options.runTestsByPath,
seed: options.seed,
shard: options.shard,
Expand Down
41 changes: 22 additions & 19 deletions packages/jest-core/src/__tests__/testSchedulerHelper.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,32 +23,34 @@ const getTestMock = () => ({
const getTestsMock = () => [getTestMock(), getTestMock()];

test.each`
tests | timings | detectOpenHandles | maxWorkers | watch | workerIdleMemoryLimit | expectedResult
${[getTestMock()]} | ${[500, 500]} | ${false} | ${undefined} | ${true} | ${undefined} | ${false}
${getTestsMock()} | ${[2000, 500]} | ${false} | ${1} | ${true} | ${undefined} | ${false}
${getTestsMock()} | ${[2000, 500]} | ${false} | ${2} | ${true} | ${undefined} | ${false}
${[getTestMock()]} | ${[2000, 500]} | ${false} | ${undefined} | ${true} | ${undefined} | ${false}
${getTestMock()} | ${[500, 500]} | ${false} | ${undefined} | ${true} | ${undefined} | ${false}
${getTestsMock()} | ${[2000, 500]} | ${false} | ${1} | ${false} | ${undefined} | ${true}
${getTestMock()} | ${[2000, 500]} | ${false} | ${2} | ${false} | ${undefined} | ${false}
${[getTestMock()]} | ${[2000]} | ${false} | ${undefined} | ${false} | ${undefined} | ${true}
${getTestsMock()} | ${[500, 500]} | ${false} | ${undefined} | ${false} | ${undefined} | ${true}
${new Array(45)} | ${[500]} | ${false} | ${undefined} | ${false} | ${undefined} | ${false}
${getTestsMock()} | ${[2000, 500]} | ${false} | ${undefined} | ${false} | ${undefined} | ${false}
${getTestsMock()} | ${[2000, 500]} | ${true} | ${undefined} | ${false} | ${undefined} | ${true}
${[getTestMock()]} | ${[500, 500]} | ${false} | ${undefined} | ${true} | ${'500MB'} | ${false}
${getTestsMock()} | ${[2000, 500]} | ${false} | ${1} | ${true} | ${'500MB'} | ${false}
${getTestsMock()} | ${[2000, 500]} | ${false} | ${1} | ${false} | ${'500MB'} | ${false}
${[getTestMock()]} | ${[2000]} | ${false} | ${undefined} | ${false} | ${'500MB'} | ${false}
${getTestsMock()} | ${[500, 500]} | ${false} | ${undefined} | ${false} | ${'500MB'} | ${false}
${getTestsMock()} | ${[2000, 500]} | ${true} | ${undefined} | ${false} | ${'500MB'} | ${true}
tests | timings | detectOpenHandles | runInBand | maxWorkers | watch | workerIdleMemoryLimit | expectedResult
${[getTestMock()]} | ${[500, 500]} | ${false} | ${false} | ${undefined} | ${true} | ${undefined} | ${false}
${getTestsMock()} | ${[2000, 500]} | ${false} | ${false} | ${1} | ${true} | ${undefined} | ${false}
${getTestsMock()} | ${[2000, 500]} | ${false} | ${false} | ${2} | ${true} | ${undefined} | ${false}
${getTestsMock()} | ${[2000, 500]} | ${false} | ${true} | ${1} | ${true} | ${undefined} | ${true}
${[getTestMock()]} | ${[2000, 500]} | ${false} | ${false} | ${undefined} | ${true} | ${undefined} | ${false}
${getTestMock()} | ${[500, 500]} | ${false} | ${false} | ${undefined} | ${true} | ${undefined} | ${false}
${getTestsMock()} | ${[2000, 500]} | ${false} | ${false} | ${1} | ${false} | ${undefined} | ${true}
${getTestMock()} | ${[2000, 500]} | ${false} | ${false} | ${2} | ${false} | ${undefined} | ${false}
${[getTestMock()]} | ${[2000]} | ${false} | ${false} | ${undefined} | ${false} | ${undefined} | ${true}
${getTestsMock()} | ${[500, 500]} | ${false} | ${false} | ${undefined} | ${false} | ${undefined} | ${true}
${new Array(45)} | ${[500]} | ${false} | ${false} | ${undefined} | ${false} | ${undefined} | ${false}
${getTestsMock()} | ${[2000, 500]} | ${false} | ${false} | ${undefined} | ${false} | ${undefined} | ${false}
${getTestsMock()} | ${[2000, 500]} | ${true} | ${false} | ${undefined} | ${false} | ${undefined} | ${true}
${[getTestMock()]} | ${[500, 500]} | ${false} | ${false} | ${undefined} | ${true} | ${'500MB'} | ${false}
${getTestsMock()} | ${[2000, 500]} | ${false} | ${false} | ${1} | ${true} | ${'500MB'} | ${false}
${getTestsMock()} | ${[2000, 500]} | ${false} | ${false} | ${1} | ${false} | ${'500MB'} | ${false}
${[getTestMock()]} | ${[2000]} | ${false} | ${false} | ${undefined} | ${false} | ${'500MB'} | ${false}
${getTestsMock()} | ${[500, 500]} | ${false} | ${false} | ${undefined} | ${false} | ${'500MB'} | ${false}
${getTestsMock()} | ${[2000, 500]} | ${true} | ${false} | ${undefined} | ${false} | ${'500MB'} | ${true}
`(
'shouldRunInBand() - should return $expectedResult for runInBand mode',
({
tests,
timings,
detectOpenHandles,
maxWorkers,
runInBand,
watch,
workerIdleMemoryLimit,
expectedResult,
Expand All @@ -57,6 +59,7 @@ test.each`
shouldRunInBand(tests, timings, {
detectOpenHandles,
maxWorkers,
runInBand,
watch,
workerIdleMemoryLimit,
}),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,7 @@ exports[`prints the config object 1`] = `
"projects": [],
"reporters": [],
"rootDir": "/test_root_dir/",
"runInBand": false,
"runTestsByPath": false,
"seed": 1234,
"silent": false,
Expand Down
7 changes: 3 additions & 4 deletions packages/jest-core/src/testSchedulerHelper.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,13 +16,15 @@ export function shouldRunInBand(
{
detectOpenHandles,
maxWorkers,
runInBand,
watch,
watchAll,
workerIdleMemoryLimit,
}: Config.GlobalConfig,
): boolean {
// If user asked for run in band, respect that.
// detectOpenHandles makes no sense without runInBand, because it cannot detect leaks in workers
if (detectOpenHandles) {
if (runInBand || detectOpenHandles) {
return true;
}

Expand All @@ -39,9 +41,6 @@ export function shouldRunInBand(
* Otherwise, run in band if we only have one test or one worker available.
* Also, if we are confident from previous runs that the tests will finish
* quickly we also run in band to reduce the overhead of spawning workers.
* Finally, the user can provide the runInBand argument in the CLI to
* force running in band, which sets maxWorkers to 1 here:
* https://github.com/facebook/jest/blob/d106643a8ee0ffa9c2f11c6bb2d12094e99135aa/packages/jest-config/src/getMaxWorkers.ts#L27-L28
*/
const areFastTests = timings.every(timing => timing < SLOW_TEST_TIME);
const oneWorkerOrLess = maxWorkers <= 1;
Expand Down
1 change: 1 addition & 0 deletions packages/jest-types/src/Config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -400,6 +400,7 @@ export type GlobalConfig = {
randomize?: boolean;
replname?: string;
reporters?: Array<ReporterConfig>;
runInBand: boolean;
runTestsByPath: boolean;
rootDir: string;
seed: number;
Expand Down
1 change: 1 addition & 0 deletions packages/test-utils/src/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ const DEFAULT_GLOBAL_CONFIG: Config.GlobalConfig = {
replname: undefined,
reporters: [],
rootDir: '/test_root_dir/',
runInBand: false,
runTestsByPath: false,
seed: 1234,
silent: false,
Expand Down