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

chore: remove checks for compileFunction #9949

Merged
merged 1 commit into from
May 2, 2020
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
chore: remove checks for compileFunction
SimenB committed May 2, 2020

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
commit 9f0f3277763806fec861d605e06413b949c8c4af
6 changes: 1 addition & 5 deletions docs/CLI.md
Original file line number Diff line number Diff line change
@@ -156,11 +156,7 @@ Alias: `--collectCoverage`. Indicates that test coverage information should be c

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` requires [`jest-environment-jsdom-sixteen`](https://www.npmjs.com/package/jest-environment-jsdom-sixteen))
1. V8 has way better data in the later versions, so using the latest versions of node (v13 at the time of this writing) will yield better results
Note that using `v8` is considered experimental. This uses V8's builtin code coverage rather than one based on Babel. It is not as well tested, and it has also improved in the last few releases of Node. Using the latest versions of node (v14 at the time of this writing) will yield better results.

### `--debug`

6 changes: 1 addition & 5 deletions docs/Configuration.md
Original file line number Diff line number Diff line change
@@ -183,11 +183,7 @@ These pattern strings match against the full path. Use the `<rootDir>` string to

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` requires [`jest-environment-jsdom-sixteen`](https://www.npmjs.com/package/jest-environment-jsdom-sixteen))
1. V8 has way better data in the later versions, so using the latest versions of node (v13 at the time of this writing) will yield better results
Note that using `v8` is considered experimental. This uses V8's builtin code coverage rather than one based on Babel. It is not as well tested, and it has also improved in the last few releases of Node. Using the latest versions of node (v14 at the time of this writing) will yield better results.

### `coverageReporters` [array\<string | [string,any]>]

12 changes: 12 additions & 0 deletions e2e/__tests__/__snapshots__/globals.test.ts.snap
Original file line number Diff line number Diff line change
@@ -18,6 +18,18 @@ Ran all test suites.
`;
exports[`cannot have describe with no implementation 1`] = `
FAIL __tests__/onlyConstructs.test.js
● Test suite failed to run
Missing second argument. It must be a callback function.
> 1 | describe('describe, no implementation');
| ^
at Object.describe (__tests__/onlyConstructs.test.js:1:1)
`;
exports[`cannot have describe with no implementation 2`] = `
Test Suites: 1 failed, 1 total
Tests: 0 total
Snapshots: 0 total
37 changes: 1 addition & 36 deletions e2e/__tests__/globals.test.ts
Original file line number Diff line number Diff line change
@@ -7,7 +7,6 @@

import * as path from 'path';
import {tmpdir} from 'os';
import {compileFunction} from 'vm';
import {wrap} from 'jest-snapshot-serializer-raw';
import runJest from '../runJest';
import {
@@ -127,41 +126,7 @@ test('cannot have describe with no implementation', () => {
const rest = cleanStderr(stderr);
const {summary} = extractSummary(stderr);

const rightTrimmedRest = rest
.split('\n')
.map(l => l.trimRight())
.join('\n')
.trim();

if (typeof compileFunction === 'function') {
expect(rightTrimmedRest).toEqual(
`
FAIL __tests__/onlyConstructs.test.js
● Test suite failed to run
Missing second argument. It must be a callback function.
> 1 | describe('describe, no implementation');
| ^
at Object.describe (__tests__/onlyConstructs.test.js:1:1)
`.trim(),
);
} else {
expect(rightTrimmedRest).toEqual(
`
FAIL __tests__/onlyConstructs.test.js
● Test suite failed to run
Missing second argument. It must be a callback function.
> 1 | describe('describe, no implementation');
| ^
at Object.<anonymous> (__tests__/onlyConstructs.test.js:1:10)
`.trim(),
);
}
expect(wrap(rest)).toMatchSnapshot();
expect(wrap(summary)).toMatchSnapshot();
});

6 changes: 2 additions & 4 deletions packages/jest-runner/src/runTest.ts
Original file line number Diff line number Diff line change
@@ -6,7 +6,6 @@
*
*/

import {compileFunction} from 'vm';
import type {Config} from '@jest/types';
import type {TestResult} from '@jest/test-result';
import {
@@ -226,11 +225,10 @@ async function runTestInternal(
};
}

// if we don't have `getVmContext` on the env,or `compileFunction` available skip coverage
// if we don't have `getVmContext` on the env skip coverage
const collectV8Coverage =
globalConfig.coverageProvider === 'v8' &&
typeof environment.getVmContext === 'function' &&
typeof compileFunction === 'function';
typeof environment.getVmContext === 'function';

try {
await environment.setup();
36 changes: 11 additions & 25 deletions packages/jest-runtime/src/index.ts
Original file line number Diff line number Diff line change
@@ -982,31 +982,17 @@ class Runtime {
const vmContext = this._environment.getVmContext();

if (vmContext) {
if (typeof compileFunction === 'function') {
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've intentionally kept the check on line 981 so people can still use runScript if they really want to

try {
compiledFunction = compileFunction(
transformedCode,
this.constructInjectedModuleParameters(),
{
filename,
parsingContext: vmContext,
},
) as ModuleWrapper;
} catch (e) {
throw handlePotentialSyntaxError(e);
}
} else {
const script = this.createScriptFromCode(transformedCode, filename);

const runScript = script.runInContext(
vmContext,
) as RunScriptEvalResult;

if (runScript === null) {
compiledFunction = null;
} else {
compiledFunction = runScript[EVAL_RESULT_VARIABLE];
}
try {
compiledFunction = compileFunction(
transformedCode,
this.constructInjectedModuleParameters(),
{
filename,
parsingContext: vmContext,
},
) as ModuleWrapper;
} catch (e) {
throw handlePotentialSyntaxError(e);
}
}
} else {