From 4643178d3ab86bc7f8b5f03216c859def81c318b Mon Sep 17 00:00:00 2001 From: Simen Bekkhus Date: Sun, 17 Nov 2019 21:27:38 +0100 Subject: [PATCH] chore: make changedFiles option optional in `shouldInstrument` (#9197) --- .../__snapshots__/showConfig.test.ts.snap | 2 +- packages/jest-config/src/normalize.ts | 2 + ...tation.test.js => instrumentation.test.ts} | 20 +++-- packages/jest-runtime/src/index.ts | 9 +- .../src/__tests__/script_transformer.test.js | 89 ++++++++++++------- ...ment.test.js => should_instrument.test.ts} | 20 +++-- .../jest-transform/src/shouldInstrument.ts | 2 +- packages/jest-transform/src/types.ts | 2 +- 8 files changed, 86 insertions(+), 60 deletions(-) rename packages/jest-runtime/src/__tests__/{instrumentation.test.js => instrumentation.test.ts} (69%) rename packages/jest-transform/src/__tests__/{should_instrument.test.js => should_instrument.test.ts} (94%) diff --git a/e2e/__tests__/__snapshots__/showConfig.test.ts.snap b/e2e/__tests__/__snapshots__/showConfig.test.ts.snap index dbafaaad3ca1..6e77a0ed28f2 100644 --- a/e2e/__tests__/__snapshots__/showConfig.test.ts.snap +++ b/e2e/__tests__/__snapshots__/showConfig.test.ts.snap @@ -85,7 +85,7 @@ exports[`--showConfig outputs config info and exits 1`] = ` "bail": 0, "changedFilesWithAncestor": false, "collectCoverage": false, - "collectCoverageFrom": null, + "collectCoverageFrom": [], "coverageDirectory": "<>/coverage", "coverageReporters": [ "json", diff --git a/packages/jest-config/src/normalize.ts b/packages/jest-config/src/normalize.ts index 4a1f9c681e64..100b4338cc06 100644 --- a/packages/jest-config/src/normalize.ts +++ b/packages/jest-config/src/normalize.ts @@ -1010,6 +1010,8 @@ export default function normalize( } newOptions.collectCoverageFrom = collectCoverageFrom; + } else if (!newOptions.collectCoverageFrom) { + newOptions.collectCoverageFrom = []; } return { diff --git a/packages/jest-runtime/src/__tests__/instrumentation.test.js b/packages/jest-runtime/src/__tests__/instrumentation.test.ts similarity index 69% rename from packages/jest-runtime/src/__tests__/instrumentation.test.js rename to packages/jest-runtime/src/__tests__/instrumentation.test.ts index 81618df6661a..a969c161e685 100644 --- a/packages/jest-runtime/src/__tests__/instrumentation.test.js +++ b/packages/jest-runtime/src/__tests__/instrumentation.test.ts @@ -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'); @@ -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 diff --git a/packages/jest-runtime/src/index.ts b/packages/jest-runtime/src/index.ts index bbffd575bbd5..e56d5c3ca113 100644 --- a/packages/jest-runtime/src/index.ts +++ b/packages/jest-runtime/src/index.ts @@ -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, diff --git a/packages/jest-transform/src/__tests__/script_transformer.test.js b/packages/jest-transform/src/__tests__/script_transformer.test.js index ddb7792b35a2..0316c6fd9daf 100644 --- a/packages/jest-transform/src/__tests__/script_transformer.test.js +++ b/packages/jest-transform/src/__tests__/script_transformer.test.js @@ -6,9 +6,7 @@ * */ -'use strict'; - -import {makeProjectConfig} from '../../../../TestUtils'; +import {makeGlobalConfig, makeProjectConfig} from '../../../../TestUtils'; jest .mock('fs', () => @@ -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(); @@ -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', { @@ -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); @@ -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( @@ -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); @@ -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( @@ -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); }); @@ -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(); diff --git a/packages/jest-transform/src/__tests__/should_instrument.test.js b/packages/jest-transform/src/__tests__/should_instrument.test.ts similarity index 94% rename from packages/jest-transform/src/__tests__/should_instrument.test.js rename to packages/jest-transform/src/__tests__/should_instrument.test.ts index 38bf9388417c..7ec00a40c52e 100644 --- a/packages/jest-transform/src/__tests__/should_instrument.test.js +++ b/packages/jest-transform/src/__tests__/should_instrument.test.ts @@ -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, + config: Partial, ) => { const result = shouldInstrument( filename, @@ -110,8 +114,8 @@ describe('shouldInstrument', () => { describe('should return false', () => { const testShouldInstrument = ( filename = defaultFilename, - options, - config, + options: Partial, + config: Partial, ) => { const result = shouldInstrument( filename, diff --git a/packages/jest-transform/src/shouldInstrument.ts b/packages/jest-transform/src/shouldInstrument.ts index 3c5c8a89508a..2d8b57da709a 100644 --- a/packages/jest-transform/src/shouldInstrument.ts +++ b/packages/jest-transform/src/shouldInstrument.ts @@ -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, diff --git a/packages/jest-transform/src/types.ts b/packages/jest-transform/src/types.ts index 42e60a3420f0..541d2b225074 100644 --- a/packages/jest-transform/src/types.ts +++ b/packages/jest-transform/src/types.ts @@ -13,7 +13,7 @@ export type ShouldInstrumentOptions = Pick< Config.GlobalConfig, 'collectCoverage' | 'collectCoverageFrom' | 'collectCoverageOnlyFrom' > & { - changedFiles: Set | undefined; + changedFiles?: Set; }; export type Options = ShouldInstrumentOptions &