Merge pull request #12 from tonerdo/master

Update fork

Author: daveMueller

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.4.4" />
+    <PackageReference Update="ReportGenerator.Core" Version="4.4.5" />
     <!--
         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

Documentation/Changelog.md

@@ -9,7 +9,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 ### Fixed
 
 -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)
+-Fix bug with nested types filtering [#689](https://github.com/tonerdo/coverlet/issues/689)  
+-Fix Coverage Issue - New Using + Async/Await + ConfigureAwait [#669](https://github.com/tonerdo/coverlet/issues/669)
+
 
 ### Improvements
 

Documentation/Troubleshooting.md

@@ -222,7 +222,7 @@ We can collect logs from trackers through an enviroment variable
  set COVERLET_ENABLETRACKERLOG=1
 ```
 When enabled, tracking event will be collected in log file near to module location.  
-File name will be something like `moduleName.dll_tracker.txt`
+File name will be something like `moduleName.dll_tracker.txt` and files with detailed hits will be in a folder named `TrackersHitsLog`.
 
 ## Enable msbuild task instrumentation debugging
 

Documentation/VSTestIntegration.md

@@ -54,7 +54,7 @@ How to specify these options via runsettings?
           <IncludeDirectory>../dir1/,../dir2/,</IncludeDirectory>
           <SingleHit>false</SingleHit>
           <UseSourceLink>true</UseSourceLink>
-          <IncludeTestAssembly>true<IncludeTestAssembly>
+          <IncludeTestAssembly>true</IncludeTestAssembly>
         </Configuration>
       </DataCollector>
     </DataCollectors>
@@ -63,9 +63,9 @@ How to specify these options via runsettings?
 ```
 This runsettings file can easily be provided using command line option as given :
 
-1. `dotnet test --settings coverletArgs.runsettings`
+1. `dotnet test --settings coverlet.runsettings`
 
-2. `dotnet vstest --settings coverletArgs.runsettings`
+2. `dotnet vstest --settings coverlet.runsettings`
 
 Take a look at our [`HelloWorld`](Examples/VSTest/HelloWorld/HowTo.md) sample.
 
@@ -83,7 +83,7 @@ The datacollectors will be bundled as a separate NuGet package, the reference to
     <PackageReference Include="Microsoft.NET.Test.Sdk" Version="x.x.x" />
     <PackageReference Include="MSTest.TestAdapter" Version="x.x.x" />
     <PackageReference Include="MSTest.TestFramework" Version="x.x.x" />
-    <PackageReference Include="coverlet.collector" Version="1.0.0" />
+    <PackageReference Include="coverlet.collector" Version="x.x.x" />
 </ItemGroup>
 ```
 

README.md

@@ -1,6 +1,6 @@
 # Coverlet
 
-[![Build Status](https://dev.azure.com/tonerdo/coverlet/_apis/build/status/tonerdo.coverlet?branchName=master)](https://dev.azure.com/tonerdo/coverlet/_build/latest?definitionId=3&branchName=master)
+[![Build Status](https://dev.azure.com/tonerdo/coverlet/_apis/build/status/tonerdo.coverlet?branchName=master)](https://dev.azure.com/tonerdo/coverlet/_build/latest?definitionId=3&branchName=master) [![License: MIT](https://img.shields.io/badge/License-MIT-yellow.svg)](https://github.com/tonerdo/coverlet/blob/master/LICENSE)
 
 Coverlet is a cross platform code coverage framework for .NET, with support for line, branch and method coverage. It works with .NET Framework on Windows and .NET Core on all supported platforms.
 
@@ -37,7 +37,7 @@ Coverlet is integrated into the Visual Studio Test Platform as a [data collector
 dotnet test --collect:"XPlat Code Coverage"
 ```
 
-After the above command is run, a `coverage.cobertura.json` file containing the results will be published to the `TestResults` directory as an attachment. A summary of the results will also be displayed in the terminal.
+After the above command is run, a `coverage.cobertura.xml` file containing the results will be published to the `TestResults` directory as an attachment.
 
 See [documentation](Documentation/VSTestIntegration.md) for advanced usage.
 
@@ -48,7 +48,13 @@ See [documentation](Documentation/VSTestIntegration.md) for advanced usage.
 ```
 <PackageReference Include="Microsoft.NET.Test.Sdk" Version="16.1.0" />
 ```
-* Important [know issue](Documentation/KnowIssues.md#2-upgrade-coverletcollector-to-version--100)
+#### Important temporary [know issue](Documentation/KnowIssues.md#2-upgrade-coverletcollector-to-version--100)
+
+*Current* recommended test sdk package to reference
+```
+<PackageReference Include="Microsoft.NET.Test.Sdk" Version="16.5.0-preview-20200116-01" />
+```
+
 ### MSBuild Integration
 
 ### Installation

coverlet.sln

@@ -46,6 +46,8 @@ Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "coverlet.integration.tests"
 EndProject
 Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "coverlet.integration.template", "test\coverlet.integration.template\coverlet.integration.template.csproj", "{F6FE7678-C662-43D3-AC6A-64F6AC5A5935}"
 EndProject
+Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "coverlet.core.tests.samples.netstandard", "test\coverlet.core.tests.samples.netstandard\coverlet.core.tests.samples.netstandard.csproj", "{5FF404AD-7C0B-465A-A1E9-558CDC642B0C}"
+EndProject
 Global
 	GlobalSection(SolutionConfigurationPlatforms) = preSolution
 		Debug|Any CPU = Debug|Any CPU
@@ -104,6 +106,10 @@ Global
 		{F6FE7678-C662-43D3-AC6A-64F6AC5A5935}.Debug|Any CPU.Build.0 = Debug|Any CPU
 		{F6FE7678-C662-43D3-AC6A-64F6AC5A5935}.Release|Any CPU.ActiveCfg = Release|Any CPU
 		{F6FE7678-C662-43D3-AC6A-64F6AC5A5935}.Release|Any CPU.Build.0 = Release|Any CPU
+		{5FF404AD-7C0B-465A-A1E9-558CDC642B0C}.Debug|Any CPU.ActiveCfg = Debug|Any CPU
+		{5FF404AD-7C0B-465A-A1E9-558CDC642B0C}.Debug|Any CPU.Build.0 = Debug|Any CPU
+		{5FF404AD-7C0B-465A-A1E9-558CDC642B0C}.Release|Any CPU.ActiveCfg = Release|Any CPU
+		{5FF404AD-7C0B-465A-A1E9-558CDC642B0C}.Release|Any CPU.Build.0 = Release|Any CPU
 	EndGlobalSection
 	GlobalSection(SolutionProperties) = preSolution
 		HideSolutionNode = FALSE
@@ -122,6 +128,7 @@ Global
 		{D6B14F2F-9E7D-4D2C-BAC8-48834F853ED6} = {2FEBDE1B-83E3-445B-B9F8-5644B0E0E134}
 		{99B4059C-B25C-4B82-8117-A0E9DC9B0949} = {2FEBDE1B-83E3-445B-B9F8-5644B0E0E134}
 		{F6FE7678-C662-43D3-AC6A-64F6AC5A5935} = {2FEBDE1B-83E3-445B-B9F8-5644B0E0E134}
+		{5FF404AD-7C0B-465A-A1E9-558CDC642B0C} = {2FEBDE1B-83E3-445B-B9F8-5644B0E0E134}
 	EndGlobalSection
 	GlobalSection(ExtensibilityGlobals) = postSolution
 		SolutionGuid = {9CA57C02-97B0-4C38-A027-EA61E8741F10}

eng/build.yml

@@ -22,5 +22,5 @@ steps:
   displayName: Test
   inputs:
     command: test
-    arguments: -c $(BuildConfiguration) --no-build /p:CollectCoverage=true /p:CoverletOutputFormat=opencover /p:Include=[coverlet.*]* /p:Exclude=[coverlet.tests.remoteexecutor]*
+    arguments: -c $(BuildConfiguration) --no-build /p:CollectCoverage=true /p:CoverletOutputFormat=opencover /p:Include="[coverlet.collector]*%2c[coverlet.core]*%2c[coverlet.msbuild.tasks]*" /p:Exclude="[coverlet.core.tests.samples.netstandard]*%2c[coverlet.tests.remoteexecutor]*"
     testRunTitle: $(Agent.JobName)

src/coverlet.core/Instrumentation/ModuleTrackerTemplate.cs

@@ -1,4 +1,5 @@
 using System;
+using System.Diagnostics;
 using System.Diagnostics.CodeAnalysis;
 using System.IO;
 using System.Reflection;
@@ -20,7 +21,7 @@ internal static class ModuleTrackerTemplate
         public static string HitsFilePath;
         public static int[] HitsArray;
         public static bool SingleHit;
-        private static bool _enableLog = int.TryParse(Environment.GetEnvironmentVariable("COVERLET_ENABLETRACKERLOG"), out int result) ? result == 1 : false;
+        private static readonly bool _enableLog = int.TryParse(Environment.GetEnvironmentVariable("COVERLET_ENABLETRACKERLOG"), out int result) ? result == 1 : false;
 
         static ModuleTrackerTemplate()
         {
@@ -116,6 +117,8 @@ public static void UnloadModule(object sender, EventArgs e)
                         using (var bw = new BinaryWriter(fs))
                         {
                             int hitsLength = br.ReadInt32();
+                            WriteLog($"Current hits found '{hitsLength}'");
+
                             if (hitsLength != hitsArray.Length)
                             {
                                 throw new InvalidOperationException(
@@ -134,6 +137,8 @@ public static void UnloadModule(object sender, EventArgs e)
                         }
                     }
 
+                    WriteHits();
+
                     // On purpose this is not under a try-finally: it is better to have an exception if there was any error writing the hits file
                     // this case is relevant when instrumenting corelib since multiple processes can be running against the same instrumented dll.
                     mutex.ReleaseMutex();
@@ -147,21 +152,38 @@ public static void UnloadModule(object sender, EventArgs e)
             }
         }
 
-        private static void WriteLog(string logText)
+        private static void WriteHits()
         {
             if (_enableLog)
             {
-                try
-                {
-                    // We don't set path as global var to keep benign possible errors inside try/catch
-                    // I'm not sure that location will be ok in every scenario
-                    string location = Assembly.GetExecutingAssembly().Location;
-                    File.AppendAllText(Path.Combine(Path.GetDirectoryName(location), Path.GetFileName(location) + "_tracker.txt"), $"[{DateTime.UtcNow} {Thread.CurrentThread.ManagedThreadId}]{logText}{Environment.NewLine}");
-                }
-                catch
+                Assembly currentAssembly = Assembly.GetExecutingAssembly();
+                DirectoryInfo location = new DirectoryInfo(Path.Combine(Path.GetDirectoryName(currentAssembly.Location), "TrackersHitsLog"));
+                location.Create();
+                string logFile = Path.Combine(location.FullName, $"{Path.GetFileName(currentAssembly.Location)}_{DateTime.UtcNow.Ticks}_{Process.GetCurrentProcess().Id}.txt");
+                using (var fs = new FileStream(HitsFilePath, FileMode.Open, FileAccess.ReadWrite, FileShare.None))
+                using (var log = new FileStream(logFile, FileMode.CreateNew, FileAccess.ReadWrite, FileShare.None))
+                using (var logWriter = new StreamWriter(log))
+                using (var br = new BinaryReader(fs))
                 {
-                    // do nothing if log fail
+                    int hitsLength = br.ReadInt32();
+                    for (int i = 0; i < hitsLength; ++i)
+                    {
+                        logWriter.WriteLine($"{i},{br.ReadInt32()}");
+                    }
                 }
+
+                File.AppendAllText(logFile, "Hits flushed");
+            }
+        }
+
+        private static void WriteLog(string logText)
+        {
+            if (_enableLog)
+            {
+                // We don't set path as global var to keep benign possible errors inside try/catch
+                // I'm not sure that location will be ok in every scenario
+                string location = Assembly.GetExecutingAssembly().Location;
+                File.AppendAllText(Path.Combine(Path.GetDirectoryName(location), Path.GetFileName(location) + "_tracker.txt"), $"[{DateTime.UtcNow} P:{Process.GetCurrentProcess().Id} T:{Thread.CurrentThread.ManagedThreadId}]{logText}{Environment.NewLine}");
             }
         }
     }

src/coverlet.core/Symbols/CecilSymbolHelper.cs

@@ -60,6 +60,25 @@ private static bool IsMoveNextInsideEnumerator(MethodDefinition methodDefinition
             return false;
         }
 
+        private static bool IsRecognizedMoveNextInsideAsyncStateMachineProlog(MethodDefinition methodDefinition)
+        {
+            /*
+                int num = <>1__state;
+                IL_0000: ldarg.0
+                IL_0001: ldfld ...::'<>1__state'
+                IL_0006: stloc.0
+            */
+            return (methodDefinition.Body.Instructions[0].OpCode == OpCodes.Ldarg_0 ||
+                    methodDefinition.Body.Instructions[0].OpCode == OpCodes.Ldarg) &&
+
+                    methodDefinition.Body.Instructions[1].OpCode == OpCodes.Ldfld &&
+                    methodDefinition.Body.Instructions[1].Operand is FieldDefinition fd && fd.Name == "<>1__state" &&
+
+                    (methodDefinition.Body.Instructions[2].OpCode == OpCodes.Stloc &&
+                    methodDefinition.Body.Instructions[2].Operand is VariableDefinition vd && vd.Index == 0) ||
+                    methodDefinition.Body.Instructions[2].OpCode == OpCodes.Stloc_0;
+        }
+
         public static List<BranchPoint> GetBranchPoints(MethodDefinition methodDefinition)
         {
             var list = new List<BranchPoint>();
@@ -69,9 +88,9 @@ public static List<BranchPoint> GetBranchPoints(MethodDefinition methodDefinitio
             UInt32 ordinal = 0;
             var instructions = methodDefinition.Body.Instructions;
 
-            // if method is a generated MoveNext skip first branch (could be a switch or a branch)
             bool isAsyncStateMachineMoveNext = IsMoveNextInsideAsyncStateMachine(methodDefinition);
-            bool skipFirstBranch = isAsyncStateMachineMoveNext || IsMoveNextInsideEnumerator(methodDefinition);
+            bool isRecognizedMoveNextInsideAsyncStateMachineProlog = isAsyncStateMachineMoveNext && IsRecognizedMoveNextInsideAsyncStateMachineProlog(methodDefinition);
+            bool skipFirstBranch = IsMoveNextInsideEnumerator(methodDefinition);
 
             foreach (Instruction instruction in instructions.Where(instruction => instruction.OpCode.FlowControl == FlowControl.Cond_Branch))
             {
@@ -83,15 +102,87 @@ public static List<BranchPoint> GetBranchPoints(MethodDefinition methodDefinitio
                         continue;
                     }
 
+                    /* 
+                       If method is a generated MoveNext we'll skip first branches (could be a switch or a series of branches) 
+                       that check state machine value to jump to correct state(for instance after a true async call)
+                       Check if it's a Cond_Branch on state machine current value int num = <>1__state;
+                       We are on branch OpCode so we need to go back by max 2 operation to reach ldloc.0 the load of "num"
+                       Max 2 because we handle following patterns
+
+                        Swich
+
+                        // switch (num)
+                        IL_0007: ldloc.0                        2
+                        // (no C# code)
+                        IL_0008: switch (IL_0037, IL_003c, ...  1
+                        ...
+
+                        Single branch
+
+                        // if (num != 0)
+                        IL_0007: ldloc.0           2
+                        // (no C# code)
+                        IL_0008: brfalse.s IL_000c 1
+                        IL_000a: br.s IL_000e
+                        IL_000c: br.s IL_0049
+                        IL_000e: nop
+                        ...
+
+                        More tha one branch
+
+                        // if (num != 0)
+                        IL_0007: ldloc.0
+                        // (no C# code)
+                        IL_0008: brfalse.s IL_0012
+                        IL_000a: br.s IL_000c
+                        // if (num == 1)
+                        IL_000c: ldloc.0       3
+                        IL_000d: ldc.i4.1      2
+                        IL_000e: beq.s IL_0014 1
+                        // (no C# code)
+                        IL_0010: br.s IL_0019
+                        IL_0012: br.s IL_0060
+                        IL_0014: br IL_00e5
+                        IL_0019: nop
+                        ...
+
+                        so we know that current branch are checking that field and we're not interested in.
+                    */
+                    if (isAsyncStateMachineMoveNext && isRecognizedMoveNextInsideAsyncStateMachineProlog)
+                    {
+                        bool skipInstruction = false;
+                        Instruction current = instruction.Previous;
+                        for (int instructionBefore = 2; instructionBefore > 0 && current.Previous != null; current = current.Previous, instructionBefore--)
+                        {
+                            if (
+                                    (current.OpCode == OpCodes.Ldloc && current.Operand is VariableDefinition vo && vo.Index == 0) ||
+                                    current.OpCode == OpCodes.Ldloc_0
+                                )
+                            {
+                                skipInstruction = true;
+                                break;
+                            }
+                        }
+                        if (skipInstruction)
+                        {
+                            continue;
+                        }
+                    }
+
                     // Skip get_IsCompleted to avoid unuseful branch due to async/await state machine
-                    if (isAsyncStateMachineMoveNext && instruction.Previous.Operand is MethodReference operand &&
+                    if (
+                            isAsyncStateMachineMoveNext && instruction.Previous.Operand is MethodReference operand &&
                             operand.Name == "get_IsCompleted" &&
                             (
                                 operand.DeclaringType.FullName.StartsWith("System.Runtime.CompilerServices.TaskAwaiter") ||
                                 operand.DeclaringType.FullName.StartsWith("System.Runtime.CompilerServices.ConfiguredTaskAwaitable")
                             )
                             &&
-                            operand.DeclaringType.Scope.Name == "System.Runtime")
+                            (
+                                operand.DeclaringType.Scope.Name == "System.Runtime" ||
+                                operand.DeclaringType.Scope.Name == "netstandard"
+                            )
+                       )
                     {
                         continue;
                     }

test/coverlet.core.tests.samples.netstandard/Instrumentation.AsyncAwait.cs

@@ -0,0 +1,17 @@
+// Remember to use full name because adding new using directives change line numbers
+
+namespace Coverlet.Core.Tests
+{
+    public class Issue_669_2
+    {
+        private readonly System.Net.Http.HttpClient _httpClient = new System.Net.Http.HttpClient();
+
+        async public System.Threading.Tasks.ValueTask<System.Net.Http.HttpResponseMessage> SendRequest()
+        {
+            using (var requestMessage = new System.Net.Http.HttpRequestMessage(System.Net.Http.HttpMethod.Get, "https://www.google.it"))
+            {
+                return await _httpClient.SendAsync(requestMessage).ConfigureAwait(false);
+            }
+        }
+    }
+}

test/coverlet.core.tests.samples.netstandard/coverlet.core.tests.samples.netstandard.csproj

@@ -0,0 +1,12 @@
+<Project Sdk="Microsoft.NET.Sdk">
+  <PropertyGroup>
+    <TargetFramework>netstandard2.0</TargetFramework>
+    <IsPackable>false</IsPackable>
+    <IsTestProject>false</IsTestProject>
+  </PropertyGroup>
+
+  <ItemGroup>
+    <PackageReference Include="System.Threading.Tasks.Extensions" Version="4.5.3" />
+  </ItemGroup>
+
+</Project>