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

Is it possible to specify a directory to exclude from coverage? #56

Closed
abe545 opened this issue Apr 19, 2018 · 49 comments
Closed

Is it possible to specify a directory to exclude from coverage? #56

abe545 opened this issue Apr 19, 2018 · 49 comments
Labels
enhancement General enhancement request

Comments

@abe545
Copy link

abe545 commented Apr 19, 2018

Hi, we would love to start using coverlet to generate code coverage, but we really don't want to include code that gets automatically generated in our coverage results. Since we generate it during CI, it wouldn't be feasible for us to apply attributes to every class that we want to exclude from coverage. We do, however, have a common folder in which the code gets generated.

Is there a flag we can pass to exclude the files in said folder from coverage?

@ryanmvt
Copy link

ryanmvt commented Apr 23, 2018

We're also using gRPC code generation which makes the attribute-based exclusion feature unrealistic. A configuration or flag based solution would really help out here.

@abe545
Copy link
Author

abe545 commented Apr 23, 2018

@tonerdo the syntax suggested in #55 looks like it could be a winner - if that were a globbing pattern (and we'd have to support multiple globs). My suggestion would be a semicolon separated list of globs (probably have to quote it)

/p:ExcludeFromCoverage="some/**/glob;c:\some\complex rooted path\*.glob"

I think it would need to support both absolute and relative paths. Thoughts?

@tonerdo tonerdo added enhancement General enhancement request help wanted labels Apr 23, 2018
@tonerdo
Copy link
Collaborator

tonerdo commented Apr 23, 2018

I totally agree! My only thought is whether to limit this feature to just whole assemblies or to extend it to allow specifying namespaces/classes/methods and how that will work.

@MarcoRossignoli
Copy link
Collaborator

In my scenario could be very useful also to have an "include" switch(a lot of auxiliary class i don't test). I like the idea of json config file(in addition) because we can address both. Another concern is the case with a lot of include/exclude, command line could become hard to use and i don't like the idea to organize code for coverage, again this isn't possible on existing code. My idea is to chose the code i want to test with minimum % of coverage or fail for new or existing code.

@abe545
Copy link
Author

abe545 commented Apr 23, 2018

@tonerdo I think the easiest would be path based. This will cover the majority of the scenarios, and devs can use the attribute method to exclude methods still. I don't see a scenario where I'd want to cover most code generated code, but not one particular method. At the very least, I'd expect such code to be generated in its own file.

@MarcoRossignoli I think the ability to both include and exclude is valuable. I think if we help out with implementing an msbuild switch, it would be easy to include an msbuild xml file in your project (via project includes), which could define these in a file for you.

@ryanmvt
Copy link

ryanmvt commented Apr 23, 2018

@abe545 Path-based meaning everything inside the directory specified in the path, as well as all sub-directories? That idea would be flexible enough to encompass generated code perfectly. It also allows you to target large swaths of code or down to a single file at a time.

@tonerdo I don't think assembly-only based exclusions would be flexible enough. It would potentially force the developer to create additional projects specifically to exclude portions of code from code coverage right?

Even the config I would think would end up path-based too? What do you guys think?

@abe545
Copy link
Author

abe545 commented Apr 23, 2018

@ryanmvt yes... except I was thinking to support globbing patterns. If you don't specify any wildcards, it would indeed exclude the directory. That way, you could even do something like **/*test*.cs and exclude all files with the word test in the filename.

@tonerdo
Copy link
Collaborator

tonerdo commented Apr 23, 2018

@abe545 @ryanmvt all good ideas. Here's what my final thoughts are:

  • Path based ignore with support for globbing patterns
  • Keep ExcludeFromCoverage attribute for fine grained control over ignoring classes/methods

Is this a good setup?

@abe545
Copy link
Author

abe545 commented Apr 23, 2018

@tonerdo sounds good to me

@tonerdo
Copy link
Collaborator

tonerdo commented Apr 23, 2018

Awesome! Now who's gonna open that PR 😄?

@abe545
Copy link
Author

abe545 commented Apr 23, 2018

My coworker @ido-namely has volunteered, if nobody else wants it.

@ryanmvt
Copy link

ryanmvt commented Apr 24, 2018

@tonerdo sounds good to me as well! Thank you for the volunteer @abe545

@MarcoRossignoli
Copy link
Collaborator

MarcoRossignoli commented Apr 24, 2018

Thank's a lot @abe545 @ido-namely !

@coenm
Copy link
Contributor

coenm commented Apr 25, 2018

I'm familiar with OpenCover and I've always liked the filter option it provides. Not saying Coverlet should use the same syntax and options though.

Their filter option can include or exclude by assembly name and/or by type. I know that this topic is about excluding directories but I think the problem really boils down to have fine grained control over what to include and exclude. Personally I've never excluded something based on physical location but I've always specified what to include/exclude using assembly names and namespaces.

Again, I'm not suggesting Coverlet should use the same syntax but it might be worth to take a look.

The following is copied/based on OpenCovers wiki page

Syntax : (+|-)[Assembly-Filter]Type-Filter
Multiple filters can be applied by seperating them with a space.

Examples

  • +[Open*]* -[Open.Test]*
    Include all classes in modules starting with Open.* but exclude all those in modules Open.Test.

  • +[Open]* -[Open]Data.*
    Include all classes in module Open but exclude all classes in the Data namespace.

  • +[Open]* -[Open]*Attribute
    Include all classes in module Open but exclude all classes ending with Attribute.

@ido-namely
Copy link
Contributor

Would appreciate your comments on : #64

@MarcoRossignoli
Copy link
Collaborator

@coenm I admit I like it...

cc: @tonerdo @abe545 @ryanmvt

@tonerdo
Copy link
Collaborator

tonerdo commented Apr 26, 2018

All this while my mind has been on ignoring assemblies (e.g. SomeAssembly.dll) from this PR it seems you guys were referring to actual source files. Is this about right?

cc @MarcoRossignoli @abe545 @ryanmvt @ido-namely

@MarcoRossignoli
Copy link
Collaborator

MarcoRossignoli commented Apr 26, 2018

We do, however, have a common folder in which the code gets generated.
Is there a flag we can pass to exclude the files in said folder from coverage?

i think so(IMHO anyway after some thought on @coenm post...i think we could be more close to opencovers accepted standard, coverlet plus is cross plat support).

@abe545 your generated class are in special namespaces?Or your issue is this(same namespace of "production classes"?

@coenm
Copy link
Contributor

coenm commented Apr 26, 2018

So this issue is about excluding source files from coverage.
My suggestion (based on OpenCover filter method) uses assembly and namespaces to filter.

As I see it, both options provide a different mechanism to have more control over what to cover and what to exclude.
Although I prefer filtering based on assembly and namespaces it doesn't mean that one option excludes the other.

When working with filenames however, please take into account that Coverlet is a cross platform product.
So:

  • Filenames and paths are different in windows and linux;
    Please test (and document) this well on both platforms.
  • Filenames can be relative and absolute.
    Do we want to support both? Why? What is the consequence?
  • Classes can be partial.
    What if one file is excluded by the filter and the other part of a partial class isn't?

@MarcoRossignoli
Copy link
Collaborator

  • Classes can be partial.
    What if one file is excluded by the filter and the other part of a partial class isn't?

very good point...

@abe545
Copy link
Author

abe545 commented Apr 26, 2018

Our use case is for code that gets generated by a tool (grpc). The tool does so based off some input files, which optionally define a namespace, and if not, will generate one based on the name of the input file. What this all means is that we would likely have to modify our coverage commands anytime an input changed (or we need to rely on a new generated file).

I'm personally trying to build a set of scripts that can work throughout our organization, and we can decide where the files are generated, but not the contents of them. For our use case, physical file paths are the best way to do it. Yes, linux/windows paths are different (but the abstractions are good at using unixy paths, and rooted paths are also easy to detect in a xplat way (Path.IsRooted).

Regarding partial classes - correct me if I'm wrong, but coverage is explicitly tied to a file. I don't think partials are too big a hurdle. If one of the class files is excluded from coverage, then the rest of the class can still have coverage generated.

If people want the ability to exclude assembly and types, I'm all for it. But for us, the path based solution is still a better fit. I don't think it would be too difficult to support both.

@ryanmvt
Copy link

ryanmvt commented Apr 26, 2018

@tonerdo yes, we were more focused on file paths than assemblies (my team is also using gRPC code generation). Assembly exclusion is probably good to exclude entire projects from coverage, but namespace/type exclusion is probably much more useful (and granular). I would be happy personally with either one of these, or both solutions. Both could definitely work, and each one is probably more helpful for different circumstances.

@tonerdo
Copy link
Collaborator

tonerdo commented Apr 26, 2018

We can definitely have both assembly based filtering and source file based filtering, we don't necessarily have to choose. My only thought is how we want that functionality to look like from a usability standpoint.

Right now I'm thinking of introducing 2 new properties:

Excluding sources:

dotnet msbuild /p:CollectCoverage=true /p:ExcludeSources=*pathglob*;*pathglob*

@ido-namely has already made some good progress on this. Much appreciated!

Excluding assemblies:

dotnet msbuild /p:CollectCoverage=true /p:ExcludeAssemblies=*fileglob*;*fileglob*

The values for ExcludeAssemblies need not be file paths but just a glob pattern to filter out the assembly filename. E.g. Open*.dll

But at this point it feels a bit verbose and I'm currently leaning towards a single Exclude property that can accept an array of both source files and assemblies but is able to distinguish between the two and do the needful. E.g if the glob pattern resolves to a .dll then obviously treat it as an assembly and ignore calculating coverage for it, but if resolves to anything else then it must be a source file and treat accordingly

@MarcoRossignoli
Copy link
Collaborator

@tonerdo i agree with you on single prop.

@ido-namely
Copy link
Contributor

I think I addressed all your comments and submitted some changes (let me know if I missed something).
@tonerdo I agree that we can use the same flag, I can make the necessary flag renaming.
However, I do think we will still need to have a flag to establish the parent dir for the searches (with default value of the project directory), this way you can use relative paths or absolute paths (by setting the parent dir to ("/" or "c:"), another alternative is to check if the path is rooted or not and decide whether to use the the parent dir arg for the given path or take it as is.

@ido-namely
Copy link
Contributor

One point I just thought of regarding the single property - what about the case where your source dir is the parent dir for your assemblies, I think in this case it wouldn't be so easy to just exclude some sources files without it excluding the assemblies, or is there an easy solution for that?

@tonerdo
Copy link
Collaborator

tonerdo commented Apr 29, 2018

#64 is all merged in. Will get to adding a Filter property for ignoring assemblies

@MarcoRossignoli
Copy link
Collaborator

MarcoRossignoli commented Apr 29, 2018

@ido-namely thank's for PR!
@tonerdo we could discuss #56 (comment) on a new issue.What do you think?I mean maybe we can filter(include/exclude) assemblies and types also with same syntax.

@mathnogueira
Copy link

mathnogueira commented May 2, 2018

First of all, thanks for the pull request @ido-namely!

I noticed that the latest nuget package does not contain this feature. Is there a release date for this feature? @tonerdo

@tonerdo
Copy link
Collaborator

tonerdo commented May 2, 2018

Thanks @ido-namely. @mathnogueira, @MarcoRossignoli, @ryanmvt, @abe545 new release here: https://www.nuget.org/packages/coverlet.msbuild/1.2.0.

@MarcoRossignoli kindly help open a new issue to track adding a Filter property to exclude assemblies.

Thanks everyone for the productive comments

@tonerdo tonerdo closed this as completed May 2, 2018
@tonerdo
Copy link
Collaborator

tonerdo commented May 17, 2018

New release available here: https://www.nuget.org/packages/coverlet.msbuild/2.0.0. Thanks for your help and patience guys

@jeffrutland
Copy link

greetings, over a year later here. :)

I'm curious if any progress was made on excluding code covered by specifying namespaces in the <Exclude> element in the runsettings file.

our codebase is built in Azure DevOps, and tested with dotnet test - I'd like the coverage analysis to exclude generated code in specific namespaces. the documentation seems to suggest that filtering via regex is possible, but this is on a page that specifically talks about vstest. is this possible using dotnet test and if so, are there some examples of this available?

@MarcoRossignoli
Copy link
Collaborator

MarcoRossignoli commented Oct 4, 2019

You could try with something like [asm]namespace.* it should exlude all types in the assembly under that namespace.
You can verify reading the logs, if you're using collectors you can enable and check inside vstest/test logs https://github.com/tonerdo/coverlet/blob/master/Documentation/Troubleshooting.md#collectors-integration, search for Included module/Excluded module string.

@jeffrutland
Copy link

You could try with something like [asm]namespace.* it should exlude all types in the assembly under that namespace.

is that the exact syntax required this? I see no different results when running with that in the <Exclude> tag.

You can verify reading the logs, if you're using collectors you can enable and check inside vstest/test logs https://github.com/tonerdo/coverlet/blob/master/Documentation/Troubleshooting.md#collectors-integration, search for Included module/Excluded module string.

thanks for the log suggestion - good to know.

@MarcoRossignoli
Copy link
Collaborator

Mmm...I'll give it a try...I never tested for namespace, but I expect that is something close to that...and I think that we should support that filter feature if missing.

@MarcoRossignoli
Copy link
Collaborator

MarcoRossignoli commented Oct 5, 2019

@jeffrutland I did some test and syntax should work...I added some test to be sure, take a look to sample #579
Let me know if you've some throuble, but please open a new issue, we'll follow up there.

@micheleissa
Copy link

Is this supported by coverlet.collector I've tried but it won't work. The build agent is linux

@abe545
Copy link
Author

abe545 commented Aug 21, 2020

@micheleissa yes, we use coverlet.collector with a coverlet.runsettings file (this is executed in a linux environment):

<?xml version="1.0" encoding="utf-8" ?>
<RunSettings>
  <DataCollectionRunSettings>
    <DataCollectors>
      <DataCollector friendlyName="XPlat code coverage">
        <Configuration>
          <Format>lcov</Format>
          <Exclude>[xunit*]*</Exclude>
          <ExcludeByFile>/app/gen/Namely.*.Grpc/*</ExcludeByFile>
        </Configuration>
      </DataCollector>
    </DataCollectors>
  </DataCollectionRunSettings>
</RunSettings>

@micheleissa
Copy link

@abe545
I'm trying in AZDO pipeline with something like the following:

      displayName: 'dotnet test'
      inputs:
        command: 'test'
        projects: '**/*.UnitTests.csproj'
        arguments: '--configuration $(configuration) /p:CollectCoverage=true /p:CoverletOutputFormat=cobertura /p:CoverletOutput=$(Build.SourcesDirectory)/TestResults/Coverage/ /p:Exclude="[*].TinyIoC.*"'
        publishTestResults: true
      
    
    - task: PublishCodeCoverageResults@1
      displayName: 'Publish code coverage report'
      inputs:
        codeCoverageTool: 'Cobertura'
        summaryFileLocation: '$(Build.SourcesDirectory)/**/coverage.cobertura.xml'

I'm trying to exclude anything in TinyIoC namespace. Does it have to be done via runsettings ?

@abe545
Copy link
Author

abe545 commented Aug 21, 2020

So those properties you're passing with /p only work for the msbuild package - at least according to the docs

I'm not a maintainer of this fantastic project, so I may not have the latest info.

@micheleissa
Copy link

micheleissa commented Aug 21, 2020

no most of them work fine, I'm following MS docs from here
When I try this:
/p:Exclude="[*]namespace1*,[*]namespace2*,[*]namespace3*"

Now the comma separated list for namespaces is giving me errors regarding msbuild something like

MSBUILD : error MS1006: Property is not valid.

Switch: [*]namespace2*

For switch syntax, type "MSBuild --help"

@MarcoRossignoli , @tonerdo
Should I open a new issue for this or what is the workaround?
according to readme the flag supports comma separated list.

@abe545
Copy link
Author

abe545 commented Aug 21, 2020

@micheleissa ah, according the msdn documentation, you're using the msbuild collector. This is how we used to use the msbuild collector (note that we escape the quotes because it was required in the dockerfile to force docker to pass the quotes to the actual command to run - you might be experiencing something similar):

dotnet test \
    --no-build \
    --configuration Release \
    -v n \
    /p:CollectCoverage=true \
    /p:CoverletOutputFormat=lcov \
    /p:Include=\"[Namely.*]*\" \	
    /p:Exclude=\"[Namely.Tests.*]*,[xunit*]*\" \
    /p:ExcludeByFile=\"/app/src/Namely.Grpc.Client/Grpc/**/*,/app/src/Namely.Kafka/Grpc/**/*\" \
    "/p:CoverletOutput=/out/coverage/coverage.info" \
    "test/Namely.Tests/Namely.Tests.csproj"

@micheleissa
Copy link

@abe545 I will try it, thanks a lot.

@micheleissa
Copy link

I tried the following:

dotnet test --configuration Develop /p:CollectCoverage=true /p:CoverletOutputFormat=cobertura /p:CoverletOutput=TestResults/Coverage/ /p:Exclude=\"[*]namespace1*,[*]namespace2*\"

getting

no matches found: /p:Exclude="[*]namespace1*,[*]namespace2*"

Any ideas?

@MarcoRossignoli
Copy link
Collaborator

MarcoRossignoli commented Aug 22, 2020

@micheleissa pls can you open a new issue and attach the console output you get when you say no matches found: /p:Exclude="[*]namespace1*,[*]namespace2*"?
As @abe545 said coverlet comes in three flavours

VSTest engine integration --collect:"Xplat Code Coverage" https://github.com/coverlet-coverage/coverlet/blob/master/Documentation/VSTestIntegration.md
MSBuild task integration -p:CollectCoverage=true https://github.com/coverlet-coverage/coverlet/blob/master/Documentation/MSBuildIntegration.md
As a .NET Global tool coverlet ... https://github.com/coverlet-coverage/coverlet/blob/master/Documentation/GlobalTool.md

The coverage engine is the same undeneath but the "entry point"(we call these drivers) can be different with different way to setup things
When you use -p:CollectCoverage=true the same of /p:CollectCoverage=true you need to follow https://github.com/coverlet-coverage/coverlet/blob/master/Documentation/MSBuildIntegration.md
Also in case of cross plat vstest integration(Xplat Code Coverage) you need to follow this ms guide https://docs.microsoft.com/en-us/dotnet/core/testing/unit-testing-code-coverage

Thanks a lot @abe545 for the help.

@jrichardsz
Copy link

@micheleissa ah, according the msdn documentation, you're using the msbuild collector. This is how we used to use the msbuild collector (note that we escape the quotes because it was required in the dockerfile to force docker to pass the quotes to the actual command to run - you might be experiencing something similar):

dotnet test \
    --no-build \
    --configuration Release \
    -v n \
    /p:CollectCoverage=true \
    /p:CoverletOutputFormat=lcov \
    /p:Include=\"[Namely.*]*\" \	
    /p:Exclude=\"[Namely.Tests.*]*,[xunit*]*\" \
    /p:ExcludeByFile=\"/app/src/Namely.Grpc.Client/Grpc/**/*,/app/src/Namely.Kafka/Grpc/**/*\" \
    "/p:CoverletOutput=/out/coverage/coverage.info" \
    "test/Namely.Tests/Namely.Tests.csproj"

/p:ExcludeByFile=\"/app/src/Common/**/*\" worked in netcore 7 + linux

/p:Exclude don't works

@micheleissa
Copy link

@jrichardsz Actually I'm using p:/Exclude in pipelines and it does work.

@jrichardsz
Copy link

Try with /p:ExcludeByFile= or share us your namespaces

@johnthagen
Copy link

After a long amount of searching around and trial and error, what I had to do to solve this problem (alluded to in previous comments) is to add the following to my .Tests .csproj:

<PackageReference Include="coverlet.msbuild" Version="6.0.0"/>

And then I had to switch from the following command:

dotnet test --collect:"Code Coverage;Format=Cobertura;CoverageFileName=coverage.xml"

To:

dotnet test /p:CollectCoverage=true /p:CoverletOutputFormat=cobertura /p:CoverletOutput="coverage.xml /p:ExcludeByFile="**/MyProject/Subfolder/**/*.cs,**/MyProject/AnotherSubFolder/**/*.cs"

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

No branches or pull requests