From 92e3dd7b72caf0f50cd756f31648336a8d2d2f7d Mon Sep 17 00:00:00 2001 From: Richard Lau Date: Thu, 23 Jul 2020 14:54:55 -0400 Subject: [PATCH 1/4] Check find binary supports the `-iname` parameter The `-iname` parameter is a non-POSIX extension and may not be supported by a POSIX compliant find binary. Add a check that the parameter is supported when determining whether to use the native find binary. --- CHANGELOG.md | 1 + .../src/crawlers/__tests__/node.test.js | 39 +++++++++++++++++++ packages/jest-haste-map/src/crawlers/node.ts | 9 ++++- 3 files changed, 48 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1d58d366357f..8e9f27c0ea84 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,7 @@ - `[expect]` Match symbols and bigints in `any()` ([#10223](https://github.com/facebook/jest/pull/10223)) - `[jest-changed-files]` Use `git diff` instead of `git log` for `--changedSince` ([#10155](https://github.com/facebook/jest/pull/10155)) - `[jest-console]` Add missing console.timeLog for compatability with Node ([#10209](https://github.com/facebook/jest/pull/10209)) +- `[jest-haste-map]` Check find binary supports the `-iname` parameter ([#10308](https://github.com/facebook/jest/pull/10308)) - `[jest-snapshot]` Strip added indentation for inline error snapshots ([#10217](https://github.com/facebook/jest/pull/10217)) ### Chore & Maintenance 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 c972f0da6dd1..ab3b894d796e 100644 --- a/packages/jest-haste-map/src/crawlers/__tests__/node.test.js +++ b/packages/jest-haste-map/src/crawlers/__tests__/node.test.js @@ -14,6 +14,11 @@ jest.mock('child_process', () => ({ spawn: jest.fn((cmd, args) => { let closeCallback; return { + on: jest.fn().mockImplementation((event, callback) => { + if (event === 'exit') { + callback(mockSpawnExit, null); + } + }), stdout: { on: jest.fn().mockImplementation((event, callback) => { if (event === 'data') { @@ -131,6 +136,7 @@ const createMap = obj => new Map(Object.keys(obj).map(key => [key, obj[key]])); const rootDir = '/project'; let mockResponse; +let mockSpawnExit; let nodeCrawl; let childProcess; @@ -148,6 +154,8 @@ describe('node crawler', () => { '/project/fruits/strawberry.js', '/project/fruits/tomato.js', ].join('\n'); + + mockSpawnExit = 0; }); it('crawls for files based on patterns', () => { @@ -293,6 +301,37 @@ describe('node crawler', () => { }); }); + it('uses node fs APIs on Unix based OS with incompatible find binary', () => { + process.platform = 'linux'; + mockResponse = ''; + mockSpawnExit = 1; + childProcess = require('child_process'); + const which = require('which'); + which.mockReturnValueOnce(Promise.resolve('/mypath/find')); + + nodeCrawl = require('../node'); + + return nodeCrawl({ + data: { + files: new Map(), + }, + extensions: ['js'], + ignore: pearMatcher, + rootDir, + roots: ['/project/fruits'], + }).then(({hasteMap, removedFiles}) => { + expect(childProcess.spawn).lastCalledWith('find', ['.', '-iname', "''"]); + 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()); + expect(which).toBeCalledWith('find'); + }); + }); + it('uses node fs APIs on Unix based OS without find binary', () => { process.platform = 'linux'; const which = require('which'); diff --git a/packages/jest-haste-map/src/crawlers/node.ts b/packages/jest-haste-map/src/crawlers/node.ts index 0266816d91fb..456ad922448e 100644 --- a/packages/jest-haste-map/src/crawlers/node.ts +++ b/packages/jest-haste-map/src/crawlers/node.ts @@ -31,7 +31,14 @@ async function hasNativeFindSupport( try { await which('find'); - return true; + return await new Promise(resolve => { + // Check the find binary supports the non-POSIX -iname parameter. + const args = ['.', '-iname', "''"]; + const child = spawn('find', args); + child.on('exit', code => { + resolve(code == 0); + }); + }); } catch { return false; } From b3b6cddeb43afe852244c0e5211d62cf2f1839fb Mon Sep 17 00:00:00 2001 From: Richard Lau Date: Fri, 24 Jul 2020 09:00:39 -0400 Subject: [PATCH 2/4] Remove use of which module --- packages/jest-haste-map/package.json | 6 ++---- .../src/crawlers/__tests__/node.test.js | 14 ++++++-------- packages/jest-haste-map/src/crawlers/node.ts | 7 ++++--- yarn.lock | 2 -- 4 files changed, 12 insertions(+), 17 deletions(-) diff --git a/packages/jest-haste-map/package.json b/packages/jest-haste-map/package.json index 76e34ab3ea23..f5cb365aec48 100644 --- a/packages/jest-haste-map/package.json +++ b/packages/jest-haste-map/package.json @@ -21,16 +21,14 @@ "jest-worker": "^26.1.0", "micromatch": "^4.0.2", "sane": "^4.0.3", - "walker": "^1.0.7", - "which": "^2.0.2" + "walker": "^1.0.7" }, "devDependencies": { "@jest/test-utils": "^26.0.0", "@types/anymatch": "^1.3.1", "@types/fb-watchman": "^2.0.0", "@types/micromatch": "^4.0.0", - "@types/sane": "^2.0.0", - "@types/which": "^1.3.2" + "@types/sane": "^2.0.0" }, "optionalDependencies": { "fsevents": "^2.1.2" 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 ab3b894d796e..903d1984fd97 100644 --- a/packages/jest-haste-map/src/crawlers/__tests__/node.test.js +++ b/packages/jest-haste-map/src/crawlers/__tests__/node.test.js @@ -129,8 +129,6 @@ jest.mock('graceful-fs', () => { }; }); -jest.mock('which', () => jest.fn().mockResolvedValue()); - const pearMatcher = path => /pear/.test(path); const createMap = obj => new Map(Object.keys(obj).map(key => [key, obj[key]])); @@ -306,8 +304,6 @@ describe('node crawler', () => { mockResponse = ''; mockSpawnExit = 1; childProcess = require('child_process'); - const which = require('which'); - which.mockReturnValueOnce(Promise.resolve('/mypath/find')); nodeCrawl = require('../node'); @@ -328,15 +324,16 @@ describe('node crawler', () => { }), ); expect(removedFiles).toEqual(new Map()); - expect(which).toBeCalledWith('find'); }); }); it('uses node fs APIs on Unix based OS without find binary', () => { process.platform = 'linux'; - const which = require('which'); - which.mockReturnValueOnce(Promise.reject()); + childProcess = require('child_process'); + childProcess.spawn.mockImplementationOnce(() => { + throw new Error(); + }); nodeCrawl = require('../node'); return nodeCrawl({ @@ -355,13 +352,13 @@ describe('node crawler', () => { }), ); expect(removedFiles).toEqual(new Map()); - expect(which).toBeCalledWith('find'); }); }); it('uses node fs APIs if "forceNodeFilesystemAPI" is set to true, regardless of platform', () => { process.platform = 'linux'; + childProcess = require('child_process'); nodeCrawl = require('../node'); const files = new Map(); @@ -373,6 +370,7 @@ describe('node crawler', () => { rootDir, roots: ['/project/fruits'], }).then(({hasteMap, removedFiles}) => { + expect(childProcess.spawn).toHaveBeenCalledTimes(0); expect(hasteMap.files).toEqual( createMap({ 'fruits/directory/strawberry.js': ['', 33, 42, 0, '', null], diff --git a/packages/jest-haste-map/src/crawlers/node.ts b/packages/jest-haste-map/src/crawlers/node.ts index 456ad922448e..f34417284d23 100644 --- a/packages/jest-haste-map/src/crawlers/node.ts +++ b/packages/jest-haste-map/src/crawlers/node.ts @@ -8,7 +8,6 @@ import * as path from 'path'; import {spawn} from 'child_process'; import * as fs from 'graceful-fs'; -import which = require('which'); import H from '../constants'; import * as fastPath from '../lib/fast_path'; import type { @@ -30,13 +29,15 @@ async function hasNativeFindSupport( } try { - await which('find'); return await new Promise(resolve => { // Check the find binary supports the non-POSIX -iname parameter. const args = ['.', '-iname', "''"]; const child = spawn('find', args); + child.on('error', () => { + resolve(false); + }); child.on('exit', code => { - resolve(code == 0); + resolve(code === 0); }); }); } catch { diff --git a/yarn.lock b/yarn.lock index 25e98d817b6d..6e44474780a7 100644 --- a/yarn.lock +++ b/yarn.lock @@ -11208,7 +11208,6 @@ fsevents@^1.2.7: "@types/micromatch": ^4.0.0 "@types/node": "*" "@types/sane": ^2.0.0 - "@types/which": ^1.3.2 anymatch: ^3.0.3 fb-watchman: ^2.0.0 fsevents: ^2.1.2 @@ -11219,7 +11218,6 @@ fsevents@^1.2.7: micromatch: ^4.0.2 sane: ^4.0.3 walker: ^1.0.7 - which: ^2.0.2 dependenciesMeta: fsevents: optional: true From 6e1dd006a1956cdc36cc1b5c84b226ff2859557c Mon Sep 17 00:00:00 2001 From: Richard Lau Date: Fri, 24 Jul 2020 16:17:08 +0100 Subject: [PATCH 3/4] Allow Windows to use compatible native find binary Enable detection of a compatible find binary on Windows. Fix tests so that they run and pass on Windows. --- packages/jest-haste-map/package.json | 3 +- .../src/crawlers/__tests__/node.test.js | 75 +++++-------------- packages/jest-haste-map/src/crawlers/node.ts | 2 +- yarn.lock | 1 + 4 files changed, 21 insertions(+), 60 deletions(-) diff --git a/packages/jest-haste-map/package.json b/packages/jest-haste-map/package.json index f5cb365aec48..1ffbf315b3f7 100644 --- a/packages/jest-haste-map/package.json +++ b/packages/jest-haste-map/package.json @@ -28,7 +28,8 @@ "@types/anymatch": "^1.3.1", "@types/fb-watchman": "^2.0.0", "@types/micromatch": "^4.0.0", - "@types/sane": "^2.0.0" + "@types/sane": "^2.0.0", + "slash": "^3.0.0" }, "optionalDependencies": { "fsevents": "^2.1.2" 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 903d1984fd97..467c0acb9195 100644 --- a/packages/jest-haste-map/src/crawlers/__tests__/node.test.js +++ b/packages/jest-haste-map/src/crawlers/__tests__/node.test.js @@ -8,8 +8,6 @@ 'use strict'; -import {skipSuiteOnWindows} from '@jest/test-utils'; - jest.mock('child_process', () => ({ spawn: jest.fn((cmd, args) => { let closeCallback; @@ -39,6 +37,7 @@ jest.mock('child_process', () => ({ let mockHasReaddirWithFileTypesSupport = false; jest.mock('graceful-fs', () => { + const slash = require('slash'); let mtime = 32; const size = 42; const stat = (path, callback) => { @@ -46,10 +45,10 @@ jest.mock('graceful-fs', () => { () => callback(null, { isDirectory() { - return path.endsWith('/directory'); + return slash(path).endsWith('/directory'); }, isSymbolicLink() { - return path.endsWith('symlink'); + return slash(path).endsWith('symlink'); }, mtime: { getTime() { @@ -75,7 +74,7 @@ jest.mock('graceful-fs', () => { } if (mockHasReaddirWithFileTypesSupport) { - if (dir === '/project/fruits') { + if (slash(dir) === '/project/fruits') { setTimeout( () => callback(null, [ @@ -97,7 +96,7 @@ jest.mock('graceful-fs', () => { ]), 0, ); - } else if (dir === '/project/fruits/directory') { + } else if (slash(dir) === '/project/fruits/directory') { setTimeout( () => callback(null, [ @@ -109,18 +108,18 @@ jest.mock('graceful-fs', () => { ]), 0, ); - } else if (dir == '/error') { + } else if (slash(dir) == '/error') { setTimeout(() => callback({code: 'ENOTDIR'}, undefined), 0); } } else { - if (dir === '/project/fruits') { + if (slash(dir) === '/project/fruits') { setTimeout( () => callback(null, ['directory', 'tomato.js', 'symlink']), 0, ); - } else if (dir === '/project/fruits/directory') { + } else if (slash(dir) === '/project/fruits/directory') { setTimeout(() => callback(null, ['strawberry.js']), 0); - } else if (dir == '/error') { + } else if (slash(dir) == '/error') { setTimeout(() => callback({code: 'ENOTDIR'}, undefined), 0); } } @@ -130,7 +129,10 @@ jest.mock('graceful-fs', () => { }); const pearMatcher = path => /pear/.test(path); -const createMap = obj => new Map(Object.keys(obj).map(key => [key, obj[key]])); +const normalize = path => + process.platform === 'win32' ? path.replace(/\//g, '\\') : path; +const createMap = obj => + new Map(Object.keys(obj).map(key => [normalize(key), obj[key]])); const rootDir = '/project'; let mockResponse; @@ -139,14 +141,9 @@ let nodeCrawl; let childProcess; describe('node crawler', () => { - skipSuiteOnWindows(); - beforeEach(() => { jest.resetModules(); - // Remove the "process.platform" property descriptor so it can be writable. - delete process.platform; - mockResponse = [ '/project/fruits/pear.js', '/project/fruits/strawberry.js', @@ -157,8 +154,6 @@ describe('node crawler', () => { }); it('crawls for files based on patterns', () => { - process.platform = 'linux'; - childProcess = require('child_process'); nodeCrawl = require('../node'); @@ -209,8 +204,6 @@ describe('node crawler', () => { }); it('updates only changed files', () => { - process.platform = 'linux'; - nodeCrawl = require('../node'); // In this test sample, strawberry is changed and tomato is unchanged @@ -235,15 +228,13 @@ describe('node crawler', () => { ); // Make sure it is the *same* unchanged object. - expect(hasteMap.files.get('fruits/tomato.js')).toBe(tomato); + expect(hasteMap.files.get(normalize('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 @@ -275,32 +266,7 @@ describe('node crawler', () => { }); }); - it('uses node fs APIs on windows', () => { - process.platform = 'win32'; - - nodeCrawl = require('../node'); - - return nodeCrawl({ - data: { - files: new Map(), - }, - extensions: ['js'], - ignore: pearMatcher, - rootDir, - roots: ['/project/fruits'], - }).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()); - }); - }); - - it('uses node fs APIs on Unix based OS with incompatible find binary', () => { - process.platform = 'linux'; + it('uses node fs APIs with incompatible find binary', () => { mockResponse = ''; mockSpawnExit = 1; childProcess = require('child_process'); @@ -327,9 +293,7 @@ describe('node crawler', () => { }); }); - it('uses node fs APIs on Unix based OS without find binary', () => { - process.platform = 'linux'; - + it('uses node fs APIs without find binary', () => { childProcess = require('child_process'); childProcess.spawn.mockImplementationOnce(() => { throw new Error(); @@ -356,8 +320,6 @@ describe('node crawler', () => { }); it('uses node fs APIs if "forceNodeFilesystemAPI" is set to true, regardless of platform', () => { - process.platform = 'linux'; - childProcess = require('child_process'); nodeCrawl = require('../node'); @@ -382,8 +344,6 @@ describe('node crawler', () => { }); it('completes with empty roots', () => { - process.platform = 'win32'; - nodeCrawl = require('../node'); const files = new Map(); @@ -401,14 +361,13 @@ describe('node crawler', () => { }); it('completes with fs.readdir throwing an error', () => { - process.platform = 'win32'; - nodeCrawl = require('../node'); const files = new Map(); return nodeCrawl({ data: {files}, extensions: ['js'], + forceNodeFilesystemAPI: true, ignore: pearMatcher, rootDir, roots: ['/error'], diff --git a/packages/jest-haste-map/src/crawlers/node.ts b/packages/jest-haste-map/src/crawlers/node.ts index f34417284d23..be6bcae43b05 100644 --- a/packages/jest-haste-map/src/crawlers/node.ts +++ b/packages/jest-haste-map/src/crawlers/node.ts @@ -24,7 +24,7 @@ type Callback = (result: Result) => void; async function hasNativeFindSupport( forceNodeFilesystemAPI: boolean, ): Promise { - if (forceNodeFilesystemAPI || process.platform === 'win32') { + if (forceNodeFilesystemAPI) { return false; } diff --git a/yarn.lock b/yarn.lock index 6e44474780a7..4569c6275937 100644 --- a/yarn.lock +++ b/yarn.lock @@ -11217,6 +11217,7 @@ fsevents@^1.2.7: jest-worker: ^26.1.0 micromatch: ^4.0.2 sane: ^4.0.3 + slash: ^3.0.0 walker: ^1.0.7 dependenciesMeta: fsevents: From 41436be27aef1b5968d5360062e5599d206f16d8 Mon Sep 17 00:00:00 2001 From: Simen Bekkhus Date: Fri, 24 Jul 2020 18:48:29 +0200 Subject: [PATCH 4/4] Update CHANGELOG.md --- CHANGELOG.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8e9f27c0ea84..7dc5d3e317b7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,8 +9,8 @@ - `[expect]` Match symbols and bigints in `any()` ([#10223](https://github.com/facebook/jest/pull/10223)) - `[jest-changed-files]` Use `git diff` instead of `git log` for `--changedSince` ([#10155](https://github.com/facebook/jest/pull/10155)) -- `[jest-console]` Add missing console.timeLog for compatability with Node ([#10209](https://github.com/facebook/jest/pull/10209)) -- `[jest-haste-map]` Check find binary supports the `-iname` parameter ([#10308](https://github.com/facebook/jest/pull/10308)) +- `[jest-console]` Add missing `console.timeLog` for compatibility with Node ([#10209](https://github.com/facebook/jest/pull/10209)) +- `[jest-haste-map]` Check `find` binary supports the `-iname` parameter ([#10308](https://github.com/facebook/jest/pull/10308)) - `[jest-snapshot]` Strip added indentation for inline error snapshots ([#10217](https://github.com/facebook/jest/pull/10217)) ### Chore & Maintenance