From ec7f120c669f0099c259f03127c7fbcc4009ad15 Mon Sep 17 00:00:00 2001 From: Marco Rossignoli Date: Sat, 22 Dec 2018 19:24:35 +0100 Subject: [PATCH 1/7] fix MergeWith report --- src/coverlet.core/CoverageResult.cs | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/src/coverlet.core/CoverageResult.cs b/src/coverlet.core/CoverageResult.cs index c23bd9c94..cd0fcdba9 100644 --- a/src/coverlet.core/CoverageResult.cs +++ b/src/coverlet.core/CoverageResult.cs @@ -3,6 +3,7 @@ using System.IO; using System.Linq; using Coverlet.Core.Enums; +using Coverlet.Core.Symbols; namespace Coverlet.Core { @@ -97,6 +98,28 @@ internal void Merge(Modules modules) else branchInfo.Hits += branch.Hits; } + + // for MoveNext() compiler autogenerated method we need to patch false positive (IAsyncStateMachine for instance) + // we'll remove all MoveNext() not covered branch + List branchesToRemove = new List(); + foreach (var branch in this.Modules[module.Key][document.Key][@class.Key][method.Key].Branches) + { + //if one branch is covered we search the other one only if it's not covered + if (CecilSymbolHelper.IsMoveNext(method.Key) && branch.Hits > 0) + { + foreach (var moveNextBranch in this.Modules[module.Key][document.Key][@class.Key][method.Key].Branches) + { + if (moveNextBranch != branch && moveNextBranch.Hits == 0) + { + branchesToRemove.Add(moveNextBranch); + } + } + } + } + foreach (var branchToRemove in branchesToRemove) + { + this.Modules[module.Key][document.Key][@class.Key][method.Key].Branches.Remove(branchToRemove); + } } } } From 26ad42ad2e2cd80ae16a76dfed640fb5195b5a2c Mon Sep 17 00:00:00 2001 From: Marco Rossignoli Date: Mon, 24 Dec 2018 17:57:02 +0100 Subject: [PATCH 2/7] updates --- src/coverlet.core/Coverage.cs | 6 +-- src/coverlet.core/CoverageResult.cs | 54 +++++++++++-------- .../Instrumentation/Instrumenter.cs | 15 +++++- .../Instrumentation/InstrumenterResult.cs | 1 + 4 files changed, 51 insertions(+), 25 deletions(-) diff --git a/src/coverlet.core/Coverage.cs b/src/coverlet.core/Coverage.cs index 452f708dd..23855036e 100644 --- a/src/coverlet.core/Coverage.cs +++ b/src/coverlet.core/Coverage.cs @@ -132,7 +132,7 @@ public CoverageResult GetCoverageResult() if (methods.TryGetValue(branch.Method, out Method method)) { method.Branches.Add(new BranchInfo - { Line = branch.Number, Hits = branch.Hits, Offset = branch.Offset, EndOffset = branch.EndOffset, Path = branch.Path, Ordinal = branch.Ordinal } + { Line = branch.Number, Hits = branch.Hits, Offset = branch.Offset, EndOffset = branch.EndOffset, Path = branch.Path, Ordinal = branch.Ordinal, IsAsyncStateMachineBranch = branch.IsAsyncStateMachineBranch } ); } else @@ -232,14 +232,14 @@ private void CalculateCoverage() } // for MoveNext() compiler autogenerated method we need to patch false positive (IAsyncStateMachine for instance) - // we'll remove all MoveNext() not covered branch + // we'll remove all MoveNext() not covered branch foreach (var document in result.Documents) { List> branchesToRemove = new List>(); foreach (var branch in document.Value.Branches) { //if one branch is covered we search the other one only if it's not covered - if (CecilSymbolHelper.IsMoveNext(branch.Value.Method) && branch.Value.Hits > 0) + if (branch.Value.IsAsyncStateMachineBranch && branch.Value.Method.EndsWith("::MoveNext()") && branch.Value.Hits > 0) { foreach (var moveNextBranch in document.Value.Branches) { diff --git a/src/coverlet.core/CoverageResult.cs b/src/coverlet.core/CoverageResult.cs index cd0fcdba9..5ffde6b27 100644 --- a/src/coverlet.core/CoverageResult.cs +++ b/src/coverlet.core/CoverageResult.cs @@ -15,6 +15,7 @@ public class BranchInfo public int Path { get; set; } public uint Ordinal { get; set; } public int Hits { get; set; } + public bool IsAsyncStateMachineBranch { get; set; } } public class Lines : SortedDictionary { } @@ -98,32 +99,43 @@ internal void Merge(Modules modules) else branchInfo.Hits += branch.Hits; } + } + } + } + } + } + } + } + } - // for MoveNext() compiler autogenerated method we need to patch false positive (IAsyncStateMachine for instance) - // we'll remove all MoveNext() not covered branch - List branchesToRemove = new List(); - foreach (var branch in this.Modules[module.Key][document.Key][@class.Key][method.Key].Branches) - { - //if one branch is covered we search the other one only if it's not covered - if (CecilSymbolHelper.IsMoveNext(method.Key) && branch.Hits > 0) - { - foreach (var moveNextBranch in this.Modules[module.Key][document.Key][@class.Key][method.Key].Branches) - { - if (moveNextBranch != branch && moveNextBranch.Hits == 0) - { - branchesToRemove.Add(moveNextBranch); - } - } - } - } - foreach (var branchToRemove in branchesToRemove) - { - this.Modules[module.Key][document.Key][@class.Key][method.Key].Branches.Remove(branchToRemove); - } + // for MoveNext() compiler autogenerated method we need to patch false positive (IAsyncStateMachine for instance) + // we'll remove all MoveNext() not covered branch + List branchesToRemove = new List(); + foreach (var module in this.Modules) + { + foreach (var document in module.Value) + { + foreach (var @class in document.Value) + { + foreach (var method in @class.Value) + { + foreach (var branch in method.Value.Branches) + { + if (branch.IsAsyncStateMachineBranch && method.Key.EndsWith("::MoveNext()") && branch.Hits > 0) + { + foreach (var moveNextBranch in method.Value.Branches) + { + if (moveNextBranch != branch && moveNextBranch.Hits == 0) + { + branchesToRemove.Add(moveNextBranch); } } } } + foreach (var branchToRemove in branchesToRemove) + { + method.Value.Branches.Remove(branchToRemove); + } } } } diff --git a/src/coverlet.core/Instrumentation/Instrumenter.cs b/src/coverlet.core/Instrumentation/Instrumenter.cs index 05169c8dc..d08d8ba6e 100644 --- a/src/coverlet.core/Instrumentation/Instrumenter.cs +++ b/src/coverlet.core/Instrumentation/Instrumenter.cs @@ -335,7 +335,8 @@ private Instruction AddInstrumentationCode(MethodDefinition method, ILProcessor Offset = branchPoint.Offset, EndOffset = branchPoint.EndOffset, Path = branchPoint.Path, - Ordinal = branchPoint.Ordinal + Ordinal = branchPoint.Ordinal, + IsAsyncStateMachineBranch = IsAsyncStateMachineBranch(method.DeclaringType, method) } ); @@ -345,6 +346,18 @@ private Instruction AddInstrumentationCode(MethodDefinition method, ILProcessor return AddInstrumentationInstructions(method, processor, instruction, _result.HitCandidates.Count - 1); } + private bool IsAsyncStateMachineBranch(TypeDefinition typeDef, MethodDefinition method) + { + foreach (InterfaceImplementation implementedInterface in typeDef.Interfaces) + { + if (implementedInterface.InterfaceType.FullName == "System.Runtime.CompilerServices.IAsyncStateMachine" && method.FullName.EndsWith("::MoveNext()")) + { + return true; + } + } + return false; + } + private Instruction AddInstrumentationInstructions(MethodDefinition method, ILProcessor processor, Instruction instruction, int hitEntryIndex) { if (_customTrackerRecordHitMethod == null) diff --git a/src/coverlet.core/Instrumentation/InstrumenterResult.cs b/src/coverlet.core/Instrumentation/InstrumenterResult.cs index e1d264a79..5954d0f64 100644 --- a/src/coverlet.core/Instrumentation/InstrumenterResult.cs +++ b/src/coverlet.core/Instrumentation/InstrumenterResult.cs @@ -16,6 +16,7 @@ internal class Branch : Line public int EndOffset; public int Path; public uint Ordinal; + public bool IsAsyncStateMachineBranch; } internal class Document From cb743bc2bede3364df191d45eb71443bf70c17f8 Mon Sep 17 00:00:00 2001 From: Marco Rossignoli Date: Mon, 24 Dec 2018 19:14:48 +0100 Subject: [PATCH 3/7] new strategy --- src/coverlet.core/Coverage.cs | 18 +++++++++++++--- src/coverlet.core/CoverageResult.cs | 17 +++++++++++++-- .../Instrumentation/Instrumenter.cs | 21 +++++++++++++++++-- .../Instrumentation/InstrumenterResult.cs | 2 +- 4 files changed, 50 insertions(+), 8 deletions(-) diff --git a/src/coverlet.core/Coverage.cs b/src/coverlet.core/Coverage.cs index 23855036e..ff93f5a33 100644 --- a/src/coverlet.core/Coverage.cs +++ b/src/coverlet.core/Coverage.cs @@ -132,7 +132,7 @@ public CoverageResult GetCoverageResult() if (methods.TryGetValue(branch.Method, out Method method)) { method.Branches.Add(new BranchInfo - { Line = branch.Number, Hits = branch.Hits, Offset = branch.Offset, EndOffset = branch.EndOffset, Path = branch.Path, Ordinal = branch.Ordinal, IsAsyncStateMachineBranch = branch.IsAsyncStateMachineBranch } + { Line = branch.Number, Hits = branch.Hits, Offset = branch.Offset, EndOffset = branch.EndOffset, Path = branch.Path, Ordinal = branch.Ordinal } ); } else @@ -168,7 +168,7 @@ public CoverageResult GetCoverageResult() InstrumentationHelper.RestoreOriginalModule(result.ModulePath, _identifier); } - var coverageResult = new CoverageResult { Identifier = _identifier, Modules = modules }; + var coverageResult = new CoverageResult { Identifier = _identifier, Modules = modules, InstrumentedResults = _results }; if (!string.IsNullOrEmpty(_mergeWith) && !string.IsNullOrWhiteSpace(_mergeWith) && File.Exists(_mergeWith)) { @@ -239,7 +239,7 @@ private void CalculateCoverage() foreach (var branch in document.Value.Branches) { //if one branch is covered we search the other one only if it's not covered - if (branch.Value.IsAsyncStateMachineBranch && branch.Value.Method.EndsWith("::MoveNext()") && branch.Value.Hits > 0) + if (IsAsyncStateMachineMethod(branch.Value.Method) && branch.Value.Hits > 0) { foreach (var moveNextBranch in document.Value.Branches) { @@ -260,6 +260,18 @@ private void CalculateCoverage() } } + private bool IsAsyncStateMachineMethod(string method) + { + foreach (var instrumentationResult in _results) + { + if (instrumentationResult.AsyncMachineStateMethod.Contains(method)) + { + return true; + } + } + return false; + } + private string GetSourceLinkUrl(Dictionary sourceLinkDocuments, string document) { if (sourceLinkDocuments.TryGetValue(document, out string url)) diff --git a/src/coverlet.core/CoverageResult.cs b/src/coverlet.core/CoverageResult.cs index 5ffde6b27..4b532af75 100644 --- a/src/coverlet.core/CoverageResult.cs +++ b/src/coverlet.core/CoverageResult.cs @@ -3,6 +3,7 @@ using System.IO; using System.Linq; using Coverlet.Core.Enums; +using Coverlet.Core.Instrumentation; using Coverlet.Core.Symbols; namespace Coverlet.Core @@ -15,7 +16,6 @@ public class BranchInfo public int Path { get; set; } public uint Ordinal { get; set; } public int Hits { get; set; } - public bool IsAsyncStateMachineBranch { get; set; } } public class Lines : SortedDictionary { } @@ -41,6 +41,7 @@ public class CoverageResult { public string Identifier; public Modules Modules; + internal List InstrumentedResults; internal CoverageResult() { } @@ -121,7 +122,7 @@ internal void Merge(Modules modules) { foreach (var branch in method.Value.Branches) { - if (branch.IsAsyncStateMachineBranch && method.Key.EndsWith("::MoveNext()") && branch.Hits > 0) + if (IsAsyncStateMachineMethod(method.Key)) { foreach (var moveNextBranch in method.Value.Branches) { @@ -142,6 +143,18 @@ internal void Merge(Modules modules) } } + private bool IsAsyncStateMachineMethod(string method) + { + foreach (var instrumentedResult in InstrumentedResults) + { + if (instrumentedResult.AsyncMachineStateMethod.Contains(method)) + { + return true; + } + } + return false; + } + public ThresholdTypeFlags GetThresholdTypesBelowThreshold(CoverageSummary summary, double threshold, ThresholdTypeFlags thresholdTypes, ThresholdStatistic thresholdStat) { var thresholdTypeFlags = ThresholdTypeFlags.None; diff --git a/src/coverlet.core/Instrumentation/Instrumenter.cs b/src/coverlet.core/Instrumentation/Instrumenter.cs index d08d8ba6e..128f4b9f1 100644 --- a/src/coverlet.core/Instrumentation/Instrumenter.cs +++ b/src/coverlet.core/Instrumentation/Instrumenter.cs @@ -30,6 +30,7 @@ internal class Instrumenter private ILProcessor _customTrackerClassConstructorIl; private TypeDefinition _customTrackerTypeDef; private MethodReference _customTrackerRecordHitMethod; + private List _asyncMachineStateMethod; public Instrumenter(string module, string identifier, string[] excludeFilters, string[] includeFilters, string[] excludedFiles, string[] excludedAttributes) { @@ -59,6 +60,8 @@ public InstrumenterResult Instrument() InstrumentModule(); + _result.AsyncMachineStateMethod = _asyncMachineStateMethod == null ? Array.Empty() : _asyncMachineStateMethod.ToArray(); + return _result; } @@ -326,6 +329,7 @@ private Instruction AddInstrumentationCode(MethodDefinition method, ILProcessor var key = (branchPoint.StartLine, (int)branchPoint.Ordinal); if (!document.Branches.ContainsKey(key)) + { document.Branches.Add(key, new Branch { @@ -335,11 +339,24 @@ private Instruction AddInstrumentationCode(MethodDefinition method, ILProcessor Offset = branchPoint.Offset, EndOffset = branchPoint.EndOffset, Path = branchPoint.Path, - Ordinal = branchPoint.Ordinal, - IsAsyncStateMachineBranch = IsAsyncStateMachineBranch(method.DeclaringType, method) + Ordinal = branchPoint.Ordinal } ); + if (IsAsyncStateMachineBranch(method.DeclaringType, method)) + { + if (_asyncMachineStateMethod == null) + { + _asyncMachineStateMethod = new List(); + } + + if (!_asyncMachineStateMethod.Contains(method.FullName)) + { + _asyncMachineStateMethod.Add(method.FullName); + } + } + } + var entry = (true, document.Index, branchPoint.StartLine, (int)branchPoint.Ordinal); _result.HitCandidates.Add(entry); diff --git a/src/coverlet.core/Instrumentation/InstrumenterResult.cs b/src/coverlet.core/Instrumentation/InstrumenterResult.cs index 5954d0f64..5c3d374e2 100644 --- a/src/coverlet.core/Instrumentation/InstrumenterResult.cs +++ b/src/coverlet.core/Instrumentation/InstrumenterResult.cs @@ -16,7 +16,6 @@ internal class Branch : Line public int EndOffset; public int Path; public uint Ordinal; - public bool IsAsyncStateMachineBranch; } internal class Document @@ -42,6 +41,7 @@ public InstrumenterResult() } public string Module; + public string[] AsyncMachineStateMethod; public string HitsFilePath; public string ModulePath; public string SourceLink; From 10ac5c4c87e012c4412cde1fb045b51aac7d7354 Mon Sep 17 00:00:00 2001 From: Marco Rossignoli Date: Mon, 24 Dec 2018 21:29:37 +0100 Subject: [PATCH 4/7] perf improvment --- src/coverlet.core/Coverage.cs | 5 +++++ src/coverlet.core/CoverageResult.cs | 5 +++++ src/coverlet.core/Instrumentation/Instrumenter.cs | 7 ++++++- 3 files changed, 16 insertions(+), 1 deletion(-) diff --git a/src/coverlet.core/Coverage.cs b/src/coverlet.core/Coverage.cs index ff93f5a33..cd1cf6af1 100644 --- a/src/coverlet.core/Coverage.cs +++ b/src/coverlet.core/Coverage.cs @@ -262,6 +262,11 @@ private void CalculateCoverage() private bool IsAsyncStateMachineMethod(string method) { + if (!method.EndsWith("::MoveNext()")) + { + return false; + } + foreach (var instrumentationResult in _results) { if (instrumentationResult.AsyncMachineStateMethod.Contains(method)) diff --git a/src/coverlet.core/CoverageResult.cs b/src/coverlet.core/CoverageResult.cs index 4b532af75..1683fb436 100644 --- a/src/coverlet.core/CoverageResult.cs +++ b/src/coverlet.core/CoverageResult.cs @@ -145,6 +145,11 @@ internal void Merge(Modules modules) private bool IsAsyncStateMachineMethod(string method) { + if (!method.EndsWith("::MoveNext()")) + { + return false; + } + foreach (var instrumentedResult in InstrumentedResults) { if (instrumentedResult.AsyncMachineStateMethod.Contains(method)) diff --git a/src/coverlet.core/Instrumentation/Instrumenter.cs b/src/coverlet.core/Instrumentation/Instrumenter.cs index 128f4b9f1..f89104f8a 100644 --- a/src/coverlet.core/Instrumentation/Instrumenter.cs +++ b/src/coverlet.core/Instrumentation/Instrumenter.cs @@ -365,9 +365,14 @@ private Instruction AddInstrumentationCode(MethodDefinition method, ILProcessor private bool IsAsyncStateMachineBranch(TypeDefinition typeDef, MethodDefinition method) { + if (!method.FullName.EndsWith("::MoveNext()")) + { + return false; + } + foreach (InterfaceImplementation implementedInterface in typeDef.Interfaces) { - if (implementedInterface.InterfaceType.FullName == "System.Runtime.CompilerServices.IAsyncStateMachine" && method.FullName.EndsWith("::MoveNext()")) + if (implementedInterface.InterfaceType.FullName == "System.Runtime.CompilerServices.IAsyncStateMachine") { return true; } From 2a5a3b8373e0cf349a941d119ef64a5e9708face Mon Sep 17 00:00:00 2001 From: Marco Rossignoli Date: Tue, 1 Jan 2019 16:48:49 +0100 Subject: [PATCH 5/7] address PR feedback --- src/coverlet.core/CoverageResult.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/coverlet.core/CoverageResult.cs b/src/coverlet.core/CoverageResult.cs index 1683fb436..b6c02eade 100644 --- a/src/coverlet.core/CoverageResult.cs +++ b/src/coverlet.core/CoverageResult.cs @@ -122,7 +122,8 @@ internal void Merge(Modules modules) { foreach (var branch in method.Value.Branches) { - if (IsAsyncStateMachineMethod(method.Key)) + //if one branch is covered we search the other one only if it's not covered + if (IsAsyncStateMachineMethod(method.Key) && branch.Hits > 0) { foreach (var moveNextBranch in method.Value.Branches) { From e39eca580e46dd77934520504fe28b2824619f1e Mon Sep 17 00:00:00 2001 From: Marco Rossignoli Date: Tue, 1 Jan 2019 17:14:51 +0100 Subject: [PATCH 6/7] rollback past changes, we don't need anymore IsMoveNext() exposed --- src/coverlet.core/Symbols/CecilSymbolHelper.cs | 5 ----- 1 file changed, 5 deletions(-) diff --git a/src/coverlet.core/Symbols/CecilSymbolHelper.cs b/src/coverlet.core/Symbols/CecilSymbolHelper.cs index 66ae83aaa..e4c9ca3ae 100644 --- a/src/coverlet.core/Symbols/CecilSymbolHelper.cs +++ b/src/coverlet.core/Symbols/CecilSymbolHelper.cs @@ -20,11 +20,6 @@ public static class CecilSymbolHelper private const int StepOverLineCode = 0xFEEFEE; private static readonly Regex IsMovenext = new Regex(@"\<[^\s>]+\>\w__\w(\w)?::MoveNext\(\)$", RegexOptions.Compiled | RegexOptions.ExplicitCapture); - public static bool IsMoveNext(string fullName) - { - return IsMovenext.IsMatch(fullName); - } - public static List GetBranchPoints(MethodDefinition methodDefinition) { var list = new List(); From e63b3f4bd1e59df44eb42d72b1a034957d04a41d Mon Sep 17 00:00:00 2001 From: Marco Rossignoli Date: Wed, 16 Jan 2019 22:51:22 +0100 Subject: [PATCH 7/7] remove IsMoveNext() --- src/coverlet.core/Symbols/CecilSymbolHelper.cs | 5 ----- 1 file changed, 5 deletions(-) diff --git a/src/coverlet.core/Symbols/CecilSymbolHelper.cs b/src/coverlet.core/Symbols/CecilSymbolHelper.cs index d8774d862..e4c9ca3ae 100644 --- a/src/coverlet.core/Symbols/CecilSymbolHelper.cs +++ b/src/coverlet.core/Symbols/CecilSymbolHelper.cs @@ -20,11 +20,6 @@ public static class CecilSymbolHelper private const int StepOverLineCode = 0xFEEFEE; private static readonly Regex IsMovenext = new Regex(@"\<[^\s>]+\>\w__\w(\w)?::MoveNext\(\)$", RegexOptions.Compiled | RegexOptions.ExplicitCapture); - public static bool IsMoveNext(string fullName) - { - return IsMovenext.IsMatch(fullName); - } - public static List GetBranchPoints(MethodDefinition methodDefinition) { var list = new List();