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

Skip instrumentation of module with embedded ppbd without local sources #510

Merged
merged 5 commits into from
Aug 14, 2019

Conversation

MarcoRossignoli
Copy link
Collaborator

closes #476

cc: @tonerdo

@MarcoRossignoli MarcoRossignoli added the enhancement General enhancement request label Aug 5, 2019
@MarcoRossignoli MarcoRossignoli changed the title Handle embedded ppbd without local sources Skip instrumentation of embedded ppbd without local sources Aug 5, 2019
@MarcoRossignoli MarcoRossignoli changed the title Skip instrumentation of embedded ppbd without local sources Skip instrumentation of module with embedded ppbd without local sources Aug 5, 2019
@MarcoRossignoli
Copy link
Collaborator Author

@ViktorHofer does it work with corelib?

@MarcoRossignoli
Copy link
Collaborator Author

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@clairernovotny
Copy link
Contributor

Will this also skip a module with an external PDB that doesn't have local source? That's another possible scenario that should probably be handled the same way.

@MarcoRossignoli
Copy link
Collaborator Author

Will this also skip a module with an external PDB that doesn't have local source? That's another possible scenario that should probably be handled the same way.

No this PR handle only embedded PDB...do you've some case with that issue?

@clairernovotny
Copy link
Contributor

If a nuget package includes the pdb's in it, default behavior will put them in the output directory, which would lead to the same issue.

@MarcoRossignoli
Copy link
Collaborator Author

MarcoRossignoli commented Aug 5, 2019

If a nuget package includes the pdb's in it, default behavior will put them in the output directory, which would lead to the same issue.

Can you point me to a package behaves like that, so I can write a test.

@clairernovotny
Copy link
Contributor

I'd have to make one as I don't know one off the top of my head.

@ViktorHofer
Copy link
Contributor

@ViktorHofer does it work with corelib?

I didn't test this but it should work fine as only changes to the ModuleTrackerTemplate are dangerous for CoreLib coverage.

@MarcoRossignoli
Copy link
Collaborator Author

@tonerdo can you take a look?
With this update I think that we could also remove fixed xunit filters...maybe in future.

@tonerdo
Copy link
Collaborator

tonerdo commented Aug 14, 2019

@MarcoRossignoli we can go ahead an remove xunit filters before I make another release this Friday

@tonerdo tonerdo merged commit 0faaeea into coverlet-coverage:master Aug 14, 2019
@MarcoRossignoli MarcoRossignoli deleted the embeddedpdb branch August 14, 2019 13:39
@MarcoRossignoli
Copy link
Collaborator Author

MarcoRossignoli commented Aug 14, 2019

@MarcoRossignoli we can go ahead an remove xunit filters before I make another release this Friday

@tonerdo I thought about a bit more...I don't know if could lead to same problem in future, I mean what if for some reason xunit will remove embedded pdb?
Maybe we could keep filter as fallback. What do you think?
EDIT: Never mind...understood I'll remove. We should also skip in case of local pdb downloaded from nuget but without source, but we need a "real sample" of such type of package to test.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement General enhancement request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Coverlet assumes coverage of 3rd party files with embedded ppdb
4 participants