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.

Sign up for GitHub

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

Jump to bottom

chore: remove checks for compileFunction #9949

Merged
merged 1 commit into from May 2, 2020
Merged

chore: remove checks for compileFunction #9949

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
6 changes: 1 addition & 5 deletions docs/CLI.md
View file
Open in desktop
Expand Up @@ -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`

Expand Down
6 changes: 1 addition & 5 deletions docs/Configuration.md
View file
Open in desktop
Expand Up @@ -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]>]

Expand Down
12 changes: 12 additions & 0 deletions e2e/__tests__/__snapshots__/globals.test.ts.snap
View file
Open in desktop
Expand Up @@ -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
Expand Down
37 changes: 1 addition & 36 deletions e2e/__tests__/globals.test.ts
View file
Open in desktop
Expand Up @@ -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 {
Expand Down Expand Up @@ -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();
});

Expand Down
6 changes: 2 additions & 4 deletions packages/jest-runner/src/runTest.ts
View file
Open in desktop
Expand Up @@ -6,7 +6,6 @@
*
*/

import {compileFunction} from 'vm';
import type {Config} from '@jest/types';
import type {TestResult} from '@jest/test-result';
import {
Expand Down Expand Up @@ -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();
Expand Down
36 changes: 11 additions & 25 deletions packages/jest-runtime/src/index.ts
View file
Open in desktop
Expand Up @@ -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 {
Expand Down