From 23eec748db0de7b6b5fcda28cc51c48ddae16545 Mon Sep 17 00:00:00 2001 From: Simen Bekkhus Date: Wed, 21 Feb 2018 11:00:30 +0100 Subject: [PATCH] [WIP] Remove usage of `retainLines` (#5594) * Remove usage of `retainLines` * update changelog * Move callsite to jest-util, add sourcemap support * This is a type * Remove unused function moved to jest-util * Fix lint * Minor clean `up * Feedback updates * fix flow violations and minimize diff * add todo * use sourcemaps in buffered console as well * fix stupid typo * Add unit tests for get_callsite * Fix tests * move changelog entry --- .babelrc | 3 +- CHANGELOG.md | 8 +++ .../__snapshots__/failures.test.js.snap | 2 +- .../__tests__/location_in_results.test.js | 4 +- .../location-in-results/__tests__/test.js | 3 +- packages/babel-jest/src/index.js | 1 - packages/jest-jasmine2/package.json | 2 +- packages/jest-jasmine2/src/index.js | 15 +++-- packages/jest-runner/src/run_test.js | 14 +++- packages/jest-runtime/src/index.js | 7 +- packages/jest-util/package.json | 3 +- .../src/__tests__/buffered_console.test.js | 2 +- .../src/__tests__/get_callsite.test.js | 56 ++++++++++++++++ packages/jest-util/src/buffered_console.js | 19 ++++-- packages/jest-util/src/get_callsite.js | 64 +++++++++++++++++++ packages/jest-util/src/index.js | 2 + types/SourceMaps.js | 10 +++ 17 files changed, 189 insertions(+), 26 deletions(-) create mode 100644 packages/jest-util/src/__tests__/get_callsite.test.js create mode 100644 packages/jest-util/src/get_callsite.js create mode 100644 types/SourceMaps.js diff --git a/.babelrc b/.babelrc index 902f1e07dbe8..7743d00e7c9a 100644 --- a/.babelrc +++ b/.babelrc @@ -8,6 +8,5 @@ "transform-async-to-generator", "transform-strict-mode", ["transform-es2015-modules-commonjs", {"allowTopLevelThis": true}] - ], - "retainLines": true + ] } diff --git a/CHANGELOG.md b/CHANGELOG.md index 6797f1f012af..a46d5a5c7e06 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,14 @@ * `[jest-runtime]` Provide `require.main` property set to module with test suite ([#5618](https://github.com/facebook/jest/pull/5618)) + +### Fixes + +* `[babel-jest]` Remove `retainLines` argument to babel. + ([#5594](https://github.com/facebook/jest/pull/5594)) + +### Chore & Maintenance + * `[docs]` Add note about Node version support ([#5622](https://github.com/facebook/jest/pull/5622)) * `[docs]` Update to use yarn diff --git a/integration-tests/__tests__/__snapshots__/failures.test.js.snap b/integration-tests/__tests__/__snapshots__/failures.test.js.snap index 80542fc0622e..6ebf606f5060 100644 --- a/integration-tests/__tests__/__snapshots__/failures.test.js.snap +++ b/integration-tests/__tests__/__snapshots__/failures.test.js.snap @@ -118,7 +118,7 @@ exports[`works with async failures 1`] = ` + \\"foo\\": \\"bar\\", } - at ../../packages/expect/build/index.js:145:57 + at ../../packages/expect/build/index.js:104:76 " `; diff --git a/integration-tests/__tests__/location_in_results.test.js b/integration-tests/__tests__/location_in_results.test.js index 19891b542a1d..2e7b78565ab4 100644 --- a/integration-tests/__tests__/location_in_results.test.js +++ b/integration-tests/__tests__/location_in_results.test.js @@ -28,6 +28,6 @@ it('adds correct location info when provided with flag', () => { const assertions = result.testResults[0].assertionResults; expect(result.success).toBe(true); expect(result.numTotalTests).toBe(2); - expect(assertions[0].location).toEqual({column: 1, line: 9}); - expect(assertions[1].location).toEqual({column: 3, line: 14}); + expect(assertions[0].location).toEqual({column: 1, line: 10}); + expect(assertions[1].location).toEqual({column: 2, line: 15}); }); diff --git a/integration-tests/location-in-results/__tests__/test.js b/integration-tests/location-in-results/__tests__/test.js index 55c08de1379c..810c033d474a 100644 --- a/integration-tests/location-in-results/__tests__/test.js +++ b/integration-tests/location-in-results/__tests__/test.js @@ -4,7 +4,8 @@ * This source code is licensed under the MIT license found in the * LICENSE file in the root directory of this source tree. */ -'use strict'; +// This file is missing 'use strict' to force babel into doing something +// as we have `transform-strict-mode` it('no ancestors', () => { expect(true).toBeTruthy(); diff --git a/packages/babel-jest/src/index.js b/packages/babel-jest/src/index.js index 810abb73a579..34ea2f1d2886 100644 --- a/packages/babel-jest/src/index.js +++ b/packages/babel-jest/src/index.js @@ -74,7 +74,6 @@ const createTransformer = (options: any): Transformer => { options = Object.assign({}, options, { plugins: (options && options.plugins) || [], presets: ((options && options.presets) || []).concat([jestPreset]), - retainLines: true, sourceMaps: 'both', }); delete options.cacheDirectory; diff --git a/packages/jest-jasmine2/package.json b/packages/jest-jasmine2/package.json index 7e539beb3f28..72312bffd713 100644 --- a/packages/jest-jasmine2/package.json +++ b/packages/jest-jasmine2/package.json @@ -8,7 +8,6 @@ "license": "MIT", "main": "build/index.js", "dependencies": { - "callsites": "^2.0.0", "chalk": "^2.0.1", "co": "^4.6.0", "expect": "^22.4.0", @@ -18,6 +17,7 @@ "jest-matcher-utils": "^22.4.0", "jest-message-util": "^22.4.0", "jest-snapshot": "^22.4.0", + "jest-util": "^22.4.0", "source-map-support": "^0.5.0" }, "devDependencies": { diff --git a/packages/jest-jasmine2/src/index.js b/packages/jest-jasmine2/src/index.js index 38422b08d5b2..3e822d1420c8 100644 --- a/packages/jest-jasmine2/src/index.js +++ b/packages/jest-jasmine2/src/index.js @@ -15,8 +15,8 @@ import type {TestResult} from 'types/TestResult'; import type Runtime from 'jest-runtime'; import path from 'path'; -import fs from 'fs'; -import callsites from 'callsites'; +import fs from 'graceful-fs'; +import {getCallsite} from 'jest-util'; import JasmineReporter from './reporter'; import {install as jasmineAsyncInstall} from './jasmine_async'; @@ -51,7 +51,7 @@ async function jasmine2( if (config.testLocationInResults === true) { const originalIt = environment.global.it; environment.global.it = (...args) => { - const stack = callsites()[1]; + const stack = getCallsite(1, runtime.getSourceMaps()); const it = originalIt(...args); it.result.__callsite = stack; @@ -125,12 +125,13 @@ async function jasmine2( environment: 'node', handleUncaughtExceptions: false, retrieveSourceMap: source => { - if (runtime._sourceMapRegistry[source]) { + const sourceMaps = runtime.getSourceMaps(); + const sourceMapSource = sourceMaps && sourceMaps[source]; + + if (sourceMapSource) { try { return { - map: JSON.parse( - fs.readFileSync(runtime._sourceMapRegistry[source]), - ), + map: JSON.parse(fs.readFileSync(sourceMapSource)), url: source, }; } catch (e) {} diff --git a/packages/jest-runner/src/run_test.js b/packages/jest-runner/src/run_test.js index 0a225dbaad1f..a209ffca7c51 100644 --- a/packages/jest-runner/src/run_test.js +++ b/packages/jest-runner/src/run_test.js @@ -74,13 +74,21 @@ async function runTestInternal( RuntimeClass, >); + let runtime = undefined; + const consoleOut = globalConfig.useStderr ? process.stderr : process.stdout; const consoleFormatter = (type, message) => getConsoleOutput( config.cwd, !!globalConfig.verbose, // 4 = the console call is buried 4 stack frames deep - BufferedConsole.write([], type, message, 4), + BufferedConsole.write( + [], + type, + message, + 4, + runtime && runtime.getSourceMaps(), + ), ); let testConsole; @@ -90,7 +98,7 @@ async function runTestInternal( } else if (globalConfig.verbose) { testConsole = new Console(consoleOut, process.stderr, consoleFormatter); } else { - testConsole = new BufferedConsole(); + testConsole = new BufferedConsole(() => runtime && runtime.getSourceMaps()); } const environment = new TestEnvironment(config, {console: testConsole}); @@ -101,7 +109,7 @@ async function runTestInternal( const cacheFS = {[path]: testSource}; setGlobal(environment.global, 'console', testConsole); - const runtime = new Runtime(config, environment, resolver, cacheFS, { + runtime = new Runtime(config, environment, resolver, cacheFS, { collectCoverage: globalConfig.collectCoverage, collectCoverageFrom: globalConfig.collectCoverageFrom, collectCoverageOnlyFrom: globalConfig.collectCoverageOnlyFrom, diff --git a/packages/jest-runtime/src/index.js b/packages/jest-runtime/src/index.js index 7b739fd4e32d..987a171b5b50 100644 --- a/packages/jest-runtime/src/index.js +++ b/packages/jest-runtime/src/index.js @@ -15,6 +15,7 @@ import type {Context} from 'types/Context'; import type {Jest, LocalModuleRequire} from 'types/Jest'; import type {ModuleMap} from 'jest-haste-map'; import type {MockFunctionMetadata, ModuleMocker} from 'types/Mock'; +import type {SourceMapRegistry} from 'types/SourceMaps'; import path from 'path'; import HasteMap from 'jest-haste-map'; @@ -100,7 +101,7 @@ class Runtime { _shouldAutoMock: boolean; _shouldMockModuleCache: BooleanObject; _shouldUnmockTransitiveDependenciesCache: BooleanObject; - _sourceMapRegistry: {[key: string]: string, __proto__: null}; + _sourceMapRegistry: SourceMapRegistry; _scriptTransformer: ScriptTransformer; _transitiveShouldMock: BooleanObject; _unmockList: ?RegExp; @@ -449,6 +450,10 @@ class Runtime { }, {}); } + getSourceMaps(): SourceMapRegistry { + return this._sourceMapRegistry; + } + setMock( from: string, moduleName: string, diff --git a/packages/jest-util/package.json b/packages/jest-util/package.json index b7530d210453..8c26aa54bba3 100644 --- a/packages/jest-util/package.json +++ b/packages/jest-util/package.json @@ -13,7 +13,8 @@ "graceful-fs": "^4.1.11", "is-ci": "^1.0.10", "jest-message-util": "^22.4.0", - "mkdirp": "^0.5.1" + "mkdirp": "^0.5.1", + "source-map": "^0.6.0" }, "devDependencies": { "jest-mock": "^22.2.0" diff --git a/packages/jest-util/src/__tests__/buffered_console.test.js b/packages/jest-util/src/__tests__/buffered_console.test.js index 103b9091a81b..ede81e84c810 100644 --- a/packages/jest-util/src/__tests__/buffered_console.test.js +++ b/packages/jest-util/src/__tests__/buffered_console.test.js @@ -19,7 +19,7 @@ describe('CustomConsole', () => { .join('\n'); beforeEach(() => { - _console = new BufferedConsole(); + _console = new BufferedConsole(() => null); }); describe('assert', () => { diff --git a/packages/jest-util/src/__tests__/get_callsite.test.js b/packages/jest-util/src/__tests__/get_callsite.test.js new file mode 100644 index 000000000000..c6f3237c126c --- /dev/null +++ b/packages/jest-util/src/__tests__/get_callsite.test.js @@ -0,0 +1,56 @@ +import fs from 'fs'; +import SourceMap from 'source-map'; +import getCallsite from '../get_callsite'; + +jest.mock('fs'); + +describe('getCallsite', () => { + test('without source map', () => { + const site = getCallsite(0); + + expect(site.getFileName()).toEqual(__filename); + expect(site.getColumnNumber()).toEqual(expect.any(Number)); + expect(site.getLineNumber()).toEqual(expect.any(Number)); + expect(fs.readFileSync).not.toHaveBeenCalled(); + }); + + test('ignores errors when fs throws', () => { + fs.readFileSync.mockImplementation(() => { + throw new Error('Mock error'); + }); + + const site = getCallsite(0, {[__filename]: 'mockedSourceMapFile'}); + + expect(site.getFileName()).toEqual(__filename); + expect(site.getColumnNumber()).toEqual(expect.any(Number)); + expect(site.getLineNumber()).toEqual(expect.any(Number)); + expect(fs.readFileSync).toHaveBeenCalledWith('mockedSourceMapFile', 'utf8'); + }); + + test('reads source map file to determine line and column', () => { + fs.readFileSync.mockImplementation(() => 'file data'); + + const sourceMapColumn = 1; + const sourceMapLine = 2; + SourceMap.SourceMapConsumer = class { + originalPositionFor(params) { + expect(params).toMatchObject({ + column: expect.any(Number), + line: expect.any(Number), + }); + + return { + column: sourceMapColumn, + line: sourceMapLine, + }; + } + }; + + const site = getCallsite(0, {[__filename]: 'mockedSourceMapFile'}); + + expect(site.getFileName()).toEqual(__filename); + expect(site.getColumnNumber()).toEqual(sourceMapColumn); + expect(site.getLineNumber()).toEqual(sourceMapLine); + expect(fs.readFileSync).toHaveBeenCalledWith('mockedSourceMapFile', 'utf8'); + }); +}); diff --git a/packages/jest-util/src/buffered_console.js b/packages/jest-util/src/buffered_console.js index 7c0ddca9f73d..5cee80665474 100644 --- a/packages/jest-util/src/buffered_console.js +++ b/packages/jest-util/src/buffered_console.js @@ -15,21 +15,28 @@ import type { LogTimers, } from 'types/Console'; +import type {SourceMapRegistry} from 'types/SourceMaps'; + import assert from 'assert'; import {Console} from 'console'; import {format} from 'util'; import chalk from 'chalk'; -import callsites from 'callsites'; +import getCallsite from './get_callsite'; export default class BufferedConsole extends Console { _buffer: ConsoleBuffer; _counters: LogCounters; _timers: LogTimers; _groupDepth: number; + _getSourceMaps: () => ?SourceMapRegistry; - constructor() { + constructor(getSourceMaps: () => ?SourceMapRegistry) { const buffer = []; - super({write: message => BufferedConsole.write(buffer, 'log', message)}); + super({ + write: message => + BufferedConsole.write(buffer, 'log', message, null, getSourceMaps()), + }); + this._getSourceMaps = getSourceMaps; this._buffer = buffer; this._counters = {}; this._timers = {}; @@ -41,9 +48,10 @@ export default class BufferedConsole extends Console { type: LogType, message: LogMessage, level: ?number, + sourceMaps: ?SourceMapRegistry, ) { - const call = callsites()[level != null ? level : 2]; - const origin = call.getFileName() + ':' + call.getLineNumber(); + const callsite = getCallsite(level != null ? level : 2, sourceMaps); + const origin = callsite.getFileName() + ':' + callsite.getLineNumber(); buffer.push({ message, @@ -60,6 +68,7 @@ export default class BufferedConsole extends Console { type, ' '.repeat(this._groupDepth) + message, 3, + this._getSourceMaps(), ); } diff --git a/packages/jest-util/src/get_callsite.js b/packages/jest-util/src/get_callsite.js new file mode 100644 index 000000000000..b3600e063405 --- /dev/null +++ b/packages/jest-util/src/get_callsite.js @@ -0,0 +1,64 @@ +/** + * Copyright (c) 2014-present, Facebook, Inc. 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. + * + * @flow + */ + +import type {SourceMapRegistry} from 'types/SourceMaps'; + +import fs from 'graceful-fs'; +import callsites from 'callsites'; +import {SourceMapConsumer} from 'source-map'; + +// Copied from https://github.com/rexxars/sourcemap-decorate-callsites/blob/5b9735a156964973a75dc62fd2c7f0c1975458e8/lib/index.js#L113-L158 +const addSourceMapConsumer = (callsite, consumer) => { + const getLineNumber = callsite.getLineNumber; + const getColumnNumber = callsite.getColumnNumber; + let position = null; + + function getPosition() { + if (!position) { + position = consumer.originalPositionFor({ + column: getColumnNumber.call(callsite), + line: getLineNumber.call(callsite), + }); + } + + return position; + } + + Object.defineProperties(callsite, { + getColumnNumber: { + value() { + return getPosition().column || getColumnNumber.call(callsite); + }, + writable: false, + }, + getLineNumber: { + value() { + return getPosition().line || getLineNumber.call(callsite); + }, + writable: false, + }, + }); +}; + +export default (level: number, sourceMaps: ?SourceMapRegistry) => { + const levelAfterThisCall = level + 1; + const stack = callsites()[levelAfterThisCall]; + const sourceMapFileName = sourceMaps && sourceMaps[stack.getFileName()]; + + if (sourceMapFileName) { + try { + const sourceMap = fs.readFileSync(sourceMapFileName, 'utf8'); + addSourceMapConsumer(stack, new SourceMapConsumer(sourceMap)); + } catch (e) { + // ignore + } + } + + return stack; +}; diff --git a/packages/jest-util/src/index.js b/packages/jest-util/src/index.js index 1b3a6cf27b20..be4389bfa810 100644 --- a/packages/jest-util/src/index.js +++ b/packages/jest-util/src/index.js @@ -19,6 +19,7 @@ import getConsoleOutput from './get_console_output'; import installCommonGlobals from './install_common_globals'; import NullConsole from './null_console'; import isInteractive from './is_interative'; +import getCallsite from './get_callsite'; import setGlobal from './set_global'; import deepCyclicCopy from './deep_cyclic_copy'; @@ -41,6 +42,7 @@ module.exports = { createDirectory, deepCyclicCopy, formatTestResults, + getCallsite, getConsoleOutput, getFailedSnapshotTests, installCommonGlobals, diff --git a/types/SourceMaps.js b/types/SourceMaps.js new file mode 100644 index 000000000000..aa21bae1252e --- /dev/null +++ b/types/SourceMaps.js @@ -0,0 +1,10 @@ +/** + * Copyright (c) 2014-present, Facebook, Inc. 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. + * + * @flow + */ + +export type SourceMapRegistry = {[key: string]: string, __proto__: null};