Add ability to exclude assemblies by expression #94

tonerdo · 2018-05-13T18:09:22Z

This PR introduces the ability to filter assemblies with an expression format closely resembling the one used by OpenCover. However, unlike OpenCover Coverlet doesn't support inclusion/exclusion syntax (+/-).

All instrument-able assemblies are inclusive by default and the Exclude property has been repurposed to accept a comma separated list of filter expressions. The ability to exclude source files (based on the work by @ido-namely) has been moved to the new ExcludeByFile property.

The filter rules can be used to exclude entire assemblies, all types in an assembly or all types in a specific namespace.

Syntax: [Assembly-Filter]Type-Filter

Examples:
/p:Exclude=[*]* => Excludes all types in all assemblies (nothing is instrumented)
/p:Exclude=[coverlet.*]Coverlet.Core.Coverage => Excludes the Coverage class in the Coverlet.Core namespace belonging to any assembly that matches coverlet.* (e.g coverlet.core)
/p:Exclude=[*]Coverlet.Core.Instrumentation.* => Excludes all types belonging to Coverlet.Core.Instrumentation namespace in any assembly

The PR introduces backwards incompatible changes and will be part of the 2.0 release this week

cc @MarcoRossignoli @ido-namely @JasonBock @SteveGilham @coenm @ryanmvt

Addresses #77

codecov · 2018-05-13T18:13:24Z

Codecov Report

Merging #94 into master will decrease coverage by 1.97%.
The diff coverage is 74.31%.

@@            Coverage Diff             @@
##           master      #94      +/-   ##
==========================================
- Coverage   98.83%   96.86%   -1.98%     
==========================================
  Files          15       15              
  Lines        1291     1372      +81     
==========================================
+ Hits         1276     1329      +53     
- Misses         15       43      +28

Impacted Files	Coverage Δ
src/coverlet.core/Instrumentation/Instrumenter.cs	`100% <100%> (ø)`	⬆️
src/coverlet.core/Coverage.cs	`100% <100%> (ø)`	⬆️
src/coverlet.core/Helpers/InstrumentationHelper.cs	`81.54% <70.21%> (-15.27%)`	⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2b1e56b...371112c. Read the comment docs.

MarcoRossignoli · 2018-05-14T09:00:54Z

src/coverlet.core/Helpers/InstrumentationHelper.cs

+            foreach (var filter in filters)
+            {
+                if (!IsValidFilterExpression(filter))
+                    continue;


why do not signal invalid espression with something like console log/error?

coenm · 2018-05-14T10:05:15Z

Nice!! Good work

I see that you already merged this but here my two cents:

In Coverage.PrepareModules method the 'InstrumentationHelper.IsModuleExcluded' is called for every module. The _filter field is passed as a parameter. Inside the IsModuleExcluded, each item in _filter variable is checked for validity (IsValidFilterExpression(filter)). I would take this up to reduce the number of calls to IsValidFilterExpression.

So something like this (did not try to compile ):

public void PrepareModules()
{
    string[] modules = InstrumentationHelper.GetCoverableModules(_module);
    string[] excludedFiles =  InstrumentationHelper.GetExcludedFiles(_rules);                              
    _filters = _filters.Where(filterItem => IsValidFilterExpression(filterItem)).ToArray();

    // at this point, _filters  contains only valid expressions.
}

You can even take if further by not storing the filters as string[] but by creating new SomeSortOfModuleTypeFilterObject containing a module and type regex.

class SomeSortOfModuleTypeFilterObject
{
    public SomeSortOfModuleTypeFilterObject ( Regex typeRegex, Regex moduleRegex)
    {
      TypeRegex = typeRegex;
      ModuleRegex = moduleRegex;
    }

    public Regex TypeRegex { get; }
    public Regex ModuleRegex { get; }
}

public void PrepareModules()
{
    string[] modules = InstrumentationHelper.GetCoverableModules(_module);
    string[] excludedFiles =  InstrumentationHelper.GetExcludedFiles(_rules);                              
    var filters = _filters
                    .Where(filterItem => IsValidFilterExpression(filterItem))
                    .Select(filter => 
                    {
                      // Code taken from InstrumentationHelper.IsModuleExcluded.
                      // Probably such code will end up in its own method.
                      string typePattern = filter.Substring(filter.IndexOf(']') + 1);
                      string modulePattern = filter.Substring(1, filter.IndexOf(']') - 1);

                      typePattern = WildcardToRegex(typePattern);
                      modulePattern = WildcardToRegex(modulePattern);
                      
                      return new SomeSortOfModuleTypeFilterObject ( 
                        new Regex(typePattern, RegexOptions.Compiled),  
                        new Regex(modulePattern, RegexOptions.Compiled));

                      // or using some sort of lazy construction..
//                      return new SomeSortOfModuleTypeFilterObject ( 
//                        new Lazy<Regex>(() => new Regex(typePattern, RegexOptions.Compiled)),  
//                        new Lazy<Regex>(() => new Regex((modulePattern, , RegexOptions.Compiled)),  

                    })
                    .ToArray();

    // at this point, filters is an array of 'SomeSortOfModuleTypeFilterObject' containing regex for object and module matching.
}

Such data can be usefull for the foreach loops in IsModuleExcluded and in IsTypeExcluded methods. Now, each filter in filters is parsed and Regex object created each single time.

Constructing compiled regex objects can be faster when executing the same regex multiple times. Also storing and reusing them puts less presure on the GC.

coenm

Inline comments related to my previous post

coenm · 2018-05-14T10:08:52Z

src/coverlet.core/Helpers/InstrumentationHelper.cs

+            foreach (var filter in filters)
+            {
+                if (!IsValidFilterExpression(filter))
+                    continue;


A lot of unnecessary calls to IsValidFilterExpression. An other approach is to make sure that the input parameter filters contains only valid filter expressions.

coenm · 2018-05-14T10:09:14Z

src/coverlet.core/Helpers/InstrumentationHelper.cs

+
+            foreach (var filter in filters)
+            {
+                if (!IsValidFilterExpression(filter))


A lot of unnecessary calls to IsValidFilterExpression. Make sure that the input parameter filters contains valid data.

coenm · 2018-05-14T10:10:21Z

src/coverlet.core/Helpers/InstrumentationHelper.cs

+                typePattern = WildcardToRegex(typePattern);
+                modulePattern = WildcardToRegex(modulePattern);
+
+                isMatch = new Regex(typePattern).IsMatch(type) && new Regex(modulePattern).IsMatch(module);


For each module, this method is called. For each filter these two regexes are created. This can be cached in some sort of object and the regex can be created using compile mode to speed up the process and to reduce GC presure

coenm · 2018-05-14T10:11:24Z

src/coverlet.core/Helpers/InstrumentationHelper.cs

+                modulePattern = WildcardToRegex(modulePattern);
+
+                var regex = new Regex(modulePattern);
+                isMatch = regex.IsMatch(module) && typePattern == "*";


For each module, this method is called. For each filter this regex is created. This objecty be cached in some sort of object and the regex can be created using compile mode to speed up the process and to reduce GC presure

coenm · 2018-05-14T10:12:16Z

src/coverlet.core/Helpers/InstrumentationHelper.cs

+            if (filter.EndsWith("]"))
+                return false;
+
+            if (new Regex(@"[^\w*]").IsMatch(filter.Replace(".", "").Replace("[", "").Replace("]", "")))


Regex object can be static field and created in compile mode

default-writer · 2018-06-18T06:47:09Z

Improvement:

I have a question why not to have the same syntax as for other tools?
Why not to use SonarScanner format

/d:sonar.coverage.exclusions="**/MyFun*.cs,**/*Test*.cs,**/*Exception*.cs,**/*Attribute*.cs"

for /p:ExcludeByFile?, for example:

/p:ExcludeByFile="**/MyFun*.cs,**/*Test*.cs,**/*Exception*.cs,**/*Attribute*.cs"

My build script:

& packages/dotnet-sonarscanner begin /d:sonar.login="$env:SONARCLOUDTOKEN" /k:"build-core" /d:sonar.host.url="https://sonarcloud.io" /n:"build" /v:"1.0" /d:sonar.cs.opencover.reportsPaths="Build.Tests/coverage.opencover.xml" /d:sonar.coverage.exclusions="**/MyFun*.cs,**/*Test*.cs,**/*Exception*.cs,**/*Attribute*.cs" /d:sonar.organization="hack2root-github" /d:sonar.sourceEncoding="UTF-8"
& dotnet build --configuration Release
& dotnet test --configuration Release --no-build Build.Tests /p:CollectCoverage=true /p:CoverletOutputFormat=opencover
& packages/dotnet-sonarscanner end /d:sonar.login="$env:SONARCLOUDTOKEN"
& packages/csmacnz.Coveralls --opencover -i Build.Tests/coverage.opencover.xml

tonerdo · 2018-06-19T08:52:10Z

@hack2root I don't see a reason why ExcludeByFile shouldn't support the format you speak of. Kindly confirm @ido-namely, thanks

Add ability to exclude assemblies by expression

tonerdo added 5 commits May 13, 2018 11:33

create backing fields for instrumentation task properties

002c0a6

add preliminary support for excluding assemblies from coverage

1dd3b70

complete filter expression evaluation

9d09fde

rename Exclude property to ExcludeByFile

ebedd70

change Filter property to Exclude

9ed0864

update README.md, add filter documentation

371112c

tonerdo merged commit 928f05d into master May 14, 2018

MarcoRossignoli reviewed May 14, 2018

View reviewed changes

MarcoRossignoli mentioned this pull request May 14, 2018

Adding a Filter property to include/exclude assemblies/types #77

Closed

coenm reviewed May 14, 2018

View reviewed changes

tonerdo deleted the filter-assemblies branch May 17, 2018 11:14

NorekZ pushed a commit to NorekZ/coverlet that referenced this pull request Nov 8, 2018

Merge pull request coverlet-coverage#94 from tonerdo/filter-assemblies

f461b0b

Add ability to exclude assemblies by expression

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Add ability to exclude assemblies by expression #94

Add ability to exclude assemblies by expression #94

Uh oh!

tonerdo commented May 13, 2018 •

edited

Loading

Uh oh!

codecov bot commented May 13, 2018 •

edited

Loading

Uh oh!

MarcoRossignoli May 14, 2018 •

edited

Loading

Uh oh!

coenm commented May 14, 2018

Uh oh!

coenm left a comment

Uh oh!

coenm May 14, 2018

Uh oh!

coenm May 14, 2018

Uh oh!

coenm May 14, 2018

Uh oh!

coenm May 14, 2018

Uh oh!

coenm May 14, 2018

Uh oh!

default-writer commented Jun 18, 2018 •

edited

Loading

Uh oh!

tonerdo commented Jun 19, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Add ability to exclude assemblies by expression #94

Add ability to exclude assemblies by expression #94

Uh oh!

Conversation

tonerdo commented May 13, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov bot commented May 13, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

MarcoRossignoli May 14, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

coenm commented May 14, 2018

Uh oh!

coenm left a comment

Choose a reason for hiding this comment

Uh oh!

coenm May 14, 2018

Choose a reason for hiding this comment

Uh oh!

coenm May 14, 2018

Choose a reason for hiding this comment

Uh oh!

coenm May 14, 2018

Choose a reason for hiding this comment

Uh oh!

coenm May 14, 2018

Choose a reason for hiding this comment

Uh oh!

coenm May 14, 2018

Choose a reason for hiding this comment

Uh oh!

default-writer commented Jun 18, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

tonerdo commented Jun 19, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

tonerdo commented May 13, 2018 •

edited

Loading

codecov bot commented May 13, 2018 •

edited

Loading

MarcoRossignoli May 14, 2018 •

edited

Loading

default-writer commented Jun 18, 2018 •

edited

Loading