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

Co-authored-by: Dan Muller <mrmeku@stairwell.com>
jestjs · Apr 2, 2021 · 6d45caa · 6d45caa
1 parent bc818b5
commit 6d45caa
Show file tree

Hide file tree

Showing 12 changed files with 217 additions and 10 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -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))

diff --git a/docs/Configuration.md b/docs/Configuration.md
@@ -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;
 };
 ```

diff --git a/e2e/__tests__/crawlSymlinks.test.ts b/e2e/__tests__/crawlSymlinks.test.ts
@@ -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';
+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);
+});
diff --git a/packages/jest-config/src/ValidConfig.ts b/packages/jest-config/src/ValidConfig.ts
@@ -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,

diff --git a/packages/jest-config/src/__tests__/normalize.test.ts b/packages/jest-config/src/__tests__/normalize.test.ts
@@ -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);
+  });
+});
diff --git a/packages/jest-config/src/normalize.ts b/packages/jest-config/src/normalize.ts
@@ -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) => {
@@ -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}]`);
   });

diff --git a/packages/jest-haste-map/src/__tests__/index.test.js b/packages/jest-haste-map/src/__tests__/index.test.js
@@ -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.
@@ -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');

diff --git a/packages/jest-haste-map/src/crawlers/node.ts b/packages/jest-haste-map/src/crawlers/node.ts
@@ -60,6 +60,7 @@ function find(
   roots: Array<string>,
   extensions: Array<string>,
   ignore: IgnoreMatcher,
+  enableSymlinks: boolean,
   callback: Callback,
 ): void {
   const result: Result = [];
@@ -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
@@ -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('(');
   }
@@ -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) {
@@ -201,6 +211,7 @@ export = async function nodeCrawl(
     forceNodeFilesystemAPI,
     ignore,
     rootDir,
+    enableSymlinks,
     roots,
   } = options;
 
@@ -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);
     }
   });
 };
diff --git a/packages/jest-haste-map/src/index.ts b/packages/jest-haste-map/src/index.ts
@@ -56,6 +56,7 @@ type Options = {
   computeSha1?: boolean;
   console?: Console;
   dependencyExtractor?: string | null;
+  enableSymlinks?: boolean;
   extensions: Array<string>;
   forceNodeFilesystemAPI?: boolean;
   hasteImplModulePath?: string;
@@ -79,6 +80,7 @@ type InternalOptions = {
   computeDependencies: boolean;
   computeSha1: boolean;
   dependencyExtractor: string | null;
+  enableSymlinks: boolean;
   extensions: Array<string>;
   forceNodeFilesystemAPI: boolean;
   hasteImplModulePath?: string;
@@ -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,
@@ -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 = '';
@@ -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,

diff --git a/packages/jest-haste-map/src/types.ts b/packages/jest-haste-map/src/types.ts
@@ -30,6 +30,7 @@ export type WorkerMetadata = {
 
 export type CrawlerOptions = {
   computeSha1: boolean;
+  enableSymlinks: boolean;
   data: InternalHasteMap;
   extensions: Array<string>;
   forceNodeFilesystemAPI: boolean;

diff --git a/packages/jest-runtime/src/index.ts b/packages/jest-runtime/src/index.ts
@@ -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,

diff --git a/packages/jest-types/src/Config.ts b/packages/jest-types/src/Config.ts
@@ -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']. */