Skip to content

Commit

Permalink
perf(changed-files): limit git and hg commands to specified roots (#6732
Browse files Browse the repository at this point in the history
)

## Summary

Given a monorepo setup under git version control, we saw `jest --watch` commands take significantly longer in a Docker container host mount (`rw:cached`) than when on the host directly. Tracked the issue down to the fact that `git` and `hg` command used to determine `lastCommit` changes were traversing the entire repository, and not just paths specified in the Jest config `roots` values. Given the mounted container latency, things like basic `lstat` gathering be intensive in a large repository. The `roots` we had specified are rather shallow themselves and only contain things we want to test with Jest. When limiting the scope of the `git` command to just the `roots` paths, we saw change determination drop from 2.5m to 3s in our container.

This implementation seem to align more closely with the [`roots` documentation](https://jestjs.io/docs/en/configuration#roots-array-string) which indicates:

> A list of paths to directories that Jest should use to search for files in.

It did seem a little unexpected that Jest would be traversing our entire repo.
  • Loading branch information
booleanbetrayal authored and thymikee committed Jul 21, 2018
1 parent bf9cbc2 commit 79089db
Show file tree
Hide file tree
Showing 6 changed files with 76 additions and 9 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
## master

### Performance

- `[jest-changed-files]` limit git and hg commands to specified roots ([#6732](https://github.com/facebook/jest/pull/6732))

### Fixes

- `[babel-jest]` Make `getCacheKey()` take into account `createTransformer` options ([#6699](https://github.com/facebook/jest/pull/6699))
Expand Down
45 changes: 45 additions & 0 deletions e2e/__tests__/jest_changed_files.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -224,6 +224,25 @@ test('gets changed files for git', async () => {
).toEqual(['file5.txt']);
});

test('monitors only root paths for git', async () => {
writeFiles(DIR, {
'file1.txt': 'file1',
'nested_dir/file2.txt': 'file2',
'nested_dir/second_nested_dir/file3.txt': 'file3',
});

run(`${GIT} init`, DIR);

const roots = [path.resolve(DIR, 'nested_dir')];

const {changedFiles: files} = await getChangedFilesForRoots(roots, {});
expect(
Array.from(files)
.map(filePath => path.basename(filePath))
.sort(),
).toEqual(['file2.txt', 'file3.txt']);
});

test('gets changed files for hg', async () => {
if (process.env.CI) {
// Circle and Travis have very old version of hg (v2, and current
Expand Down Expand Up @@ -326,3 +345,29 @@ test('gets changed files for hg', async () => {
.sort(),
).toEqual(['file5.txt']);
});

test('monitors only root paths for hg', async () => {
if (process.env.CI) {
// Circle and Travis have very old version of hg (v2, and current
// version is v4.2) and its API changed since then and not compatible
// any more. Changing the SCM version on CIs is not trivial, so we'll just
// skip this test and run it only locally.
return;
}
writeFiles(DIR, {
'file1.txt': 'file1',
'nested_dir/file2.txt': 'file2',
'nested_dir/second_nested_dir/file3.txt': 'file3',
});

run(`${HG} init`, DIR);

const roots = [path.resolve(DIR, 'nested_dir')];

const {changedFiles: files} = await getChangedFilesForRoots(roots, {});
expect(
Array.from(files)
.map(filePath => path.basename(filePath))
.sort(),
).toEqual(['file2.txt', 'file3.txt']);
});
22 changes: 17 additions & 5 deletions packages/jest-changed-files/src/git.js
Original file line number Diff line number Diff line change
Expand Up @@ -51,28 +51,40 @@ const adapter: SCMAdapter = {
const changedSince: ?string =
options && (options.withAncestor ? 'HEAD^' : options.changedSince);

const includePaths: Array<Path> = (options && options.includePaths) || [];

if (options && options.lastCommit) {
return await findChangedFilesUsingCommand(
['show', '--name-only', '--pretty=%b', 'HEAD'],
['show', '--name-only', '--pretty=%b', 'HEAD'].concat(includePaths),
cwd,
);
} else if (changedSince) {
const committed = await findChangedFilesUsingCommand(
['log', '--name-only', '--pretty=%b', 'HEAD', `^${changedSince}`],
[
'log',
'--name-only',
'--pretty=%b',
'HEAD',
`^${changedSince}`,
].concat(includePaths),
cwd,
);
const staged = await findChangedFilesUsingCommand(
['diff', '--cached', '--name-only'],
['diff', '--cached', '--name-only'].concat(includePaths),
cwd,
);
const unstaged = await findChangedFilesUsingCommand(
['ls-files', '--other', '--modified', '--exclude-standard'],
['ls-files', '--other', '--modified', '--exclude-standard'].concat(
includePaths,
),
cwd,
);
return [...committed, ...staged, ...unstaged];
} else {
return await findChangedFilesUsingCommand(
['ls-files', '--other', '--modified', '--exclude-standard'],
['ls-files', '--other', '--modified', '--exclude-standard'].concat(
includePaths,
),
cwd,
);
}
Expand Down
7 changes: 5 additions & 2 deletions packages/jest-changed-files/src/hg.js
Original file line number Diff line number Diff line change
Expand Up @@ -34,14 +34,17 @@ const adapter: SCMAdapter = {
options: Options,
): Promise<Array<Path>> =>
new Promise((resolve, reject) => {
let args = ['status', '-amnu'];
const includePaths: Array<Path> = (options && options.includePaths) || [];

const args = ['status', '-amnu'];
if (options && options.withAncestor) {
args.push('--rev', `ancestor(${ANCESTORS.join(', ')})`);
} else if (options && options.changedSince) {
args.push('--rev', `ancestor(., ${options.changedSince})`);
} else if (options && options.lastCommit === true) {
args = ['tip', '--template', '{files%"{file}\n"}'];
args.push('-A');
}
args.push(...includePaths);
const child = childProcess.spawn('hg', args, {cwd, env});
let stdout = '';
let stderr = '';
Expand Down
6 changes: 4 additions & 2 deletions packages/jest-changed-files/src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,12 +27,14 @@ export const getChangedFilesForRoots = async (
): ChangedFilesPromise => {
const repos = await findRepos(roots);

const changedFilesOptions = Object.assign({}, {includePaths: roots}, options);

const gitPromises = Array.from(repos.git).map(repo =>
git.findChangedFiles(repo, options),
git.findChangedFiles(repo, changedFilesOptions),
);

const hgPromises = Array.from(repos.hg).map(repo =>
hg.findChangedFiles(repo, options),
hg.findChangedFiles(repo, changedFilesOptions),
);

const changedFiles = (await Promise.all(
Expand Down
1 change: 1 addition & 0 deletions types/ChangedFiles.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ export type Options = {|
lastCommit?: boolean,
withAncestor?: boolean,
changedSince?: string,
includePaths?: Array<Path>,
|};

export type ChangedFiles = Set<Path>;
Expand Down

0 comments on commit 79089db

Please sign in to comment.