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

Detect memory leaks #4895

Merged
merged 2 commits into from
Nov 27, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Such clowny code.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The function wasn't async at the point it was written 🙂

}

): 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;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

muuuch cleaner.


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