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

Improve source map handling when instrumenting transformed code (#5739) #9811

Merged
merged 15 commits into from Apr 23, 2020
Merged
Show file tree
Hide file tree
Changes from 14 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
1 change: 1 addition & 0 deletions CHANGELOG.md
Expand Up @@ -9,6 +9,7 @@

- `[jest-runtime]` Support importing CJS from ESM using `import` statements ([#9850](https://github.com/facebook/jest/pull/9850))
- `[jest-runtime]` Support importing parallel dynamic `import`s ([#9858](https://github.com/facebook/jest/pull/9858))
- `[jest-transform]` Improve source map handling when instrumenting transformed code ([#9811](https://github.com/facebook/jest/pull/9811))

### Chore & Maintenance

Expand Down
@@ -0,0 +1,20 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`processes stack traces and code frames with source maps with coverage 1`] = `
FAIL __tests__/fails.ts
✕ fails

● fails

This did not work!

11 |
12 | export function error() {
> 13 | throw new Error('This did not work!');
| ^
14 | }
15 |

at Object.error (lib.ts:13:9)
at Object.<anonymous> (__tests__/fails.ts:10:3)
`;
Copy link
Member

Choose a reason for hiding this comment

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

For posterity (if we ever come back to this and wonder why we pass sourcemap into Babel), this is the error if running this test without the other changes in this PR

image

22 changes: 22 additions & 0 deletions e2e/__tests__/stackTraceSourceMapsWithCoverage.test.ts
@@ -0,0 +1,22 @@
/**
* 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 {extractSummary, run} from '../Utils';
import runJest from '../runJest';
import wrap from 'jest-snapshot-serializer-raw';
SimenB marked this conversation as resolved.
Show resolved Hide resolved

it('processes stack traces and code frames with source maps with coverage', () => {
const dir = path.resolve(
__dirname,
'../stack-trace-source-maps-with-coverage',
);
run('yarn', dir);
const {stderr} = runJest(dir, ['--no-cache', '--coverage']);

// Should report an error at source line 13 in lib.ts at line 10 of the test
expect(wrap(extractSummary(stderr).rest)).toMatchSnapshot();
});
11 changes: 11 additions & 0 deletions e2e/stack-trace-source-maps-with-coverage/__tests__/fails.ts
@@ -0,0 +1,11 @@
/**
* 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 {error} from '../lib';

it('fails', () => {
error();
});
14 changes: 14 additions & 0 deletions e2e/stack-trace-source-maps-with-coverage/lib.ts
@@ -0,0 +1,14 @@
/**
* 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.
*/
interface NotUsedButTakesUpLines {
a: number;
b: string;
}

export function error() {
throw new Error('This did not work!');
}
13 changes: 13 additions & 0 deletions e2e/stack-trace-source-maps-with-coverage/package.json
@@ -0,0 +1,13 @@
{
"jest": {
"rootDir": "./",
"transform": {
"^.+\\.(ts)$": "<rootDir>/preprocessor.js"
},
"testEnvironment": "node",
"testRegex": "fails"
},
"dependencies": {
"typescript": "^3.7.4"
}
}
21 changes: 21 additions & 0 deletions e2e/stack-trace-source-maps-with-coverage/preprocessor.js
@@ -0,0 +1,21 @@
/**
* 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.
*/

const tsc = require('typescript');

module.exports = {
process(src, path) {
return tsc.transpileModule(src, {
compilerOptions: {
inlineSourceMap: true,
module: tsc.ModuleKind.CommonJS,
target: 'es5',
},
fileName: path,
}).outputText;
},
};
8 changes: 8 additions & 0 deletions e2e/stack-trace-source-maps-with-coverage/yarn.lock
@@ -0,0 +1,8 @@
# THIS IS AN AUTOGENERATED FILE. DO NOT EDIT THIS FILE DIRECTLY.
# yarn lockfile v1


typescript@^3.7.4:
version "3.8.3"
resolved "https://registry.yarnpkg.com/typescript/-/typescript-3.8.3.tgz#409eb8544ea0335711205869ec458ab109ee1061"
integrity sha512-MYlEfn5VrLNsgudQTVJeNaQFUAI7DkhnOjdpAp4T+ku1TfQClewlbSuTVHiA+8skNBgaf02TL/kLOvig4y3G8w==
29 changes: 0 additions & 29 deletions packages/jest-reporters/src/coverage_reporter.ts
Expand Up @@ -71,28 +71,6 @@ export default class CoverageReporter extends BaseReporter {
if (testResult.coverage) {
this._coverageMap.merge(testResult.coverage);
}

const sourceMaps = testResult.sourceMaps;
if (sourceMaps) {
Object.keys(sourceMaps).forEach(sourcePath => {
let inputSourceMap: RawSourceMap | undefined;
try {
const coverage: istanbulCoverage.FileCoverage = this._coverageMap.fileCoverageFor(
sourcePath,
);
inputSourceMap = (coverage.toJSON() as any).inputSourceMap;
} finally {
if (inputSourceMap) {
this._sourceMapStore.registerMap(sourcePath, inputSourceMap);
} else {
this._sourceMapStore.registerURL(
sourcePath,
sourceMaps[sourcePath],
);
}
}
});
}
}

async onRunComplete(
Expand Down Expand Up @@ -215,13 +193,6 @@ export default class CoverageReporter extends BaseReporter {
]);
} else {
this._coverageMap.addFileCoverage(result.coverage);

if (result.sourceMapPath) {
this._sourceMapStore.registerURL(
filename,
result.sourceMapPath,
);
}
}
}
} catch (error) {
Expand Down
10 changes: 5 additions & 5 deletions packages/jest-reporters/src/generateEmptyCoverage.ts
Expand Up @@ -18,7 +18,6 @@ export type CoverageWorkerResult =
| {
kind: 'BabelCoverage';
coverage: FileCoverage;
sourceMapPath?: string | null;
}
| {
kind: 'V8Coverage';
Expand Down Expand Up @@ -66,17 +65,18 @@ export default function (
}

// Transform file with instrumentation to make sure initial coverage data is well mapped to original code.
const {code, mapCoverage, sourceMapPath} = new ScriptTransformer(
config,
).transformSource(filename, source, true);
const {code} = new ScriptTransformer(config).transformSource(
filename,
source,
true,
);
// TODO: consider passing AST
const extracted = readInitialCoverage(code);
// Check extracted initial coverage is not null, this can happen when using /* istanbul ignore file */
if (extracted) {
coverageWorkerResult = {
coverage: createFileCoverage(extracted.coverageData),
kind: 'BabelCoverage',
sourceMapPath: mapCoverage ? sourceMapPath : null,
};
}
}
Expand Down
1 change: 0 additions & 1 deletion packages/jest-runner/src/runTest.ts
Expand Up @@ -275,7 +275,6 @@ async function runTestInternal(
const coverageKeys = Object.keys(coverage);
if (coverageKeys.length) {
result.coverage = coverage;
result.sourceMaps = runtime.getSourceMapInfo(new Set(coverageKeys));
}
}

Expand Down
22 changes: 3 additions & 19 deletions packages/jest-runtime/src/index.ts
Expand Up @@ -140,7 +140,6 @@ class Runtime {
private _isolatedModuleRegistry: ModuleRegistry | null;
private _moduleRegistry: ModuleRegistry;
private _esmoduleRegistry: Map<string, Promise<VMModule>>;
private _needsCoverageMapped: Set<string>;
private _resolver: Resolver;
private _shouldAutoMock: boolean;
private _shouldMockModuleCache: BooleanObject;
Expand Down Expand Up @@ -186,7 +185,6 @@ class Runtime {
this._isolatedMockRegistry = null;
this._moduleRegistry = new Map();
this._esmoduleRegistry = new Map();
this._needsCoverageMapped = new Set();
this._resolver = resolver;
this._scriptTransformer = new ScriptTransformer(config);
this._shouldAutoMock = config.automock;
Expand Down Expand Up @@ -794,20 +792,9 @@ class Runtime {
});
}

getSourceMapInfo(coveredFiles: Set<string>): Record<string, string> {
return Object.keys(this._sourceMapRegistry).reduce<Record<string, string>>(
(result, sourcePath) => {
if (
coveredFiles.has(sourcePath) &&
this._needsCoverageMapped.has(sourcePath) &&
fs.existsSync(this._sourceMapRegistry[sourcePath])
) {
result[sourcePath] = this._sourceMapRegistry[sourcePath];
}
return result;
},
{},
);
// TODO - remove in Jest 26
getSourceMapInfo(_coveredFiles: Set<string>): Record<string, string> {
return {};
}

getSourceMaps(): SourceMapRegistry {
Expand Down Expand Up @@ -1056,9 +1043,6 @@ class Runtime {

if (transformedFile.sourceMapPath) {
this._sourceMapRegistry[filename] = transformedFile.sourceMapPath;
if (transformedFile.mapCoverage) {
this._needsCoverageMapped.add(filename);
}
}
return transformedFile;
}
Expand Down
1 change: 1 addition & 0 deletions packages/jest-test-result/src/types.ts
Expand Up @@ -104,6 +104,7 @@ export type TestResult = {
unmatched: number;
updated: number;
};
// TODO - Remove in Jest 26
sourceMaps?: {
SimenB marked this conversation as resolved.
Show resolved Hide resolved
[sourcePath: string]: string;
};
Expand Down