Fix --coverage with --findRelatedTests overwriting collectCoverageFrom options (#6736)

* add test to reproduce #6462

* fix test

* initial attempt, not happy with it

* take 2, happier with this fix

* better test case

* verify stderr is empty

* Update CHANGELOG.md

Author: stipsan

CHANGELOG.md

@@ -15,6 +15,7 @@
 - `[jest-jasmine2]` Use prettier through `require` instead of `localRequire`. Fixes `matchInlineSnapshot` where prettier dependencies like `path` and `fs` are mocked with `jest.mock`. ([#6776](https://github.com/facebook/jest/pull/6776))
 - `[docs]` Fix contributors link ([#6711](https://github.com/facebook/jest/pull/6711))
 - `[website]` Fix website versions page to link to correct language ([#6734](https://github.com/facebook/jest/pull/6734))
+- `[jest-config]` Fix `--coverage` with `--findRelatedTests` overwriting `collectCoverageFrom` options ([#6736](https://github.com/facebook/jest/pull/6736))
 
 ## 23.4.1
 

e2e/__tests__/__snapshots__/find_related_files.test.js.snap

@@ -1,5 +1,29 @@
 // Jest Snapshot v1, https://goo.gl/fbAQLP
 
+exports[`--findRelatedTests flag coverage configuration is applied correctly 1`] = `
+"Test Suites: 1 passed, 1 total
+Tests:       1 passed, 1 total
+Snapshots:   0 total
+Time:        <<REPLACED>>
+Ran all test suites related to files matching /a.js|b.js/i."
+`;
+
+exports[`--findRelatedTests flag coverage configuration is applied correctly 2`] = `
+"
+
+PASS __tests__/a.test.js
+✓ a"
+`;
+
+exports[`--findRelatedTests flag coverage configuration is applied correctly 3`] = `
+"----------|----------|----------|----------|----------|-------------------|
+File      |  % Stmts | % Branch |  % Funcs |  % Lines | Uncovered Line #s |
+----------|----------|----------|----------|----------|-------------------|
+All files |      100 |      100 |      100 |      100 |                   |
+ a.js     |      100 |      100 |      100 |      100 |                   |
+----------|----------|----------|----------|----------|-------------------|"
+`;
+
 exports[`--findRelatedTests flag generates coverage report for filename 1`] = `
 "Test Suites: 2 passed, 2 total
 Tests:       2 passed, 2 total

e2e/__tests__/find_related_files.test.js

@@ -90,4 +90,48 @@ describe('--findRelatedTests flag', () => {
     // coverage should be collected only for a.js
     expect(stdout).toMatchSnapshot();
   });
+
+  test('coverage configuration is applied correctly', () => {
+    writeFiles(DIR, {
+      '.watchmanconfig': '',
+      '__tests__/a.test.js': `
+        require('../a');
+        test('a', () => expect(1).toBe(1));
+      `,
+      'a.js': 'module.exports = {}',
+      'b.js': 'module.exports = {}',
+      'package.json': JSON.stringify({
+        jest: {
+          collectCoverage: true,
+          collectCoverageFrom: ['!b.js', 'a.js'],
+          testEnvironment: 'node',
+        },
+      }),
+    });
+
+    let stdout;
+    let stderr;
+    ({stdout, stderr} = runJest(DIR, ['--findRelatedTests', 'a.js', 'b.js']));
+
+    const {summary, rest} = extractSummary(stderr);
+    expect(summary).toMatchSnapshot();
+    expect(
+      rest
+        .split('\n')
+        .map(s => s.trim())
+        .sort()
+        .join('\n'),
+    ).toMatchSnapshot();
+
+    // Only a.js should be in the report
+    expect(stdout).toMatchSnapshot();
+    expect(stdout).toMatch('a.js');
+    expect(stdout).not.toMatch('b.js');
+
+    ({stdout, stderr} = runJest(DIR, ['--findRelatedTests', 'b.js']));
+
+    // Neither a.js or b.js should be in the report
+    expect(stdout).toMatch('No tests found');
+    expect(stderr).toBe('');
+  });
 });

packages/jest-config/package.json

@@ -20,6 +20,7 @@
     "jest-resolve": "^23.4.1",
     "jest-util": "^23.4.0",
     "jest-validate": "^23.4.0",
+    "micromatch": "^2.3.11",
     "pretty-format": "^23.2.0"
   }
 }

packages/jest-config/src/normalize.js

@@ -18,6 +18,7 @@ import validatePattern from './validate_pattern';
 import {clearLine} from 'jest-util';
 import chalk from 'chalk';
 import getMaxWorkers from './get_max_workers';
+import micromatch from 'micromatch';
 import Resolver from 'jest-resolve';
 import {replacePathSepForRegex} from 'jest-regex-util';
 import {
@@ -677,12 +678,29 @@ export default function normalize(options: InitialOptions, argv: Argv) {
   // where arguments to `--collectCoverageFrom` should be globs (or relative
   // paths to the rootDir)
   if (newOptions.collectCoverage && argv.findRelatedTests) {
-    newOptions.collectCoverageFrom = argv._.map(filename => {
+    let collectCoverageFrom = argv._.map(filename => {
       filename = replaceRootDirInPath(options.rootDir, filename);
       return path.isAbsolute(filename)
         ? path.relative(options.rootDir, filename)
         : filename;
     });
+
+    // Don't override existing collectCoverageFrom options
+    if (newOptions.collectCoverageFrom) {
+      collectCoverageFrom = collectCoverageFrom.reduce((patterns, filename) => {
+        if (
+          !micromatch(
+            [path.relative(options.rootDir, filename)],
+            newOptions.collectCoverageFrom,
+          ).length
+        ) {
+          return patterns;
+        }
+        return [...patterns, filename];
+      }, newOptions.collectCoverageFrom);
+    }
+
+    newOptions.collectCoverageFrom = collectCoverageFrom;
   }
 
   return {