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

Allow option to instrument modules for which some source files listed in the associated .pdb file are not present #1193

Closed
wyattm124 opened this issue Jul 13, 2021 · 7 comments
Labels
discussion Generic discussion on something feature-request New feature request

Comments

@wyattm124
Copy link

Hello,

Our engineering team has a sizable .dll (***.netstandard.dll sorry the name is redacted for privacy) that we would like to instrument using the coverlet.console.exe. However after we create the .dll using MSBuild, we also repack the .dll with all of its dependencies into one .dll that we release to our customers. When instrumenting this .dll, we come across the following issue :

...
Unable to instrument module: C:\***\bin\Debug\netcoreapp3.1\***.netstandard.dll, pdb without local source files, [C:\Repos\HtmlAgilityPack\HtmlAgilityPack.Shared\crc32.cs]
Instrumented module: 'C:\***\bin\Debug\netcoreapp3.1\***CoreOperations.dll'
...

Our main .dll that we would like to instrument for code coverage cannot be instrumented because the .pdb file associated with the .dll contains references to the Html agility pack .dll (which can be found here : https://www.nuget.org/packages/HtmlAgilityPack/ we use version : 1.11.24) and the source code for this package cannot be found locally. However, for some reason this problem does not occur for us with our dependencies like NLog (which can be found here : https://www.nuget.org/packages/NLog/ , we use version : 4.7.7). Is there a way that we could repack our assembly so that all the required files are available?

Or, could we submit a PR to allow the option to skip verifying that all the source files referenced in a .pdb file are present? We were able to instrument ***.netstandard.dll and receive decent code coverage results by commenting out the use of the PortablePdbHasLocalSource function from the InstrumentationHelper.cs file.

@MarcoRossignoli MarcoRossignoli added the feature-request New feature request label Jul 13, 2021
@MarcoRossignoli
Copy link
Collaborator

MarcoRossignoli commented Jul 13, 2021

Thx @wyattm124 for the proposal, we have more than one issue like that, we need to think to something, relax a bit the heuristic(% of files for instance 90%) or provide an argument, the best tradeoff between correctness and usage(no argument at all).

cc: @daveMueller @petli @tonerdo

@MarcoRossignoli MarcoRossignoli added the discussion Generic discussion on something label Jul 13, 2021
@wyattm124
Copy link
Author

Hello Marco,

Thank you for a prompt response.

If I am able to chime in, our team would prefer to be able to set a flag such as --allow-partial-instrumentation that would only log the source files that the global tool is not able to find as referenced in the .pdb file, while instrumenting all the .dll files it is able to find, or maybe only those with corresponding source files.

We have many dependencies that we are not looking for coverage in, and for performance it may be best not to instrument many of our dependencies anyway. I guess ordinarily we would exclude these dependencies from the application folder the global tool is instrumenting, but because we repack the dependencies with the code that we created into one .dll file, this is not possible.

If there are other approaches in the mix, or other related issues, feel free to link them here.

@MarcoRossignoli
Copy link
Collaborator

MarcoRossignoli commented Jul 16, 2021

Another idea would be skip heuristic in case of https://github.com/coverlet-coverage/coverlet/blob/master/Documentation/MSBuildIntegration.md#filters filters

I mean if you specify include/exclude filter means that you know what you want to instrument and so we can skip our check and go on. This can lead to some strange report(we cannot link missing source file to report) but in that case you can exclude(generated files) after using source file exclusion https://github.com/coverlet-coverage/coverlet/blob/master/Documentation/MSBuildIntegration.md#source-files

wdyt?

@MarcoRossignoli MarcoRossignoli pinned this issue Jul 16, 2021
@MarcoRossignoli
Copy link
Collaborator

cc: @sirkiza

@wyattm124
Copy link
Author

Hello @MarcoRossignoli,

Sorry for the late response. In our case if the Exclude and Include filters were extended to the coverlet global tool (coverlet.console.exe) that would be a great solution to our problem. In general, the heuristic may still be helpful but because we repack all our assemblies into one, and we only want code coverage for a small portion of the repacked assembly, we may need to skip the heuristic.

I assume that skipping the heuristic may create some odd code coverage results. I am not as familiar with this project to understand entirely how, but I could see how including the ExcludeByFile filter may help with this in creating more complete code coverage results, and would appreciate source file filters for coverlet.console.exe as well.

Overall, if these filters were included as options when running the coverlet.console.exe executable, our team would greatly appreciate the features and they would help us collect code coverage results within our test pipeline.

@petli
Copy link
Collaborator

petli commented Aug 5, 2021

I wrote an issue for a similar problem (#1164) which may also help with your specific problem.

@MarcoRossignoli
Copy link
Collaborator

MarcoRossignoli commented Aug 7, 2021

Will close this issue in favor of @petli one #1164

We can continue the discussion there.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discussion Generic discussion on something feature-request New feature request
Projects
None yet
Development

No branches or pull requests

3 participants