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

Code Coverage should not be checking the unit test projects themselves for coverage. #20

Closed
StingyJack opened this issue Apr 18, 2018 · 26 comments
Labels
enhancement New feature or request

Comments

@StingyJack
Copy link

Running the code coverage option from the test explorer will always display the coverage statistics for unit test assemblies, and this skews the overall results.

I'm aware that this can be excluded by using a runsettings/testsettings file, but that has become tedious after a hundred or so projects, especially since VS cant associate and remember the test settings file with the solution. The way it requires file naming is also easy to mess up.

There is a request open to permit the [ExcludeFromCodeCoverage] attribute to an assembly, but that is targeted to net core and not net framework.

Can unit test projects just be removed from the coverage stats by default, and still let those who want this opt in?

@smadala
Copy link

smadala commented Apr 19, 2018

Agreed with @StingyJack , To get proper coverage for product code only, Currently we have to pass runsettings. I run into same experience when try to get code coverage for vstest repo.

@clairernovotny
Copy link
Member

Any exclude should be more than *.Test*.dll. It should at least be .*Test*.

Tests can live in .exe projects (as will be the case for xUnit 3.0), and they may be plural (foo.tests)

@gioce90
Copy link

gioce90 commented Feb 19, 2022

This issue is open by 2018. I'm using collect:"Code coverage" (the new native ms dotnet coverage tool) and I create cobertura or . coverage results as well, but is painful using . runsettings configuration file for exclude tests and 3rd party libraries.

@devlie
Copy link
Member

devlie commented Mar 25, 2022

.runsettings workaround is not ideal because its default is to include all binaries including nuget dependencies, which means we have to specify both <Include> to include our namespaces and <Exclude> to exclude our tests. Worse, our dependencies share the same namespace prefix, so our regex list grows.

We tried specifying assembly level ExcludeFromCodeCoverage, but that didn't work. It doesn't look like it's respected by VS/dotnet test. This would have been an ideal solution since we can just decorate it on any test csproj.

@IvanAlekseev
Copy link

Is there any update on this one? I am trying to migrate from coverlet and using runsettings file doesn't feel good. It would be awesome to have some clever defaults. At least stop trying to cover all nuget dependencies

@devlie
Copy link
Member

devlie commented May 23, 2022

Is there any update on this one? I am trying to migrate from coverlet and using runsettings file doesn't feel good. It would be awesome to have some clever defaults. At least stop trying to cover all nuget dependencies

@IvanAlekseev , please upvote microsoft/vstest#3669 and microsoft/vstest#1050 so we can get some traction :)

@Evangelink
Copy link
Member

Cc @jakubch1 @MarcoRossignoli

@MarcoRossignoli
Copy link
Contributor

It's available today going to close microsoft/vstest#3669 (comment)

@bramborman
Copy link

@MarcoRossignoli this should IMO be reopened as it doesn't fix the original issue - to automatically exclude test projects from the coverage - that is not implemented. We still need to manually add the ExcludeFromCodeCoverageAttribute assembly-wide, but that is not possible on older target frameworks.

@jakubch1
Copy link
Member

Unfortunately, Microsoft.CodeCoverage package covers by default all assemblies with pdb existing. You can exclude using ExcludeFromCodeCoverageAttribute or runsettings. If you don't want to generate runsettings you can also use this way:

dotnet test --collect "Code Coverage" -- DataCollectionRunSettings.DataCollectors.DataCollector.Configuration.CodeCoverage.ModulePaths.Exclude.ModulePath=".*tests.dll"

@bramborman
Copy link

Yeah I know this is possible, but think it should be excluded by default, as it doesn't really make sense in almost all of the possible cases I believe. The attribute is not available to be applied assembly-wide in older frameworks, so that's not a solution, and applying it class-by-class is tedious and error-prone... Runsettings are a way, but once again, I believe almost everybody using this doesn't want to have coverage counted for the test projects, so it's just an annoying repetitive stuff to add for every test run everywhere... It should be configurable, but rather as an opt-in, than opt-out IMHO...

@jakubch1
Copy link
Member

The issue is that many teams are collecting coverage also for test projects. Changing default behavior could break many solutions.

@StingyJack
Copy link
Author

@jakubch1 - are you being serious or sarcastic?

@jakubch1
Copy link
Member

You can check how often people are using option to collect for test projects here (this is for coverlet): https://github.com/search?p=4&q=IncludeTestAssembly&type=Code

Microsoft.CodeCoverage is able to collect code coverage for both .NET and C++. In C++ world you often link statically to be tested code into test project so it is required in this case to enable coverage for test projects.

For easier usage we could add ExcludeTestAssembly flag on main level in configuration. Then usage could be for example:

dotnet test --collect "Code Coverage;ExcludeTestAssembly=true"

@StingyJack @bramborman @MarcoRossignoli WDYT?

@bramborman
Copy link

Yeah I thought that it would probably be a breaking change. But for me it was rather a surprise - I would guess the test assemblies get excluded by default. Only when I was solving an issue with report generation I found that it includes the test assemblies as well. And I guess most people think this way too - they run dotnet test --collect "Code Coverage" and don't even think of the test assemblies being included as well. I was only looking at the percents most of the time.
But I see that it's not a change that could be easily done as it would cause unexpected drops in code coverage numbers... As such it would be nice to mention the fact that test assemblies get included as well in the documentation.

For easier usage we could add ExcludeTestAssembly flag on main level in configuration. Then usage could be for example:

dotnet test --collect "Code Coverage;ExcludeTestAssembly=true"

I also wanted to suggest something similar, so I totally agree :)

@StingyJack
Copy link
Author

I think that trying to determine how well the code that is executed in production is covered by unit tests is valuable.
[Unit Tests]
[Prod Code] <-- we care about the coverage of this

I think that trying to calculate how well those unit tests are covered by an additional layer of unit tests is a waste of time.
[Unit Tests] <-- nobody is writing this, because its turtles all the way down (or up)
[Unit Tests] <-- who cares about the coverage of this
[Prod Code]

Anyone using coverage calculation in the latter example is probably doing so by mistake or has been misinformed.

@MarcoRossignoli
Copy link
Contributor

MarcoRossignoli commented Feb 28, 2023

dotnet test --collect "Code Coverage;ExcludeTestAssembly=true"

I think that this can be a good option, anyway I find also good reason sometime to have coverage of the test module, you can catch errors(mistakes in test filtering) that are not possible to catch simply looking to the production code coverage %.

We had similar discussion on coverlet side coverlet-coverage/coverlet#618

@bramborman
Copy link

@StingyJack I don't think (hope 😄) anybody writes tests that would test other tests. That would make sense probably only if you have a huge custom test framework. From what I've seen in my results, the test projects covered themselves.

Makes sense that by getting code coverage of the tests, you see if you have some parts of tests that don't get executed. But I'd call this maybe an advanced thing. And you definitely don't want to raise the overall percentage of covered code by the coverage of the tests themselves. Personally I'd like to have two values - one for production code coverage, one for the test code coverage, should I ever need to have the value for tests. But this is of course another story.

I can see that it would be a breaking change to change this behavior, wonder for how many people/projects, but still a breaking change so probably the best that can be done is to provide a simple way to control this behavior, as suggested before.

@jakubch1
Copy link
Member

We are considering adding new flag to easily exclude test projects so reopening this issue.

@jakubch1 jakubch1 reopened this Mar 14, 2023
@jakubch1 jakubch1 transferred this issue from microsoft/vstest Mar 14, 2023
@gioce90
Copy link

gioce90 commented Mar 14, 2023

We are considering adding new flag to easily exclude test projects so reopening this issue.

this would be an awesome improvement!

@jakubch1
Copy link
Member

We have added new flag. In upcoming 17.8 release of our Microsoft.CodeCoverage package you can do:

dotnet test --collect "Code Coverage;IncludeTestAssembly=false"

Name of the flag is consistent with coverlet.

@gioce90
Copy link

gioce90 commented Nov 2, 2023

Hi @jakubch1 , IncludeTestAssembly=false seems what we needs, thank you. I would ask, there is also a way to exclude also third-part libraries as well?

@fhnaseer
Copy link
Member

fhnaseer commented Nov 2, 2023

Hi @jakubch1 , IncludeTestAssembly=false seems what we needs, thank you. I would ask, there is also a way to exclude also third-part libraries as well?

You can exclude these using runsettings file. https://learn.microsoft.com/en-us/visualstudio/test/customizing-code-coverage-analysis?view=vs-2022#include-or-exclude-assemblies-and-members

@gioce90
Copy link

gioce90 commented Nov 28, 2023

Sadly I think that .runsettings file is pure evil :D to manually add and remove lines from that file.... I think that a better out-of-the-box solution can be designed (IMHO)

@jakubch1
Copy link
Member

We are planning to add feature to make those exclusions simpler. We are thinking about specifying repo root dir: dotnet test --collect "Code Coverage;RepoRoot=D:\src" and then we provide coverage only for projects in D:\src. Maybe we will be able to get it even from source control.

@gioce90
Copy link

gioce90 commented Nov 28, 2023

Interesting. However, why doesn't the code coverage focus on what we are testing as default behavior?

In my company usually we do:

- task: DotNetCoreCLI@2
  displayName: 'dotnet test'
  inputs:
    command: 'test'
    projects: '**/*.csproj'  #or *.sln
    arguments: '--no-restore --configuration $(buildConfiguration) --collect "Code Coverage;IncludeTestAssembly=false"' 

or

- script: dotnet test MySolutionFolder --configuration $(buildConfiguration) --logger "trx;LogFileName=testresults.trx" /p:CollectCoverage=true /p:CoverletOutputFormat=cobertura /p:CoverletOutput=TestResults/Coverage/
  displayName: 'dotnet test'

With projects: '**/*.csproj', or *.sln, or with MySolutionFolder we already indicate what we want to test.
It would be interesting if the code coverage only focused on these, as default behavior.

I think Coverlet is already doing something like that:

Coverlet automatically excludes third-party assemblies from the coverage analysis

they talk about it here coverlet-coverage/coverlet#1164 and also here

If there are concerns about breaking changes, perhaps a flag called, for example, IncludeThirdPartyForCoverage=false should make sense.

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

No branches or pull requests