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

Bug 1445352 - Add code coverage for .jsx files #4866

Merged
merged 3 commits into from Apr 2, 2019

Conversation

@piatra
Copy link
Collaborator

commented Mar 25, 2019

I broke down our frontend code into 3 different patterns/categories and added thresholds for each of them.

// All DS components
"content-src/components/DiscoveryStreamComponents/**/*.jsx"
// All asrouter components
"content-src/asrouter/**/*.jsx"
// Everything else
"content-src/components/**/*.jsx"

Coverage varies among these categories: asrouter baseline is now around 50% while some DS components have no tests making the baseline be 0.

The issue I ran into is that karma-coverage-istanbul-reporter does not implement a excludes (or similar) filter for the global threshold declarations. This means that the global requirements include the .jsx numbers and overall bring our global numbers down.
A solution to this issue might be to switch to karma-coverage. I don't know the benefits and trade-offs but the configuration file allows for the same options and in addition you can have an excludes field for the global threshold.

This PR is based on top of #4845

@piatra piatra requested a review from k88hudson Mar 25, 2019

@k88hudson k88hudson added the Blocked label Mar 28, 2019

@piatra piatra force-pushed the piatra:bug1445352 branch from 011598d to ad538f9 Apr 2, 2019

@piatra piatra marked this pull request as ready for review Apr 2, 2019

@piatra piatra added PR / Needs review and removed Blocked labels Apr 2, 2019

@@ -51,10 +51,36 @@ module.exports = function(config) {
// This will make karma fail if coverage reporting is less than the minimums here
thresholds: !isTDD && {
global: {
statements: 93,

This comment has been minimized.

Copy link
@piatra

piatra Apr 2, 2019

Author Collaborator

I think this is ok. Our average went down but it shouldn't affect our test quality since adding more code to .jsm files which had 100% coverage will decrease this average and will catch it.
We can slowly increase these numbers as we add more tests to .jsx files.

statements: 100,
lines: 100,
functions: 100,
branches: 90,
branches: 66,
overrides: {

This comment has been minimized.

Copy link
@k88hudson

k88hudson Apr 2, 2019

Member

Ah, this is perfect.

@k88hudson k88hudson added PR / R+ and removed PR / Needs review labels Apr 2, 2019

@k88hudson k88hudson assigned piatra and unassigned k88hudson Apr 2, 2019

@piatra piatra merged commit f0bf58e into mozilla:master Apr 2, 2019

2 checks passed

Taskcluster (pull_request) TaskGroup: success
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.