Skip to content

Commit

Permalink
Detect memory leaks (#4895)
Browse files Browse the repository at this point in the history
* Make use of jest-leak-detector to detect tests leaking memory

* Addressed feedback
  • Loading branch information
mjesun authored and cpojer committed Nov 27, 2017
1 parent e5f58a6 commit 3d2eb37
Show file tree
Hide file tree
Showing 14 changed files with 104 additions and 20 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ exports[`--showConfig outputs config info and exits 1`] = `
\\"coveragePathIgnorePatterns\\": [
\\"/node_modules/\\"
],
\\"detectLeaks\\": false,
\\"globals\\": {},
\\"haste\\": {
\\"providesModuleNodeModules\\": []
Expand Down Expand Up @@ -66,6 +67,7 @@ exports[`--showConfig outputs config info and exits 1`] = `
\\"lcov\\",
\\"clover\\"
],
\\"detectLeaks\\": false,
\\"expand\\": false,
\\"listTests\\": false,
\\"mapCoverage\\": false,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
8 changes: 8 additions & 0 deletions packages/jest-cli/src/cli/args.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 ' +
Expand Down
1 change: 1 addition & 0 deletions packages/jest-cli/src/test_result_helpers.js
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ export const buildFailureTestResult = (
console: null,
displayName: '',
failureMessage: null,
leaks: false,
numFailingTests: 0,
numPassingTests: 0,
numPendingTests: 0,
Expand Down
24 changes: 22 additions & 2 deletions packages/jest-cli/src/test_scheduler.js
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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);
Expand Down
1 change: 1 addition & 0 deletions packages/jest-config/src/defaults.js
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ export default ({
clearMocks: false,
coveragePathIgnorePatterns: [NODE_MODULES_REGEXP],
coverageReporters: ['json', 'text', 'lcov', 'clover'],
detectLeaks: false,
expand: false,
globals: {},
haste: {
Expand Down
2 changes: 2 additions & 0 deletions packages/jest-config/src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down
1 change: 1 addition & 0 deletions packages/jest-config/src/normalize.js
Original file line number Diff line number Diff line change
Expand Up @@ -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':
Expand Down
1 change: 1 addition & 0 deletions packages/jest-runner/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
72 changes: 55 additions & 17 deletions packages/jest-runner/src/run_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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(
Expand All @@ -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 {
Expand All @@ -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;
}
2 changes: 2 additions & 0 deletions test_utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ const DEFAULT_GLOBAL_CONFIG: GlobalConfig = {
coverageDirectory: 'coverage',
coverageReporters: [],
coverageThreshold: {global: {}},
detectLeaks: false,
expand: false,
findRelatedTests: false,
forceExit: false,
Expand Down Expand Up @@ -63,6 +64,7 @@ const DEFAULT_PROJECT_CONFIG: ProjectConfig = {
clearMocks: false,
coveragePathIgnorePatterns: [],
cwd: '/test_root_dir/',
detectLeaks: false,
displayName: undefined,
globals: {},
haste: {
Expand Down
4 changes: 4 additions & 0 deletions types/Config.js
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ export type DefaultOptions = {|
expand: boolean,
globals: ConfigGlobals,
haste: HasteConfig,
detectLeaks: boolean,
mapCoverage: boolean,
moduleDirectories: Array<string>,
moduleFileExtensions: Array<string>,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -197,6 +200,7 @@ export type ProjectConfig = {|
clearMocks: boolean,
coveragePathIgnorePatterns: Array<string>,
cwd: Path,
detectLeaks: boolean,
displayName: ?string,
globals: ConfigGlobals,
haste: HasteConfig,
Expand Down
3 changes: 2 additions & 1 deletion types/TestResult.js
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down

0 comments on commit 3d2eb37

Please sign in to comment.