diff --git a/CHANGELOG.md b/CHANGELOG.md index 7c9cddc2c0bd..e0de77bdd531 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -82,6 +82,7 @@ - `[jest-haste-map]` Fix `require` detection with trailing commas and ignore `import typeof` modules ([#7385](https://github.com/facebook/jest/pull/7385)) - `[jest-cli]` Fix to set prettierPath via config file ([#7412](https://github.com/facebook/jest/pull/7412)) - `[jest-cli]` Support dashed args ([#7497](https://github.com/facebook/jest/pull/7497)) +- `[jest-cli]` Fix to run in band tests if watch mode enable when runInBand arg used ([#7518](https://github.com/facebook/jest/pull/7518)) ### Chore & Maintenance diff --git a/e2e/__tests__/failures.test.js b/e2e/__tests__/failures.test.js index 8a88033c580c..5b79e1ed3cd8 100644 --- a/e2e/__tests__/failures.test.js +++ b/e2e/__tests__/failures.test.js @@ -43,7 +43,7 @@ test('works with node assert', () => { // Node 9 started to include the error for `doesNotThrow` // https://github.com/nodejs/node/pull/12167 - if (nodeMajorVersion >= 8) { + if (nodeMajorVersion >= 9) { expect(summary).toContain(` assert.doesNotThrow(function) @@ -71,7 +71,7 @@ test('works with node assert', () => { Got unwanted exception. `; - if (nodeMajorVersion === 8 || nodeMajorVersion === 9) { + if (nodeMajorVersion === 9) { const specificErrorMessage = `Message: Got unwanted exception. err! diff --git a/packages/jest-cli/src/TestScheduler.js b/packages/jest-cli/src/TestScheduler.js index 682cde647d1e..7cb71decadce 100644 --- a/packages/jest-cli/src/TestScheduler.js +++ b/packages/jest-cli/src/TestScheduler.js @@ -29,8 +29,7 @@ import SummaryReporter from './reporters/summary_reporter'; import TestRunner from 'jest-runner'; import TestWatcher from './TestWatcher'; import VerboseReporter from './reporters/verbose_reporter'; - -const SLOW_TEST_TIME = 1000; +import {shouldRunInBand} from './testSchedulerHelper'; // The default jest-runner is required because it is the default test runner // and required implicitly through the `runner` ProjectConfig option. @@ -85,25 +84,12 @@ export default class TestScheduler { getEstimatedTime(timings, this._globalConfig.maxWorkers) / 1000, ); - // 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 - // watchAll. - // - // 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. - const areFastTests = timings.every(timing => timing < SLOW_TEST_TIME); - - const runInBandWatch = tests.length <= 1 && areFastTests; - const runInBandNonWatch = - this._globalConfig.maxWorkers <= 1 || - tests.length <= 1 || - (tests.length <= 20 && timings.length > 0 && areFastTests); - - const runInBand = - this._globalConfig.watch || this._globalConfig.watchAll - ? runInBandWatch - : runInBandNonWatch; + const runInBand = shouldRunInBand( + tests, + this._globalConfig.watch || this._globalConfig.watchAll, + this._globalConfig.maxWorkers, + timings, + ); const onResult = async (test: Test, testResult: TestResult) => { if (watcher.isInterrupted()) { diff --git a/packages/jest-cli/src/__tests__/TestScheduler.test.js b/packages/jest-cli/src/__tests__/TestScheduler.test.js index d73eb9684f2d..b5a791163151 100644 --- a/packages/jest-cli/src/__tests__/TestScheduler.test.js +++ b/packages/jest-cli/src/__tests__/TestScheduler.test.js @@ -10,6 +10,7 @@ import TestScheduler from '../TestScheduler'; import SummaryReporter from '../reporters/summary_reporter'; +import * as testSchedulerHelper from '../testSchedulerHelper'; jest.mock('../reporters/default_reporter'); const mockSerialRunner = { @@ -27,8 +28,12 @@ jest.mock('jest-runner-parallel', () => jest.fn(() => mockParallelRunner), { virtual: true, }); +const spyShouldRunInBand = jest.spyOn(testSchedulerHelper, 'shouldRunInBand'); + beforeEach(() => { mockSerialRunner.runTests.mockClear(); + mockParallelRunner.runTests.mockClear(); + spyShouldRunInBand.mockClear(); }); test('config for reporters supports `default`', () => { @@ -180,3 +185,51 @@ test('should not bail if less than `n` failures', async () => { }); expect(setState).not.toBeCalled(); }); + +test('should set runInBand to run in serial', async () => { + const scheduler = new TestScheduler({}, {}); + const test = { + context: { + config: { + runner: 'jest-runner-parallel', + }, + hasteFS: { + matchFiles: jest.fn(() => []), + }, + }, + path: './test/path.js', + }; + const tests = [test, test]; + + spyShouldRunInBand.mockReturnValue(true); + + await scheduler.scheduleTests(tests, {isInterrupted: jest.fn()}); + + expect(spyShouldRunInBand).toHaveBeenCalled(); + expect(mockParallelRunner.runTests).toHaveBeenCalled(); + expect(mockParallelRunner.runTests.mock.calls[0][5].serial).toBeTruthy(); +}); + +test('should set runInBand to not run in serial', async () => { + const scheduler = new TestScheduler({}, {}); + const test = { + context: { + config: { + runner: 'jest-runner-parallel', + }, + hasteFS: { + matchFiles: jest.fn(() => []), + }, + }, + path: './test/path.js', + }; + const tests = [test, test]; + + spyShouldRunInBand.mockReturnValue(false); + + await scheduler.scheduleTests(tests, {isInterrupted: jest.fn()}); + + expect(spyShouldRunInBand).toHaveBeenCalled(); + expect(mockParallelRunner.runTests).toHaveBeenCalled(); + expect(mockParallelRunner.runTests.mock.calls[0][5].serial).toBeFalsy(); +}); diff --git a/packages/jest-cli/src/__tests__/testSchedulerHelper.test.js b/packages/jest-cli/src/__tests__/testSchedulerHelper.test.js new file mode 100644 index 000000000000..8f72b3686afd --- /dev/null +++ b/packages/jest-cli/src/__tests__/testSchedulerHelper.test.js @@ -0,0 +1,45 @@ +/** + * Copyright (c) 2014-present, Facebook, Inc. All rights reserved. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ +'use strict'; + +import {shouldRunInBand} from '../testSchedulerHelper'; + +const getTestMock = () => ({ + context: { + config: { + runner: 'jest-runner-parallel', + }, + hasteFS: { + matchFiles: jest.fn(() => []), + }, + }, + path: './test/path.js', +}); + +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} +`( + 'shouldRunInBand() - should return $expectedResult for runInBand mode', + ({tests, watch, maxWorkers, timings, expectedResult}) => { + expect(shouldRunInBand(tests, watch, maxWorkers, timings)).toBe( + expectedResult, + ); + }, +); diff --git a/packages/jest-cli/src/testSchedulerHelper.js b/packages/jest-cli/src/testSchedulerHelper.js new file mode 100644 index 000000000000..dad51249c419 --- /dev/null +++ b/packages/jest-cli/src/testSchedulerHelper.js @@ -0,0 +1,42 @@ +/** + * Copyright (c) 2014-present, Facebook, Inc. All rights reserved. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + * + * @flow + */ +'use strict'; +import type {Test} from 'types/TestRunner'; + +const SLOW_TEST_TIME = 1000; + +export function shouldRunInBand( + tests: Array, + isWatchMode: boolean, + maxWorkers: number, + timings: number[], +) { + // 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 + // watchAll. + // + // 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. + const areFastTests = timings.every(timing => timing < SLOW_TEST_TIME); + // This apply also when runInBand arg present + // https://github.com/facebook/jest/blob/700e0dadb85f5dc8ff5dac6c7e98956690049734/packages/jest-config/src/getMaxWorkers.js#L14-L17 + const oneWorkerOrLess = maxWorkers <= 1; + const oneTestOrLess = tests.length <= 1; + + if (isWatchMode) { + return oneWorkerOrLess || (oneTestOrLess && areFastTests); + } + + return ( + oneWorkerOrLess || + oneTestOrLess || + (tests.length <= 20 && timings.length > 0 && areFastTests) + ); +}