Closes #83: report JavaScript code coverage in PRs

[filmaj]

I use the codecov.io service on a lot of my JavaScript projects, as it automatically comments how test code coverage changes with each incoming PR to a project. I find that feature useful. If the owners of Hubble agree, feel free to pull this PR, but please read the sections below for some details on how it all works and what permission changes are needed.

How It Works

1. Every time Travis runs a CI job, it'll execute npm run codecov after the tests complete. This will go through and parse the JS code coverage report files that our karma test runner creates to understand what the JS code coverage level is in the PR.
2. In order to report the code coverage difference in PRs, we need to ensure that Travis-CI is building commits on the master branch, so that we have a 'base' coverage level we can compare against. Looking at autodesk/hubble's commit list, I see green check marks that link to Travis jobs, so I think that's already in place.
3. codecov.io then does a comparison between the code coverage reported in a PR and the code coverage as reported in the latest master commit. The service will then report that difference right in the PR, as a comment. I went through this flow on my fork of hubble, using @pluehne's latest aggregation function changes as an example. You can see what a comment from the codecov.io service would look like here.

Permission Requirements

We'd need to give codecov.io permission to access the repo, though. Someone with autodesk organization rights would need to head over to https://codecov.io/gh/autodesk/hubble, sign in with their GitHub credentials, and give codecov.io the necessary access. The setup is not complex - I think you just need to flip a switch for a particular repository (similar to how Travis does it).

If we do merge this in, every PR created after this gets merged in would start getting JS code coverage reported as a bot comment directly inside new PRs.

[codecov-io]

Codecov Report

❗ No coverage uploaded for pull request base (master@7d8d3d5). Click here to learn what that means.
The diff coverage is n/a.

@@            Coverage Diff            @@
##             master      #84   +/-   ##
=========================================
  Coverage          ?   11.78%           
=========================================
  Files             ?        1           
  Lines             ?      280           
  Branches          ?        0           
=========================================
  Hits              ?       33           
  Misses            ?      247           
  Partials          ?        0

---

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 7d8d3d5...4a926aa. Read the comment docs.

[filmaj]

As mentioned, this comment ☝️ has no "base" code coverage report to compare against in master. If/when these changes get merged, every commit on master moving forward will have base reports be generated and stored, so moving forward we'll have code coverage reporting in PRs.

[pluehne]

@filmaj: Thanks for taking care of the Hubble infrastructure. I agree that it’s nice to have code coverage tracked by Codecov 😃.

From my experience, Codecov tends to babble about very minor coverage changes, for instance, when doing small refactorings. Is limiting Codecov output to bigger coverage changes (for instance, more than 0.5 %) something we could or should do?

If not, that’s still a detail we could certainly live with given the benefit of being notified about more significant changes in code coverage.

[filmaj]

@pluehne I will take a look and see if that's possible. I agree with you that it can be pretty verbose 😞

A perfectly acceptable answer is also "no thanks" - consider this PR just as much soliciting feedback as recommending changes. I appreciate honesty :)

[filmaj]

Ah yes, looks like the pull request comments from codecov are completely customizable. I'm going to play around with those and see if I can create a few examples that we can look at.

[filmaj]

Couple things we could try to make codecov less verbose:

1. Set the require_changes flag so that codecov will only post if there is a change in code coverage in a PR.
2. Set the require_base flag so codecov doesn't post if no base coverage report exists.
3. We can tweak the layout section and remove some sections we don't want. There are four sections that are included by default:
   • the reach, or "coverage graph" - which in the above codecov comment is the big red blob. I think we can remove this.
   • the diff should be kept - this is the information that matters IMO.
   • the files lists the coverage change per-file. I'm ambivalent about this one - we could keep it or remove it, I don't really care.
4. We can update the behaviour section to customize how codecov behaves with its PR comments. I'd recommend using the new setting, which posts a comment in a PR (and deletes its own previous comments, if any exist) every time the coverage changes due to commits landing in a PR. This way every PR will only ever have a single comment from codecov (although if you get email notifications from PR comments, you'd still get one email for each change).

Last thing that I just learned, and is kind of cool, is codecov's flags. Seems like this allows us to set code coverage targets and have these somehow (still figuring it out!) show up as requirements or checks in GitHub PRs. That is, we can say the docs/ folder requires at least e.g. 10% code coverage, and if a PR comes in that doesn't satisfy that, a check would show up as a red X in the github PR - making it super visual. This can also be customized for specific folders, too, so in theory we could set one code coverage target for the web app (docs/ folder), and a different code coverage target for the python scripts (updater/ folder).

Let me know what you think and I can put together a proof of concept with some of this stuff on my fork.

[filmaj]

I've put the above changes into practice on my fork, you can check out a test PR I opened against my fork to see how the output will look. Worth noting that I had to merge a .codecov.yml file with all of these customizations into the master branch of my fork in order to make them visible on the PR.

A summary of the changes and things to note / discuss:

• I implemented the Flags feature I described above, setting the stage for treating the Python and JS portions of Hubble separately in terms of code coverage. I tweaked the node.js codecov task (as implemented in package.json) so that Travis-CI uploads the code coverage reports for the docs/ part of Hubble to codecov with a docs flag. Similarly, I've set the stage for the Python bits by specifying an updater flag to only look at code coverage reports generated under the updater/ folder. Down the road, once a Python testing framework is put into place, we can look at what options exist in Python for generating code coverage reports that are compatible with codecov. Codecov seems to have good support for Python so I don't expect problems there.
• If you scroll down to the green checkmark area titled "All checks have passed" and click "show all checks", you'll see there are two checks being reported by codecov: the patch coverage as well as the project/docs/ coverage. We can customize the desired level of coverage, and whether these checks succeed or fail, to either be 'auto' or a particular %, with 'auto' using the last commit as a baseline (and therefore if coverage increases, rendering a green checkmark, and if it decreases, rendering a red x). We can also set a threshold to a certain %, meaning that if coverage drops by e.g. 5%, we can still consider that a success. For my example PR above, I set the target to be 'auto' and the threshold to be 0, meaning: if coverage remains the same or improves, render a checkmark, otherwise, render a red X. Feels like a good baseline to me.
  • The project/docs/ coverage shows the global code coverage for everything under the docs/ folder (in our case, JavaScript). So it reports the current code coverage of all our JS, including the changes introduced by the PR.
  • The patch coverage shows how much of just the diff being introduced by the PR is covered with tests. This gives us a good sense of how well tested PRs are - which is a useful metric, I think. We can, by the way, tweak the target and threshold numbers for patch coverage independently of project coverage, or even remove reporting patch coverage entirely, if we want.
• The tweaked comment layout is still a bit big, but it's an improvement over the one you see in here, I think. It doesn't render the graph, nor the legend at the bottom, although it does show these "flags" i configured - but I can remove those too.

Let me know what y'all think.

[filmaj]

There's also a lot more detail about customizing the behaviour in codecov's support wiki. We could also:

• turn off codecov comments entirely and simply rely on the status commit checks. If a PR comes in reducing code coverage by a certain threshold amount, we could flag a warning on the entire PR - but not resort to a comment. This would be way less verbose, but still let us know coverage drops enough for us to care.
• we can tweak the codecov comments to include just the "header" layout, which is a lot less verbose.

[pluehne]

@filmaj: Gosh, thank you so much for all your research on Codecov!

When I said that Codecov tends to be verbose, that wasn’t meant as an absolute blocker from my side 🙂. Still, I like the Codecov options you have experimented with—in particular, the new setting in the behavior section to keep only the most recent Codecov comment and delete all previous ones.

I wouldn’t set hard limits on pull requests, though. I like the fact that @larsxschneider and I can keep track of code coverage, but I’d prefer not to enforce that strictly and rather request test coverage personally where it makes most sense.

Thanks also for figuring out how to track coverage for docs/ and updater/ separately—that’s something I hadn’t thought of immediately!

Setting a permissive threshold of, say, 1–2 % is something I’d like to have for the reason mentioned earlier (very small fluctuations in the coverage can quickly become annoying to be notified about with a red cross 🙂).

Finally, I also agree with your point that patch coverage might be very interesting to look at for individual pull requests. And I like the compact “header” layout very much—it’s concise and contains a link to the full report in case someone’s interested.

[filmaj]

I've updated this PR with the codecov behaviour details you mentioned:

• disabled reporting the patch coverage to commit statuses (but if you want it back let me know)
• set a threshold of 2% for the project code coverage commit status setting
• the comments are set to be the lightweight "header" format
• ... and will delete any previous comments whenever a PR changes
• if the base of the PR has no code coverage report baseline, then codecov won't comment on the PR nor report any commit statuses
• if the code coverage doesn't change on the PR, no comments will be posted

[pluehne]

@filmaj: That sounds great! Let’s try it out like this for some time, as we can still adjust the settings later if we wanted to.

[larsxschneider]

Crazy. Why do we need all these packages? I assume codecov references them somehow?!

[filmaj]

Yeah I guess it must come from codecov. It is indeed crazy. Not sure what to do about it though...

[larsxschneider]

No worries. I guess that's the NPM world...

[filmaj]

Indeed...

For what it's worth, all of these dependencies come out from the devDependencies section of package.json, which means these are dependencies only needed for development of the Hubble dashboard/webapp. Consumers won't need any of this. Only developers of the webapp need to run npm install and pull all these extra modules in.

[larsxschneider]

@filmaj I want to second @pluehne , thanks a lot for you work! That's super great! The only blocking issue is Codecov right now. They want write access for "Repository webhooks and services" and "Commit statuses" as well as read access for "Organizations and teams". I need to carefully check that first.

[filmaj]

@larsxschneider completely understand that, makes sense. If I recall correctly, you don't need to give codecov access to webhooks if you configure the webhook codecov requires on your own. It only asks for webhook write access if you feel lazy and don't want to muck about in the repo settings. Org read access, I believe, is so codecov can determine what repos are available and offer to set up codecov integrations for them automatically for you. The only permission that it absolutely needed is write for commit statuses - that gives codecov the ability to mark a PR as red x or green checkmark by writing code coverage status to each commit as metadata.