Skip to content

Commit

Permalink
chore: make changedFiles option optional in shouldInstrument (#9197)
Browse files Browse the repository at this point in the history
  • Loading branch information
SimenB authored Nov 17, 2019
1 parent 03b97d6 commit 4643178
Show file tree
Hide file tree
Showing 8 changed files with 86 additions and 60 deletions.
2 changes: 1 addition & 1 deletion e2e/__tests__/__snapshots__/showConfig.test.ts.snap
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ exports[`--showConfig outputs config info and exits 1`] = `
"bail": 0,
"changedFilesWithAncestor": false,
"collectCoverage": false,
"collectCoverageFrom": null,
"collectCoverageFrom": [],
"coverageDirectory": "<<REPLACED_ROOT_DIR>>/coverage",
"coverageReporters": [
"json",
Expand Down
2 changes: 2 additions & 0 deletions packages/jest-config/src/normalize.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1010,6 +1010,8 @@ export default function normalize(
}

newOptions.collectCoverageFrom = collectCoverageFrom;
} else if (!newOptions.collectCoverageFrom) {
newOptions.collectCoverageFrom = [];
}

return {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,11 @@
*
*/

'use strict';

import vm from 'vm';
import path from 'path';
import os from 'os';
import * as vm from 'vm';
import * as path from 'path';
import * as os from 'os';
import {ScriptTransformer} from '@jest/transform';
import {makeProjectConfig} from '../../../../TestUtils';
import {makeGlobalConfig, makeProjectConfig} from '../../../../TestUtils';

jest.mock('vm');

Expand All @@ -27,9 +25,13 @@ it('instruments files', () => {
cacheDirectory: os.tmpdir(),
rootDir: '/',
});
const instrumented = new ScriptTransformer(
config,
).transform(FILE_PATH_TO_INSTRUMENT, {collectCoverage: true}).script;
const instrumented = new ScriptTransformer(config).transform(
FILE_PATH_TO_INSTRUMENT,
{
...makeGlobalConfig({collectCoverage: true}),
changedFiles: undefined,
},
).script;
expect(instrumented instanceof vm.Script).toBe(true);
// We can't really snapshot the resulting coverage, because it depends on
// absolute path of the file, which will be different on different
Expand Down
9 changes: 1 addition & 8 deletions packages/jest-runtime/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -178,14 +178,7 @@ class Runtime {
}
}

// TODO: Make this `static shouldInstrument = shouldInstrument;` after https://github.com/facebook/jest/issues/7846
static shouldInstrument(
filename: Config.Path,
options: ShouldInstrumentOptions,
config: Config.ProjectConfig,
) {
return shouldInstrument(filename, options, config);
}
static shouldInstrument = shouldInstrument;

static createContext(
config: Config.ProjectConfig,
Expand Down
89 changes: 57 additions & 32 deletions packages/jest-transform/src/__tests__/script_transformer.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,7 @@
*
*/

'use strict';

import {makeProjectConfig} from '../../../../TestUtils';
import {makeGlobalConfig, makeProjectConfig} from '../../../../TestUtils';

jest
.mock('fs', () =>
Expand Down Expand Up @@ -214,9 +212,10 @@ describe('ScriptTransformer', () => {

it('transforms a file properly', () => {
const scriptTransformer = new ScriptTransformer(config);
const response = scriptTransformer.transform('/fruits/banana.js', {
collectCoverage: true,
}).script;
const response = scriptTransformer.transform(
'/fruits/banana.js',
makeGlobalConfig({collectCoverage: true}),
).script;

expect(response instanceof vm.Script).toBe(true);
expect(vm.Script.mock.calls[0][0]).toMatchSnapshot();
Expand All @@ -226,24 +225,32 @@ describe('ScriptTransformer', () => {
expect(fs.readFileSync).toBeCalledWith('/fruits/banana.js', 'utf8');

// in-memory cache
const response2 = scriptTransformer.transform('/fruits/banana.js', {
collectCoverage: true,
}).script;
const response2 = scriptTransformer.transform(
'/fruits/banana.js',
makeGlobalConfig({collectCoverage: true}),
).script;
expect(response2).toBe(response);

scriptTransformer.transform('/fruits/kiwi.js', {
collectCoverage: true,
});
scriptTransformer.transform(
'/fruits/kiwi.js',
makeGlobalConfig({collectCoverage: true}),
);
const snapshot = vm.Script.mock.calls[1][0];
expect(snapshot).toMatchSnapshot();

scriptTransformer.transform('/fruits/kiwi.js', {collectCoverage: true});
scriptTransformer.transform(
'/fruits/kiwi.js',
makeGlobalConfig({collectCoverage: true}),
);

expect(vm.Script.mock.calls[0][0]).not.toEqual(snapshot);
expect(vm.Script.mock.calls[0][0]).not.toMatch(/instrumented kiwi/);

// If we disable coverage, we get a different result.
scriptTransformer.transform('/fruits/kiwi.js', {collectCoverage: false});
scriptTransformer.transform(
'/fruits/kiwi.js',
makeGlobalConfig({collectCoverage: false}),
);
expect(vm.Script.mock.calls[1][0]).toEqual(snapshot);

scriptTransformer.transform('/fruits/banana.js', {
Expand Down Expand Up @@ -401,9 +408,12 @@ describe('ScriptTransformer', () => {
map,
});

const result = scriptTransformer.transform('/fruits/banana.js', {
collectCoverage: true,
});
const result = scriptTransformer.transform(
'/fruits/banana.js',
makeGlobalConfig({
collectCoverage: true,
}),
);
expect(result.sourceMapPath).toEqual(expect.any(String));
const mapStr = JSON.stringify(map);
expect(writeFileAtomic.sync).toBeCalledTimes(2);
Expand Down Expand Up @@ -431,9 +441,12 @@ describe('ScriptTransformer', () => {

require('preprocessor-with-sourcemaps').process.mockReturnValue(content);

const result = scriptTransformer.transform('/fruits/banana.js', {
collectCoverage: true,
});
const result = scriptTransformer.transform(
'/fruits/banana.js',
makeGlobalConfig({
collectCoverage: true,
}),
);
expect(result.sourceMapPath).toEqual(expect.any(String));
expect(writeFileAtomic.sync).toBeCalledTimes(2);
expect(writeFileAtomic.sync).toBeCalledWith(
Expand Down Expand Up @@ -468,9 +481,12 @@ describe('ScriptTransformer', () => {

require('preprocessor-with-sourcemaps').process.mockReturnValue(content);

const result = scriptTransformer.transform('/fruits/banana.js', {
collectCoverage: true,
});
const result = scriptTransformer.transform(
'/fruits/banana.js',
makeGlobalConfig({
collectCoverage: true,
}),
);
expect(result.sourceMapPath).toBeNull();
expect(writeFileAtomic.sync).toBeCalledTimes(1);

Expand All @@ -496,9 +512,12 @@ describe('ScriptTransformer', () => {
map,
});

const result = scriptTransformer.transform('/fruits/banana.js', {
collectCoverage: true,
});
const result = scriptTransformer.transform(
'/fruits/banana.js',
makeGlobalConfig({
collectCoverage: true,
}),
);
expect(result.sourceMapPath).toEqual(expect.any(String));
expect(writeFileAtomic.sync).toBeCalledTimes(2);
expect(writeFileAtomic.sync).toBeCalledWith(
Expand All @@ -522,9 +541,12 @@ describe('ScriptTransformer', () => {
map: null,
});

const result = scriptTransformer.transform('/fruits/banana.js', {
collectCoverage: true,
});
const result = scriptTransformer.transform(
'/fruits/banana.js',
makeGlobalConfig({
collectCoverage: true,
}),
);
expect(result.sourceMapPath).toBeFalsy();
expect(writeFileAtomic.sync).toHaveBeenCalledTimes(1);
});
Expand All @@ -533,9 +555,12 @@ describe('ScriptTransformer', () => {
config = {...config, transform: [['^.+\\.js$', 'test_preprocessor']]};
const scriptTransformer = new ScriptTransformer(config);

scriptTransformer.transform('/fruits/banana.js', {
collectCoverage: true,
});
scriptTransformer.transform(
'/fruits/banana.js',
makeGlobalConfig({
collectCoverage: true,
}),
);

const {getCacheKey} = require('test_preprocessor');
expect(getCacheKey.mock.calls[0][3]).toMatchSnapshot();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,23 +3,27 @@
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*
*/

import {Config} from '@jest/types';
import shouldInstrument from '../shouldInstrument';
import {makeProjectConfig} from '../../../../TestUtils';
import {makeGlobalConfig, makeProjectConfig} from '../../../../TestUtils';
import {Options} from '../types';

describe('shouldInstrument', () => {
const defaultFilename = 'source_file.test.js';
const defaultOptions = {
collectCoverage: true,
const defaultOptions: Options = {
...makeGlobalConfig({
collectCoverage: true,
}),
changedFiles: undefined,
};
const defaultConfig = makeProjectConfig({rootDir: '/'});
describe('should return true', () => {
const testShouldInstrument = (
filename = defaultFilename,
options,
config,
options: Partial<Options>,
config: Partial<Config.ProjectConfig>,
) => {
const result = shouldInstrument(
filename,
Expand Down Expand Up @@ -110,8 +114,8 @@ describe('shouldInstrument', () => {
describe('should return false', () => {
const testShouldInstrument = (
filename = defaultFilename,
options,
config,
options: Partial<Options>,
config: Partial<Config.ProjectConfig>,
) => {
const result = shouldInstrument(
filename,
Expand Down
2 changes: 1 addition & 1 deletion packages/jest-transform/src/shouldInstrument.ts
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ export default function shouldInstrument(
if (
// still cover if `only` is specified
!options.collectCoverageOnlyFrom &&
options.collectCoverageFrom &&
options.collectCoverageFrom.length &&
micromatch(
[replacePathSepForGlob(path.relative(config.rootDir, filename))],
options.collectCoverageFrom,
Expand Down
2 changes: 1 addition & 1 deletion packages/jest-transform/src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ export type ShouldInstrumentOptions = Pick<
Config.GlobalConfig,
'collectCoverage' | 'collectCoverageFrom' | 'collectCoverageOnlyFrom'
> & {
changedFiles: Set<Config.Path> | undefined;
changedFiles?: Set<Config.Path>;
};

export type Options = ShouldInstrumentOptions &
Expand Down

0 comments on commit 4643178

Please sign in to comment.