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

Uninstrumenting doesn't work when the same assembly is instrumented in multiple locations #20

Closed
Quickstream opened this issue Feb 9, 2018 · 9 comments

Comments

@Quickstream
Copy link

In my solution I have 2 test projects (unit tests and integration tests) that both reference the same main project. Each of the test projects has the same main project assembly in its bin folder. Both copies of the assembly get instrumented, but when I run "dotnet minicover uninstrument" it only uninstruments the main project assembly in one of the test's bin folders.

I think this is because the coverage.json file only has one entry for the assembly, which only tracks the location of one File, BackupFile, etc.

@lucaslorentz
Copy link
Owner

Nice diagnosis. I guess it should track assemblies by path, not just assembly name.

I will create a PR to fix it. Thanks

@lucaslorentz
Copy link
Owner

@Quickstream , it should be fixed now.

Can you please try the new nuget package: MiniCover 2.0.0-ci-20180211032654 ?

@Quickstream
Copy link
Author

Hi,

I gave the new version I try. At first, it seemed to be working, but after a few attempts I ended getting the following error:

Unhandled Exception: System.ArgumentException: An item with the same key has already been added. Key:
at System.ThrowHelper.ThrowAddingDuplicateWithKeyArgumentException(Object key)
at System.Collections.Generic.Dictionary2.TryInsert(TKey key, TValue value, InsertionBehavior behavior) at System.Linq.Enumerable.ToDictionary[TSource,TKey,TElement](IEnumerable1 source, Func2 keySelector, Func2 elementSelector, IEqualityComparer`1 comparer)
at MiniCover.Reports.BaseReport.Execute(InstrumentationResult result, Single threshold)
at Microsoft.Extensions.CommandLineUtils.CommandLineApplication.Execute(String[] args)
at MiniCover.Program.Main(String[] args)
...: warning MSB3073: The command "dotnet minicover htmlreport --workdir ../ --threshold 80" exited with code 134.

If I look at the coverage.json file, I see 2 entries for the same assembly in the "Assemblies" array, each of which has one entry in the "Locations" array. I think the expected result would be one entry in the Assemblies array with 2 entries in the Locations array.

@Quickstream
Copy link
Author

Running dotnet build immediately prior to instrumentation didn't resolve the issue, but running dotnet clean, then dotnet build prior to instrumentation did resolve the issue.

However, if I then ran dotnet build on just my IntegrationTests project and then tried instrumentation again, I was able to reproduce the issue.

@lucaslorentz
Copy link
Owner

It creates 2 entries with same assembly because the assembly files are binary different, so the instrumentation might be different. Maybe one is obsolete after you partially built the solution.

It happens that even rebuilding an assembly from same source code, it will generate a different binary because there are some compilation timestamps inside the assembly.

I can fix the report to nicely handle the duplicated assembly on coverage.json. But I'm still not sure if that's a good approach. Maybe I will need to do a more fundamental change.

@lucaslorentz
Copy link
Owner

lucaslorentz commented Feb 14, 2018

@Quickstream, that issue with report should be fixed on build: MiniCover 2.0.0-ci-20180214071708

Edit:
The only time I copied the Nuget version before the build finishes it failed to push nuget package 😄
https://travis-ci.org/lucaslorentz/minicover/builds/341312354

Use this version instead: MiniCover 2.0.0-ci-20180214203020

@lucaslorentz
Copy link
Owner

@Quickstream can we close this issue?

@Quickstream
Copy link
Author

Sorry I'm on vacation. I haven't had a chance to try the latest build yet.

@lucaslorentz
Copy link
Owner

Should be fixed by now. Please, reopen the issue if necessary.

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

No branches or pull requests

2 participants