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

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]` 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

e2e/__tests__/onlyChanged.test.js

@@ -139,6 +139,30 @@ 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, 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', () => {
   writeFiles(DIR, {
     '.watchmanconfig': '',

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';
@@ -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))
+              .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<Path>): SearchResult {
@@ -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
@@ -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;
 

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';
 
@@ -41,6 +41,7 @@ export type TestSchedulerOptions = {|
 export type TestSchedulerContext = {|
   firstRun: boolean,
   previousSuccess: boolean,
+  changedFiles?: Set<Path>,
 |};
 export default class TestScheduler {
   _dispatcher: ReporterDispatcher;
@@ -173,6 +174,7 @@ export default class TestScheduler {
         // $FlowFixMe
         testRunners[config.runner] = new (require(config.runner): TestRunner)(
           this._globalConfig,
+          {changedFiles: this._context && this._context.changedFiles},
         );
       }
     });
@@ -262,7 +264,11 @@ export default class TestScheduler {
     }
 
     if (!isDefault && collectCoverage) {
-      this.addReporter(new CoverageReporter(this._globalConfig));
+      this.addReporter(
+        new CoverageReporter(this._globalConfig, {
+          changedFiles: this._context && this._context.changedFiles,
+        }),
+      );
     }
 
     if (notify) {
@@ -288,7 +294,11 @@ export default class TestScheduler {
     );
 
     if (collectCoverage) {
-      this.addReporter(new CoverageReporter(this._globalConfig));
+      this.addReporter(
+        new CoverageReporter(this._globalConfig, {
+          changedFiles: this._context && this._context.changedFiles,
+        }),
+      );
     }
 
     this.addReporter(new SummaryReporter(this._globalConfig));

packages/jest-cli/src/__tests__/SearchSource.test.js

@@ -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', () => {

packages/jest-cli/src/generateEmptyCoverage.js

@@ -25,8 +25,10 @@ export default function(
   filename: Path,
   globalConfig: GlobalConfig,
   config: ProjectConfig,
+  changedFiles: ?Set<Path>,
 ): ?CoverageWorkerResult {
   const coverageOptions = {
+    changedFiles,
     collectCoverage: globalConfig.collectCoverage,
     collectCoverageFrom: globalConfig.collectCoverageFrom,
     collectCoverageOnlyFrom: globalConfig.collectCoverageOnlyFrom,

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);