From 38cdf0cb38a332701745a864e041614b5a05a15a Mon Sep 17 00:00:00 2001 From: Ben Kraft Date: Tue, 18 Apr 2023 14:57:33 -0700 Subject: [PATCH 1/5] fix(jest-core): don't use workers in watch mode if runInBand specified In #14059, I added code to say that if you use watch mode, even if you only run one test and one worker, we should still not run in band, because then a hard-crash in the test will crash the entire watcher. But the way the logic is written, this overrides even an explicit `--runInBand`, which can be useful if you really want to, say, attach a debugger to watch-mode; it's not the smoothest experience but it's better than nothing. In this commit I make it so that if you explicitly say `--runInBand` that overrides everything else. (But if you just say `--maxWorkers=1`, or you just only have one test to run, we still use workers.) This required some replumbing; let me know if there's a better way. --- CHANGELOG.md | 2 +- packages/jest-config/src/index.ts | 1 + .../src/__tests__/testSchedulerHelper.test.js | 41 ++++++++++--------- packages/jest-core/src/testSchedulerHelper.ts | 7 ++-- packages/jest-types/src/Config.ts | 1 + packages/test-utils/src/config.ts | 1 + 6 files changed, 29 insertions(+), 24 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 309e3d342619..9138d10da3a8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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) & TODO). - `[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)) diff --git a/packages/jest-config/src/index.ts b/packages/jest-config/src/index.ts index f68c5f19b0c0..9f4eb24e02b7 100644 --- a/packages/jest-config/src/index.ts +++ b/packages/jest-config/src/index.ts @@ -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, diff --git a/packages/jest-core/src/__tests__/testSchedulerHelper.test.js b/packages/jest-core/src/__tests__/testSchedulerHelper.test.js index d4fbc2756750..945add3f9497 100644 --- a/packages/jest-core/src/__tests__/testSchedulerHelper.test.js +++ b/packages/jest-core/src/__tests__/testSchedulerHelper.test.js @@ -23,25 +23,26 @@ 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', ({ @@ -49,6 +50,7 @@ test.each` timings, detectOpenHandles, maxWorkers, + runInBand, watch, workerIdleMemoryLimit, expectedResult, @@ -57,6 +59,7 @@ test.each` shouldRunInBand(tests, timings, { detectOpenHandles, maxWorkers, + runInBand, watch, workerIdleMemoryLimit, }), diff --git a/packages/jest-core/src/testSchedulerHelper.ts b/packages/jest-core/src/testSchedulerHelper.ts index 6582415ac5d5..121025f3746c 100644 --- a/packages/jest-core/src/testSchedulerHelper.ts +++ b/packages/jest-core/src/testSchedulerHelper.ts @@ -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; } @@ -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; diff --git a/packages/jest-types/src/Config.ts b/packages/jest-types/src/Config.ts index 54240a747037..7266c09b4191 100644 --- a/packages/jest-types/src/Config.ts +++ b/packages/jest-types/src/Config.ts @@ -400,6 +400,7 @@ export type GlobalConfig = { randomize?: boolean; replname?: string; reporters?: Array; + runInBand: boolean; runTestsByPath: boolean; rootDir: string; seed: number; diff --git a/packages/test-utils/src/config.ts b/packages/test-utils/src/config.ts index 0ef998c88636..b7b68b149887 100644 --- a/packages/test-utils/src/config.ts +++ b/packages/test-utils/src/config.ts @@ -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, From 86a4af54457115c30d3b300102a2536b00a0e777 Mon Sep 17 00:00:00 2001 From: Ben Kraft Date: Tue, 18 Apr 2023 16:30:40 -0700 Subject: [PATCH 2/5] PR number in changelog --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9138d10da3a8..760ead99e2d9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,7 +7,7 @@ ### Fixes - `[jest-config]` Handle frozen config object ([#14054](https://github.com/facebook/jest/pull/14054)) -- `[jest-core]` Use workers in watch mode by default to avoid crashes ([#14059](https://github.com/facebook/jest/pull/14059) & TODO). +- `[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)) From b6400122efc6518175977495f4c4b6f6ea1b3cb1 Mon Sep 17 00:00:00 2001 From: Ben Kraft Date: Tue, 18 Apr 2023 16:43:51 -0700 Subject: [PATCH 3/5] snapshot --- .../lib/__tests__/__snapshots__/logDebugMessages.test.ts.snap | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/jest-core/src/lib/__tests__/__snapshots__/logDebugMessages.test.ts.snap b/packages/jest-core/src/lib/__tests__/__snapshots__/logDebugMessages.test.ts.snap index f9e919b15443..54e9c406da8e 100644 --- a/packages/jest-core/src/lib/__tests__/__snapshots__/logDebugMessages.test.ts.snap +++ b/packages/jest-core/src/lib/__tests__/__snapshots__/logDebugMessages.test.ts.snap @@ -96,6 +96,7 @@ exports[`prints the config object 1`] = ` "projects": [], "reporters": [], "rootDir": "/test_root_dir/", + "runInBand": false, "runTestsByPath": false, "seed": 1234, "silent": false, From 5de5ea7423f1bce616bfbc6bef2c3f6e018932a5 Mon Sep 17 00:00:00 2001 From: Ben Kraft Date: Thu, 27 Jul 2023 13:04:48 -0700 Subject: [PATCH 4/5] prettier --- docs/Configuration.md | 1 + website/versioned_docs/version-29.6/Configuration.md | 1 + 2 files changed, 2 insertions(+) diff --git a/docs/Configuration.md b/docs/Configuration.md index f60f505bef71..0fe862f4bb3d 100644 --- a/docs/Configuration.md +++ b/docs/Configuration.md @@ -1177,6 +1177,7 @@ export default config; ``` We hope to support Prettier v3 seamlessly out of the box in a future version of Jest. See [this](https://github.com/jestjs/jest/issues/14305) tracking issue. + ### `projects` \[array<string | ProjectConfig>] diff --git a/website/versioned_docs/version-29.6/Configuration.md b/website/versioned_docs/version-29.6/Configuration.md index f60f505bef71..0fe862f4bb3d 100644 --- a/website/versioned_docs/version-29.6/Configuration.md +++ b/website/versioned_docs/version-29.6/Configuration.md @@ -1177,6 +1177,7 @@ export default config; ``` We hope to support Prettier v3 seamlessly out of the box in a future version of Jest. See [this](https://github.com/jestjs/jest/issues/14305) tracking issue. + ### `projects` \[array<string | ProjectConfig>] From a1f6a911744d2574b2883ab2d7f4baac94c0faec Mon Sep 17 00:00:00 2001 From: Simen Bekkhus Date: Wed, 16 Aug 2023 15:14:24 +0200 Subject: [PATCH 5/5] Update CHANGELOG.md --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a84ed1d61ee1..e15bad622f72 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,7 @@ ### Fixes - `[expect]` Remove `@types/node` from dependencies ([#14385](https://github.com/jestjs/jest/pull/14385)) +- `[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-reporters]` Update `istanbul-lib-instrument` dependency to v6. ([#14401](https://github.com/jestjs/jest/pull/14401)) ### Chore & Maintenance @@ -45,7 +46,6 @@ - `[jest-circus]` Prevent false test failures caused by promise rejections handled asynchronously ([#14110](https://github.com/jestjs/jest/pull/14110)) - `[jest-config]` Handle frozen config object ([#14054](https://github.com/facebook/jest/pull/14054)) - `[jest-config]` Allow `coverageDirectory` and `collectCoverageFrom` in project config ([#14180](https://github.com/jestjs/jest/pull/14180)) -- `[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-core]` Always use workers in watch mode to avoid crashes ([#14059](https://github.com/facebook/jest/pull/14059)). - `[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))