From 33acece49df3bbf07e7a3b7a2fda5b6f762c9989 Mon Sep 17 00:00:00 2001 From: Brent Dearth Date: Fri, 20 Jul 2018 12:57:55 -0600 Subject: [PATCH 1/5] perf(changed-files): limit git and hg commands to specified roots --- CHANGELOG.md | 5 +++ e2e/__tests__/jest_changed_files.test.js | 49 ++++++++++++++++++++++++ packages/jest-changed-files/src/git.js | 21 +++++++--- packages/jest-changed-files/src/hg.js | 6 ++- packages/jest-changed-files/src/index.js | 4 +- types/ChangedFiles.js | 6 ++- 6 files changed, 81 insertions(+), 10 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8b346bd6d0d5..8f050ac94290 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,10 @@ ## 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)) diff --git a/e2e/__tests__/jest_changed_files.test.js b/e2e/__tests__/jest_changed_files.test.js index 1f492585f1a1..97533322ae17 100644 --- a/e2e/__tests__/jest_changed_files.test.js +++ b/e2e/__tests__/jest_changed_files.test.js @@ -224,6 +224,27 @@ test('gets changed files for git', async () => { ).toEqual(['file5.txt']); }); +test('should only monitor 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 = ['nested_dir', 'nested_dir/second_nested_dir'].map(filename => + path.resolve(DIR, filename), + ); + + 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 @@ -326,3 +347,31 @@ test('gets changed files for hg', async () => { .sort(), ).toEqual(['file5.txt']); }); + +test('should only monitor 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 = ['nested_dir', 'nested_dir/second_nested_dir'].map(filename => + path.resolve(DIR, filename), + ); + + const {changedFiles: files} = await getChangedFilesForRoots(roots, {}); + expect( + Array.from(files) + .map(filePath => path.basename(filePath)) + .sort(), + ).toEqual(['file2.txt', 'file3.txt']); +}); diff --git a/packages/jest-changed-files/src/git.js b/packages/jest-changed-files/src/git.js index bb3d286ebe81..4f878e5878a0 100644 --- a/packages/jest-changed-files/src/git.js +++ b/packages/jest-changed-files/src/git.js @@ -46,6 +46,7 @@ const findChangedFilesUsingCommand = async ( const adapter: SCMAdapter = { findChangedFiles: async ( cwd: string, + roots: Array, options?: Options, ): Promise> => { const changedSince: ?string = @@ -53,26 +54,36 @@ const adapter: SCMAdapter = { if (options && options.lastCommit) { return await findChangedFilesUsingCommand( - ['show', '--name-only', '--pretty=%b', 'HEAD'], + ['show', '--name-only', '--pretty=%b', 'HEAD'].concat(roots), cwd, ); } else if (changedSince) { const committed = await findChangedFilesUsingCommand( - ['log', '--name-only', '--pretty=%b', 'HEAD', `^${changedSince}`], + [ + 'log', + '--name-only', + '--pretty=%b', + 'HEAD', + `^${changedSince}`, + ].concat(roots), cwd, ); const staged = await findChangedFilesUsingCommand( - ['diff', '--cached', '--name-only'], + ['diff', '--cached', '--name-only'].concat(roots), cwd, ); const unstaged = await findChangedFilesUsingCommand( - ['ls-files', '--other', '--modified', '--exclude-standard'], + ['ls-files', '--other', '--modified', '--exclude-standard'].concat( + roots, + ), cwd, ); return [...committed, ...staged, ...unstaged]; } else { return await findChangedFilesUsingCommand( - ['ls-files', '--other', '--modified', '--exclude-standard'], + ['ls-files', '--other', '--modified', '--exclude-standard'].concat( + roots, + ), cwd, ); } diff --git a/packages/jest-changed-files/src/hg.js b/packages/jest-changed-files/src/hg.js index 4f3d33552793..ebc13cc6a81c 100644 --- a/packages/jest-changed-files/src/hg.js +++ b/packages/jest-changed-files/src/hg.js @@ -31,17 +31,19 @@ const ANCESTORS = [ const adapter: SCMAdapter = { findChangedFiles: async ( cwd: string, + roots: Array, options: Options, ): Promise> => new Promise((resolve, reject) => { - let args = ['status', '-amnu']; + 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(...roots); const child = childProcess.spawn('hg', args, {cwd, env}); let stdout = ''; let stderr = ''; diff --git a/packages/jest-changed-files/src/index.js b/packages/jest-changed-files/src/index.js index 234393a7c149..f48b70642fe7 100644 --- a/packages/jest-changed-files/src/index.js +++ b/packages/jest-changed-files/src/index.js @@ -28,11 +28,11 @@ export const getChangedFilesForRoots = async ( const repos = await findRepos(roots); const gitPromises = Array.from(repos.git).map(repo => - git.findChangedFiles(repo, options), + git.findChangedFiles(repo, roots, options), ); const hgPromises = Array.from(repos.hg).map(repo => - hg.findChangedFiles(repo, options), + hg.findChangedFiles(repo, roots, options), ); const changedFiles = (await Promise.all( diff --git a/types/ChangedFiles.js b/types/ChangedFiles.js index 1b7249710a9b..6b69aca8ee9a 100644 --- a/types/ChangedFiles.js +++ b/types/ChangedFiles.js @@ -23,6 +23,10 @@ export type ChangedFilesPromise = Promise<{| |}>; export type SCMAdapter = {| - findChangedFiles: (cwd: Path, options: Options) => Promise>, + findChangedFiles: ( + cwd: Path, + roots: Array, + options: Options, + ) => Promise>, getRoot: (cwd: Path) => Promise, |}; From aac709cbe7493da6a2e5dfd06bc2c9e33e69ec57 Mon Sep 17 00:00:00 2001 From: Brent Dearth Date: Fri, 20 Jul 2018 15:33:34 -0600 Subject: [PATCH 2/5] move roots into Options as optional property and always provide via getChangedFilesForRoots --- CHANGELOG.md | 1 - packages/jest-changed-files/src/git.js | 13 +++++++------ packages/jest-changed-files/src/hg.js | 5 +++-- packages/jest-changed-files/src/index.js | 6 ++++-- types/ChangedFiles.js | 7 ++----- 5 files changed, 16 insertions(+), 16 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8f050ac94290..45cf04e641ca 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,5 @@ ## master - ### Performance - `[jest-changed-files]` limit git and hg commands to specified roots ([#6732](https://github.com/facebook/jest/pull/6732)) diff --git a/packages/jest-changed-files/src/git.js b/packages/jest-changed-files/src/git.js index 4f878e5878a0..672108ad7f96 100644 --- a/packages/jest-changed-files/src/git.js +++ b/packages/jest-changed-files/src/git.js @@ -46,15 +46,16 @@ const findChangedFilesUsingCommand = async ( const adapter: SCMAdapter = { findChangedFiles: async ( cwd: string, - roots: Array, options?: Options, ): Promise> => { const changedSince: ?string = options && (options.withAncestor ? 'HEAD^' : options.changedSince); + const includePaths: Array = (options && options.includePaths) || []; + if (options && options.lastCommit) { return await findChangedFilesUsingCommand( - ['show', '--name-only', '--pretty=%b', 'HEAD'].concat(roots), + ['show', '--name-only', '--pretty=%b', 'HEAD'].concat(includePaths), cwd, ); } else if (changedSince) { @@ -65,16 +66,16 @@ const adapter: SCMAdapter = { '--pretty=%b', 'HEAD', `^${changedSince}`, - ].concat(roots), + ].concat(includePaths), cwd, ); const staged = await findChangedFilesUsingCommand( - ['diff', '--cached', '--name-only'].concat(roots), + ['diff', '--cached', '--name-only'].concat(includePaths), cwd, ); const unstaged = await findChangedFilesUsingCommand( ['ls-files', '--other', '--modified', '--exclude-standard'].concat( - roots, + includePaths, ), cwd, ); @@ -82,7 +83,7 @@ const adapter: SCMAdapter = { } else { return await findChangedFilesUsingCommand( ['ls-files', '--other', '--modified', '--exclude-standard'].concat( - roots, + includePaths, ), cwd, ); diff --git a/packages/jest-changed-files/src/hg.js b/packages/jest-changed-files/src/hg.js index ebc13cc6a81c..029709546cdd 100644 --- a/packages/jest-changed-files/src/hg.js +++ b/packages/jest-changed-files/src/hg.js @@ -31,10 +31,11 @@ const ANCESTORS = [ const adapter: SCMAdapter = { findChangedFiles: async ( cwd: string, - roots: Array, options: Options, ): Promise> => new Promise((resolve, reject) => { + const includePaths: Array = (options && options.includePaths) || []; + const args = ['status', '-amnu']; if (options && options.withAncestor) { args.push('--rev', `ancestor(${ANCESTORS.join(', ')})`); @@ -43,7 +44,7 @@ const adapter: SCMAdapter = { } else if (options && options.lastCommit === true) { args.push('-A'); } - args.push(...roots); + args.push(...includePaths); const child = childProcess.spawn('hg', args, {cwd, env}); let stdout = ''; let stderr = ''; diff --git a/packages/jest-changed-files/src/index.js b/packages/jest-changed-files/src/index.js index f48b70642fe7..a1aedfab904e 100644 --- a/packages/jest-changed-files/src/index.js +++ b/packages/jest-changed-files/src/index.js @@ -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, roots, options), + git.findChangedFiles(repo, changedFilesOptions), ); const hgPromises = Array.from(repos.hg).map(repo => - hg.findChangedFiles(repo, roots, options), + hg.findChangedFiles(repo, changedFilesOptions), ); const changedFiles = (await Promise.all( diff --git a/types/ChangedFiles.js b/types/ChangedFiles.js index 6b69aca8ee9a..9e31be9286fb 100644 --- a/types/ChangedFiles.js +++ b/types/ChangedFiles.js @@ -13,6 +13,7 @@ export type Options = {| lastCommit?: boolean, withAncestor?: boolean, changedSince?: string, + includePaths?: Array, |}; export type ChangedFiles = Set; @@ -23,10 +24,6 @@ export type ChangedFilesPromise = Promise<{| |}>; export type SCMAdapter = {| - findChangedFiles: ( - cwd: Path, - roots: Array, - options: Options, - ) => Promise>, + findChangedFiles: (cwd: Path, options: Options) => Promise>, getRoot: (cwd: Path) => Promise, |}; From 8521eb4ee973e9f3bdd9e64155853b31ca6be858 Mon Sep 17 00:00:00 2001 From: Brent Dearth Date: Fri, 20 Jul 2018 17:13:28 -0600 Subject: [PATCH 3/5] remove redundant root --- e2e/__tests__/jest_changed_files.test.js | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/e2e/__tests__/jest_changed_files.test.js b/e2e/__tests__/jest_changed_files.test.js index 97533322ae17..1dc7bddb04d7 100644 --- a/e2e/__tests__/jest_changed_files.test.js +++ b/e2e/__tests__/jest_changed_files.test.js @@ -233,9 +233,7 @@ test('should only monitor root paths for git', async () => { run(`${GIT} init`, DIR); - const roots = ['nested_dir', 'nested_dir/second_nested_dir'].map(filename => - path.resolve(DIR, filename), - ); + const roots = [path.resolve(DIR, 'nested_dir')]; const {changedFiles: files} = await getChangedFilesForRoots(roots, {}); expect( @@ -364,9 +362,7 @@ test('should only monitor root paths for hg', async () => { run(`${HG} init`, DIR); - const roots = ['nested_dir', 'nested_dir/second_nested_dir'].map(filename => - path.resolve(DIR, filename), - ); + const roots = [path.resolve(DIR, 'nested_dir')]; const {changedFiles: files} = await getChangedFilesForRoots(roots, {}); expect( From dcec62661e5865e5b8eb8c47bbfd87fc9906807e Mon Sep 17 00:00:00 2001 From: Brent Date: Fri, 20 Jul 2018 17:41:52 -0600 Subject: [PATCH 4/5] drop the shoulds --- e2e/__tests__/jest_changed_files.test.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/e2e/__tests__/jest_changed_files.test.js b/e2e/__tests__/jest_changed_files.test.js index 1dc7bddb04d7..91b42e60fc86 100644 --- a/e2e/__tests__/jest_changed_files.test.js +++ b/e2e/__tests__/jest_changed_files.test.js @@ -224,7 +224,7 @@ test('gets changed files for git', async () => { ).toEqual(['file5.txt']); }); -test('should only monitor root paths for git', async () => { +test('monitors root paths for git', async () => { writeFiles(DIR, { 'file1.txt': 'file1', 'nested_dir/file2.txt': 'file2', @@ -346,7 +346,7 @@ test('gets changed files for hg', async () => { ).toEqual(['file5.txt']); }); -test('should only monitor root paths for hg', async () => { +test('monitors 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 From 364744667365ceb5edac278a4f81febce53a08f5 Mon Sep 17 00:00:00 2001 From: Brent Date: Sat, 21 Jul 2018 07:30:47 -0600 Subject: [PATCH 5/5] "monitors only" instead of "monitors" in test labels --- e2e/__tests__/jest_changed_files.test.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/e2e/__tests__/jest_changed_files.test.js b/e2e/__tests__/jest_changed_files.test.js index 91b42e60fc86..30d162303d3e 100644 --- a/e2e/__tests__/jest_changed_files.test.js +++ b/e2e/__tests__/jest_changed_files.test.js @@ -224,7 +224,7 @@ test('gets changed files for git', async () => { ).toEqual(['file5.txt']); }); -test('monitors root paths for git', async () => { +test('monitors only root paths for git', async () => { writeFiles(DIR, { 'file1.txt': 'file1', 'nested_dir/file2.txt': 'file2', @@ -346,7 +346,7 @@ test('gets changed files for hg', async () => { ).toEqual(['file5.txt']); }); -test('monitors root paths for hg', async () => { +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