From 6950b1eb38964877742a408f3289473c4abb5067 Mon Sep 17 00:00:00 2001 From: Tim Seckinger Date: Wed, 26 Dec 2018 14:24:11 +0100 Subject: [PATCH 1/5] dependency_resolver.test.js describe blocks --- .../src/__tests__/dependency_resolver.test.js | 111 ++++++++++-------- 1 file changed, 59 insertions(+), 52 deletions(-) diff --git a/packages/jest-resolve-dependencies/src/__tests__/dependency_resolver.test.js b/packages/jest-resolve-dependencies/src/__tests__/dependency_resolver.test.js index 3725c9bc951f..1af936b7aaed 100644 --- a/packages/jest-resolve-dependencies/src/__tests__/dependency_resolver.test.js +++ b/packages/jest-resolve-dependencies/src/__tests__/dependency_resolver.test.js @@ -40,66 +40,73 @@ beforeEach(() => { }); }); -test('resolves no dependencies for non-existent path', () => { - const resolved = dependencyResolver.resolve('/non/existent/path'); - expect(resolved.length).toEqual(0); -}); - -test('resolves dependencies for existing path', () => { - const resolved = dependencyResolver.resolve( - path.resolve(__dirname, '__fixtures__', 'file.js'), - ); - expect(resolved).toEqual([ - expect.stringContaining('jest-resolve-dependencies'), - expect.stringContaining('jest-regex-util'), - ]); -}); +describe('resolve', () => { + test('resolves no dependencies for non-existent path', () => { + const resolved = dependencyResolver.resolve('/non/existent/path'); + expect(resolved.length).toEqual(0); + }); -test('resolves dependencies for scoped packages', () => { - const resolved = dependencyResolver.resolve( - path.resolve(__dirname, '__fixtures__', 'scoped.js'), - ); - expect(resolved).toEqual([ - expect.stringContaining(path.join('@myorg', 'pkg')), - ]); -}); + test('resolves dependencies for existing path', () => { + const resolved = dependencyResolver.resolve( + path.resolve(__dirname, '__fixtures__', 'file.js'), + ); + expect(resolved).toEqual([ + expect.stringContaining('jest-resolve-dependencies'), + expect.stringContaining('jest-regex-util'), + ]); + }); -test('resolves no inverse dependencies for empty paths set', () => { - const paths = new Set(); - const resolved = dependencyResolver.resolveInverse(paths, filter); - expect(resolved.length).toEqual(0); + test('resolves dependencies for scoped packages', () => { + const resolved = dependencyResolver.resolve( + path.resolve(__dirname, '__fixtures__', 'scoped.js'), + ); + expect(resolved).toEqual([ + expect.stringContaining(path.join('@myorg', 'pkg')), + ]); + }); }); -test('resolves no inverse dependencies for set of non-existent paths', () => { - const paths = new Set(['/non/existent/path', '/another/one']); - const resolved = dependencyResolver.resolveInverse(paths, filter); - expect(resolved.length).toEqual(0); -}); +describe('resolveInverse', () => { + test('resolves no inverse dependencies for empty paths set', () => { + const paths = new Set(); + const resolved = dependencyResolver.resolveInverse(paths, filter); + expect(resolved.length).toEqual(0); + }); -test('resolves inverse dependencies for existing path', () => { - const paths = new Set([path.resolve(__dirname, '__fixtures__/file.js')]); - const resolved = dependencyResolver.resolveInverse(paths, filter); - expect(resolved).toEqual([ - expect.stringContaining( - path.join('__tests__', '__fixtures__', 'file.test.js'), - ), - ]); -}); + test('resolves no inverse dependencies for set of non-existent paths', () => { + const paths = new Set(['/non/existent/path', '/another/one']); + const resolved = dependencyResolver.resolveInverse(paths, filter); + expect(resolved.length).toEqual(0); + }); -test('resolves inverse dependencies from available snapshot', () => { - const paths = new Set([ - path.resolve(__dirname, '__fixtures__/file.js'), - path.resolve(__dirname, '__fixtures__/__snapshots__/related.test.js.snap'), - ]); - const resolved = dependencyResolver.resolveInverse(paths, filter); - expect(resolved).toEqual( - expect.arrayContaining([ + test('resolves inverse dependencies for existing path', () => { + const paths = new Set([path.resolve(__dirname, '__fixtures__/file.js')]); + const resolved = dependencyResolver.resolveInverse(paths, filter); + expect(resolved).toEqual([ expect.stringContaining( path.join('__tests__', '__fixtures__', 'file.test.js'), ), - expect.stringContaining( - path.join('__tests__', '__fixtures__', 'related.test.js'), + ]); + }); + + test('resolves inverse dependencies from available snapshot', () => { + const paths = new Set([ + path.resolve(__dirname, '__fixtures__/file.js'), + path.resolve( + __dirname, + '__fixtures__/__snapshots__/related.test.js.snap', ), - ]), - ); + ]); + const resolved = dependencyResolver.resolveInverse(paths, filter); + expect(resolved).toEqual( + expect.arrayContaining([ + expect.stringContaining( + path.join('__tests__', '__fixtures__', 'file.test.js'), + ), + expect.stringContaining( + path.join('__tests__', '__fixtures__', 'related.test.js'), + ), + ]), + ); + }); }); From 47d5477f13b39c21f00d7daba74909223981f43f Mon Sep 17 00:00:00 2001 From: Tim Seckinger Date: Wed, 26 Dec 2018 14:28:04 +0100 Subject: [PATCH 2/5] add DependencyResolver.resolveRecursive() --- CHANGELOG.md | 2 ++ .../src/__tests__/__fixtures__/circular/a.js | 10 ++++++ .../src/__tests__/__fixtures__/circular/b.js | 10 ++++++ .../src/__tests__/__fixtures__/circular/c.js | 10 ++++++ .../src/__tests__/__fixtures__/recursive/a.js | 10 ++++++ .../src/__tests__/__fixtures__/recursive/b.js | 11 ++++++ .../__tests__/__fixtures__/recursive/c1.js | 8 +++++ .../__tests__/__fixtures__/recursive/c2.js | 8 +++++ .../src/__tests__/dependency_resolver.test.js | 34 +++++++++++++++++-- .../jest-resolve-dependencies/src/index.js | 21 ++++++++++-- 10 files changed, 119 insertions(+), 5 deletions(-) create mode 100644 packages/jest-resolve-dependencies/src/__tests__/__fixtures__/circular/a.js create mode 100644 packages/jest-resolve-dependencies/src/__tests__/__fixtures__/circular/b.js create mode 100644 packages/jest-resolve-dependencies/src/__tests__/__fixtures__/circular/c.js create mode 100644 packages/jest-resolve-dependencies/src/__tests__/__fixtures__/recursive/a.js create mode 100644 packages/jest-resolve-dependencies/src/__tests__/__fixtures__/recursive/b.js create mode 100644 packages/jest-resolve-dependencies/src/__tests__/__fixtures__/recursive/c1.js create mode 100644 packages/jest-resolve-dependencies/src/__tests__/__fixtures__/recursive/c2.js diff --git a/CHANGELOG.md b/CHANGELOG.md index 0050a9f04365..7aca48592cc8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,8 @@ ### Features +- `[jest-resolve-dependencies]` Add `DependencyResolver.resolveRecursive` ([#7553](https://github.com/facebook/jest/pull/7553)) + ### Fixes - `[jest-cli]` Refactor `-o` and `--coverage` combined ([#7611](https://github.com/facebook/jest/pull/7611)) diff --git a/packages/jest-resolve-dependencies/src/__tests__/__fixtures__/circular/a.js b/packages/jest-resolve-dependencies/src/__tests__/__fixtures__/circular/a.js new file mode 100644 index 000000000000..29a5470d6c56 --- /dev/null +++ b/packages/jest-resolve-dependencies/src/__tests__/__fixtures__/circular/a.js @@ -0,0 +1,10 @@ +/** + * Copyright (c) Facebook, Inc. and its affiliates. 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'; + +require('./b'); diff --git a/packages/jest-resolve-dependencies/src/__tests__/__fixtures__/circular/b.js b/packages/jest-resolve-dependencies/src/__tests__/__fixtures__/circular/b.js new file mode 100644 index 000000000000..db51e2149010 --- /dev/null +++ b/packages/jest-resolve-dependencies/src/__tests__/__fixtures__/circular/b.js @@ -0,0 +1,10 @@ +/** + * Copyright (c) Facebook, Inc. and its affiliates. 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'; + +require('./c'); diff --git a/packages/jest-resolve-dependencies/src/__tests__/__fixtures__/circular/c.js b/packages/jest-resolve-dependencies/src/__tests__/__fixtures__/circular/c.js new file mode 100644 index 000000000000..79cdb76ea1d6 --- /dev/null +++ b/packages/jest-resolve-dependencies/src/__tests__/__fixtures__/circular/c.js @@ -0,0 +1,10 @@ +/** + * Copyright (c) Facebook, Inc. and its affiliates. 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'; + +require('./a'); diff --git a/packages/jest-resolve-dependencies/src/__tests__/__fixtures__/recursive/a.js b/packages/jest-resolve-dependencies/src/__tests__/__fixtures__/recursive/a.js new file mode 100644 index 000000000000..29a5470d6c56 --- /dev/null +++ b/packages/jest-resolve-dependencies/src/__tests__/__fixtures__/recursive/a.js @@ -0,0 +1,10 @@ +/** + * Copyright (c) Facebook, Inc. and its affiliates. 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'; + +require('./b'); diff --git a/packages/jest-resolve-dependencies/src/__tests__/__fixtures__/recursive/b.js b/packages/jest-resolve-dependencies/src/__tests__/__fixtures__/recursive/b.js new file mode 100644 index 000000000000..ede9730e2a30 --- /dev/null +++ b/packages/jest-resolve-dependencies/src/__tests__/__fixtures__/recursive/b.js @@ -0,0 +1,11 @@ +/** + * Copyright (c) Facebook, Inc. and its affiliates. 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'; + +require('./c1'); +require('./c2'); diff --git a/packages/jest-resolve-dependencies/src/__tests__/__fixtures__/recursive/c1.js b/packages/jest-resolve-dependencies/src/__tests__/__fixtures__/recursive/c1.js new file mode 100644 index 000000000000..3d797dae3666 --- /dev/null +++ b/packages/jest-resolve-dependencies/src/__tests__/__fixtures__/recursive/c1.js @@ -0,0 +1,8 @@ +/** + * Copyright (c) Facebook, Inc. and its affiliates. 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'; diff --git a/packages/jest-resolve-dependencies/src/__tests__/__fixtures__/recursive/c2.js b/packages/jest-resolve-dependencies/src/__tests__/__fixtures__/recursive/c2.js new file mode 100644 index 000000000000..3d797dae3666 --- /dev/null +++ b/packages/jest-resolve-dependencies/src/__tests__/__fixtures__/recursive/c2.js @@ -0,0 +1,8 @@ +/** + * Copyright (c) Facebook, Inc. and its affiliates. 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'; diff --git a/packages/jest-resolve-dependencies/src/__tests__/dependency_resolver.test.js b/packages/jest-resolve-dependencies/src/__tests__/dependency_resolver.test.js index 1af936b7aaed..b8406f5e849d 100644 --- a/packages/jest-resolve-dependencies/src/__tests__/dependency_resolver.test.js +++ b/packages/jest-resolve-dependencies/src/__tests__/dependency_resolver.test.js @@ -43,7 +43,7 @@ beforeEach(() => { describe('resolve', () => { test('resolves no dependencies for non-existent path', () => { const resolved = dependencyResolver.resolve('/non/existent/path'); - expect(resolved.length).toEqual(0); + expect(resolved).toHaveLength(0); }); test('resolves dependencies for existing path', () => { @@ -66,17 +66,45 @@ describe('resolve', () => { }); }); +describe('resolveRecursive', () => { + test('resolves no dependencies for non-existent path', () => { + const resolved = dependencyResolver.resolveRecursive('/non/existent/path'); + expect(Array.from(resolved)).toHaveLength(0); + }); + + test('resolves dependencies for existing path', () => { + const resolved = dependencyResolver.resolveRecursive( + path.resolve(__dirname, '__fixtures__', 'recursive', 'a.js'), + ); + expect(Array.from(resolved)).toEqual([ + expect.stringContaining('b.js'), + expect.stringContaining('c1.js'), + expect.stringContaining('c2.js'), + ]); + }); + + test('resolves circular dependencies', () => { + const resolved = dependencyResolver.resolveRecursive( + path.resolve(__dirname, '__fixtures__', 'circular', 'a.js'), + ); + expect(Array.from(resolved)).toEqual([ + expect.stringContaining('b.js'), + expect.stringContaining('c.js'), + ]); + }); +}); + describe('resolveInverse', () => { test('resolves no inverse dependencies for empty paths set', () => { const paths = new Set(); const resolved = dependencyResolver.resolveInverse(paths, filter); - expect(resolved.length).toEqual(0); + expect(resolved).toHaveLength(0); }); test('resolves no inverse dependencies for set of non-existent paths', () => { const paths = new Set(['/non/existent/path', '/another/one']); const resolved = dependencyResolver.resolveInverse(paths, filter); - expect(resolved.length).toEqual(0); + expect(resolved).toHaveLength(0); }); test('resolves inverse dependencies for existing path', () => { diff --git a/packages/jest-resolve-dependencies/src/index.js b/packages/jest-resolve-dependencies/src/index.js index bebb7a76e212..cd2752e79e0c 100644 --- a/packages/jest-resolve-dependencies/src/index.js +++ b/packages/jest-resolve-dependencies/src/index.js @@ -18,8 +18,10 @@ import type {SnapshotResolver} from 'types/SnapshotResolver'; import {isSnapshotPath} from 'jest-snapshot'; /** - * DependencyResolver is used to resolve the direct dependencies of a module or - * to retrieve a list of all transitive inverse dependencies. + * DependencyResolver is used + * to resolve the direct dependencies of a module (resolve) or + * to resolve the transitive (direct and indirect) dependencies of a module (resolveRecursive) or + * to retrieve a list of all transitive inverse dependencies (resolveInverse{,ModuleMap}). */ class DependencyResolver { _hasteFS: HasteFS; @@ -65,6 +67,21 @@ class DependencyResolver { }, []); } + resolveRecursive(rootFile: Path, options?: ResolveModuleConfig): Set { + const known = new Set(); + const recurse = (file: Path) => { + if (known.has(file)) { + return; + } + if (file !== rootFile) { + known.add(file); + } + this.resolve(file, options).forEach(recurse); + }; + recurse(rootFile); + return known; + } + resolveInverseModuleMap( paths: Set, filter: (file: Path) => boolean, From 22fee13502e860681939418bf1d23e4d4547777a Mon Sep 17 00:00:00 2001 From: Tim Seckinger Date: Mon, 31 Dec 2018 00:53:20 +0100 Subject: [PATCH 3/5] add DependencyResolver includeCoreModules option --- CHANGELOG.md | 1 + .../src/__tests__/__fixtures__/core.test.js | 10 ++++++ .../src/__tests__/dependency_resolver.test.js | 34 +++++++++++++++++++ .../jest-resolve-dependencies/src/index.js | 4 ++- types/Resolve.js | 1 + 5 files changed, 49 insertions(+), 1 deletion(-) create mode 100644 packages/jest-resolve-dependencies/src/__tests__/__fixtures__/core.test.js diff --git a/CHANGELOG.md b/CHANGELOG.md index 7aca48592cc8..2f5e4392d959 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,7 @@ ### Features - `[jest-resolve-dependencies]` Add `DependencyResolver.resolveRecursive` ([#7553](https://github.com/facebook/jest/pull/7553)) +- `[jest-resolve-dependencies]` Add `DependencyResolver includeCoreModules` option ([#7553](https://github.com/facebook/jest/pull/7553)) ### Fixes diff --git a/packages/jest-resolve-dependencies/src/__tests__/__fixtures__/core.test.js b/packages/jest-resolve-dependencies/src/__tests__/__fixtures__/core.test.js new file mode 100644 index 000000000000..89d023cba71e --- /dev/null +++ b/packages/jest-resolve-dependencies/src/__tests__/__fixtures__/core.test.js @@ -0,0 +1,10 @@ +/** + * Copyright (c) Facebook, Inc. and its affiliates. 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'; + +require('fs'); diff --git a/packages/jest-resolve-dependencies/src/__tests__/dependency_resolver.test.js b/packages/jest-resolve-dependencies/src/__tests__/dependency_resolver.test.js index b8406f5e849d..6a5284ba474e 100644 --- a/packages/jest-resolve-dependencies/src/__tests__/dependency_resolver.test.js +++ b/packages/jest-resolve-dependencies/src/__tests__/dependency_resolver.test.js @@ -64,6 +64,23 @@ describe('resolve', () => { expect.stringContaining(path.join('@myorg', 'pkg')), ]); }); + + describe('includeCoreModules', () => { + test('does not include core modules without the option', () => { + const resolved = dependencyResolver.resolve( + path.resolve(__dirname, '__fixtures__', 'core.test.js'), + ); + expect(Array.from(resolved)).toEqual([]); + }); + + test('includes core modules with the option', () => { + const resolved = dependencyResolver.resolve( + path.resolve(__dirname, '__fixtures__', 'core.test.js'), + {includeCoreModules: true}, + ); + expect(Array.from(resolved)).toEqual(['fs']); + }); + }); }); describe('resolveRecursive', () => { @@ -92,6 +109,23 @@ describe('resolveRecursive', () => { expect.stringContaining('c.js'), ]); }); + + describe('includeCoreModules', () => { + test('does not include core modules without the option', () => { + const resolved = dependencyResolver.resolveRecursive( + path.resolve(__dirname, '__fixtures__', 'core.test.js'), + ); + expect(Array.from(resolved)).toEqual([]); + }); + + test('includes core modules with the option', () => { + const resolved = dependencyResolver.resolveRecursive( + path.resolve(__dirname, '__fixtures__', 'core.test.js'), + {includeCoreModules: true}, + ); + expect(Array.from(resolved)).toEqual(['fs']); + }); + }); }); describe('resolveInverse', () => { diff --git a/packages/jest-resolve-dependencies/src/index.js b/packages/jest-resolve-dependencies/src/index.js index cd2752e79e0c..78e207739d37 100644 --- a/packages/jest-resolve-dependencies/src/index.js +++ b/packages/jest-resolve-dependencies/src/index.js @@ -44,8 +44,10 @@ class DependencyResolver { return []; } + const includeCoreModules = options && options.includeCoreModules; + return dependencies.reduce((acc, dependency) => { - if (this._resolver.isCoreModule(dependency)) { + if (!includeCoreModules && this._resolver.isCoreModule(dependency)) { return acc; } let resolvedDependency; diff --git a/types/Resolve.js b/types/Resolve.js index facd55e4fc56..fc778cb06269 100644 --- a/types/Resolve.js +++ b/types/Resolve.js @@ -13,6 +13,7 @@ import type {Path} from './Config'; export type ResolveModuleConfig = {| skipNodeResolution?: boolean, + includeCoreModules?: boolean, paths?: Path[], |}; From 1ca41b52fd4745bd99b15dd605a69e7de311fecd Mon Sep 17 00:00:00 2001 From: Tim Seckinger Date: Wed, 26 Dec 2018 14:28:48 +0100 Subject: [PATCH 4/5] use file sizes of all dependencies in TestSequencer --- CHANGELOG.md | 2 + packages/jest-cli/src/TestSequencer.js | 65 +++++++++++++++- .../src/__tests__/test_sequencer.test.js | 78 ++++++++++++++++--- 3 files changed, 132 insertions(+), 13 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2f5e4392d959..2d56697b33a6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -23,6 +23,8 @@ ### Performance +- `[jest-cli]` Better test order heuristics on first run without cache ([#7553](https://github.com/facebook/jest/pull/7553)) + ## 24.1.0 ### Features diff --git a/packages/jest-cli/src/TestSequencer.js b/packages/jest-cli/src/TestSequencer.js index b13c2e261d9e..f52e14fba715 100644 --- a/packages/jest-cli/src/TestSequencer.js +++ b/packages/jest-cli/src/TestSequencer.js @@ -10,10 +10,15 @@ 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'; // $FlowFixMe: Missing ESM export import {getCacheFilePath} from 'jest-haste-map'; +import DependencyResolver from 'jest-resolve-dependencies'; +import {buildSnapshotResolver} from 'jest-snapshot'; const FAIL = 0; const SUCCESS = 1; @@ -22,6 +27,17 @@ 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. +// The weights are in bytes, just as the file sizes of regular modules. +const coreModuleWeights = { + child_process: 1000000, + fs: 100000, + http: 10000, + https: 10000, +}; + export default class TestSequencer { _cache: Map; @@ -57,6 +73,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 @@ -66,9 +122,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]; @@ -88,7 +142,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 2e0c8d00f0ea..0793e8344a58 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 @@ jest.mock('fs'); const fs = require('fs'); const path = require('path'); +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'}, From 2920417b78889b70d559e22546c7921ade138c41 Mon Sep 17 00:00:00 2001 From: Tim Seckinger Date: Wed, 26 Dec 2018 17:42:48 +0100 Subject: [PATCH 5/5] add test case for ordering to list-tests e2e test --- .../__snapshots__/listTests.test.js.snap | 3 ++- e2e/__tests__/listTests.test.js | 10 ++++++++++ e2e/list-tests/__tests__/other.test.js | 3 +++ e2e/list-tests/__tests__/with-dep.test.js | 15 +++++++++++++++ 4 files changed, 30 insertions(+), 1 deletion(-) create mode 100644 e2e/list-tests/__tests__/with-dep.test.js diff --git a/e2e/__tests__/__snapshots__/listTests.test.js.snap b/e2e/__tests__/__snapshots__/listTests.test.js.snap index 3c12f33eab07..9133d1e0efc4 100644 --- a/e2e/__tests__/__snapshots__/listTests.test.js.snap +++ b/e2e/__tests__/__snapshots__/listTests.test.js.snap @@ -3,6 +3,7 @@ exports[`--listTests flag causes tests to be printed in different lines 1`] = ` /MOCK_ABOLUTE_PATH/e2e/list-tests/__tests__/dummy.test.js /MOCK_ABOLUTE_PATH/e2e/list-tests/__tests__/other.test.js +/MOCK_ABOLUTE_PATH/e2e/list-tests/__tests__/with-dep.test.js `; -exports[`--listTests flag causes tests to be printed out as JSON when using the --json flag 1`] = `["/MOCK_ABOLUTE_PATH/e2e/list-tests/__tests__/dummy.test.js","/MOCK_ABOLUTE_PATH/e2e/list-tests/__tests__/other.test.js"]`; +exports[`--listTests flag causes tests to be printed out as JSON when using the --json flag 1`] = `["/MOCK_ABOLUTE_PATH/e2e/list-tests/__tests__/dummy.test.js","/MOCK_ABOLUTE_PATH/e2e/list-tests/__tests__/other.test.js","/MOCK_ABOLUTE_PATH/e2e/list-tests/__tests__/with-dep.test.js"]`; diff --git a/e2e/__tests__/listTests.test.js b/e2e/__tests__/listTests.test.js index f7c4ad6cb7af..52276a27330d 100644 --- a/e2e/__tests__/listTests.test.js +++ b/e2e/__tests__/listTests.test.js @@ -35,6 +35,16 @@ describe('--listTests flag', () => { ).toMatchSnapshot(); }); + it('prints tests in the execution order determined by their dependency sizes', () => { + const {status, stdout} = runJest('list-tests', ['--listTests']); + expect(status).toBe(0); + expect(normalizePaths(stdout).split('\n')).toEqual([ + expect.stringContaining('/with-dep.test.js'), + expect.stringContaining('/other.test.js'), + expect.stringContaining('/dummy.test.js'), + ]); + }); + it('causes tests to be printed out as JSON when using the --json flag', () => { const {status, stdout} = runJest('list-tests', ['--listTests', '--json']); diff --git a/e2e/list-tests/__tests__/other.test.js b/e2e/list-tests/__tests__/other.test.js index cfc1ee266fb4..2607de475730 100644 --- a/e2e/list-tests/__tests__/other.test.js +++ b/e2e/list-tests/__tests__/other.test.js @@ -11,3 +11,6 @@ it("isn't actually run", () => { // (because it is only used for --listTests) expect(true).toBe(false); }); + +// Because of this comment, other.test.js is slightly larger than dummy.test.js. +// This matters for the order in which tests are sequenced. diff --git a/e2e/list-tests/__tests__/with-dep.test.js b/e2e/list-tests/__tests__/with-dep.test.js new file mode 100644 index 000000000000..15648e892ab6 --- /dev/null +++ b/e2e/list-tests/__tests__/with-dep.test.js @@ -0,0 +1,15 @@ +/** + * Copyright (c) Facebook, Inc. and its affiliates. 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'; + +require('./dummy.test.js'); + +it("isn't actually run", () => { + // (because it is only used for --listTests) + expect(true).toBe(false); +});