From fd40599f464c7a3d2a3b76e5de3e1326265c2c25 Mon Sep 17 00:00:00 2001 From: rickhanlonii Date: Thu, 11 Jan 2018 18:40:58 -0500 Subject: [PATCH 1/6] Fix a leak in coverage --- packages/jest-runner/src/run_test.js | 2 +- packages/jest-runtime/src/index.js | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/jest-runner/src/run_test.js b/packages/jest-runner/src/run_test.js index 4d27c5ab9689..91315d541a95 100644 --- a/packages/jest-runner/src/run_test.js +++ b/packages/jest-runner/src/run_test.js @@ -123,7 +123,7 @@ async function runTestInternal( result.perfStats = {end: Date.now(), start}; result.testFilePath = path; - result.coverage = runtime.getAllCoverageInfo(); + result.coverage = runtime.getAllCoverageInfoCopy(); result.sourceMaps = runtime.getSourceMapInfo(); result.console = testConsole.getBuffer(); result.skipped = testCount === result.numPendingTests; diff --git a/packages/jest-runtime/src/index.js b/packages/jest-runtime/src/index.js index 9a322f6fc32d..1c335c76434a 100644 --- a/packages/jest-runtime/src/index.js +++ b/packages/jest-runtime/src/index.js @@ -433,8 +433,8 @@ class Runtime { } } - getAllCoverageInfo() { - return this._environment.global.__coverage__; + getAllCoverageInfoCopy() { + return JSON.parse(JSON.stringify(this._environment.global.__coverage__)); } getSourceMapInfo() { From dfe1cc7c80d760aa8ccb7d34a42ef04d9cc6b3c3 Mon Sep 17 00:00:00 2001 From: rickhanlonii Date: Thu, 11 Jan 2018 19:30:19 -0500 Subject: [PATCH 2/6] Fix tests --- packages/jest-runtime/src/index.js | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/packages/jest-runtime/src/index.js b/packages/jest-runtime/src/index.js index 1c335c76434a..6a6723a6ce03 100644 --- a/packages/jest-runtime/src/index.js +++ b/packages/jest-runtime/src/index.js @@ -434,7 +434,10 @@ class Runtime { } getAllCoverageInfoCopy() { - return JSON.parse(JSON.stringify(this._environment.global.__coverage__)); + const coverage = this._environment.global.__coverage__; + if (!coverage) return null; + + return JSON.parse(JSON.stringify(coverage)); } getSourceMapInfo() { From 40a1c8656bebc8516ee803c86fddfc93d7e4a3b3 Mon Sep 17 00:00:00 2001 From: rickhanlonii Date: Fri, 12 Jan 2018 10:18:24 -0500 Subject: [PATCH 3/6] Return undefined instead of null --- packages/jest-runtime/src/index.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/jest-runtime/src/index.js b/packages/jest-runtime/src/index.js index 6a6723a6ce03..3aa9d4ebf6c1 100644 --- a/packages/jest-runtime/src/index.js +++ b/packages/jest-runtime/src/index.js @@ -435,7 +435,7 @@ class Runtime { getAllCoverageInfoCopy() { const coverage = this._environment.global.__coverage__; - if (!coverage) return null; + if (!coverage) return undefined; return JSON.parse(JSON.stringify(coverage)); } From e803b790d0cb83ba31f09edd459627464adfd43b Mon Sep 17 00:00:00 2001 From: rickhanlonii Date: Fri, 12 Jan 2018 10:21:38 -0500 Subject: [PATCH 4/6] Update changelog --- CHANGELOG.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 045c0580bc29..7deef9fdc396 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,4 +1,10 @@ ## master +## jest 22.0.7 + +### Fixes + +* `[jest-runner]` Fix memory leak in coverage reporting ([#5289](https://github.com/facebook/jest/pull/5289)) + ## jest 22.0.6 ### Fixes From 01128f09aad28854aafd05d1a0704512717ab3e5 Mon Sep 17 00:00:00 2001 From: rickhanlonii Date: Fri, 12 Jan 2018 21:57:04 -0500 Subject: [PATCH 5/6] Update to use deepCyclicCopy --- packages/jest-runtime/src/index.js | 7 +--- .../src/__tests__/deep_cyclic_copy.test.js | 39 ++++++++++++++++++- .../jest-util/src/create_process_object.js | 5 ++- packages/jest-util/src/deep_cyclic_copy.js | 28 ++++++++----- packages/jest-util/src/index.js | 2 + 5 files changed, 65 insertions(+), 16 deletions(-) diff --git a/packages/jest-runtime/src/index.js b/packages/jest-runtime/src/index.js index 3aa9d4ebf6c1..cc1084ff1795 100644 --- a/packages/jest-runtime/src/index.js +++ b/packages/jest-runtime/src/index.js @@ -19,7 +19,7 @@ import type {MockFunctionMetadata, ModuleMocker} from 'types/Mock'; import path from 'path'; import HasteMap from 'jest-haste-map'; import Resolver from 'jest-resolve'; -import {createDirectory} from 'jest-util'; +import {createDirectory, deepCyclicCopy} from 'jest-util'; import {escapePathForRegex} from 'jest-regex-util'; import fs from 'graceful-fs'; import stripBOM from 'strip-bom'; @@ -434,10 +434,7 @@ class Runtime { } getAllCoverageInfoCopy() { - const coverage = this._environment.global.__coverage__; - if (!coverage) return undefined; - - return JSON.parse(JSON.stringify(coverage)); + return deepCyclicCopy(this._environment.global.__coverage__); } getSourceMapInfo() { diff --git a/packages/jest-util/src/__tests__/deep_cyclic_copy.test.js b/packages/jest-util/src/__tests__/deep_cyclic_copy.test.js index c59dfabcf03b..a999edcb12d4 100644 --- a/packages/jest-util/src/__tests__/deep_cyclic_copy.test.js +++ b/packages/jest-util/src/__tests__/deep_cyclic_copy.test.js @@ -70,9 +70,46 @@ it('uses the blacklist to avoid copying properties on the first level', () => { }, }; - expect(deepCyclicCopy(obj, new Set(['blacklisted']))).toEqual({ + expect(deepCyclicCopy(obj, {blacklist: new Set(['blacklisted'])})).toEqual({ subObj: { blacklisted: 42, }, }); }); + +it('does not keep the prototype by default', () => { + const obj = new function() {}(); + obj.nestedObj = new function() {}(); + + const copy = deepCyclicCopy(obj); + expect(Object.getPrototypeOf(copy)).not.toBe(Object.getPrototypeOf(obj)); + expect(Object.getPrototypeOf(copy.nestedObj)).not.toBe( + Object.getPrototypeOf(obj.nestedObj), + ); + expect(Object.getPrototypeOf(copy)).toBe(Object.getPrototypeOf({})); +}); + +it('does not keep the prototype for keepPrototype = false', () => { + const obj = new function() {}(); + obj.nestedObj = new function() {}(); + + const copy = deepCyclicCopy(obj, {keepPrototype: false}); + + expect(Object.getPrototypeOf(copy)).not.toBe(Object.getPrototypeOf(obj)); + expect(Object.getPrototypeOf(copy.nestedObj)).not.toBe( + Object.getPrototypeOf(obj.nestedObj), + ); + expect(Object.getPrototypeOf(copy)).toBe(Object.getPrototypeOf({})); +}); + +it('keeps the prototype for keepPrototype = true', () => { + const obj = new function() {}(); + obj.nestedObj = new function() {}(); + + const copy = deepCyclicCopy(obj, {keepPrototype: true}); + + expect(Object.getPrototypeOf(copy)).toBe(Object.getPrototypeOf(obj)); + expect(Object.getPrototypeOf(copy.nestedObj)).toBe( + Object.getPrototypeOf(obj.nestedObj), + ); +}); diff --git a/packages/jest-util/src/create_process_object.js b/packages/jest-util/src/create_process_object.js index 58d55f455a05..6b6f25fe2bbc 100644 --- a/packages/jest-util/src/create_process_object.js +++ b/packages/jest-util/src/create_process_object.js @@ -13,7 +13,10 @@ const BLACKLIST = new Set(['mainModule', '_events']); export default function() { const process = require('process'); - const newProcess = deepCyclicCopy(process, BLACKLIST); + const newProcess = deepCyclicCopy(process, { + blacklist: BLACKLIST, + keepPrototype: true, + }); newProcess[Symbol.toStringTag] = 'process'; diff --git a/packages/jest-util/src/deep_cyclic_copy.js b/packages/jest-util/src/deep_cyclic_copy.js index 76affb7473a4..2a7f8b7b1674 100644 --- a/packages/jest-util/src/deep_cyclic_copy.js +++ b/packages/jest-util/src/deep_cyclic_copy.js @@ -9,6 +9,11 @@ const EMPTY = new Set(); +export type DeepCyclicCopyOptions = {| + blacklist: Set, + keepPrototype: boolean, +|}; + // Node 6 does not have gOPDs, so we define a simple polyfill for it. if (!Object.getOwnPropertyDescriptors) { // $FlowFixMe: polyfill @@ -26,7 +31,7 @@ if (!Object.getOwnPropertyDescriptors) { export default function deepCyclicCopy( value: any, - blacklist: Set = EMPTY, + options?: DeepCyclicCopyOptions = {blacklist: EMPTY, keepPrototype: false}, cycles: WeakMap = new WeakMap(), ): any { if (typeof value !== 'object' || value === null) { @@ -34,25 +39,28 @@ export default function deepCyclicCopy( } else if (cycles.has(value)) { return cycles.get(value); } else if (Array.isArray(value)) { - return deepCyclicCopyArray(value, blacklist, cycles); + return deepCyclicCopyArray(value, options, cycles); } else { - return deepCyclicCopyObject(value, blacklist, cycles); + return deepCyclicCopyObject(value, options, cycles); } } function deepCyclicCopyObject( object: Object, - blacklist: Set, + options: DeepCyclicCopyOptions, cycles: WeakMap, ): Object { - const newObject = Object.create(Object.getPrototypeOf(object)); + const newObject = options.keepPrototype + ? Object.create(Object.getPrototypeOf(object)) + : {}; + // $FlowFixMe: Object.getOwnPropertyDescriptors is polyfilled above. const descriptors = Object.getOwnPropertyDescriptors(object); cycles.set(object, newObject); Object.keys(descriptors).forEach(key => { - if (blacklist.has(key)) { + if (options.blacklist && options.blacklist.has(key)) { delete descriptors[key]; return; } @@ -60,7 +68,8 @@ function deepCyclicCopyObject( const descriptor = descriptors[key]; if (typeof descriptor.value !== 'undefined') { - descriptor.value = deepCyclicCopy(descriptor.value, EMPTY, cycles); + delete options.blacklist; + descriptor.value = deepCyclicCopy(descriptor.value, options, cycles); } descriptor.configurable = true; @@ -71,7 +80,7 @@ function deepCyclicCopyObject( function deepCyclicCopyArray( array: Array, - blacklist: Set, + options: DeepCyclicCopyOptions, cycles: WeakMap, ): Array { const newArray = []; @@ -80,7 +89,8 @@ function deepCyclicCopyArray( cycles.set(array, newArray); for (let i = 0; i < length; i++) { - newArray[i] = deepCyclicCopy(array[i], EMPTY, cycles); + delete options.blacklist; + newArray[i] = deepCyclicCopy(array[i], options, cycles); } return newArray; diff --git a/packages/jest-util/src/index.js b/packages/jest-util/src/index.js index 7d3736bdefe8..0a238fc15800 100644 --- a/packages/jest-util/src/index.js +++ b/packages/jest-util/src/index.js @@ -21,6 +21,7 @@ import NullConsole from './null_console'; import isInteractive from './is_interative'; import setGlobal from './set_global'; import validateCLIOptions from './validate_cli_options'; +import deepCyclicCopy from './deep_cyclic_copy'; const createDirectory = (path: string) => { try { @@ -39,6 +40,7 @@ module.exports = { NullConsole, clearLine, createDirectory, + deepCyclicCopy, formatTestResults, getConsoleOutput, getFailedSnapshotTests, From 704d1aadbf417651b5028340b79b498100ebacb5 Mon Sep 17 00:00:00 2001 From: rickhanlonii Date: Sat, 13 Jan 2018 13:38:06 -0500 Subject: [PATCH 6/6] Respect keepPrototype option for arrays --- .../src/__tests__/deep_cyclic_copy.test.js | 140 +++++++++++++++--- packages/jest-util/src/deep_cyclic_copy.js | 5 +- 2 files changed, 123 insertions(+), 22 deletions(-) diff --git a/packages/jest-util/src/__tests__/deep_cyclic_copy.test.js b/packages/jest-util/src/__tests__/deep_cyclic_copy.test.js index a999edcb12d4..8de6d5a4772f 100644 --- a/packages/jest-util/src/__tests__/deep_cyclic_copy.test.js +++ b/packages/jest-util/src/__tests__/deep_cyclic_copy.test.js @@ -77,39 +77,137 @@ it('uses the blacklist to avoid copying properties on the first level', () => { }); }); -it('does not keep the prototype by default', () => { - const obj = new function() {}(); - obj.nestedObj = new function() {}(); +it('does not keep the prototype by default when top level is object', () => { + const sourceObject = new function() {}(); + sourceObject.nestedObject = new function() {}(); + sourceObject.nestedArray = new function() { + this.length = 0; + }(); + + const spy = jest.spyOn(Array, 'isArray').mockImplementation(object => { + return object === sourceObject.nestedArray; + }); - const copy = deepCyclicCopy(obj); - expect(Object.getPrototypeOf(copy)).not.toBe(Object.getPrototypeOf(obj)); - expect(Object.getPrototypeOf(copy.nestedObj)).not.toBe( - Object.getPrototypeOf(obj.nestedObj), + const copy = deepCyclicCopy(sourceObject, {keepPrototype: false}); + + expect(Object.getPrototypeOf(copy)).not.toBe( + Object.getPrototypeOf(sourceObject), + ); + expect(Object.getPrototypeOf(copy.nestedObject)).not.toBe( + Object.getPrototypeOf(sourceObject.nestedObject), + ); + expect(Object.getPrototypeOf(copy.nestedArray)).not.toBe( + Object.getPrototypeOf(sourceObject.nestedArray), ); + expect(Object.getPrototypeOf(copy)).toBe(Object.getPrototypeOf({})); + expect(Object.getPrototypeOf(copy.nestedObject)).toBe( + Object.getPrototypeOf({}), + ); + expect(Object.getPrototypeOf(copy.nestedArray)).toBe( + Object.getPrototypeOf([]), + ); + + spy.mockRestore(); +}); + +it('does not keep the prototype by default when top level is array', () => { + const spy = jest.spyOn(Array, 'isArray').mockImplementation(() => true); + + const sourceArray = new function() { + this.length = 0; + }(); + + const copy = deepCyclicCopy(sourceArray); + expect(Object.getPrototypeOf(copy)).not.toBe( + Object.getPrototypeOf(sourceArray), + ); + + expect(Object.getPrototypeOf(copy)).toBe(Object.getPrototypeOf([])); + spy.mockRestore(); +}); + +it('does not keep the prototype of arrays when keepPrototype = false', () => { + const spy = jest.spyOn(Array, 'isArray').mockImplementation(() => true); + + const sourceArray = new function() { + this.length = 0; + }(); + + const copy = deepCyclicCopy(sourceArray); + expect(Object.getPrototypeOf(copy)).not.toBe( + Object.getPrototypeOf(sourceArray), + ); + + expect(Object.getPrototypeOf(copy)).toBe(Object.getPrototypeOf([])); + spy.mockRestore(); }); -it('does not keep the prototype for keepPrototype = false', () => { - const obj = new function() {}(); - obj.nestedObj = new function() {}(); +it('keeps the prototype of arrays when keepPrototype = true', () => { + const spy = jest.spyOn(Array, 'isArray').mockImplementation(() => true); + + const sourceArray = new function() { + this.length = 0; + }(); + + const copy = deepCyclicCopy(sourceArray, {keepPrototype: true}); + expect(Object.getPrototypeOf(copy)).toBe(Object.getPrototypeOf(sourceArray)); + + spy.mockRestore(); +}); + +it('does not keep the prototype for objects when keepPrototype = false', () => { + const sourceobject = new function() {}(); + sourceobject.nestedObject = new function() {}(); + sourceobject.nestedArray = new function() { + this.length = 0; + }(); + + const spy = jest.spyOn(Array, 'isArray').mockImplementation(object => { + return object === sourceobject.nestedArray; + }); - const copy = deepCyclicCopy(obj, {keepPrototype: false}); + const copy = deepCyclicCopy(sourceobject, {keepPrototype: false}); - expect(Object.getPrototypeOf(copy)).not.toBe(Object.getPrototypeOf(obj)); - expect(Object.getPrototypeOf(copy.nestedObj)).not.toBe( - Object.getPrototypeOf(obj.nestedObj), + expect(Object.getPrototypeOf(copy)).not.toBe( + Object.getPrototypeOf(sourceobject), + ); + expect(Object.getPrototypeOf(copy.nestedObject)).not.toBe( + Object.getPrototypeOf(sourceobject.nestedObject), + ); + expect(Object.getPrototypeOf(copy.nestedArray)).not.toBe( + Object.getPrototypeOf(sourceobject.nestedArray), ); expect(Object.getPrototypeOf(copy)).toBe(Object.getPrototypeOf({})); + expect(Object.getPrototypeOf(copy.nestedObject)).toBe( + Object.getPrototypeOf({}), + ); + expect(Object.getPrototypeOf(copy.nestedArray)).toBe( + Object.getPrototypeOf([]), + ); + + spy.mockRestore(); }); -it('keeps the prototype for keepPrototype = true', () => { - const obj = new function() {}(); - obj.nestedObj = new function() {}(); +it('keeps the prototype for objects when keepPrototype = true', () => { + const sourceObject = new function() {}(); + sourceObject.nestedObject = new function() {}(); + sourceObject.nestedArray = new function() { + this.length = 0; + }(); + + const spy = jest.spyOn(Array, 'isArray').mockImplementation(object => { + return object === sourceObject.nestedArray; + }); - const copy = deepCyclicCopy(obj, {keepPrototype: true}); + const copy = deepCyclicCopy(sourceObject, {keepPrototype: true}); - expect(Object.getPrototypeOf(copy)).toBe(Object.getPrototypeOf(obj)); - expect(Object.getPrototypeOf(copy.nestedObj)).toBe( - Object.getPrototypeOf(obj.nestedObj), + expect(Object.getPrototypeOf(copy)).toBe(Object.getPrototypeOf(sourceObject)); + expect(Object.getPrototypeOf(copy.nestedObject)).toBe( + Object.getPrototypeOf(sourceObject.nestedObject), + ); + expect(Object.getPrototypeOf(copy.nestedArray)).toBe( + Object.getPrototypeOf(sourceObject.nestedArray), ); + spy.mockRestore(); }); diff --git a/packages/jest-util/src/deep_cyclic_copy.js b/packages/jest-util/src/deep_cyclic_copy.js index 2a7f8b7b1674..f6e6f02c8dd5 100644 --- a/packages/jest-util/src/deep_cyclic_copy.js +++ b/packages/jest-util/src/deep_cyclic_copy.js @@ -83,7 +83,10 @@ function deepCyclicCopyArray( options: DeepCyclicCopyOptions, cycles: WeakMap, ): Array { - const newArray = []; + const newArray = options.keepPrototype + ? // $FlowFixMe: getPrototypeOf an array is OK. + new (Object.getPrototypeOf(array)).constructor(array.length) + : []; const length = array.length; cycles.set(array, newArray);