diff --git a/CHANGELOG.md b/CHANGELOG.md index b0c4dec56600..746190c9c92b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -94,6 +94,8 @@ ### Performance +- `[jest-haste-map]` Optimize haste map tracking of deleted files with Watchman. ([#8056](https://github.com/facebook/jest/pull/8056)) + ## 24.1.0 ### Features diff --git a/packages/jest-haste-map/src/__tests__/index.test.js b/packages/jest-haste-map/src/__tests__/index.test.js index 68b3aa4e226b..03b74317a210 100644 --- a/packages/jest-haste-map/src/__tests__/index.test.js +++ b/packages/jest-haste-map/src/__tests__/index.test.js @@ -40,6 +40,7 @@ jest.mock('../crawlers/watchman', () => const {data, ignore, rootDir, roots, computeSha1} = options; const list = mockChangedFiles || mockFs; + const removedFiles = new Map(); data.clocks = mockClocks; @@ -51,12 +52,19 @@ jest.mock('../crawlers/watchman', () => data.files.set(relativeFilePath, ['', 32, 42, 0, [], hash]); } else { - data.files.delete(relativeFilePath); + const fileData = data.files.get(relativeFilePath); + if (fileData) { + removedFiles.set(relativeFilePath, fileData); + data.files.delete(relativeFilePath); + } } } } - return Promise.resolve(data); + return Promise.resolve({ + hasteMap: data, + removedFiles, + }); }), ); @@ -416,7 +424,10 @@ describe('HasteMap', () => { 'vegetables/Melon.js': ['Melon', 32, 42, 0, [], null], }); - return Promise.resolve(data); + return Promise.resolve({ + hasteMap: data, + removedFiles: new Map(), + }); }); const hasteMap = new HasteMap({ @@ -543,7 +554,7 @@ describe('HasteMap', () => { ...defaultConfig, }) .build() - .catch(({__hasteMapForTest: data}) => { + .catch(() => { expect(console.error.mock.calls[0][0]).toMatchSnapshot(); }); }); @@ -979,7 +990,7 @@ describe('HasteMap', () => { mockImpl(options).then(() => { const {data} = options; data.files.set('fruits/invalid/file.js', ['', 34, 44, 0, []]); - return data; + return {hasteMap: data, removedFiles: new Map()}; }), ); return new HasteMap(defaultConfig) @@ -1077,7 +1088,10 @@ describe('HasteMap', () => { data.files = createMap({ 'fruits/Banana.js': ['', 32, 42, 0, [], null], }); - return Promise.resolve(data); + return Promise.resolve({ + hasteMap: data, + removedFiles: new Map(), + }); }); return new HasteMap(defaultConfig) @@ -1108,7 +1122,10 @@ describe('HasteMap', () => { data.files = createMap({ 'fruits/Banana.js': ['', 32, 42, 0, [], null], }); - return Promise.resolve(data); + return Promise.resolve({ + hasteMap: data, + removedFiles: new Map(), + }); }); return new HasteMap(defaultConfig) diff --git a/packages/jest-haste-map/src/crawlers/__tests__/node.test.js b/packages/jest-haste-map/src/crawlers/__tests__/node.test.js index 167cc0783d53..b1cb1f1730e2 100644 --- a/packages/jest-haste-map/src/crawlers/__tests__/node.test.js +++ b/packages/jest-haste-map/src/crawlers/__tests__/node.test.js @@ -114,7 +114,7 @@ describe('node crawler', () => { ignore: pearMatcher, rootDir, roots: ['/project/fruits', '/project/vegtables'], - }).then(data => { + }).then(({hasteMap, removedFiles}) => { expect(childProcess.spawn).lastCalledWith('find', [ '/project/fruits', '/project/vegtables', @@ -129,15 +129,17 @@ describe('node crawler', () => { ')', ]); - expect(data.files).not.toBe(null); + expect(hasteMap.files).not.toBe(null); - expect(data.files).toEqual( + expect(hasteMap.files).toEqual( createMap({ 'fruits/strawberry.js': ['', 32, 42, 0, [], null], 'fruits/tomato.js': ['', 33, 42, 0, [], null], 'vegetables/melon.json': ['', 34, 42, 0, [], null], }), ); + + expect(removedFiles).toEqual(new Map()); }); return promise; @@ -161,8 +163,8 @@ describe('node crawler', () => { ignore: pearMatcher, rootDir, roots: ['/project/fruits'], - }).then(data => { - expect(data.files).toEqual( + }).then(({hasteMap, removedFiles}) => { + expect(hasteMap.files).toEqual( createMap({ 'fruits/strawberry.js': ['', 32, 42, 0, [], null], 'fruits/tomato.js': tomato, @@ -170,7 +172,43 @@ describe('node crawler', () => { ); // Make sure it is the *same* unchanged object. - expect(data.files.get('fruits/tomato.js')).toBe(tomato); + expect(hasteMap.files.get('fruits/tomato.js')).toBe(tomato); + + expect(removedFiles).toEqual(new Map()); + }); + }); + + it('returns removed files', () => { + process.platform = 'linux'; + + nodeCrawl = require('../node'); + + // In this test sample, previouslyExisted was present before and will not be + // when crawling this directory. + const files = createMap({ + 'fruits/previouslyExisted.js': ['', 30, 40, 1, [], null], + 'fruits/strawberry.js': ['', 33, 42, 0, [], null], + 'fruits/tomato.js': ['', 32, 42, 0, [], null], + }); + + return nodeCrawl({ + data: {files}, + extensions: ['js'], + ignore: pearMatcher, + rootDir, + roots: ['/project/fruits'], + }).then(({hasteMap, removedFiles}) => { + expect(hasteMap.files).toEqual( + createMap({ + 'fruits/strawberry.js': ['', 32, 42, 0, [], null], + 'fruits/tomato.js': ['', 33, 42, 0, [], null], + }), + ); + expect(removedFiles).toEqual( + createMap({ + 'fruits/previouslyExisted.js': ['', 30, 40, 1, [], null], + }), + ); }); }); @@ -187,13 +225,14 @@ describe('node crawler', () => { ignore: pearMatcher, rootDir, roots: ['/project/fruits'], - }).then(data => { - expect(data.files).toEqual( + }).then(({hasteMap, removedFiles}) => { + expect(hasteMap.files).toEqual( createMap({ 'fruits/directory/strawberry.js': ['', 33, 42, 0, [], null], 'fruits/tomato.js': ['', 32, 42, 0, [], null], }), ); + expect(removedFiles).toEqual(new Map()); }); }); @@ -210,13 +249,14 @@ describe('node crawler', () => { ignore: pearMatcher, rootDir, roots: ['/project/fruits'], - }).then(data => { - expect(data.files).toEqual( + }).then(({hasteMap, removedFiles}) => { + expect(hasteMap.files).toEqual( createMap({ 'fruits/directory/strawberry.js': ['', 33, 42, 0, [], null], 'fruits/tomato.js': ['', 32, 42, 0, [], null], }), ); + expect(removedFiles).toEqual(new Map()); }); }); @@ -233,8 +273,9 @@ describe('node crawler', () => { ignore: pearMatcher, rootDir, roots: [], - }).then(data => { - expect(data.files).toEqual(new Map()); + }).then(({hasteMap, removedFiles}) => { + expect(hasteMap.files).toEqual(new Map()); + expect(removedFiles).toEqual(new Map()); }); }); @@ -250,8 +291,9 @@ describe('node crawler', () => { ignore: pearMatcher, rootDir, roots: ['/error'], - }).then(data => { - expect(data.files).toEqual(new Map()); + }).then(({hasteMap, removedFiles}) => { + expect(hasteMap.files).toEqual(new Map()); + expect(removedFiles).toEqual(new Map()); }); }); }); diff --git a/packages/jest-haste-map/src/crawlers/__tests__/watchman.test.js b/packages/jest-haste-map/src/crawlers/__tests__/watchman.test.js index d4f4252ca5c6..ab454171f5bd 100644 --- a/packages/jest-haste-map/src/crawlers/__tests__/watchman.test.js +++ b/packages/jest-haste-map/src/crawlers/__tests__/watchman.test.js @@ -128,7 +128,7 @@ describe('watchman watch', () => { ignore: pearMatcher, rootDir: ROOT_MOCK, roots: ROOTS, - }).then(data => { + }).then(({hasteMap, removedFiles}) => { const client = watchman.Client.mock.instances[0]; const calls = client.command.mock.calls; @@ -159,13 +159,15 @@ describe('watchman watch', () => { 'vegetables/**/*.json', ]); - expect(data.clocks).toEqual( + expect(hasteMap.clocks).toEqual( createMap({ '': 'c:fake-clock:1', }), ); - expect(data.files).toEqual(mockFiles); + expect(hasteMap.files).toEqual(mockFiles); + + expect(removedFiles).toEqual(new Map()); expect(client.end).toBeCalled(); })); @@ -208,17 +210,18 @@ describe('watchman watch', () => { : null, rootDir: ROOT_MOCK, roots: ROOTS, - }).then(data => { - expect(data.files).toEqual( + }).then(({hasteMap, removedFiles}) => { + expect(hasteMap.files).toEqual( createMap({ [path.join(DURIAN_RELATIVE, 'foo.1.js')]: ['', 33, 43, 0, [], null], [path.join(DURIAN_RELATIVE, 'foo.2.js')]: ['', 33, 43, 0, [], null], }), ); + expect(removedFiles).toEqual(new Map()); }); }); - test('updates the file object when the clock is given', () => { + test('updates file map and removedFiles when the clock is given', () => { mockResponse = { 'list-capabilities': { [undefined]: { @@ -262,27 +265,33 @@ describe('watchman watch', () => { ignore: pearMatcher, rootDir: ROOT_MOCK, roots: ROOTS, - }).then(data => { + }).then(({hasteMap, removedFiles}) => { // The object was reused. - expect(data.files).toBe(mockFiles); + expect(hasteMap.files).toBe(mockFiles); - expect(data.clocks).toEqual( + expect(hasteMap.clocks).toEqual( createMap({ '': 'c:fake-clock:2', }), ); - expect(data.files).toEqual( + expect(hasteMap.files).toEqual( createMap({ [KIWI_RELATIVE]: ['', 42, 40, 0, [], null], [MELON_RELATIVE]: ['', 33, 43, 0, [], null], [STRAWBERRY_RELATIVE]: ['', 30, 40, 0, [], null], }), ); + + expect(removedFiles).toEqual( + createMap({ + [TOMATO_RELATIVE]: ['', 31, 41, 0, [], null], + }), + ); }); }); - test('resets the file object when watchman is restarted', () => { + test('resets the file map and tracks removedFiles when watchman is fresh', () => { const mockTomatoSha1 = '321f6b7e8bf7f29aab89c5e41a555b1b0baa41a9'; mockResponse = { @@ -340,18 +349,18 @@ describe('watchman watch', () => { ignore: pearMatcher, rootDir: ROOT_MOCK, roots: ROOTS, - }).then(data => { + }).then(({hasteMap, removedFiles}) => { // The file object was *not* reused. - expect(data.files).not.toBe(mockFiles); + expect(hasteMap.files).not.toBe(mockFiles); - expect(data.clocks).toEqual( + expect(hasteMap.clocks).toEqual( createMap({ '': 'c:fake-clock:3', }), ); - // /fruits/strawberry.js was removed from the file list. - expect(data.files).toEqual( + // strawberry and melon removed from the file list. + expect(hasteMap.files).toEqual( createMap({ [BANANA_RELATIVE]: mockBananaMetadata, [KIWI_RELATIVE]: ['', 42, 52, 0, [], null], @@ -361,10 +370,17 @@ describe('watchman watch', () => { // Even though the file list was reset, old file objects are still reused // if no changes have been made - expect(data.files.get(BANANA_RELATIVE)).toBe(mockBananaMetadata); + expect(hasteMap.files.get(BANANA_RELATIVE)).toBe(mockBananaMetadata); // Old file objects are not reused if they have a different mtime - expect(data.files.get(TOMATO_RELATIVE)).not.toBe(mockTomatoMetadata); + expect(hasteMap.files.get(TOMATO_RELATIVE)).not.toBe(mockTomatoMetadata); + + expect(removedFiles).toEqual( + createMap({ + [MELON_RELATIVE]: ['', 33, 43, 0, [], null], + [STRAWBERRY_RELATIVE]: ['', 30, 40, 0, [], null], + }), + ); }); }); @@ -427,20 +443,27 @@ describe('watchman watch', () => { ignore: pearMatcher, rootDir: ROOT_MOCK, roots: ROOTS, - }).then(data => { - expect(data.clocks).toEqual( + }).then(({hasteMap, removedFiles}) => { + expect(hasteMap.clocks).toEqual( createMap({ [FRUITS_RELATIVE]: 'c:fake-clock:3', [VEGETABLES_RELATIVE]: 'c:fake-clock:4', }), ); - expect(data.files).toEqual( + expect(hasteMap.files).toEqual( createMap({ [KIWI_RELATIVE]: ['', 42, 52, 0, [], null], [MELON_RELATIVE]: ['', 33, 43, 0, [], null], }), ); + + expect(removedFiles).toEqual( + createMap({ + [STRAWBERRY_RELATIVE]: ['', 30, 40, 0, [], null], + [TOMATO_RELATIVE]: ['', 31, 41, 0, [], null], + }), + ); }); }); @@ -483,7 +506,7 @@ describe('watchman watch', () => { ignore: pearMatcher, rootDir: ROOT_MOCK, roots: [...ROOTS, ROOT_MOCK], - }).then(data => { + }).then(({hasteMap, removedFiles}) => { const client = watchman.Client.mock.instances[0]; const calls = client.command.mock.calls; @@ -509,13 +532,15 @@ describe('watchman watch', () => { expect(query[2].glob).toEqual(['**/*.js', '**/*.json']); - expect(data.clocks).toEqual( + expect(hasteMap.clocks).toEqual( createMap({ '': 'c:fake-clock:1', }), ); - expect(data.files).toEqual(createMap({})); + expect(hasteMap.files).toEqual(new Map()); + + expect(removedFiles).toEqual(new Map()); expect(client.end).toBeCalled(); }); diff --git a/packages/jest-haste-map/src/crawlers/node.ts b/packages/jest-haste-map/src/crawlers/node.ts index 2f3cb61e2c5e..07a93edf3ee4 100644 --- a/packages/jest-haste-map/src/crawlers/node.ts +++ b/packages/jest-haste-map/src/crawlers/node.ts @@ -10,7 +10,12 @@ import path from 'path'; import {spawn} from 'child_process'; import H from '../constants'; import * as fastPath from '../lib/fast_path'; -import {IgnoreMatcher, InternalHasteMap, CrawlerOptions} from '../types'; +import { + IgnoreMatcher, + InternalHasteMap, + CrawlerOptions, + FileData, +} from '../types'; type Result = Array<[/* id */ string, /* mtime */ number, /* size */ number]>; @@ -130,7 +135,10 @@ function findNative( export = function nodeCrawl( options: CrawlerOptions, -): Promise { +): Promise<{ + removedFiles: FileData; + hasteMap: InternalHasteMap; +}> { if (options.mapper) { throw new Error(`Option 'mapper' isn't supported by the Node crawler`); } @@ -147,6 +155,7 @@ export = function nodeCrawl( return new Promise(resolve => { const callback = (list: Result) => { const files = new Map(); + const removedFiles = new Map(data.files); list.forEach(fileData => { const [filePath, mtime, size] = fileData; const relativeFilePath = fastPath.relative(rootDir, filePath); @@ -157,9 +166,14 @@ export = function nodeCrawl( // See ../constants.js; SHA-1 will always be null and fulfilled later. files.set(relativeFilePath, ['', mtime, size, 0, [], null]); } + removedFiles.delete(relativeFilePath); }); data.files = files; - resolve(data); + + resolve({ + hasteMap: data, + removedFiles, + }); }; if (forceNodeFilesystemAPI || process.platform === 'win32') { diff --git a/packages/jest-haste-map/src/crawlers/watchman.ts b/packages/jest-haste-map/src/crawlers/watchman.ts index b7f5fc453f0b..2512efab7540 100644 --- a/packages/jest-haste-map/src/crawlers/watchman.ts +++ b/packages/jest-haste-map/src/crawlers/watchman.ts @@ -11,7 +11,12 @@ import {Config} from '@jest/types'; import * as fastPath from '../lib/fast_path'; import normalizePathSep from '../lib/normalizePathSep'; import H from '../constants'; -import {InternalHasteMap, CrawlerOptions, FileMetaData} from '../types'; +import { + InternalHasteMap, + CrawlerOptions, + FileMetaData, + FileData, +} from '../types'; type WatchmanRoots = Map>; @@ -27,7 +32,10 @@ function WatchmanError(error: Error): Error { export = async function watchmanCrawl( options: CrawlerOptions, -): Promise { +): Promise<{ + removedFiles: FileData; + hasteMap: InternalHasteMap; +}> { const fields = ['name', 'exists', 'mtime_ms', 'size']; const {data, extensions, ignore, rootDir, roots} = options; const defaultWatchExpression = [ @@ -139,7 +147,9 @@ export = async function watchmanCrawl( } let files = data.files; + let removedFiles = new Map(); let watchmanFiles: Map; + let isFresh = false; try { const watchmanRoots = await getWatchmanRoots(roots); const watchmanFileResults = await queryWatchmanForDirs(watchmanRoots); @@ -148,6 +158,8 @@ export = async function watchmanCrawl( // files. if (watchmanFileResults.isFresh) { files = new Map(); + removedFiles = new Map(data.files); + isFresh = true; } watchmanFiles = watchmanFileResults.files; @@ -168,9 +180,25 @@ export = async function watchmanCrawl( for (const fileData of response.files) { const filePath = fsRoot + path.sep + normalizePathSep(fileData.name); const relativeFilePath = fastPath.relative(rootDir, filePath); + const existingFileData = data.files.get(relativeFilePath); + + // If watchman is fresh, the removed files map starts with all files + // and we remove them as we verify they still exist. + if (isFresh && existingFileData && fileData.exists) { + removedFiles.delete(relativeFilePath); + } if (!fileData.exists) { - files.delete(relativeFilePath); + // No need to act on files that do not exist and were not tracked. + if (existingFileData) { + files.delete(relativeFilePath); + + // If watchman is not fresh, we will know what specific files were + // deleted since we last ran and can track only those files. + if (!isFresh) { + removedFiles.set(relativeFilePath, existingFileData); + } + } } else if (!ignore(filePath)) { const mtime = typeof fileData.mtime_ms === 'number' @@ -183,7 +211,6 @@ export = async function watchmanCrawl( sha1hex = null; } - const existingFileData = data.files.get(relativeFilePath); let nextData: FileMetaData; if (existingFileData && existingFileData[H.MTIME] === mtime) { @@ -226,5 +253,8 @@ export = async function watchmanCrawl( } data.files = files; - return data; + return { + hasteMap: data, + removedFiles, + }; }; diff --git a/packages/jest-haste-map/src/index.ts b/packages/jest-haste-map/src/index.ts index 353003a041ae..174c5e3baca2 100644 --- a/packages/jest-haste-map/src/index.ts +++ b/packages/jest-haste-map/src/index.ts @@ -42,6 +42,8 @@ import { ModuleMapData, ModuleMetaData, WorkerMetadata, + CrawlerOptions, + FileData, } from './types'; type HType = typeof H; @@ -394,7 +396,7 @@ class HasteMap extends EventEmitter { * 2. crawl the file system. */ private _buildFileMap(): Promise<{ - deprecatedFiles: Array<{moduleName: string; path: string}>; + removedFiles: FileData; hasteMap: InternalHasteMap; }> { const read = this._options.resetCache ? this._createEmptyMap : this.read; @@ -402,19 +404,7 @@ class HasteMap extends EventEmitter { return Promise.resolve() .then(() => read.call(this)) .catch(() => this._createEmptyMap()) - .then(cachedHasteMap => { - const cachedFiles: Array<{moduleName: string; path: string}> = []; - for (const [relativeFilePath, fileMetadata] of cachedHasteMap.files) { - const moduleName = fileMetadata[H.ID]; - cachedFiles.push({moduleName, path: relativeFilePath}); - } - return this._crawl(cachedHasteMap).then(hasteMap => { - const deprecatedFiles = cachedFiles.filter( - file => !hasteMap.files.has(file.path), - ); - return {deprecatedFiles, hasteMap}; - }); - }); + .then(hasteMap => this._crawl(hasteMap)); } /** @@ -629,17 +619,16 @@ class HasteMap extends EventEmitter { } private _buildHasteMap(data: { - deprecatedFiles: Array<{moduleName: string; path: string}>; + removedFiles: FileData; hasteMap: InternalHasteMap; }): Promise { - const {deprecatedFiles, hasteMap} = data; + const {removedFiles, hasteMap} = data; const map = new Map(); const mocks = new Map(); const promises = []; - for (let i = 0; i < deprecatedFiles.length; ++i) { - const file = deprecatedFiles[i]; - this._recoverDuplicates(hasteMap, file.path, file.moduleName); + for (const [relativeFilePath, fileMetadata] of removedFiles) { + this._recoverDuplicates(hasteMap, relativeFilePath, fileMetadata[H.ID]); } for (const relativeFilePath of hasteMap.files.keys()) { @@ -712,11 +701,21 @@ class HasteMap extends EventEmitter { return this._worker; } - private _crawl(hasteMap: InternalHasteMap): Promise { + private _crawl(hasteMap: InternalHasteMap) { const options = this._options; const ignore = this._ignore.bind(this); const crawl = canUseWatchman && this._options.useWatchman ? watchmanCrawl : nodeCrawl; + const crawlerOptions: CrawlerOptions = { + computeSha1: options.computeSha1, + data: hasteMap, + extensions: options.extensions, + forceNodeFilesystemAPI: options.forceNodeFilesystemAPI, + ignore, + mapper: options.mapper, + rootDir: options.rootDir, + roots: options.roots, + }; const retry = (error: Error) => { if (crawl === watchmanCrawl) { @@ -729,16 +728,7 @@ class HasteMap extends EventEmitter { ` ` + error, ); - return nodeCrawl({ - computeSha1: options.computeSha1, - data: hasteMap, - extensions: options.extensions, - forceNodeFilesystemAPI: options.forceNodeFilesystemAPI, - ignore, - mapper: options.mapper, - rootDir: options.rootDir, - roots: options.roots, - }).catch(e => { + return nodeCrawl(crawlerOptions).catch(e => { throw new Error( `Crawler retry failed:\n` + ` Original error: ${error.message}\n` + @@ -751,15 +741,7 @@ class HasteMap extends EventEmitter { }; try { - return crawl({ - computeSha1: options.computeSha1, - data: hasteMap, - extensions: options.extensions, - forceNodeFilesystemAPI: options.forceNodeFilesystemAPI, - ignore, - rootDir: options.rootDir, - roots: options.roots, - }).catch(retry); + return crawl(crawlerOptions).catch(retry); } catch (error) { return retry(error); }