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.

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

Refactor -o & --coverage implementation | Fixes #6859 #7611

Merged
merged 14 commits into from Feb 7, 2019
Merged
Show file tree
Hide file tree
Changes from 8 commits
Commits
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
1 change: 1 addition & 0 deletions CHANGELOG.md
Expand Up @@ -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))
- `[jest-cli]` Handle missing `numTodoTests` in test results ([#7779](https://github.com/facebook/jest/pull/7779))

### Chore & Maintenance
Expand Down
1 change: 1 addition & 0 deletions TestUtils.js
Expand Up @@ -13,6 +13,7 @@ import type {GlobalConfig, ProjectConfig} from 'types/Config';

const DEFAULT_GLOBAL_CONFIG: GlobalConfig = {
bail: 0,
changedFiles: null,
MarcoScabbiolo marked this conversation as resolved.
Show resolved Hide resolved
changedFilesWithAncestor: false,
changedSince: '',
collectCoverage: false,
Expand Down
23 changes: 23 additions & 0 deletions e2e/__tests__/onlyChanged.test.js
Expand Up @@ -139,6 +139,29 @@ test('report test coverage for only changed files', () => {
expect(stdout).not.toMatch('b.js');
});

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 = {}',
'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': 'require("./package.json"); it("passes", () => {})',
'package.json': JSON.stringify({name: 'new name'}),
});

const {stderr} = runJest(DIR, ['-o', '--coverage']);

expect(stderr).toEqual(
MarcoScabbiolo marked this conversation as resolved.
Show resolved Hide resolved
expect.not.stringContaining('Failed to collect coverage from'),
);
});

test('onlyChanged in config is overwritten by --all or testPathPattern', () => {
writeFiles(DIR, {
'.watchmanconfig': '',
Expand Down
91 changes: 53 additions & 38 deletions packages/jest-cli/src/SearchSource.js
Expand Up @@ -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';
Expand Down Expand Up @@ -158,29 +158,48 @@ 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))
MarcoScabbiolo marked this conversation as resolved.
Show resolved Hide resolved
.forEach(p => acc.add(p));
MarcoScabbiolo marked this conversation as resolved.
Show resolved Hide resolved
}
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<Path>): SearchResult {
Expand All @@ -207,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
Expand All @@ -221,42 +240,38 @@ export default class SearchSource {

_getTestPaths(
globalConfig: GlobalConfig,
changedFilesPromise: ?ChangedFilesPromise,
): Promise<SearchResult> {
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<SearchResult> {
const searchResult = await this._getTestPaths(
globalConfig,
changedFilesPromise,
);
const searchResult = this._getTestPaths(globalConfig, changedFiles);

const filterPath = globalConfig.filter;

Expand Down
12 changes: 12 additions & 0 deletions packages/jest-cli/src/__tests__/SearchSource.test.js
Expand Up @@ -419,6 +419,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', () => {
Expand Down
117 changes: 0 additions & 117 deletions packages/jest-cli/src/__tests__/runJestWithCoverage.test.js

This file was deleted.

1 change: 1 addition & 0 deletions packages/jest-cli/src/generateEmptyCoverage.js
Expand Up @@ -27,6 +27,7 @@ export default function(
config: ProjectConfig,
): ?CoverageWorkerResult {
const coverageOptions = {
changedFiles: globalConfig.changedFiles,
collectCoverage: globalConfig.collectCoverage,
collectCoverageFrom: globalConfig.collectCoverageFrom,
collectCoverageOnlyFrom: globalConfig.collectCoverageOnlyFrom,
Expand Down