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

Add combined coverage threshold for directories #4885

Merged
merged 4 commits into from
Nov 21, 2017

Conversation

ttmarek
Copy link
Contributor

@ttmarek ttmarek commented Nov 13, 2017

Summary

Right now you can specify a global threshold in Jest and a threshold for files that match a glob pattern. But you can't specify a threshold for a given directory. This pull request adds that functionality without taking away any functionality from before. With this pull request there will be three possible ways to set a coverage threshold:

  1. global applies the threshold to every covered file.
  2. path applies the threshold to a path (either a directory or a file).
  3. glob applies the threshold to any file matching a glob pattern.

For example, before you could do:

{
  "coverageThreshold": {
    "global": {
      "branches": 50,
      "functions": 50,
      "lines": 50,
      "statements": 50
    },
    "./src/components/**/*.js": {
      "branches": 40,
      "statements": 40
    },
    "./src/api/very-important-module.js": {
      "branches": 100,
      "functions": 100,
      "lines": 100,
      "statements": 100
    }
  }
}

And this would apply a threshold separately...

  • to every file matching the "./src/components/**/*.js" glob pattern
  • to the "./src/api/very-important-module.js" file
  • and any remaining covered files (under the global threshold).

But what if you want to set a threshold for the entire components directory? That is, what if you don't care about the threshold of each individual .js file in the components directory, but you do want to set a combined coverage threshold for all your components. Something like:

{
  "coverageThreshold": {
    "global": {
      "branches": 50,
      "functions": 50,
      "lines": 50,
      "statements": 50
    },
    "./src/components/": {
      "branches": 40,
      "statements": 40
    },
    "./src/reducers/**/*.js": {
      "statements": 90,
    },
    "./src/api/very-important-module.js": {
      "branches": 100,
      "functions": 100,
      "lines": 100,
      "statements": 100
    }
  }
}

And this would apply a threshold separately...

  • to the entire"./src/components" directory (combined coverage for each file under the directory).
  • to every file matching the "./src/reducers/**/*.js" glob pattern.
  • to the "./src/api/very-important-module.js" file
  • and any remaining covered files (under the global threshold).

Test plan

I added a few unit tests that tests this new functionality. I left every other test in the test suite alone to demonstrate that there are no regressions in functionality. Let me know if you think we need to add some more tests, or if there are some integration test that I can create/update.

Add unit test for passing directory coverage

Add test for when there is no coverage data available

Fix type errors and make code more familiar

Run prettier on changed files
@ttmarek ttmarek force-pushed the coverage-threshold-per-directory branch from 68a798d to ab1d9fa Compare November 14, 2017 17:54
@codecov-io
Copy link

codecov-io commented Nov 16, 2017

Codecov Report

Merging #4885 into master will increase coverage by 0.19%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #4885      +/-   ##
==========================================
+ Coverage   59.75%   59.95%   +0.19%     
==========================================
  Files         195      195              
  Lines        6533     6562      +29     
  Branches        4        4              
==========================================
+ Hits         3904     3934      +30     
+ Misses       2629     2628       -1
Impacted Files Coverage Δ
...ckages/jest-cli/src/reporters/coverage_reporter.js 66.4% <100%> (+10.85%) ⬆️

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 38c2cf7...cc61b81. Read the comment docs.

Copy link
Member

@SimenB SimenB left a comment

Choose a reason for hiding this comment

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

Looking good! Can you update the docs and changelog as well?

@ttmarek
Copy link
Contributor Author

ttmarek commented Nov 20, 2017

Thanks for the review @SimenB.

I've updated the changelog and docs. Let me know if you spot any areas that could be improved.

Copy link
Member

@SimenB SimenB left a comment

Choose a reason for hiding this comment

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

LGTM, but this is a part of the code base I haven't touched, so should have another set of eyes

@cpojer cpojer merged commit ddac1cf into jestjs:master Nov 21, 2017
@Ben07
Copy link

Ben07 commented Nov 30, 2017

"coverageThreshold": {
    "src/app/Market/components/Cloud/Item.js": {
      "branches": 80,
      "functions": 80,
      "lines": 80,
      "statements": 80
    }
  },

this is part of my config, then I encountered TypeError: Cannot read property 'statements' of undefined. If I replace "src/app/Market/components/Cloud/Item.js" to "global" , everything is OK

@SimenB
Copy link
Member

SimenB commented Nov 30, 2017

Itæs not released yet - use jest@test or wait for #4948

@github-actions
Copy link

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.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 13, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants