From defbc0535a46119d4682f97bf8a13b5562c1445b Mon Sep 17 00:00:00 2001 From: Rob Hogan <2590098+robhogan@users.noreply.github.com> Date: Sat, 16 Apr 2022 14:01:25 +0100 Subject: [PATCH] fix(jest-haste-map): Make watchman existence check lazy+async (#12675) --- CHANGELOG.md | 1 + .../src/__tests__/index.test.js | 14 +++++-- packages/jest-haste-map/src/index.ts | 39 ++++++++++--------- .../lib/__tests__/isWatchmanInstalled.test.js | 37 ++++++++++++++++++ .../src/lib/isWatchmanInstalled.ts | 18 +++++++++ 5 files changed, 87 insertions(+), 22 deletions(-) create mode 100644 packages/jest-haste-map/src/lib/__tests__/isWatchmanInstalled.test.js create mode 100644 packages/jest-haste-map/src/lib/isWatchmanInstalled.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index a99f215c5391..488c1f0e6e1c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -74,6 +74,7 @@ - `[jest-environment-node]` Add `structuredClone` to globals ([#12631](https://github.com/facebook/jest/pull/12631)) - `[@jest/expect-utils]` [**BREAKING**] Fix false positives when looking for `undefined` prop ([#8923](https://github.com/facebook/jest/pull/8923)) - `[jest-haste-map]` Don't use partial results if file crawl errors ([#12420](https://github.com/facebook/jest/pull/12420)) +- `[jest-haste-map]` Make watchman existence check lazy+async ([#12675](https://github.com/facebook/jest/pull/12675)) - `[jest-jasmine2, jest-types]` [**BREAKING**] Move all `jasmine` specific types from `@jest/types` to its own package ([#12125](https://github.com/facebook/jest/pull/12125)) - `[jest-jasmine2]` Do not set `duration` to `0` for skipped tests ([#12518](https://github.com/facebook/jest/pull/12518)) - `[jest-matcher-utils]` Pass maxWidth to `pretty-format` to avoid printing every element in arrays by default ([#12402](https://github.com/facebook/jest/pull/12402)) diff --git a/packages/jest-haste-map/src/__tests__/index.test.js b/packages/jest-haste-map/src/__tests__/index.test.js index 7aeb7fec76f4..a3b2c6efc923 100644 --- a/packages/jest-haste-map/src/__tests__/index.test.js +++ b/packages/jest-haste-map/src/__tests__/index.test.js @@ -13,9 +13,11 @@ function mockHashContents(contents) { return crypto.createHash('sha1').update(contents).digest('hex'); } -jest.mock('child_process', () => ({ - // If this does not throw, we'll use the (mocked) watchman crawler - execSync() {}, +const mockIsWatchmanInstalled = jest.fn().mockResolvedValue(true); + +jest.mock('../lib/isWatchmanInstalled', () => ({ + __esModule: true, + default: mockIsWatchmanInstalled, })); jest.mock('jest-worker', () => ({ @@ -612,6 +614,8 @@ describe('HasteMap', () => { }); }); + mockIsWatchmanInstalled.mockClear(); + const hasteMap = await HasteMap.create({ ...defaultConfig, computeSha1: true, @@ -621,6 +625,10 @@ describe('HasteMap', () => { const data = (await hasteMap.build()).__hasteMapForTest; + expect(mockIsWatchmanInstalled).toHaveBeenCalledTimes( + useWatchman ? 1 : 0, + ); + expect(data.files).toEqual( createMap({ [path.join('fruits', 'Banana.js')]: [ diff --git a/packages/jest-haste-map/src/index.ts b/packages/jest-haste-map/src/index.ts index 76d16bcfde5e..4254eb2c52ad 100644 --- a/packages/jest-haste-map/src/index.ts +++ b/packages/jest-haste-map/src/index.ts @@ -7,7 +7,6 @@ /* eslint-disable local/ban-types-eventually */ -import {execSync} from 'child_process'; import {createHash} from 'crypto'; import {EventEmitter} from 'events'; import {tmpdir} from 'os'; @@ -26,6 +25,7 @@ import {watchmanCrawl} from './crawlers/watchman'; import getMockName from './getMockName'; import * as fastPath from './lib/fast_path'; import getPlatformExtension from './lib/getPlatformExtension'; +import isWatchmanInstalled from './lib/isWatchmanInstalled'; import normalizePathSep from './lib/normalizePathSep'; import type { ChangeEvent, @@ -124,14 +124,6 @@ const VCS_DIRECTORIES = ['.git', '.hg'] .map(vcs => escapePathForRegex(path.sep + vcs + path.sep)) .join('|'); -const canUseWatchman = ((): boolean => { - try { - execSync('watchman --version', {stdio: ['ignore']}); - return true; - } catch {} - return false; -})(); - function invariant(condition: unknown, message?: string): asserts condition { if (!condition) { throw new Error(message); @@ -221,6 +213,7 @@ export default class HasteMap extends EventEmitter { private _cachePath: string; private _changeInterval?: ReturnType; private _console: Console; + private _isWatchmanInstalledPromise: Promise | null = null; private _options: InternalOptions; private _watchers: Array; private _worker: WorkerInterface | null; @@ -763,11 +756,10 @@ export default class HasteMap extends EventEmitter { return this._worker; } - private _crawl(hasteMap: InternalHasteMap) { + private async _crawl(hasteMap: InternalHasteMap) { const options = this._options; const ignore = this._ignore.bind(this); - const crawl = - canUseWatchman && this._options.useWatchman ? watchmanCrawl : nodeCrawl; + const crawl = (await this._shouldUseWatchman()) ? watchmanCrawl : nodeCrawl; const crawlerOptions: CrawlerOptions = { computeSha1: options.computeSha1, data: hasteMap, @@ -811,7 +803,7 @@ export default class HasteMap extends EventEmitter { /** * Watch mode */ - private _watch(hasteMap: InternalHasteMap): Promise { + private async _watch(hasteMap: InternalHasteMap): Promise { if (!this._options.watch) { return Promise.resolve(); } @@ -822,12 +814,11 @@ export default class HasteMap extends EventEmitter { this._options.retainAllFiles = true; // WatchmanWatcher > FSEventsWatcher > sane.NodeWatcher - const Watcher = - canUseWatchman && this._options.useWatchman - ? WatchmanWatcher - : FSEventsWatcher.isSupported() - ? FSEventsWatcher - : NodeWatcher; + const Watcher = (await this._shouldUseWatchman()) + ? WatchmanWatcher + : FSEventsWatcher.isSupported() + ? FSEventsWatcher + : NodeWatcher; const extensions = this._options.extensions; const ignorePattern = this._options.ignorePattern; @@ -1110,6 +1101,16 @@ export default class HasteMap extends EventEmitter { ); } + private async _shouldUseWatchman(): Promise { + if (!this._options.useWatchman) { + return false; + } + if (!this._isWatchmanInstalledPromise) { + this._isWatchmanInstalledPromise = isWatchmanInstalled(); + } + return this._isWatchmanInstalledPromise; + } + private _createEmptyMap(): InternalHasteMap { return { clocks: new Map(), diff --git a/packages/jest-haste-map/src/lib/__tests__/isWatchmanInstalled.test.js b/packages/jest-haste-map/src/lib/__tests__/isWatchmanInstalled.test.js new file mode 100644 index 000000000000..e2f9d35b3800 --- /dev/null +++ b/packages/jest-haste-map/src/lib/__tests__/isWatchmanInstalled.test.js @@ -0,0 +1,37 @@ +/** + * 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 {execFile} from 'child_process'; +import isWatchmanInstalled from '../isWatchmanInstalled'; + +jest.mock('child_process'); + +describe('isWatchmanInstalled', () => { + beforeEach(() => jest.clearAllMocks()); + + it('executes watchman --version and returns true on success', async () => { + execFile.mockImplementation((file, args, cb) => { + expect(file).toBe('watchman'); + expect(args).toStrictEqual(['--version']); + cb(null, {stdout: 'v123'}); + }); + expect(await isWatchmanInstalled()).toBe(true); + expect(execFile).toHaveBeenCalledWith( + 'watchman', + ['--version'], + expect.any(Function), + ); + }); + + it('returns false when execFile fails', async () => { + execFile.mockImplementation((file, args, cb) => { + cb(new Error()); + }); + expect(await isWatchmanInstalled()).toBe(false); + expect(execFile).toHaveBeenCalled(); + }); +}); diff --git a/packages/jest-haste-map/src/lib/isWatchmanInstalled.ts b/packages/jest-haste-map/src/lib/isWatchmanInstalled.ts new file mode 100644 index 000000000000..8dee697d61f4 --- /dev/null +++ b/packages/jest-haste-map/src/lib/isWatchmanInstalled.ts @@ -0,0 +1,18 @@ +/** + * 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 {execFile} from 'child_process'; +import {promisify} from 'util'; + +export default async function isWatchmanInstalled(): Promise { + try { + await promisify(execFile)('watchman', ['--version']); + return true; + } catch { + return false; + } +}