Skip to content

Commit

Permalink
use file sizes of all dependencies in TestSequencer
Browse files Browse the repository at this point in the history
  • Loading branch information
jeysal committed Jan 9, 2019
1 parent 28230ee commit 51bf3b9
Show file tree
Hide file tree
Showing 3 changed files with 123 additions and 13 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,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
Expand Down
57 changes: 53 additions & 4 deletions packages/jest-cli/src/TestSequencer.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,12 @@
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 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;
Expand All @@ -21,6 +24,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<Context, Cache>;

Expand Down Expand Up @@ -59,6 +72,41 @@ export default class TestSequencer {
return cache;
}

_fileSizeRecurseDependencies(test: Test, sizes: Map<Path, number>): number {
const {resolver, hasteFS, config} = test.context;

const fileSize = (path): 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;
};

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 + fileSize(path),
0,
) + fileSize(test.path);

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
Expand All @@ -68,9 +116,7 @@ export default class TestSequencer {
// subsequent runs we use that to run the slowest tests first, yielding the
// fastest results.
sort(tests: Array<Test>): Array<Test> {
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];
Expand All @@ -90,7 +136,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;
}
});
}
Expand Down
78 changes: 69 additions & 9 deletions packages/jest-cli/src/__tests__/test_sequencer.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -26,6 +55,7 @@ const context = {
hasteFS: {
getSize: path => path.length,
},
resolver,
};

const secondContext = {
Expand All @@ -37,6 +67,7 @@ const secondContext = {
hasteFS: {
getSize: path => path.length,
},
resolver,
};

const toTests = paths =>
Expand All @@ -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', () => {
Expand Down Expand Up @@ -95,29 +155,29 @@ 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(
sequencer.sort(
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'},
Expand Down

0 comments on commit 51bf3b9

Please sign in to comment.