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

Show coverage of sources related to tests in changed files #9769

Merged
1 change: 1 addition & 0 deletions CHANGELOG.md
Expand Up @@ -3,6 +3,7 @@
### Features

- `[@jest/globals]` New package so Jest's globals can be explicitly imported ([#9801](https://github.com/facebook/jest/pull/9801))
- `[jest-core]` Show coverage of sources related to tests in changed files ([#9769](https://github.com/facebook/jest/pull/9769))
- `[jest-runtime]` Populate `require.cache` ([#9841](https://github.com/facebook/jest/pull/9841))

### Fixes
Expand Down
33 changes: 33 additions & 0 deletions e2e/__tests__/onlyChanged.test.ts
Expand Up @@ -137,6 +137,39 @@ test('report test coverage for only changed files', () => {
expect(stdout).not.toMatch('b.js');
});

test('report test coverage of source on test file change under only changed files', () => {
writeFiles(DIR, {
'__tests__/a.test.js': `
require('../a');
test('a1', () => expect(1).toBe(1));
`,
'a.js': 'module.exports = {}',
'package.json': JSON.stringify({
jest: {
collectCoverage: true,
coverageReporters: ['text'],
testEnvironment: 'node',
},
}),
});

run(`${GIT} init`, DIR);
run(`${GIT} add .`, DIR);
run(`${GIT} commit --no-gpg-sign -m "first"`, DIR);

writeFiles(DIR, {
'__tests__/a.test.js': `
require('../a');
test('a1', () => expect(1).toBe(1));
test('a2', () => expect(2).toBe(2));
`,
});

const {stdout} = runJest(DIR, ['--only-changed']);

expect(stdout).toMatch('a.js');
});

test('do not pickup non-tested files when reporting coverage on only changed files', () => {
writeFiles(DIR, {
'a.js': 'module.exports = {}',
Expand Down
60 changes: 47 additions & 13 deletions packages/jest-core/src/SearchSource.ts
Expand Up @@ -51,13 +51,22 @@ const toTests = (context: Context, tests: Array<Config.Path>) =>
path,
}));

const hasSCM = (changedFilesInfo: ChangedFiles) => {
const {repos} = changedFilesInfo;
// no SCM (git/hg/...) is found in any of the roots.
const noSCM = Object.values(repos).every(scm => scm.size === 0);
return !noSCM;
};

export default class SearchSource {
private _context: Context;
private _dependencyResolver: DependencyResolver | null;
private _testPathCases: TestPathCases = [];

constructor(context: Context) {
const {config} = context;
this._context = context;
this._dependencyResolver = null;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we may not immediately use DependencyResolver in SearchSource, so at first this._dependencyResolver will be null. When we need DependencyResolver, just call _getDependencyResolver to create/get it.


const rootPattern = new RegExp(
config.roots.map(dir => escapePathForRegex(dir + path.sep)).join('|'),
Expand Down Expand Up @@ -92,6 +101,17 @@ export default class SearchSource {
}
}

private _getOrBuildDependencyResolver(): DependencyResolver {
if (!this._dependencyResolver) {
this._dependencyResolver = new DependencyResolver(
this._context.resolver,
this._context.hasteFS,
buildSnapshotResolver(this._context.config),
);
}
return this._dependencyResolver;
}

private _filterTestPathsWithStats(
allPaths: Array<Test>,
testPathPattern?: string,
Expand Down Expand Up @@ -155,11 +175,7 @@ export default class SearchSource {
allPaths: Set<Config.Path>,
collectCoverage: boolean,
): SearchResult {
const dependencyResolver = new DependencyResolver(
this._context.resolver,
this._context.hasteFS,
buildSnapshotResolver(this._context.config),
);
const dependencyResolver = this._getOrBuildDependencyResolver();

if (!collectCoverage) {
return {
Expand Down Expand Up @@ -240,14 +256,11 @@ export default class SearchSource {
changedFilesInfo: ChangedFiles,
collectCoverage: boolean,
): SearchResult {
const {repos, changedFiles} = changedFilesInfo;
// no SCM (git/hg/...) is found in any of the roots.
const noSCM = (Object.keys(repos) as Array<
keyof ChangedFiles['repos']
>).every(scm => repos[scm].size === 0);
return noSCM
? {noSCM: true, tests: []}
: this.findRelatedTests(changedFiles, collectCoverage);
if (!hasSCM(changedFilesInfo)) {
return {noSCM: true, tests: []};
}
const {changedFiles} = changedFilesInfo;
return this.findRelatedTests(changedFiles, collectCoverage);
}

private _getTestPaths(
Expand Down Expand Up @@ -328,4 +341,25 @@ export default class SearchSource {

return searchResult;
}

findRelatedSourcesFromTestsInChangedFiles(
changedFilesInfo: ChangedFiles,
): Array<string> {
if (!hasSCM(changedFilesInfo)) {
return [];
}
const {changedFiles} = changedFilesInfo;
const dependencyResolver = this._getOrBuildDependencyResolver();
const relatedSourcesSet = new Set<string>();
changedFiles.forEach(filePath => {
const isTestFile = this.isTestFilePath(filePath);
if (isTestFile) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
const isTestFile = this.isTestFilePath(filePath);
if (isTestFile) {
if (this.isTestFilePath(filePath) {

const sourcePaths = dependencyResolver.resolve(filePath, {
skipNodeResolution: this._context.config.skipNodeResolution,
});
sourcePaths.forEach(sourcePath => relatedSourcesSet.add(sourcePath));
}
});
return Array.from(relatedSourcesSet);
}
}
13 changes: 10 additions & 3 deletions packages/jest-core/src/TestScheduler.ts
Expand Up @@ -44,6 +44,7 @@ export type TestSchedulerContext = {
firstRun: boolean;
previousSuccess: boolean;
changedFiles?: Set<Config.Path>;
sourcesRelatedToTestsInChangedFiles?: Set<Config.Path>;
};
export default class TestScheduler {
private _dispatcher: ReporterDispatcher;
Expand Down Expand Up @@ -180,7 +181,9 @@ export default class TestScheduler {
if (!testRunners[config.runner]) {
const Runner: typeof TestRunner = require(config.runner);
testRunners[config.runner] = new Runner(this._globalConfig, {
changedFiles: this._context && this._context.changedFiles,
changedFiles: this._context?.changedFiles,
sourcesRelatedToTestsInChangedFiles: this._context
?.sourcesRelatedToTestsInChangedFiles,
});
}
});
Expand Down Expand Up @@ -272,7 +275,9 @@ export default class TestScheduler {
if (!isDefault && collectCoverage) {
this.addReporter(
new CoverageReporter(this._globalConfig, {
changedFiles: this._context && this._context.changedFiles,
changedFiles: this._context?.changedFiles,
sourcesRelatedToTestsInChangedFiles: this._context
?.sourcesRelatedToTestsInChangedFiles,
}),
);
}
Expand Down Expand Up @@ -302,7 +307,9 @@ export default class TestScheduler {
if (collectCoverage) {
this.addReporter(
new CoverageReporter(this._globalConfig, {
changedFiles: this._context && this._context.changedFiles,
changedFiles: this._context?.changedFiles,
sourcesRelatedToTestsInChangedFiles: this._context
?.sourcesRelatedToTestsInChangedFiles,
}),
);
}
Expand Down
61 changes: 61 additions & 0 deletions packages/jest-core/src/__tests__/SearchSource.test.ts
Expand Up @@ -531,4 +531,65 @@ describe('SearchSource', () => {
}
});
});

describe('findRelatedSourcesFromTestsInChangedFiles', () => {
const rootDir = path.resolve(
__dirname,
'../../../jest-runtime/src/__tests__/test_root',
);

beforeEach(done => {
const {options: config} = normalize(
{
haste: {
hasteImplModulePath: path.resolve(
__dirname,
'../../../jest-haste-map/src/__tests__/haste_impl.js',
),
providesModuleNodeModules: [],
},
name: 'SearchSource-findRelatedSourcesFromTestsInChangedFiles-tests',
rootDir,
},
{} as Config.Argv,
);
Runtime.createContext(config, {maxWorkers, watchman: false}).then(
context => {
searchSource = new SearchSource(context);
done();
},
);
});
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
beforeEach(done => {
const {options: config} = normalize(
{
haste: {
hasteImplModulePath: path.resolve(
__dirname,
'../../../jest-haste-map/src/__tests__/haste_impl.js',
),
providesModuleNodeModules: [],
},
name: 'SearchSource-findRelatedSourcesFromTestsInChangedFiles-tests',
rootDir,
},
{} as Config.Argv,
);
Runtime.createContext(config, {maxWorkers, watchman: false}).then(
context => {
searchSource = new SearchSource(context);
done();
},
);
});
beforeEach(async () => {
const {options: config} = normalize(
{
haste: {
hasteImplModulePath: path.resolve(
__dirname,
'../../../jest-haste-map/src/__tests__/haste_impl.js',
),
providesModuleNodeModules: [],
},
name: 'SearchSource-findRelatedSourcesFromTestsInChangedFiles-tests',
rootDir,
},
{} as Config.Argv,
);
const context = await Runtime.createContext(config, {maxWorkers, watchman: false})
searchSource = new SearchSource(context);
});

diff looks big, but async await instead of .then and done


it('return empty set if no SCM', () => {
const requireRegularModule = path.join(
rootDir,
'RequireRegularModule.js',
);
const sources = searchSource.findRelatedSourcesFromTestsInChangedFiles({
changedFiles: new Set([requireRegularModule]),
repos: {
git: new Set(),
hg: new Set(),
},
});
expect(sources).toEqual([]);
});

it('return sources required by tests', () => {
const regularModule = path.join(rootDir, 'RegularModule.js');
const requireRegularModule = path.join(
rootDir,
'RequireRegularModule.js',
);
const sources = searchSource.findRelatedSourcesFromTestsInChangedFiles({
changedFiles: new Set([requireRegularModule]),
repos: {
git: new Set('/path/to/git'),
hg: new Set(),
},
});
expect(sources).toEqual([regularModule]);
});
});
});
29 changes: 22 additions & 7 deletions packages/jest-core/src/runJest.ts
Expand Up @@ -34,13 +34,12 @@ import type {Filter, TestRunData} from './types';

const getTestPaths = async (
globalConfig: Config.GlobalConfig,
context: Context,
source: SearchSource,
outputStream: NodeJS.WriteStream,
changedFiles: ChangedFiles | undefined,
jestHooks: JestHookEmitter,
filter?: Filter,
) => {
const source = new SearchSource(context);
const data = await source.getTestPaths(globalConfig, changedFiles, filter);

if (!data.tests.length && globalConfig.onlyChanged && data.noSCM) {
Expand Down Expand Up @@ -167,11 +166,14 @@ export default async function runJest({
}
}

const searchSources = contexts.map(context => new SearchSource(context));

const testRunData: TestRunData = await Promise.all(
contexts.map(async context => {
contexts.map(async (context, index) => {
const searchSource = searchSources[index];
const matches = await getTestPaths(
globalConfig,
context,
searchSource,
outputStream,
changedFilesPromise && (await changedFilesPromise),
jestHooks,
Expand Down Expand Up @@ -242,9 +244,22 @@ export default async function runJest({
}

if (changedFilesPromise) {
testSchedulerContext.changedFiles = (
await changedFilesPromise
).changedFiles;
const changedFilesInfo = await changedFilesPromise;
if (changedFilesInfo.changedFiles) {
testSchedulerContext.changedFiles = changedFilesInfo.changedFiles;
const sourcesRelatedToTestsInChangedFilesArray = contexts
.map((_, index) => {
const searchSource = searchSources[index];
const relatedSourceFromTestsInChangedFiles = searchSource.findRelatedSourcesFromTestsInChangedFiles(
changedFilesInfo,
);
return relatedSourceFromTestsInChangedFiles;
})
.reduce((total, paths) => total.concat(paths), []);
testSchedulerContext.sourcesRelatedToTestsInChangedFiles = new Set(
sourcesRelatedToTestsInChangedFilesArray,
);
}
}

const results = await new TestScheduler(
Expand Down
Expand Up @@ -41,6 +41,7 @@ test('resolves to the result of generateEmptyCoverage upon success', async () =>
globalConfig,
config,
undefined,
undefined,
);

expect(result).toEqual(42);
Expand Down
3 changes: 3 additions & 0 deletions packages/jest-reporters/src/coverage_reporter.ts
Expand Up @@ -204,6 +204,9 @@ export default class CoverageReporter extends BaseReporter {
changedFiles:
this._options.changedFiles &&
Array.from(this._options.changedFiles),
sourcesRelatedToTestsInChangedFiles:
this._options.sourcesRelatedToTestsInChangedFiles &&
Array.from(this._options.sourcesRelatedToTestsInChangedFiles),
},
path: filename,
});
Expand Down
4 changes: 3 additions & 1 deletion packages/jest-reporters/src/coverage_worker.ts
Expand Up @@ -40,6 +40,8 @@ export function worker({
path,
globalConfig,
config,
options && options.changedFiles && new Set(options.changedFiles),
options?.changedFiles && new Set(options.changedFiles),
options?.sourcesRelatedToTestsInChangedFiles &&
new Set(options.sourcesRelatedToTestsInChangedFiles),
);
}
2 changes: 2 additions & 0 deletions packages/jest-reporters/src/generateEmptyCoverage.ts
Expand Up @@ -31,13 +31,15 @@ export default function (
globalConfig: Config.GlobalConfig,
config: Config.ProjectConfig,
changedFiles?: Set<Config.Path>,
sourcesRelatedToTestsInChangedFiles?: Set<Config.Path>,
): CoverageWorkerResult | null {
const coverageOptions = {
changedFiles,
collectCoverage: globalConfig.collectCoverage,
collectCoverageFrom: globalConfig.collectCoverageFrom,
collectCoverageOnlyFrom: globalConfig.collectCoverageOnlyFrom,
coverageProvider: globalConfig.coverageProvider,
sourcesRelatedToTestsInChangedFiles,
};
let coverageWorkerResult: CoverageWorkerResult | null = null;
if (shouldInstrument(filename, coverageOptions, config)) {
Expand Down
2 changes: 2 additions & 0 deletions packages/jest-reporters/src/types.ts
Expand Up @@ -37,10 +37,12 @@ export type CoverageWorker = {worker: typeof worker};

export type CoverageReporterOptions = {
changedFiles?: Set<Config.Path>;
sourcesRelatedToTestsInChangedFiles?: Set<Config.Path>;
};

export type CoverageReporterSerializedOptions = {
changedFiles?: Array<Config.Path>;
sourcesRelatedToTestsInChangedFiles?: Array<Config.Path>;
};

export type OnTestStart = (test: Test) => Promise<void>;
Expand Down
4 changes: 3 additions & 1 deletion packages/jest-runner/src/runTest.ts
Expand Up @@ -147,11 +147,13 @@ async function runTestInternal(
setGlobal(environment.global, 'console', testConsole);

const runtime = new Runtime(config, environment, resolver, cacheFS, {
changedFiles: context && context.changedFiles,
changedFiles: context?.changedFiles,
collectCoverage: globalConfig.collectCoverage,
collectCoverageFrom: globalConfig.collectCoverageFrom,
collectCoverageOnlyFrom: globalConfig.collectCoverageOnlyFrom,
coverageProvider: globalConfig.coverageProvider,
sourcesRelatedToTestsInChangedFiles:
context?.sourcesRelatedToTestsInChangedFiles,
});

const start = Date.now();
Expand Down
1 change: 1 addition & 0 deletions packages/jest-runner/src/types.ts
Expand Up @@ -51,6 +51,7 @@ export type TestRunnerOptions = {

export type TestRunnerContext = {
changedFiles?: Set<Config.Path>;
sourcesRelatedToTestsInChangedFiles?: Set<Config.Path>;
};

export type TestRunnerSerializedContext = {
Expand Down