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

chenesan · 2020-04-05T09:36:07Z

Summary

This PR fixes #8545 . It will show coverage of sources related to tests when jest is in watch mode (--watch) or --onlyChange.

As for implementation, this PR added findRelatedSourceFromTestsInChangedFiles in SearchSource, and apply it when there are changedFiles, then pass the results into options of shouldInstrument. When changedFiles exist and the file is not in changedFiles nor sourcesRelatedToTestsInChangedFiles, we will not instrument it.

Test plan

Add an e2e test case for the use case and some unit test case for SearchSource.

SimenB

This looks great! Thanks for the thorough tests 👍

packages/jest-runner/src/runTest.ts

packages/jest-reporters/src/coverage_worker.ts

packages/jest-core/src/runJest.ts

packages/jest-core/src/SearchSource.ts

packages/jest-core/src/TestScheduler.ts

packages/jest-core/src/__tests__/SearchSource.test.ts

e2e/__tests__/onlyChanged.test.ts

codecov-io · 2020-04-05T11:03:22Z

Codecov Report

Merging #9769 into master will decrease coverage by 0.01%.
The diff coverage is 47.72%.

@@            Coverage Diff             @@
##           master    #9769      +/-   ##
==========================================
- Coverage   64.39%   64.37%   -0.02%     
==========================================
  Files         290      290              
  Lines       12361    12391      +30     
  Branches     3056     3059       +3     
==========================================
+ Hits         7960     7977      +17     
- Misses       3761     3770       +9     
- Partials      640      644       +4

Impacted Files	Coverage Δ
packages/jest-core/src/TestScheduler.ts	`65.64% <ø> (+0.72%)`	⬆️
packages/jest-reporters/src/coverage_reporter.ts	`53.57% <0.00%> (-0.33%)`	⬇️
packages/jest-reporters/src/coverage_worker.ts	`33.33% <0.00%> (-6.67%)`	⬇️
...ckages/jest-reporters/src/generateEmptyCoverage.ts	`66.66% <ø> (ø)`
packages/jest-runner/src/runTest.ts	`2.15% <ø> (+0.02%)`	⬆️
packages/jest-runtime/src/index.ts	`58.71% <ø> (ø)`
packages/jest-transform/src/shouldInstrument.ts	`78.94% <0.00%> (-9.29%)`	⬇️
packages/jest-core/src/runJest.ts	`51.19% <27.27%> (-4.81%)`	⬇️
packages/jest-core/src/SearchSource.ts	`64.95% <75.00%> (+4.75%)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 32aaff8...0d50802. Read the comment docs.

chenesan · 2020-04-05T12:38:28Z

@SimenB Thanks for the review :-)
Just address all review comments in 2f4c55b; Also use hasSCM() utility in findTestsRelatedToChangeFiles in 39a84f3 to prevent repetition.

CHANGELOG.md

chenesan · 2020-04-07T13:15:41Z

Hi @SimenB @thymikee @jeysal any news on this?

SimenB · 2020-04-07T13:50:52Z

This is good to go from my side, I'd just like further review on it.

With Easter holidays and the coronavirus things have slowed down a bit, I'm sure they'll review this soon enough 🙂

chenesan · 2020-04-07T14:49:29Z

Oh I see. Hope you all good during this epidemic period 🙏 Take care 😷

chenesan · 2020-04-22T15:15:22Z

Hi @thymikee @jeysal Sorry for repeatedly pinging, any review on this?

thymikee

Nice improvement 👍 Left a few notes on how I think we could make it make it a bit more performant

thymikee · 2020-04-25T17:44:17Z

packages/jest-core/src/runJest.ts

+      testSchedulerContext.changedFiles = changedFilesInfo.changedFiles;
+      const sourcesRelatedToTestsInChangedFilesArray = contexts
+        .map(context => {
+          const searchSource = new SearchSource(context);


Creating the SearchSource for a given context is already done earlier, when finding related tests. We could optimize this by storing the searchSources in and array, when mapping contexts for testRunData and reuse it. I think it's worth not doubling the efforts we already made.

thymikee · 2020-04-25T17:45:43Z

packages/jest-core/src/SearchSource.ts

+      return [];
+    }
+    const {changedFiles} = changedFilesInfo;
+    const dependencyResolver = new DependencyResolver(


The same DependencyResolver is also created for findRelatedTests. Can we cache it then?

chenesan · 2020-04-26T07:42:11Z

Good idea 😸 Implement reusing SearchSource and DependencyResolver in 73271f0 and 0d50802.

chenesan · 2020-04-26T07:45:15Z

packages/jest-core/src/SearchSource.ts

  private _testPathCases: TestPathCases = [];

  constructor(context: Context) {
    const {config} = context;
    this._context = context;
+    this._dependencyResolver = null;


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.

SimenB · 2020-04-26T07:45:21Z

packages/jest-core/src/SearchSource.ts

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


Suggested change

const isTestFile = this.isTestFilePath(filePath);

if (isTestFile) {

if (this.isTestFilePath(filePath) {

SimenB · 2020-04-26T07:47:11Z

packages/jest-core/src/__tests__/SearchSource.test.ts

+    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();
+        },
+      );
+    });


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

SimenB · 2020-04-26T07:49:22Z

packages/jest-transform/src/shouldInstrument.ts

+  if (options.changedFiles) {
+    if (!options.changedFiles.has(filename)) {


Suggested change

if (options.changedFiles) {

if (!options.changedFiles.has(filename)) {

if (options.changedFiles && !options.changedFiles.has(filename)) {

* upstream/master: Prints the Symbol name into the error message with a custom asymmetric matcher (jestjs#9888) Show coverage of sources related to tests in changed files (jestjs#9769) fix: don't /// <reference types="jest" /> (jestjs#9875)

…pshots * upstream/master: (39 commits) Prints the Symbol name into the error message with a custom asymmetric matcher (jestjs#9888) Show coverage of sources related to tests in changed files (jestjs#9769) fix: don't /// <reference types="jest" /> (jestjs#9875) noCodeFrame respects noStackTrace (jestjs#9866) chore: update example to [email protected] (jestjs#9746) Improve source map handling when instrumenting transformed code (jestjs#9811) Update .vscode/launch.json settings (jestjs#9868) chore: verify all packages have the same engine requirement (jestjs#9871) fix: pass custom cached realpath function to `resolve` (jestjs#9873) chore: mock stealthy-require in tests (jestjs#9855) chore: update resolve (jestjs#9872) chore: run CircleCI on node 14 (jestjs#9870) Add an option to vscode settings to always use workspace TS (jestjs#9869) fix(esm): handle parallel imports (jestjs#9858) chore: run CI on Node 14 (jestjs#9861) feat: add `@jest/globals` package for importing globals explici… (jestjs#9849) chore: bump resolve package (jestjs#9859) chore(runtime): simplify `createJestObjectFor` (jestjs#9857) chore: fix symlink creation failures on windows in tests (jestjs#9852) chore: skip mercurial tests when no hg installed (jestjs#9840) ...

github-actions · 2021-05-11T08:06:50Z

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.
Please note this issue tracker is not a help forum. We recommend using StackOverflow or our discord channel for questions.

facebook-github-bot added the cla signed label Apr 5, 2020

chenesan force-pushed the trigger-coverage-of-deps-on-test-file-change branch from 11f20c8 to e236946 Compare April 5, 2020 10:44

SimenB approved these changes Apr 5, 2020

View reviewed changes

SimenB requested review from jeysal and thymikee April 5, 2020 10:49

SimenB reviewed Apr 5, 2020

View reviewed changes

CHANGELOG.md Outdated Show resolved Hide resolved

chenesan added 6 commits April 22, 2020 23:06

Add findRelatdSourcesFromTestsInChangedFiles in SearchSource

18d7127

check sourcesRelatedToTestsInChangedFiles in shouldInstrument

6cf2150

Add tests for findRelatedSourcesFromTestsInChangedFiles

1c80400

Update CHANGELOG.md

e2665ea

Address code review suggestion

4abe272

Use hasSCM utility in SearchSource.findTestRelatedToChangeFiles

e724874

chenesan force-pushed the trigger-coverage-of-deps-on-test-file-change branch from 754f365 to e724874 Compare April 22, 2020 15:13

thymikee reviewed Apr 25, 2020

View reviewed changes

chenesan added 2 commits April 26, 2020 15:20

Create searchSources at first for reuse in runJest

73271f0

Create and keep DependencyResolver in SearchSource for reuse

0d50802

chenesan commented Apr 26, 2020

View reviewed changes

SimenB approved these changes Apr 26, 2020

View reviewed changes

Address code review suggestion

913617b

chenesan force-pushed the trigger-coverage-of-deps-on-test-file-change branch from 9df97b2 to 913617b Compare April 26, 2020 08:04

thymikee approved these changes Apr 26, 2020

View reviewed changes

thymikee merged commit 3f107a3 into jestjs:master Apr 26, 2020

SimenB mentioned this pull request Apr 28, 2020

TypeError: options.sourcesRelatedToTestsInChangedFiles.has is not a function #9910

Closed

chenesan mentioned this pull request May 13, 2020

Run jest in --watch --coverage doesn't update coverage info when files change #8491

Closed

dependabot bot mentioned this pull request Mar 17, 2021

Bump jest from 25.3.0 to 25.5.4 newerton/gostack-template-fundamentos-node#10

Merged

github-actions bot locked as resolved and limited conversation to collaborators May 11, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

chenesan commented Apr 5, 2020 •

edited by thymikee

Loading

SimenB left a comment

codecov-io commented Apr 5, 2020 •

edited

Loading

chenesan commented Apr 5, 2020 •

edited

Loading

chenesan commented Apr 7, 2020

SimenB commented Apr 7, 2020

chenesan commented Apr 7, 2020

chenesan commented Apr 22, 2020

thymikee left a comment

thymikee Apr 25, 2020

thymikee Apr 25, 2020

chenesan commented Apr 26, 2020

chenesan Apr 26, 2020

SimenB Apr 26, 2020

SimenB Apr 26, 2020

SimenB Apr 26, 2020

github-actions bot commented May 11, 2021

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

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

		if (options.changedFiles) {
		if (!options.changedFiles.has(filename)) {

	if (options.changedFiles) {
	if (!options.changedFiles.has(filename)) {
	if (options.changedFiles && !options.changedFiles.has(filename)) {

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

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

Conversation

chenesan commented Apr 5, 2020 • edited by thymikee Loading

Summary

Test plan

SimenB left a comment

Choose a reason for hiding this comment

codecov-io commented Apr 5, 2020 • edited Loading

Codecov Report

chenesan commented Apr 5, 2020 • edited Loading

chenesan commented Apr 7, 2020

SimenB commented Apr 7, 2020

chenesan commented Apr 7, 2020

chenesan commented Apr 22, 2020

thymikee left a comment

Choose a reason for hiding this comment

thymikee Apr 25, 2020

Choose a reason for hiding this comment

thymikee Apr 25, 2020

Choose a reason for hiding this comment

chenesan commented Apr 26, 2020

chenesan Apr 26, 2020

Choose a reason for hiding this comment

SimenB Apr 26, 2020

Choose a reason for hiding this comment

SimenB Apr 26, 2020

Choose a reason for hiding this comment

SimenB Apr 26, 2020

Choose a reason for hiding this comment

github-actions bot commented May 11, 2021

chenesan commented Apr 5, 2020 •

edited by thymikee

Loading

codecov-io commented Apr 5, 2020 •

edited

Loading

chenesan commented Apr 5, 2020 •

edited

Loading