Skip to content

Commit

Permalink
fix: TestScheduler now take in count the runInBand config (#7518)
Browse files Browse the repository at this point in the history
* refactor: review - check for runInBand via worker count in test scheduler watch mode case

* test: updates tests for computeRunInBand

* refactor: code review

* doc: update CHANGELOG

* fix; failure on CI due to node8 changes

* use named exports instead of asterisk
  • Loading branch information
nasreddine-skandrani-dev authored and SimenB committed Dec 24, 2018
1 parent 85890f2 commit 3c7b110
Show file tree
Hide file tree
Showing 5 changed files with 148 additions and 21 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,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

Expand Down
28 changes: 7 additions & 21 deletions packages/jest-cli/src/TestScheduler.js
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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()) {
Expand Down
53 changes: 53 additions & 0 deletions packages/jest-cli/src/__tests__/TestScheduler.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 = {
Expand All @@ -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`', () => {
Expand Down Expand Up @@ -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();
});
45 changes: 45 additions & 0 deletions packages/jest-cli/src/__tests__/testSchedulerHelper.test.js
Original file line number Diff line number Diff line change
@@ -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,
);
},
);
42 changes: 42 additions & 0 deletions packages/jest-cli/src/testSchedulerHelper.js
Original file line number Diff line number Diff line change
@@ -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<Test>,
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)
);
}

0 comments on commit 3c7b110

Please sign in to comment.