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

1.5.0 instruments and reports on the unit test project in addition to the assembly under test. #372

Closed
scovel opened this issue Mar 26, 2019 · 13 comments · Fixed by #376
Closed

Comments

@scovel
Copy link

scovel commented Mar 26, 2019

With the exact same command-line, coverlet console 1.5.0 is instrumenting and reporting coverage for the unit test project itself, where in 1.4.1 it only instrumented the project under test as expected.

coverlet TestProject1/bin/Debug/netcoreapp2.1/TestProject1.dll --target "dotnet" --targetargs "test --no-build"

coverlet console 1.4.1

Calculating coverage result...
  Generating report '/projects/customer/coverage.json'
+-------------------------+--------+--------+--------+
| Module                 | Line   | Branch | Method |
+-------------------------+--------+--------+--------+
| customer              | 29%    | 23.1%  | 47.1%  |
+-------------------------+--------+--------+--------+

+---------+--------+--------+--------+
|         | Line   | Branch | Method |
+---------+--------+--------+--------+
| Total        | 29%    | 23.1%  | 47.1%  |
+---------+--------+--------+--------+
| Average  | 29%    | 23.1%  | 47.1%  |
+---------+--------+--------+--------+

coverlet console 1.5.0

Calculating coverage result...
  Generating report '/projects/customer/coverage.json'
+-------------------------+-------+--------+--------+
| Module                  | Line  | Branch | Method |
+-------------------------+-------+--------+--------+
| customer | 29%   | 23.1%  | 47.1%  |
+-------------------------+-------+--------+--------+
| TestProject1            | 81.6% | 16.7%  | 71.4%  |
+-------------------------+-------+--------+--------+

+---------+--------+--------+--------+
|         | Line   | Branch | Method |
+---------+--------+--------+--------+
| Total   | 36.7%  | 22.9%  | 48.3%  |
+---------+--------+--------+--------+
| Average | 18.35% | 11.45% | 24.15% |
+---------+--------+--------+--------+
@MarcoRossignoli
Copy link
Collaborator

#318 we don'ts skip test project anymore, if you don't want cover test project you can use /exclude switch.

@derhally
Copy link
Contributor

derhally commented Mar 28, 2019

with coverlet.msbuild 2.5.1 I'm seeing coverage report include info for dependencies as well. I don't know if this is because of the coverlet or dotnet test. Will have to look into it.

Edit:

Yes, it seems once we updated to 2.5.1 the coverage report includes information for 3rd party assemblies. To me that is not the expected default behavior.

Edit:

This happens with 2.5.0 as well.

@scovel
Copy link
Author

scovel commented Mar 28, 2019

I had to add the following to get the "old" behavior. Otherwise it was trying to instrument some System.* classes.

--exclude "[TestProject1]*" --exclude "[System*]*" 

You'd think a breaking change would be mentioned in some release notes somewhere.

@derhally
Copy link
Contributor

I think this is a bug. Most developers will not want this behavior. The fact that everyone has to go do this by default is wrong.

@scovel
Copy link
Author

scovel commented Mar 28, 2019

I totally agree.

I think instrumenting your unit test code is a bug as well, but I guess one guy asked for that as a "feature" and here we are.

@MarcoRossignoli
Copy link
Collaborator

You'd think a breaking change would be mentioned in some release notes somewhere.

/cc @tonerdo we should think to add a notes doc somewhere with "behavioural changes"

I think this is a bug

@scovel @derhally read all thread of PR #318 there is an important point that drove us to accept the update, the key part is #318 (comment)

How can you understand if some code is not covered due to issue with some "non run tests" and find which?

@derhally
Copy link
Contributor

I don't agree. I personally never want to instrument my unit tests. My unit test runner tells me which tests are run, skipped or failed. That should not be the default behavior of coverlet. I expect that most people care only about the SUT not what is testing the SUT.

At this point I cannot use coverlet 2.5. or later. I'm having to exclude 5 or 6 external assemblies from one project. I work with dozens of projects and having to do this on each one and everytime I add a 3rd party nuget package is quite silly.

@derhally
Copy link
Contributor

The right thing to do was to have a flag that enabled this behavior change. This behavior should not be the default.

@scovel
Copy link
Author

scovel commented Mar 28, 2019

I read all of that PR, didn't necessarily agree with it. I saw you debated adding a flag and then decided to change the default behavior instead. That's fine, but some release notes for 1.5.0 would have been good.

I think derhally is referring to some system assemblies getting instrumented as being a bug. Without excluding [System*]* . I get the following:

Instrumented module: 'bin/Debug/netcoreapp2.2/System.Interactive.Async.dll'

And then get errors about that assembly later in the process. That seems to me to be a bug.

@derhally
Copy link
Contributor

With v 2.6.0 coverlet I see errors like this

Calculating coverage result...
C:\Users\user\.nuget\packages\coverlet.msbuild\2.6.0\build\netstandard2.0\coverlet.msbuild.targets(7,5): warning : [coverlet] Hits file:'C:\Users\user\AppData\Local\Temp\BouncyCastle.Crypto_4a2436bd-62db-47bd-99cf-7a8d440f2d56' not found for module: 'BouncyCastle.Crypto' [c:\dev\project\Tests\Project.Tests\Proejct.Tests.csproj]
C:\Users\user\.nuget\packages\coverlet.msbuild\2.6.0\build\netstandard2.0\coverlet.msbuild.targets(7,5): warning : [coverlet] Hits file:'C:\Users\user\AppData\Local\Temp\McMaster.Extensions.CommandLineUtils_4a2436bd-62db-47bd-99cf-7a8d440f2d56' not found for module: 'McMaster.Extensions.CommandLineUtils' [c:\dev\project\Tests\Project.Tests\Proejct.Tests.csproj]
C:\Users\user\.nuget\packages\coverlet.msbuild\2.6.0\build\netstandard2.0\coverlet.msbuild.targets(7,5): warning : [coverlet] Hits file:'C:\Users\user\AppData\Local\Temp\Refit.HttpClientFactory_4a2436bd-62db-47bd-99cf-7a8d440f2d56' not found for module: 'Refit.HttpClientFactory' [c:\dev\project\Tests\Project.Tests\Proejct.Tests.csproj]
C:\Users\user\.nuget\packages\coverlet.msbuild\2.6.0\build\netstandard2.0\coverlet.msbuild.targets(7,5): warning : [coverlet] Hits file:'C:\Users\user\AppData\Local\Temp\System.Reactive_4a2436bd-62db-47bd-99cf-7a8d440f2d56' not found for module: 'System.Reactive' [c:\dev\project\Tests\Project.Tests\Proejct.Tests.csproj]
C:\Users\user\.nuget\packages\coverlet.msbuild\2.6.0\build\netstandard2.0\coverlet.msbuild.targets(35,5): error : One or more errors occurred. (Could not find file 'C:\Users\user\AppData\Local\Temp\BouncyCastle.Crypto_4a2436bd-62db-47bd-99cf-7a8d440f2d56.pdb'.) (Could not find file 'C:\Users\user\AppData\Local\Temp\BouncyCastle.Crypto_4a2436bd-62db-47bd-99cf-7a8d440f2d56.pdb'.) (Could not find file 'C:\Users\user\AppData\Local\Temp\BouncyCastle.Crypto_4a2436bd-62db-47bd-99cf-7a8d440f2d56.pdb'.) (Could not find file 'C:\Users\user\AppData\Local\Temp\BouncyCastle.Crypto_4a2436bd-62db-47bd-99cf-7a8d440f2d56.pdb'.) (Could not find file 'C:\Users\user\AppData\Local\Temp\BouncyCastle.Crypto_4a2436bd-62db-47bd-99cf-7a8d440f2d56.pdb'.) (Could not find file 'C:\Users\user\AppData\Local\Temp\BouncyCastle.Crypto_4a2436bd-62db-47bd-99cf-7a8d440f2d56.pdb'.) (Could not find file 'C:\Users\user\AppData\Local\Temp\BouncyCastle.Crypto_4a2436bd-62db-47bd-99cf-7a8d440f2d56.pdb'.) (Could not find file 'C:\Users\user\AppData\Local\Temp\BouncyCastle.Crypto_4a2436bd-62db-47bd-99cf-7a8d440f2d56.pdb'.) (Could not find file 'C:\Users\user\AppData\Local\Temp\BouncyCastle.Crypto_4a2436bd-62db-47bd-99cf-7a8d440f2d56.pdb'.) (Could not find file 'C:\Users\user\AppData\Local\Temp\BouncyCastle.Crypto_4a2436bd-62db-47bd-99cf-7a8d440f2d56.pdb'.) [c:\dev\project\Tests\Project.Tests\Proejct.Tests.csproj]
C:\Users\user\.nuget\packages\coverlet.msbuild\2.6.0\build\netstandard2.0\coverlet.msbuild.targets(35,5): error :    at Coverlet.Core.RetryHelper.Do[T](Func`1 action, Func`1 backoffStrategy, Int32 maxAttemptCount) in /Users/toni/Workspace/coverlet/src/coverlet.core/Helpers/RetryHelper.cs:line 58 [c:\dev\project\Tests\Project.Tests\Proejct.Tests.csproj]
C:\Users\user\.nuget\packages\coverlet.msbuild\2.6.0\build\netstandard2.0\coverlet.msbuild.targets(35,5): error :    at Coverlet.Core.RetryHelper.Retry(Action action, Func`1 backoffStrategy, Int32 maxAttemptCount) in /Users/toni/Workspace/coverlet/src/coverlet.core/Helpers/RetryHelper.cs:line 27 [c:\dev\project\Tests\Project.Tests\Proejct.Tests.csproj]
C:\Users\user\.nuget\packages\coverlet.msbuild\2.6.0\build\netstandard2.0\coverlet.msbuild.targets(35,5): error :    at Coverlet.Core.Coverage.GetCoverageResult() in /Users/toni/Workspace/coverlet/src/coverlet.core/Coverage.cs:line 108 [c:\dev\project\Tests\Project.Tests\Proejct.Tests.csproj]
C:\Users\user\.nuget\packages\coverlet.msbuild\2.6.0\build\netstandard2.0\coverlet.msbuild.targets(35,5): error :    at Coverlet.MSbuild.Tasks.CoverageResultTask.Execute() in /Users/toni/Workspace/coverlet/src/coverlet.msbuild.tasks/CoverageResultTask.cs:line 67 [c:\dev\project\Tests\Project.Tests\Proejct.Tests.csproj]
An error occurred when executing task 'Test'.
Cake.Core.CakeException: .NET Core CLI: Process returned an error (exit code 1).
   at Cake.Core.Tooling.Tool`1.ProcessExitCode(Int32 exitCode)
   at Cake.Core.Tooling.Tool`1.Run(TSettings settings, ProcessArgumentBuilder arguments, ProcessSettings processSettings, Action`1 postAction)
   at Cake.Common.Tools.DotNetCore.Test.DotNetCoreTester.Test(String project, DotNetCoreTestSettings settings)

@MarcoRossignoli
Copy link
Collaborator

MarcoRossignoli commented Mar 28, 2019

@derhally can you provide a simple repro?(Any obligation)

@tonerdo
Copy link
Collaborator

tonerdo commented Mar 28, 2019

@derhally @MarcoRossignoli That error is because Coverlet is unconditionally trying to restore the pdb file for an assembly that has it embedded. I'm also in agreement with @derhally with respect to including test assemblies by default and I'll work on putting this feature behind a flag that will be turned off by default. Will address this weekend

@MarcoRossignoli
Copy link
Collaborator

thank's for info!

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

Successfully merging a pull request may close this issue.

4 participants