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

feat: add support for v8 code coverage #8596

Merged
merged 26 commits into from
Dec 17, 2019
Merged
Show file tree
Hide file tree
Changes from 20 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 TestUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ const DEFAULT_GLOBAL_CONFIG: Config.GlobalConfig = {
collectCoverageFrom: [],
collectCoverageOnlyFrom: null,
coverageDirectory: 'coverage',
coverageProvider: 'babel',
coverageReporters: [],
coverageThreshold: {global: {}},
detectLeaks: false,
Expand Down
9 changes: 9 additions & 0 deletions docs/CLI.md
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,15 @@ Alias: `-c`. The path to a Jest config file specifying how to find and execute t

Alias: `--collectCoverage`. Indicates that test coverage information should be collected and reported in the output. Optionally pass `<boolean>` to override option set in configuration.

### `--coverageProvider=<provider>`

Indicates which provider should be used to instrument code for coverage. Allowed values are `babel` (default) or `v8`.

Note that using `v8` is considered experimental. This uses V8's builtin code coverage rather than one based on Babel and comes with a few caveats

1. Your node version must include `vm.compileFunction`, which was introduced in [node 10.10](https://nodejs.org/dist/latest-v12.x/docs/api/vm.html#vm_vm_compilefunction_code_params_options)
1. Tests needs to run in Node test environment (support for `jsdom` is in the works)
SimenB marked this conversation as resolved.
Show resolved Hide resolved

### `--debug`

Print debugging info about your Jest config.
Expand Down
9 changes: 9 additions & 0 deletions docs/Configuration.md
Original file line number Diff line number Diff line change
Expand Up @@ -185,6 +185,15 @@ An array of regexp pattern strings that are matched against all file paths befor

These pattern strings match against the full path. Use the `<rootDir>` string token to include the path to your project's root directory to prevent it from accidentally ignoring all of your files in different environments that may have different root directories. Example: `["<rootDir>/build/", "<rootDir>/node_modules/"]`.

### `coverageProvider` [string]

Indicates which provider should be used to instrument code for coverage. Allowed values are `babel` (default) or `v8`.

Note that using `v8` is considered experimental. This uses V8's builtin code coverage rather than one based on Babel and comes with a few caveats

1. Your node version must include `vm.compileFunction`, which was introduced in [node 10.10](https://nodejs.org/dist/latest-v12.x/docs/api/vm.html#vm_vm_compilefunction_code_params_options)
1. Tests needs to run in Node test environment (support for `jsdom` is in the works)

### `coverageReporters` [array\<string>]

Default: `["json", "lcov", "text", "clover"]`
Expand Down
1 change: 1 addition & 0 deletions e2e/__tests__/__snapshots__/showConfig.test.ts.snap
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,7 @@ exports[`--showConfig outputs config info and exits 1`] = `
"collectCoverage": false,
"collectCoverageFrom": [],
"coverageDirectory": "<<REPLACED_ROOT_DIR>>/coverage",
"coverageProvider": "babel",
"coverageReporters": [
"json",
"text",
Expand Down
46 changes: 46 additions & 0 deletions e2e/__tests__/v8Coverage.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
/**
* 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 {onNodeVersions} from '@jest/test-utils';
import runJest from '../runJest';

const DIR = path.resolve(__dirname, '../v8-coverage');

onNodeVersions('>=10', () => {
test('prints coverage', () => {
const sourcemapDir = path.join(DIR, 'no-sourcemap');
const {stdout, exitCode} = runJest(
sourcemapDir,
['--coverage', '--coverage-provider', 'v8'],
{
stripAnsi: true,
},
);

expect(exitCode).toBe(0);
expect(
'\n' +
stdout
.split('\n')
.map(s => s.trimRight())
.join('\n') +
'\n',
).toEqual(`
console.log __tests__/Thing.test.js:10
42

----------|---------|----------|---------|---------|-------------------
File | % Stmts | % Branch | % Funcs | % Lines | Uncovered Line #s
----------|---------|----------|---------|---------|-------------------
All files | 100 | 100 | 100 | 100 |
Thing.js | 100 | 100 | 100 | 100 |
x.css | 100 | 100 | 100 | 100 |
----------|---------|----------|---------|---------|-------------------
`);
});
});
10 changes: 10 additions & 0 deletions e2e/v8-coverage/no-sourcemap/Thing.js
Original file line number Diff line number Diff line change
@@ -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.
*/

require('./x.css');

module.exports = 42;
11 changes: 11 additions & 0 deletions e2e/v8-coverage/no-sourcemap/__tests__/Thing.test.js
Original file line number Diff line number Diff line change
@@ -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.
*/

const Thing = require('../Thing');

console.log(Thing);
test.todo('whatever');
11 changes: 11 additions & 0 deletions e2e/v8-coverage/no-sourcemap/cssTransform.js
Original file line number Diff line number Diff line change
@@ -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.
*/

module.exports = {
getCacheKey: () => 'cssTransform',
process: () => 'module.exports = {};',
};
11 changes: 11 additions & 0 deletions e2e/v8-coverage/no-sourcemap/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
{
"name": "no-sourcemap",
"version": "1.0.0",
"jest": {
"testEnvironment": "node",
"transform": {
"^.+\\.[jt]sx?$": "babel-jest",
"^.+\\.css$": "<rootDir>/cssTransform.js"
}
}
}
Empty file.
1 change: 1 addition & 0 deletions jest.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ module.exports = {
transform: {
'^.+\\.[jt]sx?$': '<rootDir>/packages/babel-jest',
},
watchPathIgnorePatterns: ['coverage'],
watchPlugins: [
'jest-watch-typeahead/filename',
'jest-watch-typeahead/testname',
Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@
"isbinaryfile": "^4.0.0",
"istanbul-lib-coverage": "^3.0.0-alpha.1",
"istanbul-lib-report": "^3.0.0-alpha.1",
"istanbul-reports": "^3.0.0-alpha.4",
"istanbul-reports": "^3.0.0-alpha.5",
"jest-junit": "^9.0.0",
"jest-silent-reporter": "^0.1.2",
"jest-snapshot-serializer-raw": "^1.1.0",
Expand Down
5 changes: 5 additions & 0 deletions packages/jest-cli/src/cli/args.ts
Original file line number Diff line number Diff line change
Expand Up @@ -202,6 +202,11 @@ export const options = {
string: true,
type: 'array',
},
coverageProvider: {
choices: ['babel', 'v8'],
default: 'babel',
description: 'Select between Babel and V8 to collect coverage',
},
coverageReporters: {
description:
'A list of reporter names that Jest uses when writing ' +
Expand Down
1 change: 1 addition & 0 deletions packages/jest-config/src/Defaults.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ const defaultOptions: Config.DefaultOptions = {
clearMocks: false,
collectCoverage: false,
coveragePathIgnorePatterns: [NODE_MODULES_REGEXP],
coverageProvider: 'babel',
coverageReporters: ['json', 'text', 'lcov', 'clover'],
errorOnDeprecated: false,
expand: false,
Expand Down
1 change: 1 addition & 0 deletions packages/jest-config/src/ValidConfig.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ const initialOptions: Config.InitialOptions = {
},
coverageDirectory: 'coverage',
coveragePathIgnorePatterns: [NODE_MODULES_REGEXP],
coverageProvider: 'v8',
coverageReporters: ['json', 'text', 'lcov', 'clover'],
coverageThreshold: {
global: {
Expand Down
1 change: 1 addition & 0 deletions packages/jest-config/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,7 @@ const groupOptions = (
collectCoverageFrom: options.collectCoverageFrom,
collectCoverageOnlyFrom: options.collectCoverageOnlyFrom,
coverageDirectory: options.coverageDirectory,
coverageProvider: options.coverageProvider,
coverageReporters: options.coverageReporters,
coverageThreshold: options.coverageThreshold,
detectLeaks: options.detectLeaks,
Expand Down
1 change: 1 addition & 0 deletions packages/jest-config/src/normalize.ts
Original file line number Diff line number Diff line change
Expand Up @@ -836,6 +836,7 @@ export default function normalize(
case 'changedFilesWithAncestor':
case 'clearMocks':
case 'collectCoverage':
case 'coverageProvider':
case 'coverageReporters':
case 'coverageThreshold':
case 'detectLeaks':
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@ exports[`prints the config object 1`] = `
"collectCoverageFrom": [],
"collectCoverageOnlyFrom": null,
"coverageDirectory": "coverage",
"coverageProvider": "babel",
"coverageReporters": [],
"coverageThreshold": {
"global": {}
Expand Down
3 changes: 3 additions & 0 deletions packages/jest-coverage/.npmignore
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
**/__mocks__/**
**/__tests__/**
src
22 changes: 22 additions & 0 deletions packages/jest-coverage/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
{
"name": "@jest/coverage",
Copy link
Member Author

Choose a reason for hiding this comment

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

this module does not (and should not) have to live here. Maybe we can make it part of istanbul-lib-instrument? All it does is start and stop the coverage collection using the v8 inspector API.

/cc @bcoe @coreyfarrell

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't have time to really look at this currently but I wouldn't object to creating a new module under the istanbuljs umbrella to accomplish this. Maybe @istanbuljs/v8-coverage would be a good name? I think it should be a separate module from istanbul-lib-instrument which pulls in a bunch of babel modules.

FWIW under the istanbuljs org I would not want TS source code but I don't have a problem with having the module include an index.d.ts with public definitions.

Copy link
Contributor

Choose a reason for hiding this comment

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

👍 I honestly have less concerns about TypeScript, since I've been using it at work these days; would be happy to put up my hand and help take on maintenance burden for this one.

Copy link
Member Author

Choose a reason for hiding this comment

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

Feel free to take the code from this. Or create the repo and I can push it there.

If you want a separate d.ts. file, here's the built definition:

/// <reference types="node" />
import { Profiler } from 'inspector';
export declare type V8Coverage = ReadonlyArray<Profiler.ScriptCoverage>;
export default class CoverageInstrumenter {
    private readonly session;
    startInstrumenting(): Promise<void>;
    stopInstrumenting(): Promise<V8Coverage>;
}

Copy link
Member Author

Choose a reason for hiding this comment

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

"version": "24.9.0",
"repository": {
"type": "git",
"url": "https://github.com/facebook/jest.git",
"directory": "packages/jest-coverage"
},
"license": "MIT",
"main": "build/index.js",
"types": "build/index.d.ts",
"devDependencies": {
"@types/node": "*"
},
"engines": {
"node": ">= 8"
},
"publishConfig": {
"access": "public"
},
"gitHead": "0efb1d7809cb96ae87a7601e7802f1dab3774280"
}
76 changes: 76 additions & 0 deletions packages/jest-coverage/src/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@
/**
* 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 {Profiler, Session} from 'inspector';

export type V8Coverage = ReadonlyArray<Profiler.ScriptCoverage>;

export class CoverageInstrumenter {
private readonly session = new Session();

async startInstrumenting() {
this.session.connect();

await new Promise((resolve, reject) => {
Copy link
Member Author

Choose a reason for hiding this comment

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

I lost type info if I used util.promisify, so I just wrapped everything in promises manually

Copy link
Contributor

Choose a reason for hiding this comment

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

This is an interesting problem to solve. It's possible to strongly type the function returned by util.promisify by using some of the techniques described in this issue or this SO answer (kudos to their authors btw, ingenious ideas).

I tried this myself after reading both of the links above and managed to get it working this way:

  1. Create a conditional type which will be the union type of the variadic arguments.

    export type GetOverloadArgs<T> =
    T extends {
        (...o: infer U) : void,
        (...o: infer U2) : void,
        (...o: infer U3) : void,
        (...o: infer U4) : void,
        (...o: infer U5) : void,
        (...o: infer U6) : void,
        (...o: infer U7) : void
    } ? U | U2 | U3 | U4 | U5 | U6 | U7:
    T extends {
        (...o: infer U) : void,
        (...o: infer U2) : void,
        (...o: infer U3) : void,
        (...o: infer U4) : void,
        (...o: infer U5) : void,
        (...o: infer U6) : void,
    } ? U | U2 | U3 | U4 | U5 | U6:
    T extends {
        (...o: infer U) : void,
        (...o: infer U2) : void,
        (...o: infer U3) : void,
        (...o: infer U4) : void,
        (...o: infer U5) : void,
    } ? U | U2 | U3 | U4 | U5:
    T extends {
        (...o: infer U) : void,
        (...o: infer U2) : void,
        (...o: infer U3) : void,
        (...o: infer U4) : void,
    } ? U | U2 | U3 | U4 :
    T extends {
        (...o: infer U) : void,
        (...o: infer U2) : void,
        (...o: infer U3) : void,
    } ? U | U2 | U3 :
    T extends {
        (...o: infer U) : void,
        (...o: infer U2) : void,
    } ? U | U2 :
    T extends {
        (...o: infer U) : void,
    } ? U :
    never;
  2. Add type aliases for the Callbacks and the promisify function itself.
    PromisifyOne is just a conditional type which depends on the number of arguments passed (as they're variadic) and the Callback's generic type for reply.
    This makes sure that the return type will have the same type as the Callbacks second argument.

      export type Callback<T> = (err: Error | null, reply: T) => void;
    
      export type Promisify<T extends any[]> =
          T extends [Callback<infer U>?] ? () => Promise<U> :
          T extends [infer T1, Callback<infer U>?] ? (arg1: T1) => Promise<U> :
          T extends [infer T1, infer T2, Callback<infer U>?] ? (arg1: T1, arg2: T2) => Promise<U> :
          T extends [infer T1, infer T2, infer T3, Callback<infer U>?]? (arg1: T1, arg2: T2, arg3: T3) => Promise<U> :
          T extends [infer T1, infer T2, infer T3, infer T4, Callback<infer U>?] ? (arg1: T1, arg2: T2, arg3: T3, arg4: T4) => Promise<U> :
          never;
  3. Use the type of util.promisify's argument to obtain it's return type.

    declare module 'util' {
        function promisify<T>(fn: T): Promisify<T>;
    }

With the tree snippets above you can get the type info for func correctly in the example below:

import { promisify } from 'util';

const asyncWithCallback = (s: string, callback: (error: Error | null, r: number) => void): void => {
    let err = null;
    if (s === "err") err = new Error();
    callback(err, 10);
}

const func = promisify(asyncWithCallback);
func("not an error");
// `:YcmCompleter GetType` (which uses TSServer) gives me:
// const func: (arg1: string) => Promise<number>

I'm not sure why the other examples had to use UnionToIntersection to achieve that so I might have missed something there.

I'm also not sure if the tradeoff between having these complex type definitions versus manually wrapping these in promises is worth it.

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks for looking into it! I don't think it's worth the complexity here, but maybe you could upstream those changes to DT? The definitions are here: https://github.com/DefinitelyTyped/DefinitelyTyped/blob/796b838a15fad73287bad7a88707a9ca04e60640/types/node/util.d.ts#L86-L105

Copy link
Contributor

Choose a reason for hiding this comment

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

I completely agree, I think this actually belongs to DT's codebase since it's essentially covering what the type defs should be there.

I'll have a look at that, thanks @SimenB 😊

this.session.post('Profiler.enable', err => {
if (err) {
reject(err);
} else {
resolve();
}
});
});

await new Promise((resolve, reject) => {
this.session.post(
'Profiler.startPreciseCoverage',
{callCount: true, detailed: true},
err => {
if (err) {
reject(err);
} else {
resolve();
}
},
);
});
}

async stopInstrumenting(): Promise<V8Coverage> {
const preciseCoverage = await new Promise<V8Coverage>((resolve, reject) => {
this.session.post('Profiler.takePreciseCoverage', (err, res) => {
if (err) {
reject(err);
} else {
resolve(res.result);
}
});
});

await new Promise((resolve, reject) => {
this.session.post('Profiler.stopPreciseCoverage', err => {
if (err) {
reject(err);
} else {
resolve();
}
});
});

await new Promise((resolve, reject) => {
this.session.post('Profiler.disable', err => {
if (err) {
reject(err);
} else {
resolve();
}
});
});

return preciseCoverage;
}
}
7 changes: 7 additions & 0 deletions packages/jest-coverage/tsconfig.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"extends": "../../tsconfig.json",
"compilerOptions": {
"rootDir": "src",
"outDir": "build"
}
}
7 changes: 5 additions & 2 deletions packages/jest-reporters/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,9 @@
"main": "build/index.js",
"types": "build/index.d.ts",
"dependencies": {
"@bcoe/v8-coverage": "^0.2.3",
Copy link
Contributor

Choose a reason for hiding this comment

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

Perhaps we could move this to "Istanbul/merge-v8-coverage" or "merge-v8-coverage", @demurgos do you have a preference? Would you like to work on this library within the Istanbul organization?

Copy link

Choose a reason for hiding this comment

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

Hi,
As discussed in the v8-coverage repo, I am working on a 1.0.0 release of the lib. I agree that it would make sense to move it the istanbul org.

SimenB marked this conversation as resolved.
Show resolved Hide resolved
"@jest/console": "^24.9.0",
"@jest/coverage": "^24.9.0",
"@jest/environment": "^24.9.0",
"@jest/test-result": "^24.9.0",
"@jest/transform": "^24.9.0",
Expand All @@ -17,7 +19,7 @@
"istanbul-lib-instrument": "^4.0.0-alpha.2",
"istanbul-lib-report": "^3.0.0-alpha.1",
"istanbul-lib-source-maps": "^4.0.0-alpha.4",
"istanbul-reports": "^3.0.0-alpha.4",
"istanbul-reports": "^3.0.0-alpha.5",
"jest-haste-map": "^24.9.0",
"jest-resolve": "^24.9.0",
"jest-runtime": "^24.9.0",
Expand All @@ -26,7 +28,8 @@
"slash": "^3.0.0",
"source-map": "^0.6.0",
"string-length": "^3.1.0",
"terminal-link": "^2.0.0"
"terminal-link": "^2.0.0",
"v8-to-istanbul": "^4.0.0"
},
"devDependencies": {
"@types/exit": "^0.1.30",
Expand Down