Skip to content
Prev Previous commit
Next Next commit
Removed usage of mapCoverage from codebase
  • Loading branch information
SimenB authored and Aftabnack committed Feb 15, 2018
commit f927798ba637352ae29d08ff00e60561a1494764
1 change: 0 additions & 1 deletion TestUtils.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@ const DEFAULT_GLOBAL_CONFIG: GlobalConfig = {
lastCommit: false,
listTests: false,
logHeapUsage: false,
mapCoverage: false,
maxWorkers: 2,
noSCM: null,
noStackTrace: false,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
exports[`maps code coverage against original source 1`] = `
Object {
"covered.js": Object {
"_coverageSchema": "332fd63041d2c1bcb487cc26dd0d5f7d97098a6c",
"b": Object {},
"branchMap": Object {},
"f": Object {
Expand All @@ -12,18 +13,19 @@ Object {
"0": Object {
"decl": Object {
"end": Object {
"column": 17,
"column": 36,
"line": 2,
},
"start": Object {
"column": 26,
"line": 2,
},
},
"line": 2,
"loc": Object {
"end": Object {
"column": 0,
"line": 2,
"column": 1,
"line": 5,
},
"start": Object {
"column": 58,
Expand All @@ -42,8 +44,8 @@ Object {
"statementMap": Object {
"0": Object {
"end": Object {
"column": -1,
"line": null,
"column": 2,
"line": 5,
},
"start": Object {
"column": 0,
Expand All @@ -52,18 +54,18 @@ Object {
},
"1": Object {
"end": Object {
"column": 24,
"column": 41,
"line": 3,
},
"start": Object {
"column": 9,
"column": 34,
"line": 3,
},
},
"2": Object {
"end": Object {
"column": -1,
"line": null,
"column": 33,
"line": 4,
},
"start": Object {
"column": 2,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,6 @@ exports[`--showConfig outputs config info and exits 1`] = `
\\"globalSetup\\": null,
\\"globalTeardown\\": null,
\\"listTests\\": false,
\\"mapCoverage\\": false,
\\"maxWorkers\\": \\"[maxWorkers]\\",
\\"noStackTrace\\": false,
\\"nonFlagArgs\\": [],
Expand Down
31 changes: 23 additions & 8 deletions packages/babel-jest/src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,12 @@
*/

import type {Path, ProjectConfig} from 'types/Config';
import type {CacheKeyOptions, TransformOptions} from 'types/Transform';
import type {
CacheKeyOptions,
Transformer,
TransformOptions,
TransformedSource,
} from 'types/Transform';

import crypto from 'crypto';
import fs from 'fs';
Expand All @@ -23,7 +28,7 @@ const BABEL_CONFIG_KEY = 'babel';
const PACKAGE_JSON = 'package.json';
const THIS_FILE = fs.readFileSync(__filename);

const createTransformer = (options: any) => {
const createTransformer = (options: any): Transformer => {
const cache = Object.create(null);

const getBabelRC = filename => {
Expand Down Expand Up @@ -69,8 +74,6 @@ const createTransformer = (options: any) => {
options = Object.assign({}, options, {
plugins: (options && options.plugins) || [],
presets: ((options && options.presets) || []).concat([jestPreset]),
retainLines: true,
Copy link
Member

Choose a reason for hiding this comment

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

retainLines needs to remain for now. We'll look into removing it after getting this merged

sourceMaps: 'inline',
});
delete options.cacheDirectory;
delete options.filename;
Expand Down Expand Up @@ -102,16 +105,23 @@ const createTransformer = (options: any) => {
src: string,
filename: Path,
config: ProjectConfig,
transformOptions: TransformOptions,
): string {
transformOptions?: TransformOptions,
): string | TransformedSource {
const altExts = config.moduleFileExtensions.map(
extension => '.' + extension,
);
const shouldUseInline =
Copy link
Member

Choose a reason for hiding this comment

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

as mentioned, inline sourcemaps are useful for IDEs and debugging, so I don't think they should be removed

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We will revert to using both?

Copy link
Member

Choose a reason for hiding this comment

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

I can't think of a reason not to :)

transformOptions &&
(transformOptions.returnSourceString == null ||
transformOptions.returnSourceString);
const sourceMapOpts = {
sourceMaps: shouldUseInline ? 'inline' : true,
};
if (babelUtil && !babelUtil.canCompile(filename, altExts)) {
return src;
}

const theseOptions = Object.assign({filename}, options);
const theseOptions = Object.assign({filename}, options, sourceMapOpts);
if (transformOptions && transformOptions.instrument) {
theseOptions.auxiliaryCommentBefore = ' istanbul ignore next ';
// Copied from jest-runtime transform.js
Expand All @@ -129,7 +139,12 @@ const createTransformer = (options: any) => {

// babel v7 might return null in the case when the file has been ignored.
const transformResult = babelTransform(src, theseOptions);
return transformResult ? transformResult.code : src;

if (!transformResult) {
return src;
}

return shouldUseInline ? transformResult.code : transformResult;
},
};
};
Expand Down
3 changes: 1 addition & 2 deletions packages/jest-cli/src/generate_empty_coverage.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,14 +27,13 @@ export default function(
collectCoverage: globalConfig.collectCoverage,
collectCoverageFrom: globalConfig.collectCoverageFrom,
collectCoverageOnlyFrom: globalConfig.collectCoverageOnlyFrom,
mapCoverage: globalConfig.mapCoverage,
};
if (Runtime.shouldInstrument(filename, coverageOptions, config)) {
// Transform file without instrumentation first, to make sure produced
// source code is ES6 (no flowtypes etc.) and can be instrumented
const transformResult = new Runtime.ScriptTransformer(
config,
).transformSource(filename, source, false, globalConfig.mapCoverage);
).transformSource(filename, source, false);
const instrumenter = createInstrumenter();
instrumenter.instrumentSync(transformResult.code, filename);
return {
Expand Down
10 changes: 9 additions & 1 deletion packages/jest-cli/src/reporters/coverage_reporter.js
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,15 @@ export default class CoverageReporter extends BaseReporter {
await this._addUntestedFiles(this._globalConfig, contexts);
let map = this._coverageMap;
let sourceFinder: Object;
({map, sourceFinder} = this._sourceMapStore.transformCoverage(map)); //eslint-disable-line
const transformedCoverage = this._sourceMapStore.transformCoverage(map);
Copy link
Member

Choose a reason for hiding this comment

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

might actually be better to keep mapCoverage here? I don't really understand why it's necessary 🙁


if (
transformedCoverage.map &&
transformedCoverage.map.data &&
Object.keys(transformedCoverage.map.data).length > 0
) {
({map, sourceFinder} = transformedCoverage);
}

const reporter = createReporter();
try {
Expand Down
1 change: 0 additions & 1 deletion packages/jest-config/src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,6 @@ const getConfigs = (
lastCommit: options.lastCommit,
listTests: options.listTests,
logHeapUsage: options.logHeapUsage,
mapCoverage: options.mapCoverage,
maxWorkers: options.maxWorkers,
noSCM: undefined,
noStackTrace: options.noStackTrace,
Expand Down
1 change: 0 additions & 1 deletion packages/jest-runner/src/run_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,6 @@ async function runTestInternal(
collectCoverage: globalConfig.collectCoverage,
collectCoverageFrom: globalConfig.collectCoverageFrom,
collectCoverageOnlyFrom: globalConfig.collectCoverageOnlyFrom,
mapCoverage: globalConfig.mapCoverage,
});

const start = Date.now();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
exports[`ScriptTransformer passes expected transform options to getCacheKey 1`] = `
Object {
"instrument": true,
"mapCoverage": true,
"rootDir": "/",
}
`;
Expand Down
3 changes: 0 additions & 3 deletions packages/jest-runtime/src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,6 @@ type CoverageOptions = {
collectCoverage: boolean,
collectCoverageFrom: Array<Glob>,
collectCoverageOnlyFrom: ?{[key: string]: boolean, __proto__: null},
mapCoverage: boolean,
};

type ModuleRegistry = {[key: string]: Module, __proto__: null};
Expand Down Expand Up @@ -181,7 +180,6 @@ class Runtime {
collectCoverage: options.collectCoverage,
collectCoverageFrom: options.collectCoverageFrom,
collectCoverageOnlyFrom: options.collectCoverageOnlyFrom,
mapCoverage: options.mapCoverage,
},
config,
);
Expand Down Expand Up @@ -529,7 +527,6 @@ class Runtime {
collectCoverageFrom: this._coverageOptions.collectCoverageFrom,
collectCoverageOnlyFrom: this._coverageOptions.collectCoverageOnlyFrom,
isInternalModule,
mapCoverage: this._coverageOptions.mapCoverage,
},
this._cacheFS[filename],
);
Expand Down
34 changes: 5 additions & 29 deletions packages/jest-runtime/src/script_transformer.js
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,6 @@ export type Options = {|
collectCoverageOnlyFrom: ?{[key: string]: boolean, __proto__: null},
isCoreModule?: boolean,
isInternalModule?: boolean,
mapCoverage: boolean,
|};

const cache: Map<string, TransformResult> = new Map();
Expand All @@ -57,12 +56,7 @@ export default class ScriptTransformer {
this._transformCache = new Map();
}

_getCacheKey(
fileData: string,
filename: Path,
instrument: boolean,
mapCoverage: boolean,
): string {
_getCacheKey(fileData: string, filename: Path, instrument: boolean): string {
if (!configToJsonMap.has(this._config)) {
// We only need this set of config options that can likely influence
// cached output instead of all config options.
Expand All @@ -77,7 +71,6 @@ export default class ScriptTransformer {
.update(
transformer.getCacheKey(fileData, filename, configString, {
instrument,
mapCoverage,
rootDir: this._config.rootDir,
}),
)
Expand All @@ -89,7 +82,6 @@ export default class ScriptTransformer {
.update(fileData)
.update(configString)
.update(instrument ? 'instrument' : '')
.update(mapCoverage ? 'mapCoverage' : '')
.update(CACHE_VERSION)
.digest('hex');
}
Expand All @@ -99,19 +91,13 @@ export default class ScriptTransformer {
filename: Path,
content: string,
instrument: boolean,
mapCoverage: boolean,
): Path {
const baseCacheDir = HasteMap.getCacheFilePath(
this._config.cacheDirectory,
'jest-transform-cache-' + this._config.name,
VERSION,
);
const cacheKey = this._getCacheKey(
content,
filename,
instrument,
mapCoverage,
);
const cacheKey = this._getCacheKey(content, filename, instrument);
// Create sub folders based on the cacheKey to avoid creating one
// directory with many files.
const cacheDir = path.join(baseCacheDir, cacheKey[0] + cacheKey[1]);
Expand Down Expand Up @@ -191,20 +177,10 @@ export default class ScriptTransformer {
}
}

transformSource(
filepath: Path,
content: string,
instrument: boolean,
mapCoverage: boolean,
) {
transformSource(filepath: Path, content: string, instrument: boolean) {
const filename = this._getRealPath(filepath);
const transform = this._getTransformer(filename);
const cacheFilePath = this._getFileCachePath(
filename,
content,
instrument,
mapCoverage,
);
const cacheFilePath = this._getFileCachePath(filename, content, instrument);
let sourceMapPath = cacheFilePath + '.map';
// Ignore cache if `config.cache` is set (--no-cache)
let code = this._config.cache ? readCodeCacheFile(cacheFilePath) : null;
Expand All @@ -228,6 +204,7 @@ export default class ScriptTransformer {
if (transform && shouldTransform(filename, this._config)) {
const processed = transform.process(content, filename, this._config, {
instrument,
returnSourceString: false,
});

if (typeof processed === 'string') {
Expand Down Expand Up @@ -305,7 +282,6 @@ export default class ScriptTransformer {
filename,
content,
instrument,
!!(options && options.mapCoverage),
);

wrappedCode = wrap(transformedSource.code);
Expand Down
1 change: 0 additions & 1 deletion types/Config.js
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,6 @@ export type GlobalConfig = {|
lastCommit: boolean,
logHeapUsage: boolean,
listTests: boolean,
mapCoverage: boolean,
maxWorkers: number,
noStackTrace: boolean,
nonFlagArgs: Array<string>,
Expand Down
4 changes: 2 additions & 2 deletions types/Transform.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,17 +22,17 @@ export type TransformResult = {|

export type TransformOptions = {|
instrument: boolean,
returnSourceString?: boolean,
|};

export type CacheKeyOptions = {|
instrument: boolean,
mapCoverage: boolean,
rootDir: string,
|};

export type Transformer = {|
canInstrument?: boolean,
createTransformer(options: any): Transformer,
createTransformer?: (options: any) => Transformer,

getCacheKey: (
fileData: string,
Expand Down