From 3ce345b9be783f3ef4a81ae42ef026d088e684b5 Mon Sep 17 00:00:00 2001 From: Stian Didriksen Date: Sun, 22 Jul 2018 17:59:34 +0200 Subject: [PATCH 1/7] add test to reproduce #6462 --- .../find_related_files.test.js.snap | 24 ++++++++++++ e2e/__tests__/find_related_files.test.js | 39 +++++++++++++++++++ 2 files changed, 63 insertions(+) diff --git a/e2e/__tests__/__snapshots__/find_related_files.test.js.snap b/e2e/__tests__/__snapshots__/find_related_files.test.js.snap index 233600c71a45..54dee8b1e1f6 100644 --- a/e2e/__tests__/__snapshots__/find_related_files.test.js.snap +++ b/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: <> +Ran all test suites related to files matching /a.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 diff --git a/e2e/__tests__/find_related_files.test.js b/e2e/__tests__/find_related_files.test.js index 01828d3b4be3..a2b855278c02 100644 --- a/e2e/__tests__/find_related_files.test.js +++ b/e2e/__tests__/find_related_files.test.js @@ -90,4 +90,43 @@ 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: ['a.js', '!b.js'], + testEnvironment: 'node', + }, + }), + }); + + const {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(); + expect(stdout).toMatchSnapshot(); + + // Only a.js should be in the report + const summaryMsg = 'Ran all test suites related to files matching /a.js/i.'; + expect(stderr).toMatch(summaryMsg); + }); }); From 583ffd3842fc03c330993b0e63c388fc2b490e40 Mon Sep 17 00:00:00 2001 From: Stian Didriksen Date: Sun, 22 Jul 2018 18:29:15 +0200 Subject: [PATCH 2/7] fix test --- e2e/__tests__/__snapshots__/find_related_files.test.js.snap | 2 +- e2e/__tests__/find_related_files.test.js | 5 ++--- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/e2e/__tests__/__snapshots__/find_related_files.test.js.snap b/e2e/__tests__/__snapshots__/find_related_files.test.js.snap index 54dee8b1e1f6..bde29e5dd035 100644 --- a/e2e/__tests__/__snapshots__/find_related_files.test.js.snap +++ b/e2e/__tests__/__snapshots__/find_related_files.test.js.snap @@ -5,7 +5,7 @@ exports[`--findRelatedTests flag coverage configuration is applied correctly 1`] Tests: 1 passed, 1 total Snapshots: 0 total Time: <> -Ran all test suites related to files matching /a.js/i." +Ran all test suites related to files matching /a.js|b.js/i." `; exports[`--findRelatedTests flag coverage configuration is applied correctly 2`] = ` diff --git a/e2e/__tests__/find_related_files.test.js b/e2e/__tests__/find_related_files.test.js index a2b855278c02..7194262ff979 100644 --- a/e2e/__tests__/find_related_files.test.js +++ b/e2e/__tests__/find_related_files.test.js @@ -123,10 +123,9 @@ describe('--findRelatedTests flag', () => { .sort() .join('\n'), ).toMatchSnapshot(); - expect(stdout).toMatchSnapshot(); // Only a.js should be in the report - const summaryMsg = 'Ran all test suites related to files matching /a.js/i.'; - expect(stderr).toMatch(summaryMsg); + expect(stdout).toMatchSnapshot(); + expect(stdout).not.toMatch('b.js'); }); }); From 966282595e15e39988555b5dcd8785960d34c738 Mon Sep 17 00:00:00 2001 From: Stian Didriksen Date: Sun, 22 Jul 2018 18:32:28 +0200 Subject: [PATCH 3/7] initial attempt, not happy with it --- packages/jest-config/package.json | 1 + packages/jest-config/src/normalize.js | 16 +++++++++++++--- 2 files changed, 14 insertions(+), 3 deletions(-) diff --git a/packages/jest-config/package.json b/packages/jest-config/package.json index c08cddf532e4..13a2939327f0 100644 --- a/packages/jest-config/package.json +++ b/packages/jest-config/package.json @@ -18,6 +18,7 @@ "jest-jasmine2": "^23.4.1", "jest-regex-util": "^23.3.0", "jest-resolve": "^23.4.1", + "jest-runtime": "^23.4.1", "jest-util": "^23.4.0", "jest-validate": "^23.4.0", "pretty-format": "^23.2.0" diff --git a/packages/jest-config/src/normalize.js b/packages/jest-config/src/normalize.js index 5753d1dd9e55..a21304951d6c 100644 --- a/packages/jest-config/src/normalize.js +++ b/packages/jest-config/src/normalize.js @@ -19,6 +19,7 @@ import {clearLine} from 'jest-util'; import chalk from 'chalk'; import getMaxWorkers from './get_max_workers'; import Resolver from 'jest-resolve'; +import Runtime from 'jest-runtime'; import {replacePathSepForRegex} from 'jest-regex-util'; import { BULLET, @@ -677,12 +678,21 @@ 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 => { + const coverageOptions = { + collectCoverage: newOptions.collectCoverage, + collectCoverageFrom: newOptions.collectCoverageFrom, + collectCoverageOnlyFrom: newOptions.collectCoverageOnlyFrom, + }; + newOptions.collectCoverageFrom = argv._.reduce((sources, filename) => { filename = replaceRootDirInPath(options.rootDir, filename); - return path.isAbsolute(filename) + const source = path.isAbsolute(filename) ? path.relative(options.rootDir, filename) : filename; - }); + if (Runtime.shouldInstrument(filename, coverageOptions, newOptions)) { + return [...sources, source]; + } + return sources; + }, []); } return { From d391ee2432b8c534551ab540e28627285e29184f Mon Sep 17 00:00:00 2001 From: Stian Didriksen Date: Sun, 22 Jul 2018 19:07:31 +0200 Subject: [PATCH 4/7] take 2, happier with this fix --- packages/jest-config/package.json | 2 +- packages/jest-config/src/normalize.js | 34 +++++++++++++++++---------- 2 files changed, 22 insertions(+), 14 deletions(-) diff --git a/packages/jest-config/package.json b/packages/jest-config/package.json index 13a2939327f0..5fa356572722 100644 --- a/packages/jest-config/package.json +++ b/packages/jest-config/package.json @@ -18,9 +18,9 @@ "jest-jasmine2": "^23.4.1", "jest-regex-util": "^23.3.0", "jest-resolve": "^23.4.1", - "jest-runtime": "^23.4.1", "jest-util": "^23.4.0", "jest-validate": "^23.4.0", + "micromatch": "^2.3.11", "pretty-format": "^23.2.0" } } diff --git a/packages/jest-config/src/normalize.js b/packages/jest-config/src/normalize.js index a21304951d6c..5581f68247a7 100644 --- a/packages/jest-config/src/normalize.js +++ b/packages/jest-config/src/normalize.js @@ -18,8 +18,8 @@ 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 Runtime from 'jest-runtime'; import {replacePathSepForRegex} from 'jest-regex-util'; import { BULLET, @@ -678,21 +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) { - const coverageOptions = { - collectCoverage: newOptions.collectCoverage, - collectCoverageFrom: newOptions.collectCoverageFrom, - collectCoverageOnlyFrom: newOptions.collectCoverageOnlyFrom, - }; - newOptions.collectCoverageFrom = argv._.reduce((sources, filename) => { + let collectCoverageFrom = argv._.map(filename => { filename = replaceRootDirInPath(options.rootDir, filename); - const source = path.isAbsolute(filename) + return path.isAbsolute(filename) ? path.relative(options.rootDir, filename) : filename; - if (Runtime.shouldInstrument(filename, coverageOptions, newOptions)) { - return [...sources, source]; - } - return sources; - }, []); + }); + + // 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 { From 8e4d4828846a4f96205f8c750183a9cea88a631e Mon Sep 17 00:00:00 2001 From: Stian Didriksen Date: Sun, 22 Jul 2018 19:08:19 +0200 Subject: [PATCH 5/7] better test case --- e2e/__tests__/find_related_files.test.js | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/e2e/__tests__/find_related_files.test.js b/e2e/__tests__/find_related_files.test.js index 7194262ff979..a04c52d0e21b 100644 --- a/e2e/__tests__/find_related_files.test.js +++ b/e2e/__tests__/find_related_files.test.js @@ -103,17 +103,16 @@ describe('--findRelatedTests flag', () => { 'package.json': JSON.stringify({ jest: { collectCoverage: true, - collectCoverageFrom: ['a.js', '!b.js'], + collectCoverageFrom: ['!b.js', 'a.js'], testEnvironment: 'node', }, }), }); - const {stdout, stderr} = runJest(DIR, [ - '--findRelatedTests', - 'a.js', - 'b.js', - ]); + let stdout; + let stderr; + ({stdout, stderr} = runJest(DIR, ['--findRelatedTests', 'a.js', 'b.js'])); + const {summary, rest} = extractSummary(stderr); expect(summary).toMatchSnapshot(); expect( @@ -126,6 +125,12 @@ describe('--findRelatedTests flag', () => { // 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'); }); }); From e496c06851ba8c2d2cc264d8ac538a2bb29a7bbd Mon Sep 17 00:00:00 2001 From: Stian Didriksen Date: Sun, 22 Jul 2018 19:46:50 +0200 Subject: [PATCH 6/7] verify stderr is empty --- e2e/__tests__/find_related_files.test.js | 1 + 1 file changed, 1 insertion(+) diff --git a/e2e/__tests__/find_related_files.test.js b/e2e/__tests__/find_related_files.test.js index a04c52d0e21b..45b8e52f0c3d 100644 --- a/e2e/__tests__/find_related_files.test.js +++ b/e2e/__tests__/find_related_files.test.js @@ -132,5 +132,6 @@ describe('--findRelatedTests flag', () => { // Neither a.js or b.js should be in the report expect(stdout).toMatch('No tests found'); + expect(stderr).toBe(''); }); }); From 427cffee51e0d0a7f63626c8118582ef1b0f4305 Mon Sep 17 00:00:00 2001 From: Cody Olsen Date: Wed, 25 Jul 2018 19:32:55 +0200 Subject: [PATCH 7/7] Update CHANGELOG.md --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5687739574bb..cfda808444f3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,7 @@ - `[babel-jest]` Make `getCacheKey()` take into account `createTransformer` options ([#6699](https://github.com/facebook/jest/pull/6699)) - `[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