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
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
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
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
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
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
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
@@ -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,
);
});
63 changes: 63 additions & 0 deletions packages/jest-util/src/globsToMatcher.ts
@@ -0,0 +1,63 @@
/**
* 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 globsMatchers = new Map<
string,
{
isMatch: (str: string) => boolean;
negated: boolean;
}
>();

export default function globsToMatcher(
globs: Array<Config.Glob>,
): (path: Config.Path) => boolean {
if (globs.length === 0) {
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 (!globsMatchers.has(glob)) {
const state = micromatch.scan(glob, {dot: true});
const matcher = {
isMatch: micromatch.matcher(glob, {dot: true}),
negated: state.negated,
};
globsMatchers.set(glob, matcher);
}
return globsMatchers.get(glob)!;
});

return (path: Config.Path): boolean => {
const replacedPath = replacePathSepForGlob(path);
let kept = false;
let omitted = false;
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) {
kept = false;
omitted = true;
} else if (matched && !negated) {
kept = true;
omitted = false;
}
}

return negatives === matchers.length ? !omitted : kept && !omitted;
};
}
1 change: 1 addition & 0 deletions packages/jest-util/src/index.ts
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