Skip to content

Commit

Permalink
detectOpenHandles imply runInBand (#8283)
Browse files Browse the repository at this point in the history
* pass config into shouldRunInBand

* detectOpenHandles imply runInBand

* Update CHANGELOG.md

* docs
  • Loading branch information
jeysal committed Apr 7, 2019
1 parent 650c0b5 commit f435f01
Show file tree
Hide file tree
Showing 6 changed files with 30 additions and 27 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Expand Up @@ -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

Expand Down
2 changes: 1 addition & 1 deletion docs/CLI.md
Expand Up @@ -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=<environment>`

Expand Down
2 changes: 1 addition & 1 deletion packages/jest-cli/src/cli/args.ts
Expand Up @@ -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: {
Expand Down
7 changes: 1 addition & 6 deletions packages/jest-core/src/TestScheduler.ts
Expand Up @@ -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()) {
Expand Down
33 changes: 17 additions & 16 deletions packages/jest-core/src/__tests__/testSchedulerHelper.test.js
Expand Up @@ -23,23 +23,24 @@ 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 | 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, watch, maxWorkers, timings, expectedResult}) => {
expect(shouldRunInBand(tests, watch, maxWorkers, timings)).toBe(
expectedResult,
);
({tests, timings, detectOpenHandles, maxWorkers, watch, expectedResult}) => {
expect(
shouldRunInBand(tests, timings, {detectOpenHandles, maxWorkers, watch}),
).toBe(expectedResult);
},
);
12 changes: 9 additions & 3 deletions packages/jest-core/src/testSchedulerHelper.ts
Expand Up @@ -5,17 +5,22 @@
* 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<Test>,
isWatchMode: boolean,
maxWorkers: number,
timings: Array<number>,
{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
* we cannot schedule anything in the main thread. Same logic applies to
Expand All @@ -26,6 +31,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;
Expand Down

0 comments on commit f435f01

Please sign in to comment.