Skip to content

Commit

Permalink
[WIP] Remove usage of retainLines (#5594)
Browse files Browse the repository at this point in the history
* 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
  • Loading branch information
SimenB authored and cpojer committed Feb 21, 2018
1 parent 6ab04b7 commit 23eec74
Show file tree
Hide file tree
Showing 17 changed files with 189 additions and 26 deletions.
3 changes: 1 addition & 2 deletions .babelrc
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,5 @@
"transform-async-to-generator",
"transform-strict-mode",
["transform-es2015-modules-commonjs", {"allowTopLevelThis": true}]
],
"retainLines": true
]
}
8 changes: 8 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
"
`;
Expand Down
4 changes: 2 additions & 2 deletions integration-tests/__tests__/location_in_results.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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});
});
3 changes: 2 additions & 1 deletion integration-tests/location-in-results/__tests__/test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
1 change: 0 additions & 1 deletion packages/babel-jest/src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
2 changes: 1 addition & 1 deletion packages/jest-jasmine2/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand All @@ -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": {
Expand Down
15 changes: 8 additions & 7 deletions packages/jest-jasmine2/src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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) {}
Expand Down
14 changes: 11 additions & 3 deletions packages/jest-runner/src/run_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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});
Expand All @@ -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,
Expand Down
7 changes: 6 additions & 1 deletion packages/jest-runtime/src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -449,6 +450,10 @@ class Runtime {
}, {});
}

getSourceMaps(): SourceMapRegistry {
return this._sourceMapRegistry;
}

setMock(
from: string,
moduleName: string,
Expand Down
3 changes: 2 additions & 1 deletion packages/jest-util/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
2 changes: 1 addition & 1 deletion packages/jest-util/src/__tests__/buffered_console.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ describe('CustomConsole', () => {
.join('\n');

beforeEach(() => {
_console = new BufferedConsole();
_console = new BufferedConsole(() => null);
});

describe('assert', () => {
Expand Down
56 changes: 56 additions & 0 deletions packages/jest-util/src/__tests__/get_callsite.test.js
Original file line number Diff line number Diff line change
@@ -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');
});
});
19 changes: 14 additions & 5 deletions packages/jest-util/src/buffered_console.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 = {};
Expand All @@ -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,
Expand All @@ -60,6 +68,7 @@ export default class BufferedConsole extends Console {
type,
' '.repeat(this._groupDepth) + message,
3,
this._getSourceMaps(),
);
}

Expand Down
64 changes: 64 additions & 0 deletions packages/jest-util/src/get_callsite.js
Original file line number Diff line number Diff line change
@@ -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;
};
2 changes: 2 additions & 0 deletions packages/jest-util/src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand All @@ -41,6 +42,7 @@ module.exports = {
createDirectory,
deepCyclicCopy,
formatTestResults,
getCallsite,
getConsoleOutput,
getFailedSnapshotTests,
installCommonGlobals,
Expand Down
Loading

0 comments on commit 23eec74

Please sign in to comment.