Skip to content

Commit

Permalink
chore: drop usage of compileFunction (#10586)
Browse files Browse the repository at this point in the history
  • Loading branch information
SimenB authored Oct 5, 2020
1 parent 97e683b commit a2090a0
Show file tree
Hide file tree
Showing 11 changed files with 61 additions and 69 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
- `[jest-circus, jest-jasmine2]` Find correct location for `test.each` tests ([#10413](https://github.com/facebook/jest/pull/10413))
- `[jest-console]` Add `Console` constructor to `console` object ([#10502](https://github.com/facebook/jest/pull/10502))
- `[jest-globals]` Fix lifecycle hook function types ([#10480](https://github.com/facebook/jest/pull/10480))
- `[jest-runtime]` Remove usage of `vm.compileFunction` due to a performance issue ([#10586](https://github.com/facebook/jest/pull/10586))

### Chore & Maintenance

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,6 @@ exports[`prints console.logs when run with forceExit 3`] = `
console.log
Hey
at Object.log (__tests__/a-banana.js:1:30)
at Object.<anonymous> (__tests__/a-banana.js:1:1)
`;
4 changes: 2 additions & 2 deletions e2e/__tests__/__snapshots__/globals.test.ts.snap
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,9 @@ FAIL __tests__/onlyConstructs.test.js
Missing second argument. It must be a callback function.
> 1 | describe('describe, no implementation');
| ^
| ^
at Object.describe (__tests__/onlyConstructs.test.js:1:1)
at Object.<anonymous> (__tests__/onlyConstructs.test.js:1:10)
`;
exports[`cannot have describe with no implementation 2`] = `
Expand Down
16 changes: 15 additions & 1 deletion e2e/__tests__/consoleLogOutputWhenRunInBand.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@ const DIR = path.resolve(__dirname, '../console-log-output-when-run-in-band');
beforeEach(() => cleanup(DIR));
afterAll(() => cleanup(DIR));

const nodeMajorVersion = Number(process.versions.node.split('.')[0]);

test('prints console.logs when run with forceExit', () => {
writeFiles(DIR, {
'__tests__/a-banana.js': `
Expand All @@ -23,14 +25,26 @@ test('prints console.logs when run with forceExit', () => {
'package.json': '{}',
});

const {stderr, stdout, exitCode} = runJest(DIR, [
const {stderr, exitCode, ...res} = runJest(DIR, [
'-i',
'--ci=false',
'--forceExit',
]);
let {stdout} = res;

const {rest, summary} = extractSummary(stderr);

if (nodeMajorVersion < 12) {
expect(stdout).toContain(
'at Object.<anonymous>.test (__tests__/a-banana.js:1:1)',
);

stdout = stdout.replace(
'at Object.<anonymous>.test (__tests__/a-banana.js:1:1)',
'at Object.<anonymous> (__tests__/a-banana.js:1:1)',
);
}

expect(exitCode).toBe(0);
expect(wrap(rest)).toMatchSnapshot();
expect(wrap(summary)).toMatchSnapshot();
Expand Down
24 changes: 9 additions & 15 deletions e2e/__tests__/errorOnDeprecated.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,24 +43,18 @@ testFiles.forEach(testFile => {
expect(result.exitCode).toBe(1);
let {rest} = extractSummary(result.stderr);

if (testFile === 'defaultTimeoutInterval.test.js') {
if (
nodeMajorVersion < 12 &&
testFile === 'defaultTimeoutInterval.test.js'
) {
const lineEntry = '(__tests__/defaultTimeoutInterval.test.js:10:3)';

if (nodeMajorVersion < 10) {
expect(rest).toContain(`at Object.<anonymous>.test ${lineEntry}`);
expect(rest).toContain(`at Object.<anonymous>.test ${lineEntry}`);

rest = rest.replace(
`at Object.<anonymous>.test ${lineEntry}`,
`at Object.<anonymous> ${lineEntry}`,
);
} else if (nodeMajorVersion < 12) {
expect(rest).toContain(`at Object.test ${lineEntry}`);

rest = rest.replace(
`at Object.test ${lineEntry}`,
`at Object.<anonymous> ${lineEntry}`,
);
}
rest = rest.replace(
`at Object.<anonymous>.test ${lineEntry}`,
`at Object.<anonymous> ${lineEntry}`,
);
}

expect(wrap(rest)).toMatchSnapshot();
Expand Down
11 changes: 1 addition & 10 deletions e2e/__tests__/failures.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ test('not throwing Error objects', () => {
expect(wrap(cleanStderr(stderr))).toMatchSnapshot();
stderr = runJest(dir, ['duringTests.test.js']).stderr;

if (nodeMajorVersion < 10) {
if (nodeMajorVersion < 12) {
const lineEntry = '(__tests__/duringTests.test.js:38:8)';

expect(stderr).toContain(`at Object.<anonymous>.done ${lineEntry}`);
Expand All @@ -48,15 +48,6 @@ test('not throwing Error objects', () => {
`at Object.<anonymous>.done ${lineEntry}`,
`at Object.<anonymous> ${lineEntry}`,
);
} else if (nodeMajorVersion < 12) {
const lineEntry = '(__tests__/duringTests.test.js:38:8)';

expect(stderr).toContain(`at Object.done ${lineEntry}`);

stderr = stderr.replace(
`at Object.done ${lineEntry}`,
`at Object.<anonymous> ${lineEntry}`,
);
}

expect(wrap(cleanStderr(stderr))).toMatchSnapshot();
Expand Down
6 changes: 3 additions & 3 deletions packages/jest-reporters/src/coverage_reporter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import * as fs from 'graceful-fs';
import type {Config} from '@jest/types';
import type {
AggregatedResult,
RuntimeTransformResult,
TestResult,
V8CoverageResult,
} from '@jest/test-result';
Expand All @@ -24,7 +25,6 @@ import Worker from 'jest-worker';
import glob = require('glob');
import v8toIstanbul = require('v8-to-istanbul');
import type {RawSourceMap} from 'source-map';
import type {TransformResult} from '@jest/transform';
import BaseReporter from './base_reporter';
import type {
Context,
Expand Down Expand Up @@ -425,7 +425,7 @@ export default class CoverageReporter extends BaseReporter {
this._v8CoverageResults.map(cov => ({result: cov.map(r => r.result)})),
);

const fileTransforms = new Map<string, TransformResult>();
const fileTransforms = new Map<string, RuntimeTransformResult>();

this._v8CoverageResults.forEach(res =>
res.forEach(r => {
Expand Down Expand Up @@ -453,7 +453,7 @@ export default class CoverageReporter extends BaseReporter {

const converter = v8toIstanbul(
res.url,
0,
fileTransform?.wrapperLength ?? 0,
fileTransform && sourcemapContent
? {
originalSource: fileTransform.originalCode,
Expand Down
56 changes: 21 additions & 35 deletions packages/jest-runtime/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ import {
Context as VMContext,
// @ts-expect-error: experimental, not added to the types
Module as VMModule,
compileFunction,
} from 'vm';
import * as nativeModule from 'module';
import type {Config, Global} from '@jest/types';
Expand All @@ -35,12 +34,11 @@ import {escapePathForRegex} from 'jest-regex-util';
import {
ScriptTransformer,
ShouldInstrumentOptions,
TransformResult,
TransformationOptions,
handlePotentialSyntaxError,
shouldInstrument,
} from '@jest/transform';
import type {V8CoverageResult} from '@jest/test-result';
import type {RuntimeTransformResult, V8CoverageResult} from '@jest/test-result';
import {CoverageInstrumenter, V8Coverage} from 'collect-v8-coverage';
import * as fs from 'graceful-fs';
import jestMock = require('jest-mock');
Expand Down Expand Up @@ -170,7 +168,7 @@ class Runtime {
private _shouldUnmockTransitiveDependenciesCache: BooleanMap;
private _sourceMapRegistry: StringMap;
private _scriptTransformer: ScriptTransformer;
private _fileTransforms: Map<string, TransformResult>;
private _fileTransforms: Map<string, RuntimeTransformResult>;
private _v8CoverageInstrumenter: CoverageInstrumenter | undefined;
private _v8CoverageResult: V8Coverage | undefined;
private _transitiveShouldMock: BooleanMap;
Expand Down Expand Up @@ -830,7 +828,7 @@ class Runtime {
});
}

// TODO - remove in Jest 26
// TODO - remove in Jest 27
getSourceMapInfo(_coveredFiles: Set<string>): Record<string, string> {
return {};
}
Expand Down Expand Up @@ -1005,36 +1003,23 @@ class Runtime {

let compiledFunction: ModuleWrapper | null = null;

const script = this.createScriptFromCode(transformedCode, filename);

let runScript: RunScriptEvalResult | null = null;

// Use this if available instead of deprecated `JestEnvironment.runScript`
if (typeof this._environment.getVmContext === 'function') {
const vmContext = this._environment.getVmContext();

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

const runScript = this._environment.runScript<RunScriptEvalResult>(
script,
);
runScript = this._environment.runScript<RunScriptEvalResult>(script);
}

if (runScript === null) {
compiledFunction = null;
} else {
compiledFunction = runScript[EVAL_RESULT_VARIABLE];
}
if (runScript !== null) {
compiledFunction = runScript[EVAL_RESULT_VARIABLE];
}

if (compiledFunction === null) {
Expand Down Expand Up @@ -1097,7 +1082,10 @@ class Runtime {
source,
);

this._fileTransforms.set(filename, transformedFile);
this._fileTransforms.set(filename, {
...transformedFile,
wrapperLength: this.constructModuleWrapperStart().length,
});

if (transformedFile.sourceMapPath) {
this._sourceMapRegistry.set(filename, transformedFile.sourceMapPath);
Expand Down Expand Up @@ -1602,15 +1590,13 @@ class Runtime {
}

private wrapCodeInModuleWrapper(content: string) {
return this.constructModuleWrapperStart() + content + '\n}});';
}

private constructModuleWrapperStart() {
const args = this.constructInjectedModuleParameters();

return (
'({"' +
EVAL_RESULT_VARIABLE +
`":function(${args.join(',')}){` +
content +
'\n}});'
);
return '({"' + EVAL_RESULT_VARIABLE + `":function(${args.join(',')}){`;
}

private constructInjectedModuleParameters(): Array<string> {
Expand Down
1 change: 1 addition & 0 deletions packages/jest-test-result/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ export type {
FailedAssertion,
FormattedTestResults,
Milliseconds,
RuntimeTransformResult,
SerializableError,
SnapshotSummary,
Status,
Expand Down
7 changes: 6 additions & 1 deletion packages/jest-test-result/src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,13 @@ import type {ConsoleBuffer} from '@jest/console';
import type {Config, TestResult, TransformTypes} from '@jest/types';
import type {V8Coverage} from 'collect-v8-coverage';

export interface RuntimeTransformResult extends TransformTypes.TransformResult {
// TODO: Make mandatory in Jest 27
wrapperLength?: number;
}

export type V8CoverageResult = Array<{
codeTransformResult: TransformTypes.TransformResult | undefined;
codeTransformResult: RuntimeTransformResult | undefined;
result: V8Coverage[number];
}>;

Expand Down
2 changes: 1 addition & 1 deletion packages/jest-types/src/Transform.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,6 @@
export type TransformResult = {
code: string;
originalCode: string;
mapCoverage?: boolean; // TODO - Remove in Jest 26
mapCoverage?: boolean; // TODO - Remove in Jest 27
sourceMapPath: string | null;
};

0 comments on commit a2090a0

Please sign in to comment.