Detect memory leaks (#4895)

mjesun · cpojer · commit 3d2eb37e757c · 2017-11-27T09:01:51.000Z
* Make use of jest-leak-detector to detect tests leaking memory

* Addressed feedback
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -51,6 +51,8 @@
 
 ### Features
 
+* `[jest-runner]` Enable experimental detection of leaked contexts
+  ([#4895](https://github.com/facebook/jest/pull/4895))
 * `[jest-cli]` Add combined coverage threshold for directories.
   ([#4885](https://github.com/facebook/jest/pull/4885))
 * `[jest-mock]` Add `timestamps` to mock state.
diff --git a/integration_tests/__tests__/__snapshots__/show_config.test.js.snap b/integration_tests/__tests__/__snapshots__/show_config.test.js.snap
@@ -12,6 +12,7 @@ exports[`--showConfig outputs config info and exits 1`] = `
       \\"coveragePathIgnorePatterns\\": [
         \\"/node_modules/\\"
       ],
+      \\"detectLeaks\\": false,
       \\"globals\\": {},
       \\"haste\\": {
         \\"providesModuleNodeModules\\": []
@@ -66,6 +67,7 @@ exports[`--showConfig outputs config info and exits 1`] = `
       \\"lcov\\",
       \\"clover\\"
     ],
+    \\"detectLeaks\\": false,
     \\"expand\\": false,
     \\"listTests\\": false,
     \\"mapCoverage\\": false,
diff --git a/packages/jest-circus/src/legacy_code_todo_rewrite/jest_adapter_init.js b/packages/jest-circus/src/legacy_code_todo_rewrite/jest_adapter_init.js
@@ -129,6 +129,7 @@ export const runAndTransformResultsToJestFormat = async ({
     console: null,
     displayName: config.displayName,
     failureMessage,
+    leaks: false, // That's legacy code, just adding it so Flow is happy.
     numFailingTests,
     numPassingTests,
     numPendingTests,
diff --git a/packages/jest-cli/src/cli/args.js b/packages/jest-cli/src/cli/args.js
@@ -202,6 +202,14 @@ export const options = {
     description: 'Print debugging info about your jest config.',
     type: 'boolean',
   },
+  detectLeaks: {
+    default: false,
+    description:
+      '**EXPERIMENTAL**: Detect memory leaks in tests. After executing a ' +
+      'test, it will try to garbage collect the global object used, and fail ' +
+      'if it was leaked',
+    type: 'boolean',
+  },
   env: {
     description:
       'The test environment used for all tests. This can point to ' +
diff --git a/packages/jest-cli/src/test_result_helpers.js b/packages/jest-cli/src/test_result_helpers.js
@@ -54,6 +54,7 @@ export const buildFailureTestResult = (
     console: null,
     displayName: '',
     failureMessage: null,
+    leaks: false,
     numFailingTests: 0,
     numPassingTests: 0,
     numPendingTests: 0,
diff --git a/packages/jest-cli/src/test_scheduler.js b/packages/jest-cli/src/test_scheduler.js
@@ -12,6 +12,7 @@ import type {GlobalConfig, ReporterConfig} from 'types/Config';
 import type {Context} from 'types/Context';
 import type {Reporter, Test} from 'types/TestRunner';
 
+import chalk from 'chalk';
 import {formatExecError} from 'jest-message-util';
 import {
   addResult,
@@ -88,14 +89,33 @@ export default class TestScheduler {
       if (watcher.isInterrupted()) {
         return Promise.resolve();
       }
+
       if (testResult.testResults.length === 0) {
         const message = 'Your test suite must contain at least one test.';
-        await onFailure(test, {
+
+        return onFailure(test, {
           message,
           stack: new Error(message).stack,
         });
-        return Promise.resolve();
       }
+
+      // Throws when the context is leaked after executinga test.
+      if (testResult.leaks) {
+        const message =
+          chalk.red.bold('EXPERIMENTAL FEATURE!\n') +
+          'Your test suite is leaking memory. Please ensure all references are cleaned.\n' +
+          '\n' +
+          'There is a number of things that can leak memory:\n' +
+          '  - Async operations that have not finished (e.g. fs.readFile).\n' +
+          '  - Timers not properly mocked (e.g. setInterval, setTimeout).\n' +
+          '  - Keeping references to the global scope.';
+
+        return onFailure(test, {
+          message,
+          stack: new Error(message).stack,
+        });
+      }
+
       addResult(aggregatedResults, testResult);
       await this._dispatcher.onTestResult(test, testResult, aggregatedResults);
       return this._bailIfNeeded(contexts, aggregatedResults, watcher);
diff --git a/packages/jest-config/src/defaults.js b/packages/jest-config/src/defaults.js
@@ -36,6 +36,7 @@ export default ({
   clearMocks: false,
   coveragePathIgnorePatterns: [NODE_MODULES_REGEXP],
   coverageReporters: ['json', 'text', 'lcov', 'clover'],
+  detectLeaks: false,
   expand: false,
   globals: {},
   haste: {
diff --git a/packages/jest-config/src/index.js b/packages/jest-config/src/index.js
@@ -81,6 +81,7 @@ const getConfigs = (
       coverageDirectory: options.coverageDirectory,
       coverageReporters: options.coverageReporters,
       coverageThreshold: options.coverageThreshold,
+      detectLeaks: options.detectLeaks,
       expand: options.expand,
       findRelatedTests: options.findRelatedTests,
       forceExit: options.forceExit,
@@ -123,6 +124,7 @@ const getConfigs = (
       clearMocks: options.clearMocks,
       coveragePathIgnorePatterns: options.coveragePathIgnorePatterns,
       cwd: options.cwd,
+      detectLeaks: options.detectLeaks,
       displayName: options.displayName,
       globals: options.globals,
       haste: options.haste,
diff --git a/packages/jest-config/src/normalize.js b/packages/jest-config/src/normalize.js
@@ -455,6 +455,7 @@ export default function normalize(options: InitialOptions, argv: Argv) {
       case 'collectCoverage':
       case 'coverageReporters':
       case 'coverageThreshold':
+      case 'detectLeaks':
       case 'displayName':
       case 'expand':
       case 'globals':
diff --git a/packages/jest-runner/package.json b/packages/jest-runner/package.json
@@ -12,6 +12,7 @@
     "jest-docblock": "^21.2.0",
     "jest-haste-map": "^21.2.0",
     "jest-jasmine2": "^21.2.1",
+    "jest-leak-detector": "^21.2.1",
     "jest-message-util": "^21.2.1",
     "jest-runtime": "^21.2.1",
     "jest-util": "^21.2.1",
diff --git a/packages/jest-runner/src/run_test.js b/packages/jest-runner/src/run_test.js
@@ -23,29 +23,38 @@ import {
   setGlobal,
 } from 'jest-util';
 import jasmine2 from 'jest-jasmine2';
+import LeakDetector from 'jest-leak-detector';
 import {getTestEnvironment} from 'jest-config';
 import * as docblock from 'jest-docblock';
 
+type RunTestInternalResult = {
+  leakDetector: ?LeakDetector,
+  result: TestResult,
+};
+
 // The default jest-runner is required because it is the default test runner
 // and required implicitly through the `testRunner` ProjectConfig option.
 jasmine2;
 
-export default (async function runTest(
+// Keeping the core of "runTest" as a separate function (as "runTestInternal")
+// is key to be able to detect memory leaks. Since all variables are local to
+// the function, when "runTestInternal" finishes its execution, they can all be
+// freed, UNLESS something else is leaking them (and that's why we can detect
+// the leak!).
+//
+// If we had all the code in a single function, we should manually nullify all
+// references to verify if there is a leak, which is not maintainable and error
+// prone. That's why "runTestInternal" CANNOT be inlined inside "runTest".
+async function runTestInternal(
   path: Path,
   globalConfig: GlobalConfig,
   config: ProjectConfig,
   resolver: Resolver,
-) {
-  let testSource;
-
-  try {
-    testSource = fs.readFileSync(path, 'utf8');
-  } catch (e) {
-    return Promise.reject(e);
-  }
-
+): Promise<RunTestInternalResult> {
+  const testSource = fs.readFileSync(path, 'utf8');
   const parsedDocblock = docblock.parse(docblock.extract(testSource));
   const customEnvironment = parsedDocblock['jest-environment'];
+
   let testEnvironment = config.testEnvironment;
 
   if (customEnvironment) {
@@ -66,6 +75,10 @@ export default (async function runTest(
   >);
 
   const environment = new TestEnvironment(config);
+  const leakDetector = config.detectLeaks
+    ? new LeakDetector(environment)
+    : null;
+
   const consoleOut = globalConfig.useStderr ? process.stderr : process.stdout;
   const consoleFormatter = (type, message) =>
     getConsoleOutput(
@@ -76,24 +89,25 @@ export default (async function runTest(
     );
 
   let testConsole;
+
   if (globalConfig.silent) {
     testConsole = new NullConsole(consoleOut, process.stderr, consoleFormatter);
+  } else if (globalConfig.verbose) {
+    testConsole = new Console(consoleOut, process.stderr, consoleFormatter);
   } else {
-    if (globalConfig.verbose) {
-      testConsole = new Console(consoleOut, process.stderr, consoleFormatter);
-    } else {
-      testConsole = new BufferedConsole();
-    }
+    testConsole = new BufferedConsole();
   }
 
   const cacheFS = {[path]: testSource};
   setGlobal(environment.global, 'console', testConsole);
+
   const runtime = new Runtime(config, environment, resolver, cacheFS, {
     collectCoverage: globalConfig.collectCoverage,
     collectCoverageFrom: globalConfig.collectCoverageFrom,
     collectCoverageOnlyFrom: globalConfig.collectCoverageOnlyFrom,
     mapCoverage: globalConfig.mapCoverage,
   });
+
   const start = Date.now();
   await environment.setup();
   try {
@@ -106,22 +120,46 @@ export default (async function runTest(
     );
     const testCount =
       result.numPassingTests + result.numFailingTests + result.numPendingTests;
+
     result.perfStats = {end: Date.now(), start};
     result.testFilePath = path;
     result.coverage = runtime.getAllCoverageInfo();
     result.sourceMaps = runtime.getSourceMapInfo();
     result.console = testConsole.getBuffer();
     result.skipped = testCount === result.numPendingTests;
     result.displayName = config.displayName;
+
     if (globalConfig.logHeapUsage) {
       if (global.gc) {
         global.gc();
       }
       result.memoryUsage = process.memoryUsage().heapUsed;
     }
+
     // Delay the resolution to allow log messages to be output.
-    return new Promise(resolve => setImmediate(() => resolve(result)));
+    return new Promise(resolve => {
+      setImmediate(() => resolve({leakDetector, result}));
+    });
   } finally {
     await environment.teardown();
   }
-});
+}
+
+export default async function runTest(
+  path: Path,
+  globalConfig: GlobalConfig,
+  config: ProjectConfig,
+  resolver: Resolver,
+): Promise<TestResult> {
+  const {leakDetector, result} = await runTestInternal(
+    path,
+    globalConfig,
+    config,
+    resolver,
+  );
+
+  // Resolve leak detector, outside the "runTestInternal" closure.
+  result.leaks = leakDetector ? leakDetector.isLeaking() : false;
+
+  return result;
+}
diff --git a/test_utils.js b/test_utils.js
@@ -20,6 +20,7 @@ const DEFAULT_GLOBAL_CONFIG: GlobalConfig = {
   coverageDirectory: 'coverage',
   coverageReporters: [],
   coverageThreshold: {global: {}},
+  detectLeaks: false,
   expand: false,
   findRelatedTests: false,
   forceExit: false,
@@ -63,6 +64,7 @@ const DEFAULT_PROJECT_CONFIG: ProjectConfig = {
   clearMocks: false,
   coveragePathIgnorePatterns: [],
   cwd: '/test_root_dir/',
+  detectLeaks: false,
   displayName: undefined,
   globals: {},
   haste: {
diff --git a/types/Config.js b/types/Config.js
@@ -33,6 +33,7 @@ export type DefaultOptions = {|
   expand: boolean,
   globals: ConfigGlobals,
   haste: HasteConfig,
+  detectLeaks: boolean,
   mapCoverage: boolean,
   moduleDirectories: Array<string>,
   moduleFileExtensions: Array<string>,
@@ -78,6 +79,7 @@ export type InitialOptions = {
   coveragePathIgnorePatterns?: Array<string>,
   coverageReporters?: Array<string>,
   coverageThreshold?: {global: {[key: string]: number}},
+  detectLeaks?: boolean,
   displayName?: string,
   expand?: boolean,
   findRelatedTests?: boolean,
@@ -154,6 +156,7 @@ export type GlobalConfig = {|
   coverageDirectory: string,
   coverageReporters: Array<string>,
   coverageThreshold: {global: {[key: string]: number}},
+  detectLeaks: boolean,
   expand: boolean,
   findRelatedTests: boolean,
   forceExit: boolean,
@@ -197,6 +200,7 @@ export type ProjectConfig = {|
   clearMocks: boolean,
   coveragePathIgnorePatterns: Array<string>,
   cwd: Path,
+  detectLeaks: boolean,
   displayName: ?string,
   globals: ConfigGlobals,
   haste: HasteConfig,
diff --git a/types/TestResult.js b/types/TestResult.js
@@ -138,8 +138,9 @@ export type TestResult = {|
   console: ?ConsoleBuffer,
   coverage?: RawCoverage,
   displayName: ?string,
-  memoryUsage?: Bytes,
   failureMessage: ?string,
+  leaks: boolean,
+  memoryUsage?: Bytes,
   numFailingTests: number,
   numPassingTests: number,
   numPendingTests: number,
