Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve Jest startup time and test runtime, particularly when running with coverage, by caching micromatch and avoiding recreating RegExp instances #10131

Merged
merged 8 commits into from
Jun 23, 2020
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@

### Performance

- `[jest-core, jest-transform, jest-haste-map]` Improve Jest startup time and test runtime, particularly when running with coverage, by caching micromatch and avoiding recreating RegExp instances ([#10131](https://github.com/facebook/jest/pull/10131))

## 26.0.1

### Fixes
Expand Down
19 changes: 13 additions & 6 deletions packages/jest-core/src/SearchSource.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ import DependencyResolver = require('jest-resolve-dependencies');
import {escapePathForRegex} from 'jest-regex-util';
import {replaceRootDirInPath} from 'jest-config';
import {buildSnapshotResolver} from 'jest-snapshot';
import {replacePathSepForGlob, testPathPatternToRegExp} from 'jest-util';
import {globsToMatcher, testPathPatternToRegExp} from 'jest-util';
import type {Filter, Stats, TestPathCases} from './types';

export type SearchResult = {
Expand All @@ -37,12 +37,19 @@ export type TestSelectionConfig = {
watch?: boolean;
};

const globsToMatcher = (globs: Array<Config.Glob>) => (path: Config.Path) =>
micromatch([replacePathSepForGlob(path)], globs, {dot: true}).length > 0;
const regexToMatcher = (testRegex: Config.ProjectConfig['testRegex']) => {
const regexes = testRegex.map(testRegex => new RegExp(testRegex));

const regexToMatcher = (testRegex: Config.ProjectConfig['testRegex']) => (
path: Config.Path,
) => testRegex.some(testRegex => new RegExp(testRegex).test(path));
return (path: Config.Path) =>
regexes.some(regex => {
const result = regex.test(path);

// prevent stateful regexes from breaking, just in case
regex.lastIndex = 0;

return result;
});
};

const toTests = (context: Context, tests: Array<Config.Path>) =>
tests.map(path => ({
Expand Down
28 changes: 28 additions & 0 deletions packages/jest-core/src/__tests__/SearchSource.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -206,6 +206,34 @@ describe('SearchSource', () => {
});
});

it('finds tests matching a JS with overriding glob patterns', () => {
const {options: config} = normalize(
{
moduleFileExtensions: ['js', 'jsx'],
name,
rootDir,
testMatch: [
'**/*.js?(x)',
'!**/test.js?(x)',
'**/test.js',
'!**/test.js',
],
testRegex: '',
},
{} as Config.Argv,
);

return findMatchingTests(config).then(data => {
SimenB marked this conversation as resolved.
Show resolved Hide resolved
const relPaths = toPaths(data.tests).map(absPath =>
path.relative(rootDir, absPath),
);
expect(relPaths.sort()).toEqual([
path.normalize('module.jsx'),
path.normalize('no_tests.js'),
]);
});
});

it('finds tests with default file extensions using testRegex', () => {
const {options: config} = normalize(
{
Expand Down
7 changes: 4 additions & 3 deletions packages/jest-haste-map/src/HasteFS.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,7 @@
* LICENSE file in the root directory of this source tree.
*/

import micromatch = require('micromatch');
import {replacePathSepForGlob} from 'jest-util';
import {globsToMatcher, replacePathSepForGlob} from 'jest-util';
import type {Config} from '@jest/types';
import type {FileData} from './types';
import * as fastPath from './lib/fast_path';
Expand Down Expand Up @@ -84,9 +83,11 @@ export default class HasteFS {
root: Config.Path | null,
): Set<Config.Path> {
const files = new Set<string>();
const matcher = globsToMatcher(globs);

for (const file of this.getAbsoluteFileIterator()) {
const filePath = root ? fastPath.relative(root, file) : file;
if (micromatch([replacePathSepForGlob(filePath)], globs).length > 0) {
if (matcher(replacePathSepForGlob(filePath))) {
files.add(file);
}
}
Expand Down
31 changes: 22 additions & 9 deletions packages/jest-transform/src/shouldInstrument.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,14 +8,28 @@
import * as path from 'path';
import type {Config} from '@jest/types';
import {escapePathForRegex} from 'jest-regex-util';
import {replacePathSepForGlob} from 'jest-util';
import {globsToMatcher, replacePathSepForGlob} from 'jest-util';
import micromatch = require('micromatch');
import type {ShouldInstrumentOptions} from './types';

const MOCKS_PATTERN = new RegExp(
escapePathForRegex(path.sep + '__mocks__' + path.sep),
);

const cachedRegexes = new Map<string, RegExp>();
const getRegex = (regexStr: string) => {
if (!cachedRegexes.has(regexStr)) {
cachedRegexes.set(regexStr, new RegExp(regexStr));
}

const regex = cachedRegexes.get(regexStr)!;

// prevent stateful regexes from breaking, just in case
regex.lastIndex = 0;

return regex;
};

export default function shouldInstrument(
filename: Config.Path,
options: ShouldInstrumentOptions,
Expand All @@ -33,15 +47,15 @@ export default function shouldInstrument(
}

if (
!config.testPathIgnorePatterns.some(pattern => !!filename.match(pattern))
!config.testPathIgnorePatterns.some(pattern =>
getRegex(pattern).test(filename),
)
) {
if (config.testRegex.some(regex => new RegExp(regex).test(filename))) {
return false;
}

if (
micromatch([replacePathSepForGlob(filename)], config.testMatch).length
) {
if (globsToMatcher(config.testMatch)(replacePathSepForGlob(filename))) {
return false;
}
}
Expand All @@ -59,10 +73,9 @@ export default function shouldInstrument(
// still cover if `only` is specified
!options.collectCoverageOnlyFrom &&
options.collectCoverageFrom.length &&
micromatch(
[replacePathSepForGlob(path.relative(config.rootDir, filename))],
options.collectCoverageFrom,
).length === 0
!globsToMatcher(options.collectCoverageFrom)(
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just noticed that this will end up using the dot: true option where before that wasn't used here (and above in this file). This seems like it should be fine, but I don't really have enough context here to know for sure.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah, seems fine. HasteMap should filter out any dotfiles we don't want already (I think)

replacePathSepForGlob(path.relative(config.rootDir, filename)),
)
) {
return false;
}
Expand Down
4 changes: 3 additions & 1 deletion packages/jest-util/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,13 @@
"chalk": "^4.0.0",
"graceful-fs": "^4.2.4",
"is-ci": "^2.0.0",
"make-dir": "^3.0.0"
"make-dir": "^3.0.0",
"micromatch": "^4.0.2"
lencioni marked this conversation as resolved.
Show resolved Hide resolved
},
"devDependencies": {
"@types/graceful-fs": "^4.1.2",
"@types/is-ci": "^2.0.0",
"@types/micromatch": "^4.0.0",
"@types/node": "*"
},
"engines": {
Expand Down
72 changes: 72 additions & 0 deletions packages/jest-util/src/__tests__/globsToMatcher.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
/**
* 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.
*/

import micromatch = require('micromatch');
import globsToMatcher from '../globsToMatcher';

it('works like micromatch with only positive globs', () => {
const globs = ['**/*.test.js', '**/*.test.jsx'];
const matcher = globsToMatcher(globs);

expect(matcher('some-module.js')).toBe(
micromatch(['some-module.js'], globs).length > 0,
);

expect(matcher('some-module.test.js')).toBe(
micromatch(['some-module.test.js'], globs).length > 0,
);
});

it('works like micromatch with a mix of overlapping positive and negative globs', () => {
const globs = ['**/*.js', '!**/*.test.js', '**/*.test.js'];
const matcher = globsToMatcher(globs);

expect(matcher('some-module.js')).toBe(
micromatch(['some-module.js'], globs).length > 0,
);

expect(matcher('some-module.test.js')).toBe(
micromatch(['some-module.test.js'], globs).length > 0,
);

const globs2 = ['**/*.js', '!**/*.test.js', '**/*.test.js', '!**/*.test.js'];
const matcher2 = globsToMatcher(globs2);

expect(matcher2('some-module.js')).toBe(
micromatch(['some-module.js'], globs2).length > 0,
);

expect(matcher2('some-module.test.js')).toBe(
micromatch(['some-module.test.js'], globs2).length > 0,
);
});

it('works like micromatch with only negative globs', () => {
const globs = ['!**/*.test.js', '!**/*.test.jsx'];
const matcher = globsToMatcher(globs);

expect(matcher('some-module.js')).toBe(
micromatch(['some-module.js'], globs).length > 0,
);

expect(matcher('some-module.test.js')).toBe(
micromatch(['some-module.test.js'], globs).length > 0,
);
});

it('works like micromatch with empty globs', () => {
const globs = [];
const matcher = globsToMatcher(globs);

expect(matcher('some-module.js')).toBe(
micromatch(['some-module.js'], globs).length > 0,
);

expect(matcher('some-module.test.js')).toBe(
micromatch(['some-module.test.js'], globs).length > 0,
);
});
97 changes: 97 additions & 0 deletions packages/jest-util/src/globsToMatcher.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,97 @@
/**
* 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.
*/

import micromatch = require('micromatch');
import type {Config} from '@jest/types';
import replacePathSepForGlob from './replacePathSepForGlob';

const globsToMatchersMap = new Map<
string,
{
isMatch: (str: string) => boolean;
negated: boolean;
}
>();

const micromatchOptions = {dot: true};

/**
* Converts a list of globs into a function that matches a path against the
* globs.
*
* Every time micromatch is called, it will parse the glob strings and turn
* them into regexp instances. Instead of calling micromatch repeatedly with
* the same globs, we can use this function which will build the micromatch
* matchers ahead of time and then have an optimized path for determining
* whether an individual path matches.
*
* This function is intended to match the behavior of `micromatch()`.
*
* @example
* const isMatch = globsToMatcher(['*.js', '!*.test.js']);
* isMatch('pizza.js'); // true
* isMatch('pizza.test.js'); // false
*/
export default function globsToMatcher(
globs: Array<Config.Glob>,
): (path: Config.Path) => boolean {
if (globs.length === 0) {
// Since there were no globs given, we can simply have a fast path here and
// return with a very simple function.
return (_: Config.Path): boolean => false;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are these type annotations needed? It's not inferred from the outer function

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't expect them to be needed, but vscode showed me an eslint error when I didn't have them:

Missing return type on function. eslint@typescript-eslint/explicit-module-boundary-types

Copy link
Member

@SimenB SimenB Jun 8, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, wonder if that's fixed by the fresh 3.2 release? Either typescript-eslint/typescript-eslint#2169 or typescript-eslint/typescript-eslint#2176

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, seems promising. It looks like Jest is still on v2.30 though, so I think I'd like to say that updating the eslint plugin is out of scope for this PR if that's alright with you.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FWIW the breaking changes seem to be pretty okay for this repo: https://github.com/typescript-eslint/typescript-eslint/releases/tag/v3.0.0

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah, that's out of scope for this 😅

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@SimenB I'm happy to tackle that upgrade, if you want to create an issue and assign it to me :)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @G-Rath! #10177

}

const matchers = globs.map(glob => {
if (!globsToMatchersMap.has(glob)) {
// Matchers that are negated have different behavior than matchers that
// are not negated, so we need to store this information ahead of time.
const {negated} = micromatch.scan(glob, micromatchOptions);

const matcher = {
isMatch: micromatch.matcher(glob, micromatchOptions),
negated,
};

globsToMatchersMap.set(glob, matcher);
}

return globsToMatchersMap.get(glob)!;
});

return (path: Config.Path): boolean => {
const replacedPath = replacePathSepForGlob(path);
let kept = undefined;
let negatives = 0;

for (let i = 0; i < matchers.length; i++) {
const {isMatch, negated} = matchers[i];

if (negated) negatives++;

const matched = isMatch(replacedPath);

if (!matched && negated) {
// The path was not matched, and the matcher is a negated matcher, so we
// want to omit the path. This means that the negative matcher is
// filtering the path out.
kept = false;
} else if (matched && !negated) {
// The path was matched, and the matcher is not a negated matcher, so we
// want to keep the path.
kept = true;
}
}

// If all of the globs were negative globs, then we want to include the path
// as long as it was not explicitly not kept. Otherwise only include
// the path if it was kept. This allows sets of globs that are all negated
// to allow some paths to be matched, while sets of globs that are mixed
// negated and non-negated to cause the negated matchers to only omit paths
// and not keep them.
return negatives === matchers.length ? kept !== false : !!kept;
};
}
1 change: 1 addition & 0 deletions packages/jest-util/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ export {default as convertDescriptorToString} from './convertDescriptorToString'
import * as specialChars from './specialChars';
export {default as replacePathSepForGlob} from './replacePathSepForGlob';
export {default as testPathPatternToRegExp} from './testPathPatternToRegExp';
export {default as globsToMatcher} from './globsToMatcher';
import * as preRunMessage from './preRunMessage';
export {default as pluralize} from './pluralize';
export {default as formatTime} from './formatTime';
Expand Down