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

Fix source file check for non portable pdb #558

Merged

Conversation

MarcoRossignoli
Copy link
Collaborator

@MarcoRossignoli MarcoRossignoli commented Sep 19, 2019

Thank's to @adrianhara #524 (comment) I found an issue on new local source file check during instrumentation.
If pdf is not a portable pdb metadataReaderProvider.GetMetadataReader() throws exception Invalid COR20 header signature. because we can read only portable pdb.

65247903-87b0df80-daf1-11e9-9187-74e256b94283

For now in case of exception for this reason we'll skip check so we'll instument module.
Return false would block user and in case of issue we'll can skip bad module with exclusion filter, so I think it's a good solution.

cc: @tonerdo @petli

Thank's a lot Adrian for the repro!

NB: minor change, I fixed redundant verbosity log.
New verbose output

image

@MarcoRossignoli MarcoRossignoli added the bug Something isn't working label Sep 19, 2019
@MarcoRossignoli MarcoRossignoli merged commit 82a9208 into coverlet-coverage:master Sep 19, 2019
@MarcoRossignoli MarcoRossignoli deleted the handlenonportablepdb branch September 19, 2019 14:31
@MarcoRossignoli
Copy link
Collaborator Author

MarcoRossignoli commented Sep 19, 2019

@adrianhara could be great if tomorrow you'll give it a try(with nightly) with non portable pdb and tell us if it works as expected!

@adrianhara
Copy link

@MarcoRossignoli thanks for the quick fix. I'll give it a try and let you know.

@adrianhara
Copy link

I tried the latest nightly and it works fine. Thanks @MarcoRossignoli ! 👍

@MarcoRossignoli
Copy link
Collaborator Author

@adrianhara we released new package https://www.nuget.org/packages/coverlet.msbuild/2.7.0 you can switch to production!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants