Skip to content
This repository has been archived by the owner on Jan 11, 2024. It is now read-only.

Allow Code Coverage to work on Portable PDBs #1740

Merged
merged 4 commits into from
Oct 19, 2017

Conversation

vancem
Copy link

@vancem vancem commented Oct 13, 2017

Currently OpenCover requires MSPDBs. This change makes the coverage task generate MSPDBs from
portable ones so code coverage continues to work (soon OpenCover will support Portable PDBS directly).

We need this so we can switch CoreFX over to using Portable PDBs.

Basically I add a new Target that creates the MSPDB in the WindowsPDB directory, and then add that directory to the search path the OpenCover uses.

@dnfclas
Copy link

dnfclas commented Oct 13, 2017

@vancem,
Thanks for having already signed the Contribution License Agreement. Your agreement was validated by .NET Foundation. We will now review your pull request.
Thanks,
.NET Foundation Pull Request Bot

@@ -6,7 +6,7 @@
-->
<PropertyGroup>
<OpenCoverVersion>4.6.519</OpenCoverVersion>
<ReportGeneratorVersion>2.5.0</ReportGeneratorVersion>
<ReportGeneratorVersion>3.0.1</ReportGeneratorVersion>

This comment was marked as spam.

Currently OpenCover requires MSPDBs.  This change makes the coverage task generate MSPDBs from
portable ones so code coverage continues to work (soon OpenCover will support Portable PDBS directly).

We need this so we can switch CoreFX over to using Portable PDBs.
@vancem vancem force-pushed the CodeCoverageWithPortablePdbs.10-13-17 branch from 47ff5a3 to 2573996 Compare October 16, 2017 22:16
@vancem
Copy link
Author

vancem commented Oct 18, 2017

@dagood @stephentoub I have tested this change in CoreFX (which as far as I know is the only repo that does code coverage). I have also confirmed that it works properly if you use Windows instead of portable (thus it works both ways).

@vancem
Copy link
Author

vancem commented Oct 19, 2017

Including @weshaggard @danmosemsft @karajas for visibility.

At a high level, this is here so that we can move CoreFX to use Portable PDBs instead of Windows PDBs.

Currently OpenCover requires Windows PDBS, so this generates them on the fly during coverage runs.

This task can be REMOVED in a few months when OpenCover supports Portable PDBs.

The change is conceptually very simple. Before running OpenCover convert the portable PDB to a Windows PDB (that is what the new task does).

It also works if the build generates PortablePDBs or the conversion already happened (it does nothing).

I want to check this in later today, so speak now if you have issues.

@danmoseley
Copy link
Member

We also need System.Private.Corelib.pdb to work with code coverage. If you change that to portable, we'll need the same trick.

are generated by the GenFacades task and lack a Debug diretory (see https://github.com/dotnet/buildtools/issues/1739)
This causes ConvertPortablePdbsToWindowsPdbs to fail. Work around it right now by using WarnAndContinue
Soon we won't need this code, (since OpenCover will support portable PDBs) so it is OK to just give up for now. -->
<ConvertPortablePdbsToWindowsPdbs Files="@(ExistingPortableDllsToConvert)" ContinueOnError="WarnAndContinue"/>

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

@vancem
Copy link
Author

vancem commented Oct 19, 2017

We also need System.Private.Corelib.pdb to work with code coverage. If you change that to portable, we'll need the same trick.

I did some snooping around on this, and there does not seem to be much automation for coverage support for System.Private.Corelib directly. Here is some docs in coreFX that tell you about it.

https://github.com/dotnet/corefx/blob/master/Documentation/building/code-coverage.md#code-coverage-with-systemprivatecorelib-code

And I could not find anything in CoreCLR (there is coverage, but it is for native (and seems to be Linux only).

If anyone knows of anything that I should be testing let me know, but to the best of my knowledge the only thing are these instructions for doing coverage of things with System.private.Corelib by hand.

I can update those instructions, but I will do that as part of the CoreFX checkin.

@danmoseley
Copy link
Member

@vancem yes correct, coverage with s.p.corelib requires manual steps. It's improving (eg., we now get the PDB for you) but there are still issues (eg., will not load crossgen PDB last I tried). My goal is to make it "just work". If temporarily the corelib PDB needs to be manually rebuilt with a flag we could live with that if we have to.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants