Code coverage service for shields

[platan]

What do you think about adding a code coverage service (like https://codecov.io/#features or https://coveralls.io/features, an example results https://codecov.io/gh/babel/babel) to this project? I know that our service tests are not 100% reliable, but in my opinion such service would be helpful in finding untested parts of the code in PR. What are your experiences with this kind of services?

[paulmelnikow]

Yes, I'd love it! I haven't used Codecov, though with IcedFrisby I've used Coveralls and it works well. A scoreboard might be a big motivator – both to get coverage up in general, and to complete the rewrite (#1358).

Once we choose, we might need to ask Thaddée to set it up for us. I don't have "owner" access to the Shields organization so there are certain apps I'm not able to set up.

[RedSparr0w]

👍 Definitely agree it would be useful to have coverage checked on PR's

[chris48s]

I think for something like coveralls it is a problem that we don't run all of our tests each merge/PR. i.e: unless we prefix every PR with "[amo, ansible, appveyor, bitbucket....." or just run all the service tests on every PR (which would be slow among other things ), I think something like coveralls will just generate a meaningless number that goes up and down, rather than telling us anything useful. Maybe codecov is more configurable in that respect.

If not, maybe we could set up a config that defines a subset of the codebase where we are measuring coverage which excludes the services?

[paulmelnikow]

I think something like coveralls will just generate a meaningless number that goes up and down, rather than telling us anything useful. Maybe codecov is more configurable in that respect.

Hmm, that's a great point!

If not, maybe we could set up a config that defines a subset of the codebase where we are measuring coverage which excludes the services?

This seems like a good way to go.

When you're writing tests for a service, it would also be good to see coverage of the service code. When I look at https://coveralls.io/github/IcedFrisby/IcedFrisby, I see it does a nice job of showing coverage across different files and/or parts of the tree. Though it wouldn't be as useful for the kind of "single number" coverage we'd want to track over time for master, it might be useful enough.

Another hack option would be to use codecov for core coverage, and coveralls for services coverage, or vice versa. 😁 The core number should be reliable, and trending upward, while for the services the meaningful information is at the file level.

I did set up some coverage tooling using istanbul/nyc which similarly breaks out the core code from the service tests. I'm not sure if it's working right now, though. I haven't run it in a while.

[platan]

1. We could send coverage results (code + services) only if all service tests are passing in daily builds. This will provide stable coverage results.
2. I don't know how to handle PRs. For me it's important to have coverage results online for PR. But as you said running all service tests is not a good idea. And without them we cannot produce stable coverage number. Maybe we are able to configure coveralls/codecov to only show link to coverage results without a coverage number.

@paulmelnikow nyc configuration in shields is working great for me :-)

[chris48s]

We could send coverage results (code + services) only if all service tests are passing in daily builds. This will provide stable coverage results.

Agreed. The right solution for total repo coverage would be to calculate based off a daily build which runs the full service test suite.

Combining that with @paulmelnikow 's idea of using 2 services, we could use coveralls to measure the whole repo coverage based on a daily build and then use codecov on PRs. I think that would work because codecov can provide a file-level coverage report as a PR comment:

Although the top-level number/difference from parent would fluctuate, once we're splitting the service code out of server.js and each service integration lives in its own file, the file-level coverage number would be meaningful enough to be useful. If we set up codecov to ignore server.js I think that would give us a config that provides useful information in most cases (at least moving forwards, as we migrate). By the looks of it, it is also possible to customise the PR template a bit: https://docs.codecov.io/docs/pull-request-comments

I'm not sure we can sensibly attempt to measure the coverage of server.js while only running part of the test suite that covers it, but that is more incentive to make progress with the migration :)

[paulmelnikow]

After a little nudge via #2314, Coveralls is now running daily on master, via another repo with Circle. It's a full test run, including all the unit tests and service tests.

https://coveralls.io/builds/20137843

The reason I chose Circle is because #979 was a long-standing issue solved by Circle, and I didn't want to solve that problem again.

[paulmelnikow]

Coveralls is working well.

Does it still seem helpful to set up Codecov for PRs?

[paulmelnikow]

If we want to pick that up let's open a new issue.