coverlet-coverage / coverlet

Cross platform code coverage for .NET
MIT License
2.94k stars 386 forks source link

Code coverage rewrite of dll/pdb make crash/hang DMPs less diagnosable #985

Open AArnott opened 3 years ago

AArnott commented 3 years ago

When my tests hang or crash in Azure Pipelines, I collect a DMP of the test process along with all the dll's and pdb's so I can analyze the failure on my dev box. However the VS debugger tells me that the PDB doesn't match the dll in memory. But debugger tools also tell me the dll and pdb on disk that I have match.

After a few hours of painstaking investigation, @auott and I found that it was code coverage that was changing the dll/pdb during the test run as part of instrumentation and then restoring the original dll/pdb at the conclusion of the test run, such that recovering the pdb that matches the DMP is now impossible.

Can the instrumented PDB please be left somewhere so I can capture it as a pipeline artifact and therefore diagnose test run failures better?

petli commented 3 years ago

If possible I'd recommend running a pure unit test run without code coverage enabled and then a second run that collects code coverage. That way any issues that code coverage measurement may throw up will not give you false errors on the unit tests themselves, and also ensures that you can analyse test failures on unmodified DLLs.

AArnott commented 3 years ago

I may have to do that, but as many of our repos take several minutes to run tests, I'd rather not have to run them twice. Also, they're not all entirely stable, as much as I'd like them to be (thus the DMP analysis being important). So running them twice just gives them twice the chance to fail, but only half the chance to analyze those failures.

MarcoRossignoli commented 3 years ago

We could add an argument to specify a new folder where put the instrumented dll/pdb...could be useful also for debugging.

@tonerdo @petli what do you think?

petli commented 3 years ago

It sounds good, I think.

An extension of that thought: would it be possible to always run the coverage test from a new folder containing instrumented DLLs (and all the ones they depend on, I reckon), rather than replacing the original files in the first place? I don't know if that's possible to do in the vstest or msbuild drivers, though.

MarcoRossignoli commented 3 years ago

I think we could add a switch for "do not restore to unistrumented dll/pdb" and during the "instrumentation" phase did a simple check if the tracker is present inside a dll. Should be fast using reflection only dll load. So after first run no instrumentation will be done but run test/get report only.

github-actions[bot] commented 8 months ago

This issue is stale because it has been open for 3 months with no activity.