Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(jest-haste-map): Enable crawling for symlink test files #9351

Merged
merged 14 commits into from
Apr 2, 2021
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
- `[jest-environment-node]` Add AbortController to globals ([#11182](https://github.com/facebook/jest/pull/11182))
- `[@jest/fake-timers]` Update to `@sinonjs/fake-timers` to v7 ([#11198](https://github.com/facebook/jest/pull/11198))
- `[jest-haste-map]` Handle injected scm clocks ([#10966](https://github.com/facebook/jest/pull/10966))
- `[jest-haste-map]` Add `enableSymlinks` configuration option to follow symlinks for test files ([#9351](https://github.com/facebook/jest/pull/9351))
- `[jest-repl, jest-runner]` [**BREAKING**] Run transforms over environment ([#8751](https://github.com/facebook/jest/pull/8751))
- `[jest-runner]` [**BREAKING**] set exit code to 1 if test logs after teardown ([#10728](https://github.com/facebook/jest/pull/10728))
- `[jest-runner]` [**BREAKING**] Run transforms over `runnner` ([#8823](https://github.com/facebook/jest/pull/8823))
Expand Down
16 changes: 11 additions & 5 deletions docs/Configuration.md
Original file line number Diff line number Diff line change
Expand Up @@ -491,15 +491,21 @@ This will be used to configure the behavior of `jest-haste-map`, Jest's internal

```ts
type HasteConfig = {
// Whether to hash files using SHA-1.
/** Whether to hash files using SHA-1. */
computeSha1?: boolean;
// The platform to use as the default, e.g. 'ios'.
/** The platform to use as the default, e.g. 'ios'. */
defaultPlatform?: string | null;
// Path to a custom implementation of Haste.
/**
* Whether to follow symlinks when crawling for files.
* This options cannot be used in projects which use watchman.
* Projects with `watchman` set to true will error if this option is set to true.
*/
enableSymlinks?: boolean;
/** Path to a custom implementation of Haste. */
hasteImplModulePath?: string;
// All platforms to target, e.g ['ios', 'android'].
/** All platforms to target, e.g ['ios', 'android']. */
platforms?: Array<string>;
// Whether to throw on error on module collision.
/** Whether to throw on error on module collision. */
throwOnModuleCollision?: boolean;
};
```
Expand Down
84 changes: 84 additions & 0 deletions e2e/__tests__/crawlSymlinks.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,84 @@
/**
* 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 {tmpdir} from 'os';
import * as path from 'path';
SimenB marked this conversation as resolved.
Show resolved Hide resolved
import {wrap} from 'jest-snapshot-serializer-raw';
import {cleanup, writeFiles, writeSymlinks} from '../Utils';
import runJest from '../runJest';

const DIR = path.resolve(tmpdir(), 'crawl-symlinks-test');

beforeEach(() => {
cleanup(DIR);

writeFiles(DIR, {
'package.json': JSON.stringify({
jest: {
testMatch: ['<rootDir>/test-files/test.js'],
},
}),
'symlinked-files/test.js': `
test('1+1', () => {
expect(1).toBe(1);
});
`,
});

writeSymlinks(DIR, {
'symlinked-files/test.js': 'test-files/test.js',
});
});

afterEach(() => {
cleanup(DIR);
});

test('Node crawler picks up symlinked files when option is set as flag', () => {
// Symlinks are only enabled on windows with developer mode.
// https://blogs.windows.com/windowsdeveloper/2016/12/02/symlinks-windows-10/
if (process.platform === 'win32') {
return;
}

const {stdout, stderr, exitCode} = runJest(DIR, [
'--haste={"enableSymlinks": true}',
'--no-watchman',
]);

expect(stdout).toEqual('');
expect(stderr).toContain('Test Suites: 1 passed, 1 total');
expect(exitCode).toEqual(0);
});

test('Node crawler does not pick up symlinked files by default', () => {
const {stdout, stderr, exitCode} = runJest(DIR, ['--no-watchman']);
expect(stdout).toContain('No tests found, exiting with code 1');
expect(stderr).toEqual('');
expect(exitCode).toEqual(1);
});

test('Should throw if watchman used with haste.enableSymlinks', () => {
// it should throw both if watchman is explicitly provided and not
const run1 = runJest(DIR, ['--haste={"enableSymlinks": true}']);
const run2 = runJest(DIR, ['--haste={"enableSymlinks": true}', '--watchman']);

expect(run1.exitCode).toEqual(run2.exitCode);
expect(run1.stderr).toEqual(run2.stderr);
expect(run1.stdout).toEqual(run2.stdout);

const {exitCode, stderr, stdout} = run1;

expect(stdout).toEqual('');
expect(wrap(stderr)).toMatchInlineSnapshot(`
Validation Error:

haste.enableSymlinks is incompatible with watchman

Either set haste.enableSymlinks to false or do not use watchman
`);
expect(exitCode).toEqual(1);
});
1 change: 1 addition & 0 deletions packages/jest-config/src/ValidConfig.ts
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ const initialOptions: Config.InitialOptions = {
haste: {
computeSha1: true,
defaultPlatform: 'ios',
enableSymlinks: false,
hasteImplModulePath: '<rootDir>/haste_impl.js',
platforms: ['ios', 'android'],
throwOnModuleCollision: false,
Expand Down
23 changes: 23 additions & 0 deletions packages/jest-config/src/__tests__/normalize.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1842,3 +1842,26 @@ describe('extensionsToTreatAsEsm', () => {
);
});
});

describe('haste.enableSymlinks', () => {
it('should throw if watchman is not disabled', async () => {
await expect(
normalize({haste: {enableSymlinks: true}, rootDir: '/root/'}, {}),
).rejects.toThrow('haste.enableSymlinks is incompatible with watchman');

await expect(
normalize(
{haste: {enableSymlinks: true}, rootDir: '/root/', watchman: true},
{},
),
).rejects.toThrow('haste.enableSymlinks is incompatible with watchman');

const {options} = await normalize(
{haste: {enableSymlinks: true}, rootDir: '/root/', watchman: false},
{},
);

expect(options.haste.enableSymlinks).toBe(true);
expect(options.watchman).toBe(false);
});
});
12 changes: 12 additions & 0 deletions packages/jest-config/src/normalize.ts
Original file line number Diff line number Diff line change
Expand Up @@ -648,6 +648,10 @@ export default async function normalize(

validateExtensionsToTreatAsEsm(options.extensionsToTreatAsEsm);

if (options.watchman == null) {
options.watchman = DEFAULT_CONFIG.watchman;
}

const optionKeys = Object.keys(options) as Array<keyof Config.InitialOptions>;

optionKeys.reduce((newOptions, key: keyof Config.InitialOptions) => {
Expand Down Expand Up @@ -1023,6 +1027,14 @@ export default async function normalize(
return newOptions;
}, newOptions);

if (options.watchman && options.haste?.enableSymlinks) {
throw new ValidationError(
'Validation Error',
'haste.enableSymlinks is incompatible with watchman',
'Either set haste.enableSymlinks to false or do not use watchman',
);
}

newOptions.roots.forEach((root, i) => {
verifyDirectoryExists(root, `roots[${i}]`);
});
Expand Down
49 changes: 49 additions & 0 deletions packages/jest-haste-map/src/__tests__/index.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,19 @@ let mockChangedFiles;
let mockFs;

jest.mock('graceful-fs', () => ({
existsSync: jest.fn(path => {
// A file change can be triggered by writing into the
// mockChangedFiles object.
if (mockChangedFiles && path in mockChangedFiles) {
return true;
}

if (mockFs[path]) {
return true;
}

return false;
}),
readFileSync: jest.fn((path, options) => {
// A file change can be triggered by writing into the
// mockChangedFiles object.
Expand Down Expand Up @@ -494,6 +507,42 @@ describe('HasteMap', () => {
expect(useBuitinsInContext(hasteMap.read())).toEqual(data);
});

it('throws if both symlinks and watchman is enabled', () => {
expect(
() => new HasteMap({...defaultConfig, enableSymlinks: true}),
).toThrow(
'Set either `enableSymlinks` to false or `useWatchman` to false.',
);
expect(
() =>
new HasteMap({
...defaultConfig,
enableSymlinks: true,
useWatchman: true,
}),
).toThrow(
'Set either `enableSymlinks` to false or `useWatchman` to false.',
);

expect(
() =>
new HasteMap({
...defaultConfig,
enableSymlinks: false,
useWatchman: true,
}),
).not.toThrow();

expect(
() =>
new HasteMap({
...defaultConfig,
enableSymlinks: true,
useWatchman: false,
}),
).not.toThrow();
});

describe('builds a haste map on a fresh cache with SHA-1s', () => {
it.each([false, true])('uses watchman: %s', async useWatchman => {
const node = require('../crawlers/node');
Expand Down
21 changes: 16 additions & 5 deletions packages/jest-haste-map/src/crawlers/node.ts
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ function find(
roots: Array<string>,
extensions: Array<string>,
ignore: IgnoreMatcher,
enableSymlinks: boolean,
callback: Callback,
): void {
const result: Result = [];
Expand Down Expand Up @@ -98,7 +99,9 @@ function find(

activeCalls++;

fs.lstat(file, (err, stat) => {
const stat = enableSymlinks ? fs.stat : fs.lstat;

stat(file, (err, stat) => {
activeCalls--;

// This logic is unnecessary for node > v10.10, but leaving it in
Expand Down Expand Up @@ -137,10 +140,16 @@ function findNative(
roots: Array<string>,
extensions: Array<string>,
ignore: IgnoreMatcher,
enableSymlinks: boolean,
callback: Callback,
): void {
const args = Array.from(roots);
args.push('-type', 'f');
if (enableSymlinks) {
args.push('(', '-type', 'f', '-o', '-type', 'l', ')');
} else {
args.push('-type', 'f');
}

if (extensions.length) {
args.push('(');
}
Expand Down Expand Up @@ -177,7 +186,8 @@ function findNative(
} else {
lines.forEach(path => {
fs.stat(path, (err, stat) => {
if (!err && stat) {
// Filter out symlinks that describe directories
if (!err && stat && !stat.isDirectory()) {
result.push([path, stat.mtime.getTime(), stat.size]);
}
if (--count === 0) {
Expand All @@ -201,6 +211,7 @@ export = async function nodeCrawl(
forceNodeFilesystemAPI,
ignore,
rootDir,
enableSymlinks,
roots,
} = options;

Expand Down Expand Up @@ -231,9 +242,9 @@ export = async function nodeCrawl(
};

if (useNativeFind) {
findNative(roots, extensions, ignore, callback);
findNative(roots, extensions, ignore, enableSymlinks, callback);
} else {
find(roots, extensions, ignore, callback);
find(roots, extensions, ignore, enableSymlinks, callback);
}
});
};
12 changes: 12 additions & 0 deletions packages/jest-haste-map/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ type Options = {
computeSha1?: boolean;
console?: Console;
dependencyExtractor?: string | null;
enableSymlinks?: boolean;
extensions: Array<string>;
forceNodeFilesystemAPI?: boolean;
hasteImplModulePath?: string;
Expand All @@ -79,6 +80,7 @@ type InternalOptions = {
computeDependencies: boolean;
computeSha1: boolean;
dependencyExtractor: string | null;
enableSymlinks: boolean;
extensions: Array<string>;
forceNodeFilesystemAPI: boolean;
hasteImplModulePath?: string;
Expand Down Expand Up @@ -227,6 +229,7 @@ export default class HasteMap extends EventEmitter {
: options.computeDependencies,
computeSha1: options.computeSha1 || false,
dependencyExtractor: options.dependencyExtractor || null,
enableSymlinks: options.enableSymlinks || false,
extensions: options.extensions,
forceNodeFilesystemAPI: !!options.forceNodeFilesystemAPI,
hasteImplModulePath: options.hasteImplModulePath,
Expand Down Expand Up @@ -262,6 +265,14 @@ export default class HasteMap extends EventEmitter {
this._options.ignorePattern = new RegExp(VCS_DIRECTORIES);
}

if (this._options.enableSymlinks && this._options.useWatchman) {
throw new Error(
'jest-haste-map: enableSymlinks config option was set, but ' +
'is incompatible with watchman.\n' +
'Set either `enableSymlinks` to false or `useWatchman` to false.',
);
}

const rootDirHash = createHash('md5').update(options.rootDir).digest('hex');
let hasteImplHash = '';
let dependencyExtractorHash = '';
Expand Down Expand Up @@ -725,6 +736,7 @@ export default class HasteMap extends EventEmitter {
const crawlerOptions: CrawlerOptions = {
computeSha1: options.computeSha1,
data: hasteMap,
enableSymlinks: options.enableSymlinks,
extensions: options.extensions,
forceNodeFilesystemAPI: options.forceNodeFilesystemAPI,
ignore,
Expand Down
1 change: 1 addition & 0 deletions packages/jest-haste-map/src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ export type WorkerMetadata = {

export type CrawlerOptions = {
computeSha1: boolean;
enableSymlinks: boolean;
data: InternalHasteMap;
extensions: Array<string>;
forceNodeFilesystemAPI: boolean;
Expand Down
1 change: 1 addition & 0 deletions packages/jest-runtime/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -319,6 +319,7 @@ export default class Runtime {
computeSha1: config.haste.computeSha1,
console: options && options.console,
dependencyExtractor: config.dependencyExtractor,
enableSymlinks: config.haste.enableSymlinks,
extensions: [Snapshot.EXTENSION].concat(config.moduleFileExtensions),
hasteImplModulePath: config.haste.hasteImplModulePath,
ignorePattern,
Expand Down
6 changes: 6 additions & 0 deletions packages/jest-types/src/Config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,12 @@ export type HasteConfig = {
computeSha1?: boolean;
/** The platform to use as the default, e.g. 'ios'. */
defaultPlatform?: string | null;
/**
* Whether to follow symlinks when crawling for files.
* This options cannot be used in projects which use watchman.
* Projects with `watchman` set to true will error if this option is set to true.
*/
enableSymlinks?: boolean;
/** Path to a custom implementation of Haste. */
hasteImplModulePath?: string;
/** All platforms to target, e.g ['ios', 'android']. */
Expand Down