From 49fc6d4b92e75f47d757a6010b9bceccdf0b0a26 Mon Sep 17 00:00:00 2001 From: Tim Seckinger Date: Wed, 26 Dec 2018 14:28:48 +0100 Subject: [PATCH] use file sizes of all dependencies in TestSequencer --- CHANGELOG.md | 1 + packages/jest-cli/src/TestSequencer.js | 64 ++++++++++++++- .../src/__tests__/test_sequencer.test.js | 78 ++++++++++++++++--- 3 files changed, 130 insertions(+), 13 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5557bd0a9a23..988d39e75045 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -156,6 +156,7 @@ ### Performance +- `[jest-cli]` Better test order heuristics on first run without cache ([#7553](https://github.com/facebook/jest/pull/7553)) - `[jest-mock]` Improve `getType` function performance. ([#7159](https://github.com/facebook/jest/pull/7159)) ## 23.6.0 diff --git a/packages/jest-cli/src/TestSequencer.js b/packages/jest-cli/src/TestSequencer.js index fe51829fa58c..7b3120d2646b 100644 --- a/packages/jest-cli/src/TestSequencer.js +++ b/packages/jest-cli/src/TestSequencer.js @@ -10,9 +10,14 @@ import type {AggregatedResult} from 'types/TestResult'; import type {Context} from 'types/Context'; import type {Test} from 'types/TestRunner'; +import type {Path} from 'types/Config'; +import type {Resolver} from 'types/Resolve'; +import type {HasteFS} from 'types/HasteMap'; import fs from 'fs'; import HasteMap from 'jest-haste-map'; +import DependencyResolver from 'jest-resolve-dependencies'; +import {buildSnapshotResolver} from 'jest-snapshot'; const FAIL = 0; const SUCCESS = 1; @@ -21,6 +26,16 @@ type Cache = { [key: string]: [0 | 1, number], }; +// If a test depends on one of these core modules, +// we assume that the test may be slower because it is an "integration test" +// that spawn child processes, accesses the file system, etc. +const coreModuleWeights = { + child_process: 1000000, + fs: 100000, + http: 10000, + https: 10000, +}; + export default class TestSequencer { _cache: Map; @@ -59,6 +74,46 @@ export default class TestSequencer { return cache; } + _fileSize( + path: Path, + sizes: Map, + resolver: Resolver, + hasteFS: HasteFS, + ): number { + const cachedSize = sizes.get(path); + if (cachedSize != null) { + return cachedSize; + } + + const size = + (resolver.isCoreModule(path) + ? coreModuleWeights[path] + : hasteFS.getSize(path)) || 0; + sizes.set(path, size); + return size; + } + + _fileSizeRecurseDependencies(test: Test, sizes: Map): number { + const {resolver, hasteFS, config} = test.context; + + const dependencyResolver = new DependencyResolver( + resolver, + hasteFS, + buildSnapshotResolver(config), + ); + const recursiveDependencies = dependencyResolver.resolveRecursive( + test.path, + {includeCoreModules: true}, + ); + const size = + Array.from(recursiveDependencies).reduce( + (sum, path) => sum + this._fileSize(path, sizes, resolver, hasteFS), + 0, + ) + this._fileSize(test.path, sizes, resolver, hasteFS); + + return size; + } + // When running more tests than we have workers available, sort the tests // by size - big test files usually take longer to complete, so we run // them first in an effort to minimize worker idle time at the end of a @@ -68,9 +123,7 @@ export default class TestSequencer { // subsequent runs we use that to run the slowest tests first, yielding the // fastest results. sort(tests: Array): Array { - const stats = {}; - const fileSize = ({path, context: {hasteFS}}) => - stats[path] || (stats[path] = hasteFS.getSize(path) || 0); + const sizes = new Map(); const hasFailed = (cache, test) => cache[test.path] && cache[test.path][0] === FAIL; const time = (cache, test) => cache[test.path] && cache[test.path][1]; @@ -90,7 +143,10 @@ export default class TestSequencer { } else if (testA.duration != null && testB.duration != null) { return testA.duration < testB.duration ? 1 : -1; } else { - return fileSize(testA) < fileSize(testB) ? 1 : -1; + return this._fileSizeRecurseDependencies(testA, sizes) < + this._fileSizeRecurseDependencies(testB, sizes) + ? 1 + : -1; } }); } diff --git a/packages/jest-cli/src/__tests__/test_sequencer.test.js b/packages/jest-cli/src/__tests__/test_sequencer.test.js index 95c8000c3191..66eb730915cb 100644 --- a/packages/jest-cli/src/__tests__/test_sequencer.test.js +++ b/packages/jest-cli/src/__tests__/test_sequencer.test.js @@ -12,11 +12,40 @@ import TestSequencer from '../TestSequencer'; jest.mock('fs'); +const mockResolveRecursive = jest.fn( + (path, options) => + ({ + '/test-a.js': ['/sut-a.js', '/test-util.js'], + '/test-ab.js': ['/sut-ab.js'], + '/test-b.js': ['/sut-b0.js'], + '/test-cp.js': ['child_process'], + '/test-e.js': ['/sut-e.js'], + '/test-fs.js': ['fs'], + '/test-http.js': ['http'], + }[path] || []), +); +afterEach(() => { + mockResolveRecursive.mockClear(); +}); +jest.mock( + 'jest-resolve-dependencies', + () => + class { + constructor() { + this.resolveRecursive = mockResolveRecursive; + } + }, +); + const FAIL = 0; const SUCCESS = 1; let sequencer; +const resolver = { + isCoreModule: path => !path.startsWith('/'), +}; + const context = { config: { cache: true, @@ -26,6 +55,7 @@ const context = { hasteFS: { getSize: path => path.length, }, + resolver, }; const secondContext = { @@ -37,6 +67,7 @@ const secondContext = { hasteFS: { getSize: path => path.length, }, + resolver, }; const toTests = paths => @@ -54,11 +85,40 @@ beforeEach(() => { fs.writeFileSync = jest.fn(); }); -test('sorts by file size if there is no timing information', () => { - expect(sequencer.sort(toTests(['/test-a.js', '/test-ab.js']))).toEqual([ +test('sorts by dependency file sizes if there is no timing information', () => { + expect(sequencer.sort(toTests(['/test-ab.js', '/test-a.js']))).toEqual([ + {context, duration: undefined, path: '/test-a.js'}, + {context, duration: undefined, path: '/test-ab.js'}, + ]); + expect(mockResolveRecursive).toHaveBeenCalledWith(expect.any(String), { + includeCoreModules: true, + }); +}); + +test('includes the test file itself during size calculation', () => { + expect(sequencer.sort(toTests(['/test-b.js', '/test-ab.js']))).toEqual([ {context, duration: undefined, path: '/test-ab.js'}, + {context, duration: undefined, path: '/test-b.js'}, + ]); + expect(mockResolveRecursive).toHaveBeenCalledWith(expect.any(String), { + includeCoreModules: true, + }); +}); + +test('prioritizes tests that depend on certain core modules', () => { + expect( + sequencer.sort( + toTests(['/test-a.js', '/test-cp.js', '/test-fs.js', '/test-http.js']), + ), + ).toEqual([ + {context, duration: undefined, path: '/test-cp.js'}, + {context, duration: undefined, path: '/test-fs.js'}, + {context, duration: undefined, path: '/test-http.js'}, {context, duration: undefined, path: '/test-a.js'}, ]); + expect(mockResolveRecursive).toHaveBeenCalledWith(expect.any(String), { + includeCoreModules: true, + }); }); test('sorts based on timing information', () => { @@ -95,14 +155,14 @@ test('sorts based on failures and timing information', () => { ]); }); -test('sorts based on failures, timing information and file size', () => { +test('sorts based on failures, timing information and dependency file sizes', () => { fs.readFileSync = jest.fn(() => JSON.stringify({ '/test-a.js': [SUCCESS, 5], '/test-ab.js': [FAIL, 1], - '/test-c.js': [FAIL], + '/test-cd.js': [FAIL], '/test-d.js': [SUCCESS, 2], - '/test-efg.js': [FAIL], + '/test-e.js': [FAIL], }), ); expect( @@ -110,14 +170,14 @@ test('sorts based on failures, timing information and file size', () => { toTests([ '/test-a.js', '/test-ab.js', - '/test-c.js', + '/test-cd.js', '/test-d.js', - '/test-efg.js', + '/test-e.js', ]), ), ).toEqual([ - {context, duration: undefined, path: '/test-efg.js'}, - {context, duration: undefined, path: '/test-c.js'}, + {context, duration: undefined, path: '/test-e.js'}, + {context, duration: undefined, path: '/test-cd.js'}, {context, duration: 1, path: '/test-ab.js'}, {context, duration: 5, path: '/test-a.js'}, {context, duration: 2, path: '/test-d.js'},