From f12facbefd386bfc8717fc493d65a68aec72e589 Mon Sep 17 00:00:00 2001 From: Marco Scabbiolo Date: Fri, 11 Jan 2019 23:43:19 -0300 Subject: [PATCH 01/10] Added regression e2e test for #6859 --- e2e/__tests__/onlyChanged.test.js | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/e2e/__tests__/onlyChanged.test.js b/e2e/__tests__/onlyChanged.test.js index 1e0c36834b15..88924ffd21f2 100644 --- a/e2e/__tests__/onlyChanged.test.js +++ b/e2e/__tests__/onlyChanged.test.js @@ -141,6 +141,29 @@ test('report test coverage for only changed files', () => { expect(stdout).not.toMatch('b.js'); }); +test.skip('do not pickup non-tested files when reporting coverage on only changed files', () => { + writeFiles(DIR, { + 'a.js': 'module.exports = {}', + 'b.test.js': 'module.exports = {}', + 'package.json': JSON.stringify({name: 'original name'}), + }); + + run(`${GIT} init`, DIR); + run(`${GIT} add .`, DIR); + run(`${GIT} commit --no-gpg-sign -m "first"`, DIR); + + writeFiles(DIR, { + 'b.test.js': 'it("passes", () => {})', + 'package.json': JSON.stringify({name: 'new name'}), + }); + + const {stderr} = runJest(DIR, ['-o', '--coverage']); + + expect(stderr).toEqual( + expect.not.stringContaining('Failed to collect coverage from'), + ); +}); + test('onlyChanged in config is overwritten by --all or testPathPattern', () => { writeFiles(DIR, { '.watchmanconfig': '', From 9da44a12286f8a6f089b654fe487146373d94338 Mon Sep 17 00:00:00 2001 From: Marco Scabbiolo Date: Sat, 2 Feb 2019 02:34:14 -0300 Subject: [PATCH 02/10] expose module map in reverse dependency resolution to use test dependencies to calculate collectCoverageFrom, fixes #6859 --- e2e/__tests__/onlyChanged.test.js | 2 +- packages/jest-cli/src/SearchSource.js | 57 +++++++++++++------ .../src/__tests__/SearchSource.test.js | 12 ++++ .../jest-resolve-dependencies/src/index.js | 32 ++++++++--- types/Resolve.js | 5 ++ 5 files changed, 82 insertions(+), 26 deletions(-) diff --git a/e2e/__tests__/onlyChanged.test.js b/e2e/__tests__/onlyChanged.test.js index 88924ffd21f2..8a3112f149b5 100644 --- a/e2e/__tests__/onlyChanged.test.js +++ b/e2e/__tests__/onlyChanged.test.js @@ -141,7 +141,7 @@ test('report test coverage for only changed files', () => { expect(stdout).not.toMatch('b.js'); }); -test.skip('do not pickup non-tested files when reporting coverage on only changed files', () => { +test('do not pickup non-tested files when reporting coverage on only changed files', () => { writeFiles(DIR, { 'a.js': 'module.exports = {}', 'b.test.js': 'module.exports = {}', diff --git a/packages/jest-cli/src/SearchSource.js b/packages/jest-cli/src/SearchSource.js index 27862d6a7dac..3f305481f7a7 100644 --- a/packages/jest-cli/src/SearchSource.js +++ b/packages/jest-cli/src/SearchSource.js @@ -156,29 +156,52 @@ export default class SearchSource { buildSnapshotResolver(this._context.config), ); - const tests = toTests( - this._context, - dependencyResolver.resolveInverse( - allPaths, - this.isTestFilePath.bind(this), - { - skipNodeResolution: this._context.config.skipNodeResolution, - }, - ), + if (!collectCoverage) { + return { + tests: toTests( + this._context, + dependencyResolver.resolveInverse( + allPaths, + this.isTestFilePath.bind(this), + { + skipNodeResolution: this._context.config.skipNodeResolution, + }, + ), + ), + }; + } + + const testModulesMap = dependencyResolver.resolveInverseModuleMap( + allPaths, + this.isTestFilePath.bind(this), + { + skipNodeResolution: this._context.config.skipNodeResolution, + }, ); - let collectCoverageFrom; - // If we are collecting coverage, also return collectCoverageFrom patterns - if (collectCoverage) { - collectCoverageFrom = Array.from(allPaths).map(filename => { + const allPathsAbsolute = Array.from(allPaths).map(p => path.resolve(p)); + + return { + collectCoverageFrom: Array.from( + testModulesMap.reduce((acc, testModule) => { + if (testModule.dependencies) { + testModule.dependencies + .filter(p => allPathsAbsolute.some(ap => ap === p)) + .forEach(p => acc.add(p)); + } + return acc; + }, new Set()), + ).map(filename => { filename = replaceRootDirInPath(this._context.config.rootDir, filename); return path.isAbsolute(filename) ? path.relative(this._context.config.rootDir, filename) : filename; - }); - } - - return {collectCoverageFrom, tests}; + }), + tests: toTests( + this._context, + testModulesMap.map(testModule => testModule.file), + ), + }; } findTestsByPaths(paths: Array): SearchResult { diff --git a/packages/jest-cli/src/__tests__/SearchSource.test.js b/packages/jest-cli/src/__tests__/SearchSource.test.js index 45b94705c573..7f42c8298c10 100644 --- a/packages/jest-cli/src/__tests__/SearchSource.test.js +++ b/packages/jest-cli/src/__tests__/SearchSource.test.js @@ -421,6 +421,18 @@ describe('SearchSource', () => { rootPath, ]); }); + + it('excludes untested files from coverage', () => { + const unrelatedFile = path.join(rootDir, 'JSONFile.json'); + const regular = path.join(rootDir, 'RegularModule.js'); + const requireRegular = path.join(rootDir, 'RequireRegularMode.js'); + + const data = searchSource.findRelatedTests( + new Set([regular, requireRegular, unrelatedFile]), + true, + ); + expect(data.collectCoverageFrom).toEqual(['RegularModule.js']); + }); }); describe('findRelatedTestsFromPattern', () => { diff --git a/packages/jest-resolve-dependencies/src/index.js b/packages/jest-resolve-dependencies/src/index.js index ff1dce7bc612..bebb7a76e212 100644 --- a/packages/jest-resolve-dependencies/src/index.js +++ b/packages/jest-resolve-dependencies/src/index.js @@ -9,7 +9,11 @@ import type {HasteFS} from 'types/HasteMap'; import type {Path} from 'types/Config'; -import type {Resolver, ResolveModuleConfig} from 'types/Resolve'; +import type { + Resolver, + ResolveModuleConfig, + ResolvedModule, +} from 'types/Resolve'; import type {SnapshotResolver} from 'types/SnapshotResolver'; import {isSnapshotPath} from 'jest-snapshot'; @@ -61,17 +65,18 @@ class DependencyResolver { }, []); } - resolveInverse( + resolveInverseModuleMap( paths: Set, filter: (file: Path) => boolean, options?: ResolveModuleConfig, - ): Array { + ): Array { if (!paths.size) { return []; } - const collectModules = (relatedPaths, moduleMap, changed) => { + const collectModules = (related, moduleMap, changed) => { const visitedModules = new Set(); + const result: Array = []; while (changed.size) { changed = new Set( moduleMap.reduce((acc, module) => { @@ -84,7 +89,8 @@ class DependencyResolver { const file = module.file; if (filter(file)) { - relatedPaths.add(file); + result.push(module); + related.delete(file); } visitedModules.add(file); acc.push(module.file); @@ -92,10 +98,10 @@ class DependencyResolver { }, []), ); } - return relatedPaths; + return result.concat(Array.from(related).map(file => ({file}))); }; - const relatedPaths = new Set(); + const relatedPaths = new Set(); const changed = new Set(); for (const path of paths) { if (this._hasteFS.exists(path)) { @@ -115,7 +121,17 @@ class DependencyResolver { file, }); } - return Array.from(collectModules(relatedPaths, modules, changed)); + return collectModules(relatedPaths, modules, changed); + } + + resolveInverse( + paths: Set, + filter: (file: Path) => boolean, + options?: ResolveModuleConfig, + ): Array { + return this.resolveInverseModuleMap(paths, filter, options).map( + module => module.file, + ); } } diff --git a/types/Resolve.js b/types/Resolve.js index 1d849797cbc4..facd55e4fc56 100644 --- a/types/Resolve.js +++ b/types/Resolve.js @@ -16,4 +16,9 @@ export type ResolveModuleConfig = {| paths?: Path[], |}; +export type ResolvedModule = { + file: string, + dependencies?: string[], +}; + export type Resolver = _Resolver; From 6e820c79c97dfca94481c49f2864ce42e825d381 Mon Sep 17 00:00:00 2001 From: Simen Bekkhus Date: Sat, 2 Feb 2019 08:57:08 +0100 Subject: [PATCH 03/10] add chaneglog --- CHANGELOG.md | 1 + packages/jest-cli/src/SearchSource.js | 8 ++------ 2 files changed, 3 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 815f452fb361..8f9d44d26c7b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -20,6 +20,7 @@ - `[jest-config]` Make sure `normalize` can consume `Defaults` without warnings ([#7742](https://github.com/facebook/jest/pull/7742)) - `[jest-config]` Allow `moduleFileExtensions` without 'js' for custom runners ([#7751](https://github.com/facebook/jest/pull/7751)) - `[jest-cli]` Load transformers before installing require hooks ([#7752](https://github.com/facebook/jest/pull/7752) +- `[jest-cli]` Only consider test files and their dependencies for coverage ([#7611](https://github.com/facebook/jest/pull/7611)) ### Chore & Maintenance diff --git a/packages/jest-cli/src/SearchSource.js b/packages/jest-cli/src/SearchSource.js index 38294a26fbc2..f62ddc288984 100644 --- a/packages/jest-cli/src/SearchSource.js +++ b/packages/jest-cli/src/SearchSource.js @@ -165,9 +165,7 @@ export default class SearchSource { dependencyResolver.resolveInverse( allPaths, this.isTestFilePath.bind(this), - { - skipNodeResolution: this._context.config.skipNodeResolution, - }, + {skipNodeResolution: this._context.config.skipNodeResolution}, ), ), }; @@ -176,9 +174,7 @@ export default class SearchSource { const testModulesMap = dependencyResolver.resolveInverseModuleMap( allPaths, this.isTestFilePath.bind(this), - { - skipNodeResolution: this._context.config.skipNodeResolution, - }, + {skipNodeResolution: this._context.config.skipNodeResolution}, ); const allPathsAbsolute = Array.from(allPaths).map(p => path.resolve(p)); From 2b66fe1b62a523e05df6a20d955ca2768ace5d90 Mon Sep 17 00:00:00 2001 From: Marco Scabbiolo Date: Sat, 2 Feb 2019 15:20:31 -0300 Subject: [PATCH 04/10] added scenario for files that need to be excluded even if required --- e2e/__tests__/onlyChanged.test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/e2e/__tests__/onlyChanged.test.js b/e2e/__tests__/onlyChanged.test.js index 8f85d0dd9d13..b3180ca4f7f6 100644 --- a/e2e/__tests__/onlyChanged.test.js +++ b/e2e/__tests__/onlyChanged.test.js @@ -151,7 +151,7 @@ test('do not pickup non-tested files when reporting coverage on only changed fil run(`${GIT} commit --no-gpg-sign -m "first"`, DIR); writeFiles(DIR, { - 'b.test.js': 'it("passes", () => {})', + 'b.test.js': 'require("./package.json"); it("passes", () => {})', 'package.json': JSON.stringify({name: 'new name'}), }); From 5f8c7a51f5b52ba564579d2f01dc362f3706ce33 Mon Sep 17 00:00:00 2001 From: Marco Scabbiolo Date: Sun, 3 Feb 2019 20:15:32 -0300 Subject: [PATCH 05/10] Added changedFiles to globalConfig, refactored its exclusion from coverage --- TestUtils.js | 1 + packages/jest-cli/src/SearchSource.js | 38 +++--- .../src/__tests__/runJestWithCoverage.test.js | 117 ------------------ .../jest-cli/src/generateEmptyCoverage.js | 1 + packages/jest-cli/src/runJest.js | 55 +++----- packages/jest-config/src/index.js | 1 + packages/jest-runner/src/runTest.js | 1 + .../jest-runtime/src/ScriptTransformer.js | 1 + packages/jest-runtime/src/index.js | 4 + packages/jest-runtime/src/shouldInstrument.js | 7 ++ types/ChangedFiles.js | 5 +- types/Config.js | 1 + 12 files changed, 52 insertions(+), 180 deletions(-) delete mode 100644 packages/jest-cli/src/__tests__/runJestWithCoverage.test.js diff --git a/TestUtils.js b/TestUtils.js index 1c588f05822a..124156a7622f 100644 --- a/TestUtils.js +++ b/TestUtils.js @@ -13,6 +13,7 @@ import type {GlobalConfig, ProjectConfig} from 'types/Config'; const DEFAULT_GLOBAL_CONFIG: GlobalConfig = { bail: 0, + changedFiles: null, changedFilesWithAncestor: false, changedSince: '', collectCoverage: false, diff --git a/packages/jest-cli/src/SearchSource.js b/packages/jest-cli/src/SearchSource.js index f62ddc288984..055211b7bec0 100644 --- a/packages/jest-cli/src/SearchSource.js +++ b/packages/jest-cli/src/SearchSource.js @@ -10,7 +10,7 @@ import type {Context} from 'types/Context'; import type {Glob, GlobalConfig, Path} from 'types/Config'; import type {Test} from 'types/TestRunner'; -import type {ChangedFilesPromise} from 'types/ChangedFiles'; +import type {ChangedFilesInfo} from 'types/ChangedFiles'; import path from 'path'; import micromatch from 'micromatch'; @@ -226,11 +226,11 @@ export default class SearchSource { return {tests: []}; } - async findTestRelatedToChangedFiles( - changedFilesPromise: ChangedFilesPromise, + findTestRelatedToChangedFiles( + changedFilesInfo: ChangedFilesInfo, collectCoverage: boolean, ) { - const {repos, changedFiles} = await changedFilesPromise; + const {repos, changedFiles} = changedFilesInfo; // no SCM (git/hg/...) is found in any of the roots. const noSCM = Object.keys(repos).every(scm => repos[scm].size === 0); return noSCM @@ -240,42 +240,38 @@ export default class SearchSource { _getTestPaths( globalConfig: GlobalConfig, - changedFilesPromise: ?ChangedFilesPromise, - ): Promise { + changedFiles: ?ChangedFilesInfo, + ): SearchResult { const paths = globalConfig.nonFlagArgs; if (globalConfig.onlyChanged) { - if (!changedFilesPromise) { - throw new Error('This promise must be present when running with -o.'); + if (!changedFiles) { + throw new Error('Changed files must be set when running with -o.'); } return this.findTestRelatedToChangedFiles( - changedFilesPromise, + changedFiles, globalConfig.collectCoverage, ); } else if (globalConfig.runTestsByPath && paths && paths.length) { - return Promise.resolve(this.findTestsByPaths(paths)); + return this.findTestsByPaths(paths); } else if (globalConfig.findRelatedTests && paths && paths.length) { - return Promise.resolve( - this.findRelatedTestsFromPattern(paths, globalConfig.collectCoverage), + return this.findRelatedTestsFromPattern( + paths, + globalConfig.collectCoverage, ); } else if (globalConfig.testPathPattern != null) { - return Promise.resolve( - this.findMatchingTests(globalConfig.testPathPattern), - ); + return this.findMatchingTests(globalConfig.testPathPattern); } else { - return Promise.resolve({tests: []}); + return {tests: []}; } } async getTestPaths( globalConfig: GlobalConfig, - changedFilesPromise: ?ChangedFilesPromise, + changedFiles: ?ChangedFilesInfo, ): Promise { - const searchResult = await this._getTestPaths( - globalConfig, - changedFilesPromise, - ); + const searchResult = this._getTestPaths(globalConfig, changedFiles); const filterPath = globalConfig.filter; diff --git a/packages/jest-cli/src/__tests__/runJestWithCoverage.test.js b/packages/jest-cli/src/__tests__/runJestWithCoverage.test.js deleted file mode 100644 index e0ceb5266755..000000000000 --- a/packages/jest-cli/src/__tests__/runJestWithCoverage.test.js +++ /dev/null @@ -1,117 +0,0 @@ -// Copyright (c) Facebook, Inc. and its affiliates. All Rights Reserved. - -import runJest from '../runJest'; - -jest.mock('jest-util', () => { - const util = jest.requireActual('jest-util'); - return { - ...jest.genMockFromModule('jest-util'), - replacePathSepForGlob: util.replacePathSepForGlob, - }; -}); - -jest.mock( - '../TestScheduler', - () => - class { - constructor(globalConfig) { - this._globalConfig = globalConfig; - } - - scheduleTests() { - return {_globalConfig: this._globalConfig}; - } - }, -); - -jest.mock( - '../TestSequencer', - () => - class { - sort(allTests) { - return allTests; - } - cacheResults() {} - }, -); - -jest.mock( - '../SearchSource', - () => - class { - constructor(context) { - this._context = context; - } - - async getTestPaths(globalConfig, changedFilesPromise) { - const {files} = await changedFilesPromise; - const paths = files.filter(path => path.match(/__tests__/)); - - return { - collectCoverageFrom: files.filter(path => !path.match(/__tests__/)), - tests: paths.map(path => ({ - context: this._context, - duration: null, - path, - })), - }; - } - }, -); - -const config = {roots: [], testPathIgnorePatterns: [], testRegex: []}; -let globalConfig; -const defaults = { - changedFilesPromise: Promise.resolve({ - files: ['foo.js', '__tests__/foo-test.js', 'dont/cover.js'], - }), - contexts: [{config}], - onComplete: runResults => (globalConfig = runResults._globalConfig), - outputStream: {}, - startRun: {}, - testWatcher: {isInterrupted: () => false}, -}; - -describe('collectCoverageFrom patterns', () => { - it('should apply collectCoverageFrom patterns coming from SearchSource', async () => { - expect.assertions(1); - - await runJest({ - ...defaults, - globalConfig: { - rootDir: '', - }, - }); - expect(globalConfig.collectCoverageFrom).toEqual([ - 'foo.js', - 'dont/cover.js', - ]); - }); - - it('excludes coverage from files outside the global collectCoverageFrom config', async () => { - expect.assertions(1); - - await runJest({ - ...defaults, - globalConfig: { - collectCoverageFrom: ['**/dont/*.js'], - rootDir: '', - }, - }); - expect(globalConfig.collectCoverageFrom).toEqual(['dont/cover.js']); - }); - - it('respects coveragePathIgnorePatterns', async () => { - expect.assertions(1); - - await runJest({ - ...defaults, - globalConfig: { - collectCoverageFrom: ['**/*.js'], - coveragePathIgnorePatterns: ['dont'], - rootDir: '', - }, - }); - expect(globalConfig.collectCoverageFrom).toEqual(['foo.js']); - }); -}); diff --git a/packages/jest-cli/src/generateEmptyCoverage.js b/packages/jest-cli/src/generateEmptyCoverage.js index 65c29fcfb1c9..338337ca4af4 100644 --- a/packages/jest-cli/src/generateEmptyCoverage.js +++ b/packages/jest-cli/src/generateEmptyCoverage.js @@ -27,6 +27,7 @@ export default function( config: ProjectConfig, ): ?CoverageWorkerResult { const coverageOptions = { + changedFiles: globalConfig.changedFiles, collectCoverage: globalConfig.collectCoverage, collectCoverageFrom: globalConfig.collectCoverageFrom, collectCoverageOnlyFrom: globalConfig.collectCoverageOnlyFrom, diff --git a/packages/jest-cli/src/runJest.js b/packages/jest-cli/src/runJest.js index 7b4894e6ac0a..236815d0cb83 100644 --- a/packages/jest-cli/src/runJest.js +++ b/packages/jest-cli/src/runJest.js @@ -15,11 +15,10 @@ import type {TestRunData} from 'types/TestRunner'; import type {JestHookEmitter} from 'types/JestHooks'; import type TestWatcher from './TestWatcher'; -import micromatch from 'micromatch'; import chalk from 'chalk'; import path from 'path'; import {sync as realpath} from 'realpath-native'; -import {Console, formatTestResults, replacePathSepForGlob} from 'jest-util'; +import {Console, formatTestResults} from 'jest-util'; import exit from 'exit'; import fs from 'graceful-fs'; import getNoTestsFoundMessage from './getNoTestsFoundMessage'; @@ -36,11 +35,11 @@ const getTestPaths = async ( globalConfig, context, outputStream, - changedFilesPromise, + changedFiles, jestHooks, ) => { const source = new SearchSource(context); - const data = await source.getTestPaths(globalConfig, changedFilesPromise); + const data = await source.getTestPaths(globalConfig, changedFiles); if (!data.tests.length && globalConfig.onlyChanged && data.noSCM) { new Console(outputStream, outputStream).log( @@ -135,6 +134,7 @@ export default (async function runJest({ if (changedFilesPromise && globalConfig.watch) { const {repos} = await changedFilesPromise; + const noSCM = Object.keys(repos).every(scm => repos[scm].size === 0); if (noSCM) { process.stderr.write( @@ -147,7 +147,16 @@ export default (async function runJest({ } } - let collectCoverageFrom = []; + if (changedFilesPromise) { + const {changedFiles} = await changedFilesPromise; + const newConfig: GlobalConfig = { + ...globalConfig, + changedFiles: + changedFiles && + new Set(Array.from(changedFiles).map(p => path.resolve(p))), + }; + globalConfig = Object.freeze(newConfig); + } const testRunData: TestRunData = await Promise.all( contexts.map(async context => { @@ -155,49 +164,15 @@ export default (async function runJest({ globalConfig, context, outputStream, - changedFilesPromise, + changedFilesPromise && (await changedFilesPromise), jestHooks, ); allTests = allTests.concat(matches.tests); - if (matches.collectCoverageFrom) { - collectCoverageFrom = collectCoverageFrom.concat( - matches.collectCoverageFrom.filter(filename => { - if ( - globalConfig.collectCoverageFrom && - !micromatch.some( - replacePathSepForGlob( - path.relative(globalConfig.rootDir, filename), - ), - globalConfig.collectCoverageFrom, - ) - ) { - return false; - } - - if ( - globalConfig.coveragePathIgnorePatterns && - globalConfig.coveragePathIgnorePatterns.some(pattern => - filename.match(pattern), - ) - ) { - return false; - } - - return true; - }), - ); - } - return {context, matches}; }), ); - if (collectCoverageFrom.length) { - const newConfig: GlobalConfig = {...globalConfig, collectCoverageFrom}; - globalConfig = Object.freeze(newConfig); - } - allTests = sequencer.sort(allTests); if (globalConfig.listTests) { diff --git a/packages/jest-config/src/index.js b/packages/jest-config/src/index.js index 81d6970908c5..29241f15e180 100644 --- a/packages/jest-config/src/index.js +++ b/packages/jest-config/src/index.js @@ -108,6 +108,7 @@ const groupOptions = ( ): {globalConfig: GlobalConfig, projectConfig: ProjectConfig} => ({ globalConfig: Object.freeze({ bail: options.bail, + changedFiles: options.changedFiles, changedFilesWithAncestor: options.changedFilesWithAncestor, changedSince: options.changedSince, collectCoverage: options.collectCoverage, diff --git a/packages/jest-runner/src/runTest.js b/packages/jest-runner/src/runTest.js index 2ff916f45abd..b463d66cbeb1 100644 --- a/packages/jest-runner/src/runTest.js +++ b/packages/jest-runner/src/runTest.js @@ -143,6 +143,7 @@ async function runTestInternal( setGlobal(environment.global, 'console', testConsole); runtime = new Runtime(config, environment, resolver, cacheFS, { + changedFiles: globalConfig.changedFiles, collectCoverage: globalConfig.collectCoverage, collectCoverageFrom: globalConfig.collectCoverageFrom, collectCoverageOnlyFrom: globalConfig.collectCoverageOnlyFrom, diff --git a/packages/jest-runtime/src/ScriptTransformer.js b/packages/jest-runtime/src/ScriptTransformer.js index 92d0cb46714b..72ba88f9fabd 100644 --- a/packages/jest-runtime/src/ScriptTransformer.js +++ b/packages/jest-runtime/src/ScriptTransformer.js @@ -33,6 +33,7 @@ import {sync as realpath} from 'realpath-native'; import {enhanceUnexpectedTokenMessage} from './helpers'; export type Options = {| + changedFiles: ?Set, collectCoverage: boolean, collectCoverageFrom: Array, collectCoverageOnlyFrom: ?{[key: string]: boolean, __proto__: null}, diff --git a/packages/jest-runtime/src/index.js b/packages/jest-runtime/src/index.js index 91d6e6a19927..f49d35f10a26 100644 --- a/packages/jest-runtime/src/index.js +++ b/packages/jest-runtime/src/index.js @@ -56,6 +56,7 @@ type InternalModuleOptions = {| |}; type CoverageOptions = { + changedFiles: ?Set, collectCoverage: boolean, collectCoverageFrom: Array, collectCoverageOnlyFrom: ?{[key: string]: boolean, __proto__: null}, @@ -122,6 +123,7 @@ class Runtime { this._cacheFS = cacheFS || Object.create(null); this._config = config; this._coverageOptions = coverageOptions || { + changedFiles: null, collectCoverage: false, collectCoverageFrom: [], collectCoverageOnlyFrom: null, @@ -185,6 +187,7 @@ class Runtime { return shouldInstrument( filename, { + changedFiles: options.changedFiles, collectCoverage: options.collectCoverage, collectCoverageFrom: options.collectCoverageFrom, collectCoverageOnlyFrom: options.collectCoverageOnlyFrom, @@ -665,6 +668,7 @@ class Runtime { const transformedFile = this._scriptTransformer.transform( filename, { + changedFiles: this._coverageOptions.changedFiles, collectCoverage: this._coverageOptions.collectCoverage, collectCoverageFrom: this._coverageOptions.collectCoverageFrom, collectCoverageOnlyFrom: this._coverageOptions.collectCoverageOnlyFrom, diff --git a/packages/jest-runtime/src/shouldInstrument.js b/packages/jest-runtime/src/shouldInstrument.js index 6717fbea3e52..619020575720 100644 --- a/packages/jest-runtime/src/shouldInstrument.js +++ b/packages/jest-runtime/src/shouldInstrument.js @@ -88,5 +88,12 @@ export default function shouldInstrument( return false; } + if ( + options.changedFiles && + !options.changedFiles.has(path.resolve(filename)) + ) { + return false; + } + return true; } diff --git a/types/ChangedFiles.js b/types/ChangedFiles.js index 53b116b8783a..9e0914c01016 100644 --- a/types/ChangedFiles.js +++ b/types/ChangedFiles.js @@ -18,10 +18,11 @@ export type Options = {| export type ChangedFiles = Set; export type Repos = {|git: Set, hg: Set|}; -export type ChangedFilesPromise = Promise<{| +export type ChangedFilesInfo = {| repos: Repos, changedFiles: ChangedFiles, -|}>; +|}; +export type ChangedFilesPromise = Promise; export type SCMAdapter = {| findChangedFiles: (cwd: Path, options: Options) => Promise>, diff --git a/types/Config.js b/types/Config.js index 375278326abb..2e5d9103c946 100644 --- a/types/Config.js +++ b/types/Config.js @@ -194,6 +194,7 @@ export type GlobalConfig = {| bail: number, changedSince: string, changedFilesWithAncestor: boolean, + changedFiles: ?Set, collectCoverage: boolean, collectCoverageFrom: Array, collectCoverageOnlyFrom: ?{[key: string]: boolean}, From 71ce303865121d6da87bb7bb64b38b7576ba80fb Mon Sep 17 00:00:00 2001 From: Marco Scabbiolo Date: Mon, 4 Feb 2019 21:38:56 -0300 Subject: [PATCH 06/10] weaved changedFilesPromise result into Runtime.shouldInstrument --- CHANGELOG.md | 2 +- TestUtils.js | 1 - packages/jest-cli/src/TestScheduler.js | 11 ++++++++--- packages/jest-cli/src/generateEmptyCoverage.js | 3 ++- .../src/reporters/__tests__/coverage_worker.test.js | 1 + .../jest-cli/src/reporters/coverage_reporter.js | 7 +++++-- packages/jest-cli/src/reporters/coverage_worker.js | 3 +++ packages/jest-cli/src/runJest.js | 13 ++----------- packages/jest-config/src/index.js | 1 - packages/jest-runner/src/index.js | 7 +++++-- packages/jest-runner/src/runTest.js | 5 ++++- packages/jest-runtime/src/shouldInstrument.js | 5 +---- types/Config.js | 1 - 13 files changed, 32 insertions(+), 28 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b42930dcdd5a..3e1c9a15ed99 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -20,7 +20,7 @@ - `[jest-config]` Make sure `normalize` can consume `Defaults` without warnings ([#7742](https://github.com/facebook/jest/pull/7742)) - `[jest-config]` Allow `moduleFileExtensions` without 'js' for custom runners ([#7751](https://github.com/facebook/jest/pull/7751)) - `[jest-cli]` Load transformers before installing require hooks ([#7752](https://github.com/facebook/jest/pull/7752) -- `[jest-cli]` Only consider test files and their dependencies for coverage ([#7611](https://github.com/facebook/jest/pull/7611)) +- `[jest-cli]` Refactor `-o` and `--coverage` combined ([#7611](https://github.com/facebook/jest/pull/7611)) - `[jest-cli]` Handle missing `numTodoTests` in test results ([#7779](https://github.com/facebook/jest/pull/7779)) ### Chore & Maintenance diff --git a/TestUtils.js b/TestUtils.js index 124156a7622f..1c588f05822a 100644 --- a/TestUtils.js +++ b/TestUtils.js @@ -13,7 +13,6 @@ import type {GlobalConfig, ProjectConfig} from 'types/Config'; const DEFAULT_GLOBAL_CONFIG: GlobalConfig = { bail: 0, - changedFiles: null, changedFilesWithAncestor: false, changedSince: '', collectCoverage: false, diff --git a/packages/jest-cli/src/TestScheduler.js b/packages/jest-cli/src/TestScheduler.js index f183ab33b867..890e768ac6e8 100644 --- a/packages/jest-cli/src/TestScheduler.js +++ b/packages/jest-cli/src/TestScheduler.js @@ -8,7 +8,7 @@ */ import type {AggregatedResult, TestResult} from 'types/TestResult'; -import type {GlobalConfig, ReporterConfig} from 'types/Config'; +import type {GlobalConfig, ReporterConfig, Path} from 'types/Config'; import type {Context} from 'types/Context'; import type {Reporter, Test} from 'types/TestRunner'; @@ -37,6 +37,7 @@ TestRunner; export type TestSchedulerOptions = {| startRun: (globalConfig: GlobalConfig) => *, + changedFiles: ?Set, |}; export type TestSchedulerContext = {| firstRun: boolean, @@ -262,7 +263,9 @@ export default class TestScheduler { } if (!isDefault && collectCoverage) { - this.addReporter(new CoverageReporter(this._globalConfig)); + this.addReporter( + new CoverageReporter(this._globalConfig, this._options.changedFiles), + ); } if (notify) { @@ -288,7 +291,9 @@ export default class TestScheduler { ); if (collectCoverage) { - this.addReporter(new CoverageReporter(this._globalConfig)); + this.addReporter( + new CoverageReporter(this._globalConfig, this._options.changedFiles), + ); } this.addReporter(new SummaryReporter(this._globalConfig)); diff --git a/packages/jest-cli/src/generateEmptyCoverage.js b/packages/jest-cli/src/generateEmptyCoverage.js index 338337ca4af4..d43f2de65dab 100644 --- a/packages/jest-cli/src/generateEmptyCoverage.js +++ b/packages/jest-cli/src/generateEmptyCoverage.js @@ -25,9 +25,10 @@ export default function( filename: Path, globalConfig: GlobalConfig, config: ProjectConfig, + changedFiles: ?Set, ): ?CoverageWorkerResult { const coverageOptions = { - changedFiles: globalConfig.changedFiles, + changedFiles, collectCoverage: globalConfig.collectCoverage, collectCoverageFrom: globalConfig.collectCoverageFrom, collectCoverageOnlyFrom: globalConfig.collectCoverageOnlyFrom, diff --git a/packages/jest-cli/src/reporters/__tests__/coverage_worker.test.js b/packages/jest-cli/src/reporters/__tests__/coverage_worker.test.js index 8f0fe1c0e7d7..fe9d1f1bb5db 100644 --- a/packages/jest-cli/src/reporters/__tests__/coverage_worker.test.js +++ b/packages/jest-cli/src/reporters/__tests__/coverage_worker.test.js @@ -40,6 +40,7 @@ test('resolves to the result of generateEmptyCoverage upon success', async () => 'banana.js', globalConfig, config, + undefined, ); expect(result).toEqual(42); diff --git a/packages/jest-cli/src/reporters/coverage_reporter.js b/packages/jest-cli/src/reporters/coverage_reporter.js index 81e45ec08a1e..b37611f3adf5 100644 --- a/packages/jest-cli/src/reporters/coverage_reporter.js +++ b/packages/jest-cli/src/reporters/coverage_reporter.js @@ -16,7 +16,7 @@ import type { } from 'types/TestResult'; import typeof {worker} from './coverage_worker'; -import type {GlobalConfig} from 'types/Config'; +import type {GlobalConfig, Path} from 'types/Config'; import type {Context} from 'types/Context'; import type {Test} from 'types/TestRunner'; @@ -39,12 +39,14 @@ export default class CoverageReporter extends BaseReporter { _coverageMap: CoverageMap; _globalConfig: GlobalConfig; _sourceMapStore: any; + _changedFiles: ?Set; - constructor(globalConfig: GlobalConfig) { + constructor(globalConfig: GlobalConfig, changedFiles: ?Set) { super(); this._coverageMap = istanbulCoverage.createCoverageMap({}); this._globalConfig = globalConfig; this._sourceMapStore = libSourceMaps.createSourceMapStore(); + this._changedFiles = changedFiles; } onTestResult( @@ -168,6 +170,7 @@ export default class CoverageReporter extends BaseReporter { if (!this._coverageMap.data[filename]) { try { const result = await worker.worker({ + changedFiles: this._changedFiles, config, globalConfig, path: filename, diff --git a/packages/jest-cli/src/reporters/coverage_worker.js b/packages/jest-cli/src/reporters/coverage_worker.js index c3d53016f248..b9c0e01117a9 100644 --- a/packages/jest-cli/src/reporters/coverage_worker.js +++ b/packages/jest-cli/src/reporters/coverage_worker.js @@ -18,6 +18,7 @@ export type CoverageWorkerData = {| globalConfig: GlobalConfig, config: ProjectConfig, path: Path, + changedFiles: ?Set, |}; export type {CoverageWorkerResult}; @@ -32,11 +33,13 @@ export function worker({ config, globalConfig, path, + changedFiles, }: CoverageWorkerData): ?CoverageWorkerResult { return generateEmptyCoverage( fs.readFileSync(path, 'utf8'), path, globalConfig, config, + changedFiles, ); } diff --git a/packages/jest-cli/src/runJest.js b/packages/jest-cli/src/runJest.js index 236815d0cb83..bd856d9db65f 100644 --- a/packages/jest-cli/src/runJest.js +++ b/packages/jest-cli/src/runJest.js @@ -147,17 +147,6 @@ export default (async function runJest({ } } - if (changedFilesPromise) { - const {changedFiles} = await changedFilesPromise; - const newConfig: GlobalConfig = { - ...globalConfig, - changedFiles: - changedFiles && - new Set(Array.from(changedFiles).map(p => path.resolve(p))), - }; - globalConfig = Object.freeze(newConfig); - } - const testRunData: TestRunData = await Promise.all( contexts.map(async context => { const matches = await getTestPaths( @@ -234,6 +223,8 @@ export default (async function runJest({ const results = await new TestScheduler( globalConfig, { + changedFiles: + changedFilesPromise && (await changedFilesPromise).changedFiles, startRun, }, testSchedulerContext, diff --git a/packages/jest-config/src/index.js b/packages/jest-config/src/index.js index 29241f15e180..81d6970908c5 100644 --- a/packages/jest-config/src/index.js +++ b/packages/jest-config/src/index.js @@ -108,7 +108,6 @@ const groupOptions = ( ): {globalConfig: GlobalConfig, projectConfig: ProjectConfig} => ({ globalConfig: Object.freeze({ bail: options.bail, - changedFiles: options.changedFiles, changedFilesWithAncestor: options.changedFilesWithAncestor, changedSince: options.changedSince, collectCoverage: options.collectCoverage, diff --git a/packages/jest-runner/src/index.js b/packages/jest-runner/src/index.js index ca4ccc141895..b3ce38c460b1 100644 --- a/packages/jest-runner/src/index.js +++ b/packages/jest-runner/src/index.js @@ -7,7 +7,7 @@ * @flow */ -import type {GlobalConfig} from 'types/Config'; +import type {GlobalConfig, Path} from 'types/Config'; import type { OnTestFailure, OnTestStart, @@ -30,9 +30,11 @@ type WorkerInterface = Worker & {worker: worker}; class TestRunner { _globalConfig: GlobalConfig; + _changedFiles: ?Set; - constructor(globalConfig: GlobalConfig) { + constructor(globalConfig: GlobalConfig, changedFiles: ?Set) { this._globalConfig = globalConfig; + this._changedFiles = changedFiles; } async runTests( @@ -78,6 +80,7 @@ class TestRunner { this._globalConfig, test.context.config, test.context.resolver, + this._changedFiles, ); }) .then(result => onResult(test, result)) diff --git a/packages/jest-runner/src/runTest.js b/packages/jest-runner/src/runTest.js index b463d66cbeb1..80e04817fdc7 100644 --- a/packages/jest-runner/src/runTest.js +++ b/packages/jest-runner/src/runTest.js @@ -78,6 +78,7 @@ async function runTestInternal( globalConfig: GlobalConfig, config: ProjectConfig, resolver: Resolver, + changedFiles: ?Set, ): Promise { const testSource = fs.readFileSync(path, 'utf8'); const parsedDocblock = docblock.parse(docblock.extract(testSource)); @@ -143,7 +144,7 @@ async function runTestInternal( setGlobal(environment.global, 'console', testConsole); runtime = new Runtime(config, environment, resolver, cacheFS, { - changedFiles: globalConfig.changedFiles, + changedFiles, collectCoverage: globalConfig.collectCoverage, collectCoverageFrom: globalConfig.collectCoverageFrom, collectCoverageOnlyFrom: globalConfig.collectCoverageOnlyFrom, @@ -269,12 +270,14 @@ export default async function runTest( globalConfig: GlobalConfig, config: ProjectConfig, resolver: Resolver, + changedFiles: ?Set, ): Promise { const {leakDetector, result} = await runTestInternal( path, globalConfig, config, resolver, + changedFiles, ); if (leakDetector) { diff --git a/packages/jest-runtime/src/shouldInstrument.js b/packages/jest-runtime/src/shouldInstrument.js index 619020575720..f3153df6239a 100644 --- a/packages/jest-runtime/src/shouldInstrument.js +++ b/packages/jest-runtime/src/shouldInstrument.js @@ -88,10 +88,7 @@ export default function shouldInstrument( return false; } - if ( - options.changedFiles && - !options.changedFiles.has(path.resolve(filename)) - ) { + if (options.changedFiles && !options.changedFiles.has(filename)) { return false; } diff --git a/types/Config.js b/types/Config.js index 2e5d9103c946..375278326abb 100644 --- a/types/Config.js +++ b/types/Config.js @@ -194,7 +194,6 @@ export type GlobalConfig = {| bail: number, changedSince: string, changedFilesWithAncestor: boolean, - changedFiles: ?Set, collectCoverage: boolean, collectCoverageFrom: Array, collectCoverageOnlyFrom: ?{[key: string]: boolean}, From 2058b2e592b43eeb1ffe6784de9f39893f602992 Mon Sep 17 00:00:00 2001 From: Marco Scabbiolo Date: Mon, 4 Feb 2019 22:55:26 -0300 Subject: [PATCH 07/10] missing changed files constructor paramter in TestScheduler construction of TestRunner --- packages/jest-cli/src/TestScheduler.js | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/jest-cli/src/TestScheduler.js b/packages/jest-cli/src/TestScheduler.js index 890e768ac6e8..b41b1263d23c 100644 --- a/packages/jest-cli/src/TestScheduler.js +++ b/packages/jest-cli/src/TestScheduler.js @@ -174,6 +174,7 @@ export default class TestScheduler { // $FlowFixMe testRunners[config.runner] = new (require(config.runner): TestRunner)( this._globalConfig, + this._options.changedFiles, ); } }); From f5ea6b712af2c013c0e73d3eb34c940fc63f12cc Mon Sep 17 00:00:00 2001 From: Marco Scabbiolo Date: Thu, 7 Feb 2019 11:09:42 -0300 Subject: [PATCH 08/10] pass changedFiles inside context/options objects instead of as a single parameter --- e2e/__tests__/onlyChanged.test.js | 3 ++- packages/jest-cli/src/TestScheduler.js | 12 ++++++++---- .../jest-cli/src/reporters/coverage_reporter.js | 12 ++++++++---- .../jest-cli/src/reporters/coverage_worker.js | 7 ++++--- packages/jest-cli/src/runJest.js | 9 ++++++--- .../src/__tests__/testRunner.test.js | 17 +++++++++++++++-- packages/jest-runner/src/index.js | 12 +++++++----- packages/jest-runner/src/runTest.js | 10 +++++----- packages/jest-runner/src/testWorker.js | 4 ++++ types/TestRunner.js | 4 ++++ 10 files changed, 63 insertions(+), 27 deletions(-) diff --git a/e2e/__tests__/onlyChanged.test.js b/e2e/__tests__/onlyChanged.test.js index b3180ca4f7f6..1f351e28d81d 100644 --- a/e2e/__tests__/onlyChanged.test.js +++ b/e2e/__tests__/onlyChanged.test.js @@ -155,11 +155,12 @@ test('do not pickup non-tested files when reporting coverage on only changed fil 'package.json': JSON.stringify({name: 'new name'}), }); - const {stderr} = runJest(DIR, ['-o', '--coverage']); + const {stderr, stdout} = runJest(DIR, ['-o', '--coverage']); expect(stderr).toEqual( expect.not.stringContaining('Failed to collect coverage from'), ); + expect(stdout).toEqual(expect.not.stringContaining('package.json')); }); test('onlyChanged in config is overwritten by --all or testPathPattern', () => { diff --git a/packages/jest-cli/src/TestScheduler.js b/packages/jest-cli/src/TestScheduler.js index b41b1263d23c..28511803998a 100644 --- a/packages/jest-cli/src/TestScheduler.js +++ b/packages/jest-cli/src/TestScheduler.js @@ -37,11 +37,11 @@ TestRunner; export type TestSchedulerOptions = {| startRun: (globalConfig: GlobalConfig) => *, - changedFiles: ?Set, |}; export type TestSchedulerContext = {| firstRun: boolean, previousSuccess: boolean, + changedFiles?: Set, |}; export default class TestScheduler { _dispatcher: ReporterDispatcher; @@ -174,7 +174,7 @@ export default class TestScheduler { // $FlowFixMe testRunners[config.runner] = new (require(config.runner): TestRunner)( this._globalConfig, - this._options.changedFiles, + {changedFiles: this._context && this._context.changedFiles}, ); } }); @@ -265,7 +265,9 @@ export default class TestScheduler { if (!isDefault && collectCoverage) { this.addReporter( - new CoverageReporter(this._globalConfig, this._options.changedFiles), + new CoverageReporter(this._globalConfig, { + changedFiles: this._context && this._context.changedFiles, + }), ); } @@ -293,7 +295,9 @@ export default class TestScheduler { if (collectCoverage) { this.addReporter( - new CoverageReporter(this._globalConfig, this._options.changedFiles), + new CoverageReporter(this._globalConfig, { + changedFiles: this._context && this._context.changedFiles, + }), ); } diff --git a/packages/jest-cli/src/reporters/coverage_reporter.js b/packages/jest-cli/src/reporters/coverage_reporter.js index b37611f3adf5..f2fad46f9d27 100644 --- a/packages/jest-cli/src/reporters/coverage_reporter.js +++ b/packages/jest-cli/src/reporters/coverage_reporter.js @@ -35,18 +35,22 @@ const RUNNING_TEST_COLOR = chalk.bold.dim; type CoverageWorker = {worker: worker}; +export type CoverageReporterOptions = { + changedFiles?: Set, +}; + export default class CoverageReporter extends BaseReporter { _coverageMap: CoverageMap; _globalConfig: GlobalConfig; _sourceMapStore: any; - _changedFiles: ?Set; + _options: CoverageReporterOptions; - constructor(globalConfig: GlobalConfig, changedFiles: ?Set) { + constructor(globalConfig: GlobalConfig, options?: CoverageReporterOptions) { super(); this._coverageMap = istanbulCoverage.createCoverageMap({}); this._globalConfig = globalConfig; this._sourceMapStore = libSourceMaps.createSourceMapStore(); - this._changedFiles = changedFiles; + this._options = options || {}; } onTestResult( @@ -170,9 +174,9 @@ export default class CoverageReporter extends BaseReporter { if (!this._coverageMap.data[filename]) { try { const result = await worker.worker({ - changedFiles: this._changedFiles, config, globalConfig, + options: this._options, path: filename, }); diff --git a/packages/jest-cli/src/reporters/coverage_worker.js b/packages/jest-cli/src/reporters/coverage_worker.js index b9c0e01117a9..6a7feded80e1 100644 --- a/packages/jest-cli/src/reporters/coverage_worker.js +++ b/packages/jest-cli/src/reporters/coverage_worker.js @@ -8,6 +8,7 @@ */ import type {GlobalConfig, ProjectConfig, Path} from 'types/Config'; +import type {CoverageReporterOptions} from './coverage_reporter'; import exit from 'exit'; import fs from 'fs'; @@ -18,7 +19,7 @@ export type CoverageWorkerData = {| globalConfig: GlobalConfig, config: ProjectConfig, path: Path, - changedFiles: ?Set, + options?: CoverageReporterOptions, |}; export type {CoverageWorkerResult}; @@ -33,13 +34,13 @@ export function worker({ config, globalConfig, path, - changedFiles, + options, }: CoverageWorkerData): ?CoverageWorkerResult { return generateEmptyCoverage( fs.readFileSync(path, 'utf8'), path, globalConfig, config, - changedFiles, + options && options.changedFiles, ); } diff --git a/packages/jest-cli/src/runJest.js b/packages/jest-cli/src/runJest.js index bd856d9db65f..a403ac57ca8a 100644 --- a/packages/jest-cli/src/runJest.js +++ b/packages/jest-cli/src/runJest.js @@ -14,6 +14,7 @@ import type {AggregatedResult} from 'types/TestResult'; import type {TestRunData} from 'types/TestRunner'; import type {JestHookEmitter} from 'types/JestHooks'; import type TestWatcher from './TestWatcher'; +import type {TestSchedulerContext} from './TestScheduler'; import chalk from 'chalk'; import path from 'path'; @@ -103,7 +104,7 @@ const processResults = (runResults, options) => { return onComplete && onComplete(runResults); }; -const testSchedulerContext = { +const testSchedulerContext: TestSchedulerContext = { firstRun: true, previousSuccess: true, }; @@ -220,11 +221,13 @@ export default (async function runJest({ await runGlobalHook({allTests, globalConfig, moduleName: 'globalSetup'}); } + if (changedFilesPromise) { + testSchedulerContext.changedFiles = (await changedFilesPromise).changedFiles; + } + const results = await new TestScheduler( globalConfig, { - changedFiles: - changedFilesPromise && (await changedFilesPromise).changedFiles, startRun, }, testSchedulerContext, diff --git a/packages/jest-runner/src/__tests__/testRunner.test.js b/packages/jest-runner/src/__tests__/testRunner.test.js index d375d91a43a3..c4ce341398cd 100644 --- a/packages/jest-runner/src/__tests__/testRunner.test.js +++ b/packages/jest-runner/src/__tests__/testRunner.test.js @@ -31,6 +31,7 @@ test('injects the serializable module map into each worker in watch mode', () => const globalConfig = {maxWorkers: 2, watch: true}; const config = {rootDir: '/path/'}; const serializableModuleMap = jest.fn(); + const runContext = {}; const context = { config, moduleMap: {toJSON: () => serializableModuleMap}, @@ -46,10 +47,19 @@ test('injects the serializable module map into each worker in watch mode', () => ) .then(() => { expect(mockWorkerFarm.worker.mock.calls).toEqual([ - [{config, globalConfig, path: './file.test.js', serializableModuleMap}], [ { config, + context: runContext, + globalConfig, + path: './file.test.js', + serializableModuleMap, + }, + ], + [ + { + config, + context: runContext, globalConfig, path: './file2.test.js', serializableModuleMap, @@ -63,8 +73,9 @@ test('does not inject the serializable module map in serial mode', () => { const globalConfig = {maxWorkers: 1, watch: false}; const config = {rootDir: '/path/'}; const context = {config}; + const runContext = {}; - return new TestRunner(globalConfig) + return new TestRunner(globalConfig, runContext) .runTests( [{context, path: './file.test.js'}, {context, path: './file2.test.js'}], new TestWatcher({isWatchMode: globalConfig.watch}), @@ -78,6 +89,7 @@ test('does not inject the serializable module map in serial mode', () => { [ { config, + context: runContext, globalConfig, path: './file.test.js', serializableModuleMap: null, @@ -86,6 +98,7 @@ test('does not inject the serializable module map in serial mode', () => { [ { config, + context: runContext, globalConfig, path: './file2.test.js', serializableModuleMap: null, diff --git a/packages/jest-runner/src/index.js b/packages/jest-runner/src/index.js index b3ce38c460b1..cd339cd37e5c 100644 --- a/packages/jest-runner/src/index.js +++ b/packages/jest-runner/src/index.js @@ -7,12 +7,13 @@ * @flow */ -import type {GlobalConfig, Path} from 'types/Config'; +import type {GlobalConfig} from 'types/Config'; import type { OnTestFailure, OnTestStart, OnTestSuccess, Test, + TestRunnerContext, TestRunnerOptions, TestWatcher, } from 'types/TestRunner'; @@ -30,11 +31,11 @@ type WorkerInterface = Worker & {worker: worker}; class TestRunner { _globalConfig: GlobalConfig; - _changedFiles: ?Set; + _context: TestRunnerContext; - constructor(globalConfig: GlobalConfig, changedFiles: ?Set) { + constructor(globalConfig: GlobalConfig, context?: TestRunnerContext) { this._globalConfig = globalConfig; - this._changedFiles = changedFiles; + this._context = context || {}; } async runTests( @@ -80,7 +81,7 @@ class TestRunner { this._globalConfig, test.context.config, test.context.resolver, - this._changedFiles, + this._context, ); }) .then(result => onResult(test, result)) @@ -122,6 +123,7 @@ class TestRunner { return worker.worker({ config: test.context.config, + context: this._context, globalConfig: this._globalConfig, path: test.path, serializableModuleMap: watcher.isWatchMode() diff --git a/packages/jest-runner/src/runTest.js b/packages/jest-runner/src/runTest.js index 80e04817fdc7..f609e09d4c8d 100644 --- a/packages/jest-runner/src/runTest.js +++ b/packages/jest-runner/src/runTest.js @@ -10,7 +10,7 @@ import type {EnvironmentClass} from 'types/Environment'; import type {GlobalConfig, Path, ProjectConfig} from 'types/Config'; import type {Resolver} from 'types/Resolve'; -import type {TestFramework} from 'types/TestRunner'; +import type {TestFramework, TestRunnerContext} from 'types/TestRunner'; import type {TestResult} from 'types/TestResult'; import type RuntimeClass from 'jest-runtime'; @@ -78,7 +78,7 @@ async function runTestInternal( globalConfig: GlobalConfig, config: ProjectConfig, resolver: Resolver, - changedFiles: ?Set, + context: ?TestRunnerContext, ): Promise { const testSource = fs.readFileSync(path, 'utf8'); const parsedDocblock = docblock.parse(docblock.extract(testSource)); @@ -144,7 +144,7 @@ async function runTestInternal( setGlobal(environment.global, 'console', testConsole); runtime = new Runtime(config, environment, resolver, cacheFS, { - changedFiles, + changedFiles: context && context.changedFiles, collectCoverage: globalConfig.collectCoverage, collectCoverageFrom: globalConfig.collectCoverageFrom, collectCoverageOnlyFrom: globalConfig.collectCoverageOnlyFrom, @@ -270,14 +270,14 @@ export default async function runTest( globalConfig: GlobalConfig, config: ProjectConfig, resolver: Resolver, - changedFiles: ?Set, + context: ?TestRunnerContext, ): Promise { const {leakDetector, result} = await runTestInternal( path, globalConfig, config, resolver, - changedFiles, + context, ); if (leakDetector) { diff --git a/packages/jest-runner/src/testWorker.js b/packages/jest-runner/src/testWorker.js index c16f2f749a37..1651803861e6 100644 --- a/packages/jest-runner/src/testWorker.js +++ b/packages/jest-runner/src/testWorker.js @@ -11,6 +11,7 @@ import type {GlobalConfig, Path, ProjectConfig} from 'types/Config'; import type {SerializableError, TestResult} from 'types/TestResult'; import type {SerializableModuleMap} from 'types/HasteMap'; import type {ErrorWithCode} from 'types/Errors'; +import type {TestRunnerContext} from 'types/TestRunner'; import exit from 'exit'; import HasteMap from 'jest-haste-map'; @@ -23,6 +24,7 @@ export type WorkerData = {| globalConfig: GlobalConfig, path: Path, serializableModuleMap: ?SerializableModuleMap, + context?: TestRunnerContext, |}; // Make sure uncaught errors are logged before we exit. @@ -74,6 +76,7 @@ export async function worker({ globalConfig, path, serializableModuleMap, + context, }: WorkerData): Promise { try { const moduleMap = serializableModuleMap @@ -84,6 +87,7 @@ export async function worker({ globalConfig, config, getResolver(config, moduleMap), + context, ); } catch (error) { throw formatError(error); diff --git a/types/TestRunner.js b/types/TestRunner.js index 2eac8afcd349..2aa7005c6f26 100644 --- a/types/TestRunner.js +++ b/types/TestRunner.js @@ -61,6 +61,10 @@ export type TestRunnerOptions = { serial: boolean, }; +export type TestRunnerContext = { + changedFiles?: Set, +}; + export type TestRunData = Array<{ context: Context, matches: {allTests: number, tests: Array, total: number}, From 7c1ffebcad0b457ab7ac453c93133453b6fcfe34 Mon Sep 17 00:00:00 2001 From: Marco Scabbiolo Date: Thu, 7 Feb 2019 12:15:50 -0300 Subject: [PATCH 09/10] more clear syntax in seachsource coverageFrom calculation, turned the result into a set --- packages/jest-cli/src/SearchSource.js | 39 +++++++++++-------- .../src/__tests__/SearchSource.test.js | 4 +- 2 files changed, 26 insertions(+), 17 deletions(-) diff --git a/packages/jest-cli/src/SearchSource.js b/packages/jest-cli/src/SearchSource.js index 055211b7bec0..52d83bdb3f5a 100644 --- a/packages/jest-cli/src/SearchSource.js +++ b/packages/jest-cli/src/SearchSource.js @@ -24,7 +24,7 @@ import {replacePathSepForGlob} from 'jest-util'; type SearchResult = {| noSCM?: boolean, stats?: {[key: string]: number}, - collectCoverageFrom?: Array, + collectCoverageFrom?: Set, tests: Array, total?: number, |}; @@ -179,22 +179,29 @@ export default class SearchSource { const allPathsAbsolute = Array.from(allPaths).map(p => path.resolve(p)); + const collectCoverageFrom = new Set(); + + testModulesMap.forEach(testModule => { + if (!testModule.dependencies) { + return; + } + + testModule.dependencies + .filter(p => allPathsAbsolute.includes(p)) + .map(filename => { + filename = replaceRootDirInPath( + this._context.config.rootDir, + filename, + ); + return path.isAbsolute(filename) + ? path.relative(this._context.config.rootDir, filename) + : filename; + }) + .forEach(filename => collectCoverageFrom.add(filename)); + }); + return { - collectCoverageFrom: Array.from( - testModulesMap.reduce((acc, testModule) => { - if (testModule.dependencies) { - testModule.dependencies - .filter(p => allPathsAbsolute.some(ap => ap === p)) - .forEach(p => acc.add(p)); - } - return acc; - }, new Set()), - ).map(filename => { - filename = replaceRootDirInPath(this._context.config.rootDir, filename); - return path.isAbsolute(filename) - ? path.relative(this._context.config.rootDir, filename) - : filename; - }), + collectCoverageFrom, tests: toTests( this._context, testModulesMap.map(testModule => testModule.file), diff --git a/packages/jest-cli/src/__tests__/SearchSource.test.js b/packages/jest-cli/src/__tests__/SearchSource.test.js index e10c76cbe2ba..0f010a136e3d 100644 --- a/packages/jest-cli/src/__tests__/SearchSource.test.js +++ b/packages/jest-cli/src/__tests__/SearchSource.test.js @@ -429,7 +429,9 @@ describe('SearchSource', () => { new Set([regular, requireRegular, unrelatedFile]), true, ); - expect(data.collectCoverageFrom).toEqual(['RegularModule.js']); + expect(Array.from(data.collectCoverageFrom)).toEqual([ + 'RegularModule.js', + ]); }); }); From 39fcebc02737a33f18dad9b27c2dbb3d46e183f2 Mon Sep 17 00:00:00 2001 From: Marco Scabbiolo Date: Thu, 7 Feb 2019 12:20:49 -0300 Subject: [PATCH 10/10] updated changelog --- CHANGELOG.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2431e1ea8786..0a516378ae79 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,8 @@ ### Fixes +- `[jest-cli]` Refactor `-o` and `--coverage` combined ([#7611](https://github.com/facebook/jest/pull/7611)) + ### Chore & Maintenance - `[*]`: Setup building, linting and testing of TypeScript ([#7808](https://github.com/facebook/jest/pull/7808)) @@ -39,7 +41,6 @@ - `[jest-config]` Make sure `normalize` can consume `Defaults` without warnings ([#7742](https://github.com/facebook/jest/pull/7742)) - `[jest-config]` Allow `moduleFileExtensions` without 'js' for custom runners ([#7751](https://github.com/facebook/jest/pull/7751)) - `[jest-cli]` Load transformers before installing require hooks ([#7752](https://github.com/facebook/jest/pull/7752) -- `[jest-cli]` Refactor `-o` and `--coverage` combined ([#7611](https://github.com/facebook/jest/pull/7611)) - `[jest-cli]` Handle missing `numTodoTests` in test results ([#7779](https://github.com/facebook/jest/pull/7779)) - `[jest-runtime]` Exclude setup/teardown files from coverage report ([#7790](https://github.com/facebook/jest/pull/7790) - `[babel-jest]` Throw an error if `babel-jest` tries to transform a file ignored by Babel ([#7797](https://github.com/facebook/jest/pull/7797))