From 194535618abb5fc17deac34288efdaa23c8be597 Mon Sep 17 00:00:00 2001 From: Tim Seckinger Date: Sun, 7 Apr 2019 17:22:37 +0200 Subject: [PATCH 1/4] pass config into shouldRunInBand --- packages/jest-core/src/TestScheduler.ts | 7 +---- .../src/__tests__/testSchedulerHelper.test.js | 28 +++++++++---------- packages/jest-core/src/testSchedulerHelper.ts | 7 +++-- 3 files changed, 19 insertions(+), 23 deletions(-) diff --git a/packages/jest-core/src/TestScheduler.ts b/packages/jest-core/src/TestScheduler.ts index 62c9ae2ceedd..3c7724b2d3ea 100644 --- a/packages/jest-core/src/TestScheduler.ts +++ b/packages/jest-core/src/TestScheduler.ts @@ -86,12 +86,7 @@ export default class TestScheduler { getEstimatedTime(timings, this._globalConfig.maxWorkers) / 1000, ); - const runInBand = shouldRunInBand( - tests, - this._globalConfig.watch || this._globalConfig.watchAll, - this._globalConfig.maxWorkers, - timings, - ); + const runInBand = shouldRunInBand(tests, timings, this._globalConfig); const onResult = async (test: Test, testResult: TestResult) => { if (watcher.isInterrupted()) { diff --git a/packages/jest-core/src/__tests__/testSchedulerHelper.test.js b/packages/jest-core/src/__tests__/testSchedulerHelper.test.js index 3fc2043f3eb4..09660548160c 100644 --- a/packages/jest-core/src/__tests__/testSchedulerHelper.test.js +++ b/packages/jest-core/src/__tests__/testSchedulerHelper.test.js @@ -23,22 +23,22 @@ const getTestMock = () => ({ const getTestsMock = () => [getTestMock(), getTestMock()]; test.each` - tests | watch | maxWorkers | timings | expectedResult - ${[getTestMock()]} | ${true} | ${undefined} | ${[500, 500]} | ${true} - ${getTestsMock()} | ${true} | ${1} | ${[2000, 500]} | ${true} - ${getTestsMock()} | ${true} | ${2} | ${[2000, 500]} | ${false} - ${[getTestMock()]} | ${true} | ${undefined} | ${[2000, 500]} | ${false} - ${getTestMock()} | ${true} | ${undefined} | ${[500, 500]} | ${false} - ${getTestsMock()} | ${false} | ${1} | ${[2000, 500]} | ${true} - ${getTestMock()} | ${false} | ${2} | ${[2000, 500]} | ${false} - ${[getTestMock()]} | ${false} | ${undefined} | ${[2000]} | ${true} - ${getTestsMock()} | ${false} | ${undefined} | ${[500, 500]} | ${true} - ${new Array(45)} | ${false} | ${undefined} | ${[500]} | ${false} - ${getTestsMock()} | ${false} | ${undefined} | ${[2000, 500]} | ${false} + tests | timings | maxWorkers | watch | expectedResult + ${[getTestMock()]} | ${[500, 500]} | ${undefined} | ${true} | ${true} + ${getTestsMock()} | ${[2000, 500]} | ${1} | ${true} | ${true} + ${getTestsMock()} | ${[2000, 500]} | ${2} | ${true} | ${false} + ${[getTestMock()]} | ${[2000, 500]} | ${undefined} | ${true} | ${false} + ${getTestMock()} | ${[500, 500]} | ${undefined} | ${true} | ${false} + ${getTestsMock()} | ${[2000, 500]} | ${1} | ${false} | ${true} + ${getTestMock()} | ${[2000, 500]} | ${2} | ${false} | ${false} + ${[getTestMock()]} | ${[2000]} | ${undefined} | ${false} | ${true} + ${getTestsMock()} | ${[500, 500]} | ${undefined} | ${false} | ${true} + ${new Array(45)} | ${[500]} | ${undefined} | ${false} | ${false} + ${getTestsMock()} | ${[2000, 500]} | ${undefined} | ${false} | ${false} `( 'shouldRunInBand() - should return $expectedResult for runInBand mode', - ({tests, watch, maxWorkers, timings, expectedResult}) => { - expect(shouldRunInBand(tests, watch, maxWorkers, timings)).toBe( + ({tests, timings, maxWorkers, watch, expectedResult}) => { + expect(shouldRunInBand(tests, timings, {maxWorkers, watch})).toBe( expectedResult, ); }, diff --git a/packages/jest-core/src/testSchedulerHelper.ts b/packages/jest-core/src/testSchedulerHelper.ts index b14e677cd20b..aefe59d7d8c7 100644 --- a/packages/jest-core/src/testSchedulerHelper.ts +++ b/packages/jest-core/src/testSchedulerHelper.ts @@ -5,17 +5,17 @@ * LICENSE file in the root directory of this source tree. */ +import {Config} from '@jest/types'; import {Test} from 'jest-runner'; const SLOW_TEST_TIME = 1000; export function shouldRunInBand( tests: Array, - isWatchMode: boolean, - maxWorkers: number, timings: Array, + {maxWorkers, watch, watchAll}: Config.GlobalConfig, ) { - /** + /* * Run in band if we only have one test or one worker available, unless we * are using the watch mode, in which case the TTY has to be responsive and * we cannot schedule anything in the main thread. Same logic applies to @@ -26,6 +26,7 @@ export function shouldRunInBand( * force running in band. * https://github.com/facebook/jest/blob/700e0dadb85f5dc8ff5dac6c7e98956690049734/packages/jest-config/src/getMaxWorkers.js#L14-L17 */ + const isWatchMode = watch || watchAll; const areFastTests = timings.every(timing => timing < SLOW_TEST_TIME); const oneWorkerOrLess = maxWorkers <= 1; const oneTestOrLess = tests.length <= 1; From a281212498f9e3ffc86446eda60d4b21e93da6cb Mon Sep 17 00:00:00 2001 From: Tim Seckinger Date: Sun, 7 Apr 2019 17:32:09 +0200 Subject: [PATCH 2/4] detectOpenHandles imply runInBand --- .../src/__tests__/testSchedulerHelper.test.js | 33 ++++++++++--------- packages/jest-core/src/testSchedulerHelper.ts | 7 +++- 2 files changed, 23 insertions(+), 17 deletions(-) diff --git a/packages/jest-core/src/__tests__/testSchedulerHelper.test.js b/packages/jest-core/src/__tests__/testSchedulerHelper.test.js index 09660548160c..d3cc137013c4 100644 --- a/packages/jest-core/src/__tests__/testSchedulerHelper.test.js +++ b/packages/jest-core/src/__tests__/testSchedulerHelper.test.js @@ -23,23 +23,24 @@ const getTestMock = () => ({ const getTestsMock = () => [getTestMock(), getTestMock()]; test.each` - tests | timings | maxWorkers | watch | expectedResult - ${[getTestMock()]} | ${[500, 500]} | ${undefined} | ${true} | ${true} - ${getTestsMock()} | ${[2000, 500]} | ${1} | ${true} | ${true} - ${getTestsMock()} | ${[2000, 500]} | ${2} | ${true} | ${false} - ${[getTestMock()]} | ${[2000, 500]} | ${undefined} | ${true} | ${false} - ${getTestMock()} | ${[500, 500]} | ${undefined} | ${true} | ${false} - ${getTestsMock()} | ${[2000, 500]} | ${1} | ${false} | ${true} - ${getTestMock()} | ${[2000, 500]} | ${2} | ${false} | ${false} - ${[getTestMock()]} | ${[2000]} | ${undefined} | ${false} | ${true} - ${getTestsMock()} | ${[500, 500]} | ${undefined} | ${false} | ${true} - ${new Array(45)} | ${[500]} | ${undefined} | ${false} | ${false} - ${getTestsMock()} | ${[2000, 500]} | ${undefined} | ${false} | ${false} + tests | timings | detectOpenHandles | maxWorkers | watch | expectedResult + ${[getTestMock()]} | ${[500, 500]} | ${false} | ${undefined} | ${true} | ${true} + ${getTestsMock()} | ${[2000, 500]} | ${false} | ${1} | ${true} | ${true} + ${getTestsMock()} | ${[2000, 500]} | ${false} | ${2} | ${true} | ${false} + ${[getTestMock()]} | ${[2000, 500]} | ${false} | ${undefined} | ${true} | ${false} + ${getTestMock()} | ${[500, 500]} | ${false} | ${undefined} | ${true} | ${false} + ${getTestsMock()} | ${[2000, 500]} | ${false} | ${1} | ${false} | ${true} + ${getTestMock()} | ${[2000, 500]} | ${false} | ${2} | ${false} | ${false} + ${[getTestMock()]} | ${[2000]} | ${false} | ${undefined} | ${false} | ${true} + ${getTestsMock()} | ${[500, 500]} | ${false} | ${undefined} | ${false} | ${true} + ${new Array(45)} | ${[500]} | ${false} | ${undefined} | ${false} | ${false} + ${getTestsMock()} | ${[2000, 500]} | ${false} | ${undefined} | ${false} | ${false} + ${getTestsMock()} | ${[2000, 500]} | ${true} | ${undefined} | ${false} | ${true} `( 'shouldRunInBand() - should return $expectedResult for runInBand mode', - ({tests, timings, maxWorkers, watch, expectedResult}) => { - expect(shouldRunInBand(tests, timings, {maxWorkers, watch})).toBe( - expectedResult, - ); + ({tests, timings, detectOpenHandles, maxWorkers, watch, expectedResult}) => { + expect( + shouldRunInBand(tests, timings, {detectOpenHandles, maxWorkers, watch}), + ).toBe(expectedResult); }, ); diff --git a/packages/jest-core/src/testSchedulerHelper.ts b/packages/jest-core/src/testSchedulerHelper.ts index aefe59d7d8c7..8fe17525aec6 100644 --- a/packages/jest-core/src/testSchedulerHelper.ts +++ b/packages/jest-core/src/testSchedulerHelper.ts @@ -13,8 +13,13 @@ const SLOW_TEST_TIME = 1000; export function shouldRunInBand( tests: Array, timings: Array, - {maxWorkers, watch, watchAll}: Config.GlobalConfig, + {detectOpenHandles, maxWorkers, watch, watchAll}: Config.GlobalConfig, ) { + // detectOpenHandles makes no sense without runInBand, because it cannot detect leaks in workers + if (detectOpenHandles) { + return true; + } + /* * Run in band if we only have one test or one worker available, unless we * are using the watch mode, in which case the TTY has to be responsive and From 2d17dd122680b7e28467dbefde44f9833af88d3e Mon Sep 17 00:00:00 2001 From: Tim Seckinger Date: Sun, 7 Apr 2019 17:40:30 +0200 Subject: [PATCH 3/4] Update CHANGELOG.md --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 549d7efa84ec..59d17e8a8817 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,7 @@ ### Fixes - `[jest-snapshot]` Inline snapshots: do not indent empty lines ([#8277](https://github.com/facebook/jest/pull/8277)) +- `[jest-core]` Make `detectOpenHandles` imply `runInBand` ([#8283](https://github.com/facebook/jest/pull/8283)) ### Chore & Maintenance From 28b18fe66ea713f0254b865a0f9a7d5e80ec28d1 Mon Sep 17 00:00:00 2001 From: Tim Seckinger Date: Sun, 7 Apr 2019 20:30:30 +0200 Subject: [PATCH 4/4] docs --- docs/CLI.md | 2 +- packages/jest-cli/src/cli/args.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/CLI.md b/docs/CLI.md index f6b483391f7b..2530a940879b 100644 --- a/docs/CLI.md +++ b/docs/CLI.md @@ -158,7 +158,7 @@ Print debugging info about your Jest config. ### `--detectOpenHandles` -Attempt to collect and print open handles preventing Jest from exiting cleanly. Use this in cases where you need to use `--forceExit` in order for Jest to exit to potentially track down the reason. Implemented using [`async_hooks`](https://nodejs.org/api/async_hooks.html), so it only works in Node 8 and newer. +Attempt to collect and print open handles preventing Jest from exiting cleanly. Use this in cases where you need to use `--forceExit` in order for Jest to exit to potentially track down the reason. This implies `--runInBand`, making tests run serially. Implemented using [`async_hooks`](https://nodejs.org/api/async_hooks.html), so it only works in Node 8 and newer. This option has a significant performance penalty and should only be used for debugging. ### `--env=` diff --git a/packages/jest-cli/src/cli/args.ts b/packages/jest-cli/src/cli/args.ts index 4c68b6e1a9c9..2f7c38aa0279 100644 --- a/packages/jest-cli/src/cli/args.ts +++ b/packages/jest-cli/src/cli/args.ts @@ -229,7 +229,7 @@ export const options = { default: false, description: 'Print out remaining open handles preventing Jest from exiting at the ' + - 'end of a test run.', + 'end of a test run. Implies `runInBand`.', type: 'boolean' as 'boolean', }, env: {