Skip to content

Commit 79089db

Browse files
booleanbetrayalthymikee
authored andcommitted
perf(changed-files): limit git and hg commands to specified roots (jestjs#6732)
## 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.
1 parent bf9cbc2 commit 79089db

File tree

6 files changed

+76
-9
lines changed

6 files changed

+76
-9
lines changed

CHANGELOG.md

+4
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,9 @@
11
## master
22

3+
### Performance
4+
5+
- `[jest-changed-files]` limit git and hg commands to specified roots ([#6732](https://github.com/facebook/jest/pull/6732))
6+
37
### Fixes
48

59
- `[babel-jest]` Make `getCacheKey()` take into account `createTransformer` options ([#6699](https://github.com/facebook/jest/pull/6699))

e2e/__tests__/jest_changed_files.test.js

+45
Original file line numberDiff line numberDiff line change
@@ -224,6 +224,25 @@ test('gets changed files for git', async () => {
224224
).toEqual(['file5.txt']);
225225
});
226226

227+
test('monitors only root paths for git', async () => {
228+
writeFiles(DIR, {
229+
'file1.txt': 'file1',
230+
'nested_dir/file2.txt': 'file2',
231+
'nested_dir/second_nested_dir/file3.txt': 'file3',
232+
});
233+
234+
run(`${GIT} init`, DIR);
235+
236+
const roots = [path.resolve(DIR, 'nested_dir')];
237+
238+
const {changedFiles: files} = await getChangedFilesForRoots(roots, {});
239+
expect(
240+
Array.from(files)
241+
.map(filePath => path.basename(filePath))
242+
.sort(),
243+
).toEqual(['file2.txt', 'file3.txt']);
244+
});
245+
227246
test('gets changed files for hg', async () => {
228247
if (process.env.CI) {
229248
// Circle and Travis have very old version of hg (v2, and current
@@ -326,3 +345,29 @@ test('gets changed files for hg', async () => {
326345
.sort(),
327346
).toEqual(['file5.txt']);
328347
});
348+
349+
test('monitors only root paths for hg', async () => {
350+
if (process.env.CI) {
351+
// Circle and Travis have very old version of hg (v2, and current
352+
// version is v4.2) and its API changed since then and not compatible
353+
// any more. Changing the SCM version on CIs is not trivial, so we'll just
354+
// skip this test and run it only locally.
355+
return;
356+
}
357+
writeFiles(DIR, {
358+
'file1.txt': 'file1',
359+
'nested_dir/file2.txt': 'file2',
360+
'nested_dir/second_nested_dir/file3.txt': 'file3',
361+
});
362+
363+
run(`${HG} init`, DIR);
364+
365+
const roots = [path.resolve(DIR, 'nested_dir')];
366+
367+
const {changedFiles: files} = await getChangedFilesForRoots(roots, {});
368+
expect(
369+
Array.from(files)
370+
.map(filePath => path.basename(filePath))
371+
.sort(),
372+
).toEqual(['file2.txt', 'file3.txt']);
373+
});

packages/jest-changed-files/src/git.js

+17-5
Original file line numberDiff line numberDiff line change
@@ -51,28 +51,40 @@ const adapter: SCMAdapter = {
5151
const changedSince: ?string =
5252
options && (options.withAncestor ? 'HEAD^' : options.changedSince);
5353

54+
const includePaths: Array<Path> = (options && options.includePaths) || [];
55+
5456
if (options && options.lastCommit) {
5557
return await findChangedFilesUsingCommand(
56-
['show', '--name-only', '--pretty=%b', 'HEAD'],
58+
['show', '--name-only', '--pretty=%b', 'HEAD'].concat(includePaths),
5759
cwd,
5860
);
5961
} else if (changedSince) {
6062
const committed = await findChangedFilesUsingCommand(
61-
['log', '--name-only', '--pretty=%b', 'HEAD', `^${changedSince}`],
63+
[
64+
'log',
65+
'--name-only',
66+
'--pretty=%b',
67+
'HEAD',
68+
`^${changedSince}`,
69+
].concat(includePaths),
6270
cwd,
6371
);
6472
const staged = await findChangedFilesUsingCommand(
65-
['diff', '--cached', '--name-only'],
73+
['diff', '--cached', '--name-only'].concat(includePaths),
6674
cwd,
6775
);
6876
const unstaged = await findChangedFilesUsingCommand(
69-
['ls-files', '--other', '--modified', '--exclude-standard'],
77+
['ls-files', '--other', '--modified', '--exclude-standard'].concat(
78+
includePaths,
79+
),
7080
cwd,
7181
);
7282
return [...committed, ...staged, ...unstaged];
7383
} else {
7484
return await findChangedFilesUsingCommand(
75-
['ls-files', '--other', '--modified', '--exclude-standard'],
85+
['ls-files', '--other', '--modified', '--exclude-standard'].concat(
86+
includePaths,
87+
),
7688
cwd,
7789
);
7890
}

packages/jest-changed-files/src/hg.js

+5-2
Original file line numberDiff line numberDiff line change
@@ -34,14 +34,17 @@ const adapter: SCMAdapter = {
3434
options: Options,
3535
): Promise<Array<Path>> =>
3636
new Promise((resolve, reject) => {
37-
let args = ['status', '-amnu'];
37+
const includePaths: Array<Path> = (options && options.includePaths) || [];
38+
39+
const args = ['status', '-amnu'];
3840
if (options && options.withAncestor) {
3941
args.push('--rev', `ancestor(${ANCESTORS.join(', ')})`);
4042
} else if (options && options.changedSince) {
4143
args.push('--rev', `ancestor(., ${options.changedSince})`);
4244
} else if (options && options.lastCommit === true) {
43-
args = ['tip', '--template', '{files%"{file}\n"}'];
45+
args.push('-A');
4446
}
47+
args.push(...includePaths);
4548
const child = childProcess.spawn('hg', args, {cwd, env});
4649
let stdout = '';
4750
let stderr = '';

packages/jest-changed-files/src/index.js

+4-2
Original file line numberDiff line numberDiff line change
@@ -27,12 +27,14 @@ export const getChangedFilesForRoots = async (
2727
): ChangedFilesPromise => {
2828
const repos = await findRepos(roots);
2929

30+
const changedFilesOptions = Object.assign({}, {includePaths: roots}, options);
31+
3032
const gitPromises = Array.from(repos.git).map(repo =>
31-
git.findChangedFiles(repo, options),
33+
git.findChangedFiles(repo, changedFilesOptions),
3234
);
3335

3436
const hgPromises = Array.from(repos.hg).map(repo =>
35-
hg.findChangedFiles(repo, options),
37+
hg.findChangedFiles(repo, changedFilesOptions),
3638
);
3739

3840
const changedFiles = (await Promise.all(

types/ChangedFiles.js

+1
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ export type Options = {|
1313
lastCommit?: boolean,
1414
withAncestor?: boolean,
1515
changedSince?: string,
16+
includePaths?: Array<Path>,
1617
|};
1718

1819
export type ChangedFiles = Set<Path>;

0 commit comments

Comments
 (0)