Runtime: Road to code coverage

Created on 27 Jan 2019  路  15Comments  路  Source: dotnet/runtime

The list of remaining work necessary for automated code coverage:

Full code coverage

Currently cc for System.Private.CoreLib is broken. These steps are necessary to fix that:

  • [x] Merge https://github.com/tonerdo/coverlet/pull/322
  • [x] Consume a new released version of coverlet and use the IL version of S.P.CoreLib. (https://github.com/dotnet/corefx/pull/37172)
  • [ ] Port official cc run to AzDO (and potentially helix?) and add a Linux (Ubuntu) and OSX leg (https://github.com/dotnet/corefx/pull/35296)

Make code coverage faster (optional)

Integrate codecov into corefx

  • [x] Add codecov token to Azure Pipelines (https://github.com/dotnet/corefx/pull/35296)
  • [ ] Publish refactor codecov-exe as AzDO task (External dependency, needs PR, blocked)
  • [x] Make global tools work with helix runs (https://github.com/dotnet/corefx/pull/34913)
  • [x] Setup codecov page for corefx (https://codecov.io/gh/dotnet/corefx/)
  • [x] Add codecov.yml file (https://github.com/dotnet/corefx/pull/34896)
  • [ ] Add badge to README.md (https://github.com/dotnet/corefx/pull/35296)

Integrate cc and codecov into continuous PRs

  • [ ] Add a Windows, Linux (Ubuntu) and OSX leg to AzDO that runs with every PR and enable codecov uploads.

cc @danmosemsft @safern @ericstj @stephentoub

area-Infrastructure-libraries

All 15 comments

@safern do we use Helix for official runs too? It would be nice if we could use helix for the official code coverage run(s).

We also need to solve the early termination problem right? I see in the vstest issue they say it needs to be a DiagnosticCollector to avoid early termination.

Re speed, I dont believe we often use branch coverage today . Is there a flag to only instrument for block coverage, would that help?

No that issue doesn't affect us as we don't use vstest right now.

Re speed, I dont believe we often use branch coverage today . Is there a flag to only instrument for block coverage, would that help?

There isn't such a flag yet. Fixing the above linked issue would be the ideal first step to increase perf. The current instrumentation logic is suboptimal.

I dont believe we often use branch coverage today

@danmosemsft, what do you mean by this? Where do we "use" line/block coverage but don't branch coverage? I personally look at both.

If it made a significant difference in perf it might mean we could afford to get at least some coverage data automatically in CI rather than none. If we can get both great.

Ok

Re speed, I dont believe we often use branch coverage today . Is there a flag to only instrument for block coverage, would that help?

I strongly recommend _not_ going down this path. Branch and/or sequence point coverage is an extremely valuable metric for the primary value-add of code coverage (guided code reviews).

If it made a significant difference in perf ...

It will not have a substantial impact on performance. Your biggest performance gains will come from --single-hit (tonerdo/coverlet#309) and, if required, selective exclusions of extremely-frequently-hit helpers. The latter is a PerfView-guided process that I'm happy to help with after coverage tooling is set up.

Add a Windows, Linux (Ubuntu) and OSX leg to AzDO that runs with every PR and enable codecov uploads.

I recommend starting with Windows Debug and observing stable uploads for _at least_ a week. After that, we can add one additional Debug upload (Linux Debug or Mac Debug), and let it stablize for _at least_ a week. Our uploads will stress the report merge process used by codecov on multiple fronts:

  1. The reports will be large, possibly approaching or exceeding the system limits even without multiple uploads. Reporting a single report avoids the need to merge reports on codecov servers, which improves our chances of success with all the different features they offer.
  2. The merge process used by codecov is a complicated process intended to accurately represent the combined coverage of multiple runs. For example, if you have an if (IsWindows) statement with a true branch covered in one build and a false branch covered in a different build, the combined report should show that the statement is fully covered by the combined report but only partially covered when you filter the view to just Windows or Linux. However, the merge process assumes that the IL offset of the branch will not change between the two uploads, and does a suboptimal job of handling cases where it does change. This is especially problematic when attempting to upload Debug and Release reports, and less problematic when uploading reports combining deterministic+unconditional compilation.

@safern do we use Helix for official runs too? It would be nice if we could use helix for the official code coverage run(s).

Yes, we do use Helix for official runs. Yes we should figure out how to hook up code coverage with helix 馃槃

The codecov dotnet tool is now available: https://www.nuget.org/packages/Codecov.Tool/

Remaining external blocker is the release of an updated coverlet.console package.

Updated issue with current status.

@ViktorHofer new package with your fixes https://www.nuget.org/packages/coverlet.console/

Status: dotnet test 3.x soon supports coverlet inbox which would help a lot here. Maybe we should give https://github.com/dotnet/corefx/issues/34338 another try.

@ViktorHofer we have also a nightly build https://www.myget.org/feed/coverlet-dev/package/nuget/coverlet.collector
Could be interesting if "sometimes" we can "propose" in some way(throught a PR on some deps config) a usage of particular nightly version to catch issue asap, I mean corefx repo has got a lot of users and if we introduce some bug it's highly probable that someone here hit issue immediatly.
Clearly should be possible quickly "revert" to stable one.
If it's too invasive risky, complex or simply a bad idea, never mind.

Use single hit option for non official builds: tonerdo/coverlet#309

Done https://github.com/tonerdo/coverlet/blob/master/Documentation/VSTestIntegration.md#advanced-options-supported-via-runsettings

cc: @tonerdo and @AArnott that helping with build worfklow.

Was this page helpful?
0 / 5 - 0 ratings