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

Fix coverage reporter for uncovered files without transformers #9724

Merged
merged 1 commit into from Mar 29, 2020
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 2 additions & 1 deletion CHANGELOG.md
Expand Up @@ -6,7 +6,8 @@

### Fixes

- `[jest-circus]` Fix type ellision of jest-runtime imports ([#9717](https://github.com/facebook/jest/pull/9717))
- `[jest-circus]` Fix type elision of jest-runtime imports ([#9717](https://github.com/facebook/jest/pull/9717))
- `[@jest/transform]` Fix coverage reporter for uncovered files without transformers, reverting ([#9460](https://github.com/facebook/jest/pull/9460)) ([#9724](https://github.com/facebook/jest/pull/9724))

### Chore & Maintenance

Expand Down
10 changes: 10 additions & 0 deletions e2e/__tests__/__snapshots__/coverageWithoutTransform.test.ts.snap
@@ -0,0 +1,10 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`produces code coverage for uncovered files without transformer 1`] = `
---------------------|---------|----------|---------|---------|-------------------
File | % Stmts | % Branch | % Funcs | % Lines | Uncovered Line #s
---------------------|---------|----------|---------|---------|-------------------
All files | 0 | 100 | 0 | 0 |
some-random-file.js | 0 | 100 | 0 | 0 | 8,10,11
---------------------|---------|----------|---------|---------|-------------------
`;
29 changes: 29 additions & 0 deletions e2e/__tests__/coverageWithoutTransform.test.ts
@@ -0,0 +1,29 @@
/**
* Copyright (c) Facebook, Inc. and its affiliates. 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.
*/

import * as path from 'path';
import {wrap} from 'jest-snapshot-serializer-raw';
import {cleanup} from '../Utils';
import runJest from '../runJest';

const dir = path.resolve(__dirname, '../coverage-without-transform');
const coverageDir = path.join(dir, 'coverage');

beforeAll(() => {
cleanup(coverageDir);
});

afterAll(() => {
cleanup(coverageDir);
});

it('produces code coverage for uncovered files without transformer', () => {
const {exitCode, stdout} = runJest(dir, ['--coverage', '--no-cache']);

expect(exitCode).toBe(0);
expect(wrap(stdout)).toMatchSnapshot();
});
10 changes: 10 additions & 0 deletions e2e/coverage-without-transform/__tests__/test.js
@@ -0,0 +1,10 @@
/**
* Copyright (c) Facebook, Inc. and its affiliates. 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.
*/

it('dummy test', () => {
expect(1).toBe(1);
});
7 changes: 7 additions & 0 deletions e2e/coverage-without-transform/package.json
@@ -0,0 +1,7 @@
{
"jest": {
"collectCoverageFrom": ["*.js"],
"transform": {},
"testEnvironment": "node"
}
}
12 changes: 12 additions & 0 deletions e2e/coverage-without-transform/some-random-file.js
@@ -0,0 +1,12 @@
/**
* Copyright (c) Facebook, Inc. and its affiliates. 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.
*/

module.exports = function doES6Stuff(testObj, multiplier) {
// eslint-disable-next-line no-unused-vars
const {someNumber, ...others} = testObj;
return someNumber * multiplier;
};
57 changes: 14 additions & 43 deletions packages/jest-transform/src/ScriptTransformer.ts
Expand Up @@ -191,12 +191,8 @@ export default class ScriptTransformer {
return transform;
}

private _instrumentFile(
filename: Config.Path,
input: TransformedSource,
canMapToInput: boolean,
): TransformedSource {
const result = babelTransform(input.code, {
private _instrumentFile(filename: Config.Path, content: string): string {
const result = babelTransform(content, {
auxiliaryCommentBefore: ' istanbul ignore next ',
babelrc: false,
caller: {
Expand All @@ -214,27 +210,21 @@ export default class ScriptTransformer {
cwd: this._config.rootDir,
exclude: [],
extension: false,
// Needed for correct coverage as soon as we start storing a source map of the instrumented code
inputSourceMap: input.map,
useInlineSourceMaps: false,
},
],
],
/**
* It's necessary to be able to map back to original source from the instrumented code.
* The inline map is needed for debugging functionality, and exposing it as a separate file is needed
* for mapping stack traces. It's convenient to use 'both' here and avoid extracting the source map.
*
* Previous behavior of emitting no map when we can't map back to original source is preserved.
*/
sourceMaps: canMapToInput ? 'both' : false,
Copy link
Member Author

Choose a reason for hiding this comment

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

specifically this line that breaks. I can keep everything else and just remove this line (or set it to false) and everything works again. But I thought a full revert makes more sense than a partial one

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks! Revert seems appropriate given how confusing this area is.

Hopefully can re-attempt this one soon.

Copy link
Member Author

Choose a reason for hiding this comment

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

Sounds good, thanks!

});

if (result && result.code) {
return result as TransformResult;
if (result) {
const {code} = result;

if (code) {
return code;
}
}

return {code: input.code};
return content;
}

private _getRealPath(filepath: Config.Path): Config.Path {
Expand Down Expand Up @@ -328,36 +318,17 @@ export default class ScriptTransformer {
}
}

// Apply instrumentation to the code if necessary, keeping the instrumented code and new map
let map = transformed.map;
if (!transformWillInstrument && instrument) {
/**
* We can map the original source code to the instrumented code ONLY if
* - the process of transforming the code produced a source map e.g. ts-jest
* - we did not transform the source code
*
* Otherwise we cannot make any statements about how the instrumented code corresponds to the original code,
* and we should NOT emit any source maps
*
*/
const shouldEmitSourceMaps = (!!transform && !!map) || !transform;
const instrumented = this._instrumentFile(
filename,
transformed,
shouldEmitSourceMaps,
);
code = instrumented.code;

if (instrumented.map) {
map = instrumented.map;
}
code = this._instrumentFile(filename, transformed.code);
} else {
code = transformed.code;
}

if (map) {
if (transformed.map) {
const sourceMapContent =
typeof map === 'string' ? map : JSON.stringify(map);
typeof transformed.map === 'string'
? transformed.map
: JSON.stringify(transformed.map);
writeCacheFile(sourceMapPath, sourceMapContent);
} else {
sourceMapPath = null;
Expand Down
Expand Up @@ -78,7 +78,7 @@ exports[`ScriptTransformer transforms a file properly 1`] = `
/* istanbul ignore next */
function cov_25u22311x4() {
var path = "/fruits/banana.js";
var hash = "3f8e915bed83285455a8a16aa04dc0cf5242d755";
var hash = "4be0f6184160be573fc43f7c2a5877c28b7ce249";
var global = new Function("return this")();
var gcv = "__coverage__";
var coverageData = {
Expand All @@ -102,9 +102,8 @@ function cov_25u22311x4() {
},
f: {},
b: {},
inputSourceMap: null,
_coverageSchema: "1a1c01bbd47fc00a2c39e90264f33305004495a9",
hash: "3f8e915bed83285455a8a16aa04dc0cf5242d755"
hash: "4be0f6184160be573fc43f7c2a5877c28b7ce249"
};
var coverage = global[gcv] || (global[gcv] = {});

Expand All @@ -124,14 +123,13 @@ function cov_25u22311x4() {
cov_25u22311x4();
cov_25u22311x4().s[0]++;
module.exports = "banana";
//# sourceMappingURL=data:application/json;charset=utf-8;base64,eyJ2ZXJzaW9uIjozLCJzb3VyY2VzIjpbImJhbmFuYS5qcyJdLCJuYW1lcyI6WyJtb2R1bGUiLCJleHBvcnRzIl0sIm1hcHBpbmdzIjoiOzs7Ozs7Ozs7Ozs7Ozs7Ozs7Ozs7Ozs7Ozs7Ozs7Ozs7Ozs7Ozs7Ozs7Ozs7Ozs7QUFBQUEsTUFBTSxDQUFDQyxPQUFQLEdBQWlCLFFBQWpCIiwic291cmNlc0NvbnRlbnQiOlsibW9kdWxlLmV4cG9ydHMgPSBcImJhbmFuYVwiOyJdfQ==
`;

exports[`ScriptTransformer transforms a file properly 2`] = `
/* istanbul ignore next */
function cov_23yvu8etmu() {
var path = "/fruits/kiwi.js";
var hash = "8b5afd38d79008f13ebc229b89ef82b12ee9447a";
var hash = "7705dd5fcfbc884dcea7062944cfb8cc5d141d1a";
var global = new Function("return this")();
var gcv = "__coverage__";
var coverageData = {
Expand Down Expand Up @@ -193,9 +191,8 @@ function cov_23yvu8etmu() {
"0": 0
},
b: {},
inputSourceMap: null,
_coverageSchema: "1a1c01bbd47fc00a2c39e90264f33305004495a9",
hash: "8b5afd38d79008f13ebc229b89ef82b12ee9447a"
hash: "7705dd5fcfbc884dcea7062944cfb8cc5d141d1a"
};
var coverage = global[gcv] || (global[gcv] = {});

Expand All @@ -221,7 +218,6 @@ module.exports = () => {
cov_23yvu8etmu().s[1]++;
return "kiwi";
};
//# sourceMappingURL=data:application/json;charset=utf-8;base64,eyJ2ZXJzaW9uIjozLCJzb3VyY2VzIjpbImtpd2kuanMiXSwibmFtZXMiOlsibW9kdWxlIiwiZXhwb3J0cyJdLCJtYXBwaW5ncyI6Ijs7Ozs7Ozs7Ozs7Ozs7Ozs7Ozs7Ozs7Ozs7Ozs7Ozs7Ozs7Ozs7Ozs7Ozs7Ozs7Ozs7Ozs7Ozs7Ozs7Ozs7Ozs7Ozs7Ozs7Ozs7Ozs7Ozs7Ozs7Ozs7O0FBQUFBLE1BQU0sQ0FBQ0MsT0FBUCxHQUFpQixNQUFNO0FBQUE7QUFBQTtBQUFBO0FBQUE7QUFBTSxDQUE3QiIsInNvdXJjZXNDb250ZW50IjpbIm1vZHVsZS5leHBvcnRzID0gKCkgPT4gXCJraXdpXCI7Il19
`;

exports[`ScriptTransformer uses multiple preprocessors 1`] = `
Expand Down
90 changes: 0 additions & 90 deletions packages/jest-transform/src/__tests__/script_transformer.test.js
Expand Up @@ -535,96 +535,6 @@ describe('ScriptTransformer', () => {
expect(writeFileAtomic.sync).toHaveBeenCalledTimes(1);
});

it('should write a source map for the instrumented file when transformed', () => {
const transformerConfig = {
...config,
transform: [['^.+\\.js$', 'preprocessor-with-sourcemaps']],
};
const scriptTransformer = new ScriptTransformer(transformerConfig);

const map = {
mappings: ';AAAA',
version: 3,
};

// A map from the original source to the instrumented output
/* eslint-disable sort-keys */
const instrumentedCodeMap = {
version: 3,
sources: ['banana.js'],
names: ['content'],
mappings: ';;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;AAAAA,OAAO',
sourcesContent: ['content'],
};
/* eslint-enable */

require('preprocessor-with-sourcemaps').process.mockReturnValue({
code: 'content',
map,
});

const result = scriptTransformer.transform(
'/fruits/banana.js',
makeGlobalConfig({
collectCoverage: true,
}),
);
expect(result.sourceMapPath).toEqual(expect.any(String));
expect(writeFileAtomic.sync).toBeCalledTimes(2);
expect(writeFileAtomic.sync).toBeCalledWith(
result.sourceMapPath,
JSON.stringify(instrumentedCodeMap),
{
encoding: 'utf8',
fsync: false,
},
);

// Inline source map allows debugging of original source when running instrumented code
expect(result.code).toContain('//# sourceMappingURL');
});

it('should write a source map for the instrumented file when not transformed', () => {
const scriptTransformer = new ScriptTransformer(config);

// A map from the original source to the instrumented output
/* eslint-disable sort-keys */
const instrumentedCodeMap = {
version: 3,
sources: ['banana.js'],
names: ['module', 'exports'],
mappings:
';;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;AAAAA,MAAM,CAACC,OAAP,GAAiB,QAAjB',
sourcesContent: ['module.exports = "banana";'],
};
/* eslint-enable */

require('preprocessor-with-sourcemaps').process.mockReturnValue({
code: 'content',
map: null,
});

const result = scriptTransformer.transform(
'/fruits/banana.js',
makeGlobalConfig({
collectCoverage: true,
}),
);
expect(result.sourceMapPath).toEqual(expect.any(String));
expect(writeFileAtomic.sync).toBeCalledTimes(2);
expect(writeFileAtomic.sync).toBeCalledWith(
result.sourceMapPath,
JSON.stringify(instrumentedCodeMap),
{
encoding: 'utf8',
fsync: false,
},
);

// Inline source map allows debugging of original source when running instrumented code
expect(result.code).toContain('//# sourceMappingURL');
});

it('passes expected transform options to getCacheKey', () => {
config = {...config, transform: [['^.+\\.js$', 'test_preprocessor']]};
const scriptTransformer = new ScriptTransformer(config);
Expand Down