Merge pull request #11 from tonerdo/master

Update fork
daveMueller · Jan 21, 2020 · 566262b · 566262b
2 parents 8b8505b + a63b67c
commit 566262b
Show file tree

Hide file tree

Showing 15 changed files with 517 additions and 117 deletions.
diff --git a/Directory.Build.targets b/Directory.Build.targets
@@ -14,7 +14,7 @@
     <!-- Do not upgrade this version or we won't support old SDK -->
     <PackageReference Update="Newtonsoft.Json" Version="9.0.1" />
     <PackageReference Update="NuGet.Packaging" Version="5.4.0" />
-    <PackageReference Update="ReportGenerator.Core" Version="4.2.15" />
+    <PackageReference Update="ReportGenerator.Core" Version="4.4.4" />
     <!--
         Do not change System.Reflection.Metadata version since we need to support VSTest DataCollectors. Goto https://www.nuget.org/packages/System.Reflection.Metadata to check versions.
         We need to load assembly version 1.4.2.0 to properly work

diff --git a/Documentation/Changelog.md b/Documentation/Changelog.md
@@ -8,7 +8,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Fixed
 
--Fixed ExcludeFromCodeCoverage attribute bugs [#129](https://github.com/tonerdo/coverlet/issues/129) and [#670](https://github.com/tonerdo/coverlet/issues/670) with [#671](https://github.com/tonerdo/coverlet/pull/671) by https://github.com/matteoerigozzi
+-Fix ExcludeFromCodeCoverage attribute bugs [#129](https://github.com/tonerdo/coverlet/issues/129) and [#670](https://github.com/tonerdo/coverlet/issues/670) with [#671](https://github.com/tonerdo/coverlet/pull/671) by https://github.com/matteoerigozzi  
+-Fix bug with nested types filtering [#689](https://github.com/tonerdo/coverlet/issues/689)
 
 ### Improvements
 

diff --git a/src/coverlet.core/Instrumentation/Instrumenter.cs b/src/coverlet.core/Instrumentation/Instrumenter.cs
@@ -141,6 +141,28 @@ public InstrumenterResult Instrument()
             return _result;
         }
 
+        // If current type or one of his parent is excluded we'll exclude it
+        // If I'm out every my children and every children of my children will be out
+        private bool IsTypeExcluded(TypeDefinition type)
+        {
+            for (TypeDefinition current = type; current != null; current = current.DeclaringType)
+            {
+                // Check exclude attribute and filters
+                if (current.CustomAttributes.Any(IsExcludeAttribute) || _instrumentationHelper.IsTypeExcluded(_module, current.FullName, _excludeFilters))
+                {
+                    return true;
+                }
+            }
+
+            return false;
+        }
+
+        // Instrumenting Interlocked which is used for recording hits would cause an infinite loop.
+        private bool Is_System_Threading_Interlocked_CoreLib_Type(TypeDefinition type)
+        {
+            return _isCoreLibrary && type.FullName == "System.Threading.Interlocked";
+        }
+
         private void InstrumentModule()
         {
             using (var stream = _fileSystem.NewFileStream(_module, FileMode.Open, FileAccess.ReadWrite))
@@ -177,18 +199,15 @@ private void InstrumentModule()
 
                     foreach (TypeDefinition type in types)
                     {
-                        var actualType = type.DeclaringType ?? type;
-                        if (!actualType.CustomAttributes.Any(IsExcludeAttribute)
-                            // Instrumenting Interlocked which is used for recording hits would cause an infinite loop.
-                            && (!_isCoreLibrary || actualType.FullName != "System.Threading.Interlocked")
-                            && !_instrumentationHelper.IsTypeExcluded(_module, actualType.FullName, _excludeFilters)
-                            && _instrumentationHelper.IsTypeIncluded(_module, actualType.FullName, _includeFilters)
+                        if (
+                            !Is_System_Threading_Interlocked_CoreLib_Type(type) &&
+                            !IsTypeExcluded(type) &&
+                            _instrumentationHelper.IsTypeIncluded(_module, type.FullName, _includeFilters)
                             )
                         {
                             if (IsSynthesizedMemberToBeExcluded(type))
                             {
-                                _excludedCompilerGeneratedTypes ??= new List<string>();
-                                _excludedCompilerGeneratedTypes.Add(type.FullName);
+                                (_excludedCompilerGeneratedTypes ??= new List<string>()).Add(type.FullName);
                             }
                             else
                             {
@@ -378,8 +397,7 @@ private void InstrumentType(TypeDefinition type)
                 }
                 else
                 {
-                    _excludedMethods ??= new List<(MethodDefinition, int)>();
-                    _excludedMethods.Add((method, ordinal));
+                    (_excludedMethods ??= new List<(MethodDefinition, int)>()).Add((method, ordinal));
                 }
             }
 

diff --git a/src/coverlet.core/Instrumentation/ModuleTrackerTemplate.cs b/src/coverlet.core/Instrumentation/ModuleTrackerTemplate.cs
@@ -78,7 +78,7 @@ public static void UnloadModule(object sender, EventArgs e)
             {
                 WriteLog($"Unload called for '{Assembly.GetExecutingAssembly().Location}'");
                 // Claim the current hits array and reset it to prevent double-counting scenarios.
-                var hitsArray = Interlocked.Exchange(ref HitsArray, new int[HitsArray.Length]);
+                int[] hitsArray = Interlocked.Exchange(ref HitsArray, new int[HitsArray.Length]);
 
                 // The same module can be unloaded multiple times in the same process via different app domains.
                 // Use a global mutex to ensure no concurrent access.
@@ -101,8 +101,9 @@ public static void UnloadModule(object sender, EventArgs e)
                             }
                         }
                     }
-                    catch
+                    catch (Exception ex)
                     {
+                        WriteLog($"Failed to create new hits file '{HitsFilePath}'\n{ex}");
                         failedToCreateNewHitsFile = true;
                     }
 

diff --git a/test/coverlet.core.tests/Coverage/CoverageTests.cs b/test/coverlet.core.tests/Coverage/CoverageTests.cs
@@ -1,6 +1,7 @@
 using System;
 using System.IO;
 using System.Linq;
+using System.Reflection;
 using System.Threading.Tasks;
 
 using Coverlet.Core.Abstracts;
@@ -99,7 +100,7 @@ public void SelectionStatements_If()
 
                         // For now we have only async Run helper
                         return Task.CompletedTask;
-                    }, pathSerialize);
+                    }, persistPrepareResultToFile: pathSerialize);
 
                     // we return 0 if we return something different assert fail
                     return 0;
@@ -138,7 +139,7 @@ public void SelectionStatements_Switch()
                     {
                         instance.Switch(1);
                         return Task.CompletedTask;
-                    }, pathSerialize);
+                    }, persistPrepareResultToFile: pathSerialize);
                     return 0;
                 }, path).Dispose();
 
@@ -176,7 +177,7 @@ public void AsyncAwait()
                         res = ((Task<int>)instance.ConfigureAwait()).ConfigureAwait(false).GetAwaiter().GetResult();
 
                         return Task.CompletedTask;
-                    }, pathSerialize);
+                    }, persistPrepareResultToFile: pathSerialize);
                     return 0;
                 }, path).Dispose();
 
@@ -227,7 +228,7 @@ public void Lambda_Issue343()
                         instance.InvokeAnonymous_Test();
                         ((Task<bool>)instance.InvokeAnonymousAsync_Test()).ConfigureAwait(false).GetAwaiter().GetResult();
                         return Task.CompletedTask;
-                    }, pathSerialize);
+                    }, persistPrepareResultToFile: pathSerialize);
                     return 0;
                 }, path).Dispose();
 
@@ -273,7 +274,7 @@ public void ExcludeFromCodeCoverage_CompilerGeneratedMethodsAndTypes()
                     {
                         ((Task<int>)instance.Test("test")).ConfigureAwait(false).GetAwaiter().GetResult();
                         return Task.CompletedTask;
-                    }, pathSerialize);
+                    }, persistPrepareResultToFile: pathSerialize);
 
                     return 0;
 
@@ -314,7 +315,7 @@ public void ExcludeFromCodeCoverage_CompilerGeneratedMethodsAndTypes_NestedMembe
                     {
                         instance.Test();
                         return Task.CompletedTask;
-                    }, pathSerialize);
+                    }, persistPrepareResultToFile: pathSerialize);
 
                     return 0;
 
@@ -344,7 +345,7 @@ public void ExcludeFromCodeCoverageCompilerGeneratedMethodsAndTypes_Issue670()
                     {
                         instance.Test("test");
                         return Task.CompletedTask;
-                    }, pathSerialize);
+                    }, persistPrepareResultToFile: pathSerialize);
 
                     return 0;
 
@@ -361,5 +362,150 @@ public void ExcludeFromCodeCoverageCompilerGeneratedMethodsAndTypes_Issue670()
                 File.Delete(path);
             }
         }
+
+        [Fact]
+        public void ExcludeFromCodeCoverageNextedTypes()
+        {
+            string path = Path.GetTempFileName();
+            try
+            {
+                RemoteExecutor.Invoke(async pathSerialize =>
+                {
+                    CoveragePrepareResult coveragePrepareResult = await TestInstrumentationHelper.Run<ExcludeFromCoverageAttrFilterClass1>(instance =>
+                    {
+                        Assert.Equal(42, instance.Run());
+                        return Task.CompletedTask;
+                    }, persistPrepareResultToFile: pathSerialize);
+
+                    return 0;
+                }, path).Dispose();
+
+                TestInstrumentationHelper.GetCoverageResult(path)
+                .Document("Instrumentation.ExcludeFromCoverage.cs")
+                .AssertLinesCovered(BuildConfiguration.Debug, (143, 1))
+                .AssertNonInstrumentedLines(BuildConfiguration.Debug, 146, 160);
+            }
+            finally
+            {
+                File.Delete(path);
+            }
+        }
+
+        [Fact]
+        public void ExcludeFilteredNestedAutogeneratedTypes()
+        {
+            string path = Path.GetTempFileName();
+            try
+            {
+                RemoteExecutor.Invoke(async pathSerialize =>
+                {
+                    CoveragePrepareResult coveragePrepareResult = await TestInstrumentationHelper.Run<ExcludeFilterNestedAutogeneratedTypes>(instance =>
+                    {
+                        instance.Run();
+
+                        PropertyInfo stateProp = null;
+                        foreach (Type type in ((Type)instance.GetType()).Assembly.GetTypes())
+                        {
+                            if (typeof(Issue_689).FullName == type.FullName)
+                            {
+                                Assert.Equal(0, (stateProp = type.GetProperty("State")).GetValue(null));
+                                break;
+                            }
+                        }
+
+                        foreach (Type type in ((Type)instance.GetType()).Assembly.GetTypes())
+                        {
+                            if (typeof(EventSource_Issue_689).FullName == type.FullName)
+                            {
+                                type.GetMethod("RaiseEvent").Invoke(null, null);
+                                break;
+                            }
+                        }
+
+                        Assert.Equal(2, stateProp.GetValue(null));
+
+                        return Task.CompletedTask;
+                    },
+                    includeFilter: moduleFileName => new string[] { $"[{Path.GetFileNameWithoutExtension(moduleFileName)}*]*ExcludeFilterNestedAutogeneratedTypes", $"[{Path.GetFileNameWithoutExtension(moduleFileName)}*]*Issue_689" },
+                    excludeFilter: moduleFileName => new string[] { $"[{Path.GetFileNameWithoutExtension(moduleFileName)}*]*NestedToFilterOut", $"[{Path.GetFileNameWithoutExtension(moduleFileName)}*]*Uncoverlet" },
+                    persistPrepareResultToFile: pathSerialize);
+
+                    return 0;
+                }, path).Dispose();
+
+               TestInstrumentationHelper.GetCoverageResult(path)
+               .Document("Instrumentation.ExcludeFilter.cs")
+               .AssertLinesCovered(BuildConfiguration.Debug, (12, 1), (13, 1), (14, 1))
+               .AssertLinesCovered(BuildConfiguration.Debug, (27, 1), (28, 1), (29, 1), (30, 1), (31, 1))
+               .AssertLinesCovered(BuildConfiguration.Debug, (39, 2), (40, 2), (41, 2), (43, 5))
+               .AssertLinesCovered(BuildConfiguration.Debug, (50, 1), (51, 1), (52, 1))
+               .AssertNonInstrumentedLines(BuildConfiguration.Debug, 17, 21)
+               .AssertNonInstrumentedLines(BuildConfiguration.Debug, 33, 36);
+            }
+            finally
+            {
+                File.Delete(path);
+            }
+        }
+
+        [Fact]
+        public void ExcludeFilteredTypes()
+        {
+            string path = Path.GetTempFileName();
+            try
+            {
+                RemoteExecutor.Invoke(async pathSerialize =>
+                {
+                    CoveragePrepareResult coveragePrepareResult = await TestInstrumentationHelper.Run<ExcludeFilterOuterTypes>(instance =>
+                    {
+                        Assert.Equal(42, instance.Run());
+                        return Task.CompletedTask;
+                    },
+                    excludeFilter: moduleFileName => new string[] { $"[{Path.GetFileNameWithoutExtension(moduleFileName)}*]*ExcludeFilterOuterTypes" },
+                    persistPrepareResultToFile: pathSerialize);
+
+                    return 0;
+                }, path).Dispose();
+
+                TestInstrumentationHelper.GetCoverageResult(path)
+                .Document("Instrumentation.ExcludeFilter.cs")
+                .AssertNonInstrumentedLines(BuildConfiguration.Debug, 1, 62)
+                .AssertLinesCovered(BuildConfiguration.Debug, (66, 1), (68, 1));
+            }
+            finally
+            {
+                File.Delete(path);
+            }
+        }
+
+        [Fact]
+        public void ExcludeFilteredNestedTypes()
+        {
+            string path = Path.GetTempFileName();
+            try
+            {
+                RemoteExecutor.Invoke(async pathSerialize =>
+                {
+                    CoveragePrepareResult coveragePrepareResult = await TestInstrumentationHelper.Run<ExcludeFilterClass1>(instance =>
+                    {
+                        Assert.Equal(42, instance.Run());
+                        return Task.CompletedTask;
+                    },
+                    excludeFilter: moduleFileName => new string[] { $"[{Path.GetFileNameWithoutExtension(moduleFileName)}*]*ExcludeFilterClass2" },
+                    persistPrepareResultToFile: pathSerialize);
+
+                    return 0;
+                }, path).Dispose();
+
+                TestInstrumentationHelper.GetCoverageResult(path)
+                .Document("Instrumentation.ExcludeFilter.cs")
+                .AssertLinesCovered(BuildConfiguration.Debug, (73, 1))
+                .AssertNonInstrumentedLines(BuildConfiguration.Debug, 75, 93);
+            }
+            finally
+            {
+                File.Delete(path);
+            }
+        }
     }
 }