diff --git a/src/coverlet.core/Coverage.cs b/src/coverlet.core/Coverage.cs index 530b1081c..64c864429 100644 --- a/src/coverlet.core/Coverage.cs +++ b/src/coverlet.core/Coverage.cs @@ -217,6 +217,63 @@ public CoverageResult GetCoverageResult() _instrumentationHelper.RestoreOriginalModule(result.ModulePath, _identifier); } + // In case of anonymous delegate compiler generate a custom class and passes it as type.method delegate. + // If in delegate method we've a branches we need to move these to "actual" class/method that use it. + // We search "method" with same "Line" of closure class method and add missing branches to it, + // in this way we correctly report missing branch inside compiled generated anonymous delegate. + List compileGeneratedClassToRemove = null; + foreach (var module in 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 (BranchInCompilerGeneratedClass(method.Key)) + { + Method actualMethod = GetMethodWithSameLineInSameDocument(document.Value, @class.Key, branch.Line); + + if (actualMethod is null) + { + continue; + } + + actualMethod.Branches.Add(branch); + + if (compileGeneratedClassToRemove is null) + { + compileGeneratedClassToRemove = new List(); + } + + if (!compileGeneratedClassToRemove.Contains(@class.Key)) + { + compileGeneratedClassToRemove.Add(@class.Key); + } + } + } + } + } + } + } + + // After method/branches analysis of compiled generated class we can remove noise from reports + if (!(compileGeneratedClassToRemove is null)) + { + foreach (var module in modules) + { + foreach (var document in module.Value) + { + foreach (var classToRemove in compileGeneratedClassToRemove) + { + document.Value.Remove(classToRemove); + } + } + } + } + var coverageResult = new CoverageResult { Identifier = _identifier, Modules = modules, InstrumentedResults = _results }; if (!string.IsNullOrEmpty(_mergeWith) && !string.IsNullOrWhiteSpace(_mergeWith) && _fileSystem.Exists(_mergeWith)) @@ -228,6 +285,41 @@ public CoverageResult GetCoverageResult() return coverageResult; } + private bool BranchInCompilerGeneratedClass(string methodName) + { + foreach (var instrumentedResult in _results) + { + if (instrumentedResult.BranchesInCompiledGeneratedClass.Contains(methodName)) + { + return true; + } + } + return false; + } + + private Method GetMethodWithSameLineInSameDocument(Classes documentClasses, string compilerGeneratedClassName, int branchLine) + { + foreach (var @class in documentClasses) + { + if (@class.Key == compilerGeneratedClassName) + { + continue; + } + + foreach (var method in @class.Value) + { + foreach (var line in method.Value.Lines) + { + if (line.Key == branchLine) + { + return method.Value; + } + } + } + } + return null; + } + private void CalculateCoverage() { foreach (var result in _results) @@ -253,6 +345,8 @@ private void CalculateCoverage() } } + List<(int docIndex, int line)> zeroHitsLines = new List<(int docIndex, int line)>(); + var documentsList = result.Documents.Values.ToList(); using (var fs = _fileSystem.NewFileStream(result.HitsFilePath, FileMode.Open)) using (var br = new BinaryReader(fs)) { @@ -260,8 +354,6 @@ private void CalculateCoverage() // TODO: hitCandidatesCount should be verified against result.HitCandidates.Count - var documentsList = result.Documents.Values.ToList(); - for (int i = 0; i < hitCandidatesCount; ++i) { var hitLocation = result.HitCandidates[i]; @@ -279,10 +371,29 @@ private void CalculateCoverage() { var line = document.Lines[j]; line.Hits += hits; + + // We register 0 hit lines for later cleanup false positive of nested lambda closures + if (hits == 0) + { + zeroHitsLines.Add((hitLocation.docIndex, line.Number)); + } } } } } + + // Cleanup nested state machine false positive hits + foreach (var (docIndex, line) in zeroHitsLines) + { + foreach (var lineToCheck in documentsList[docIndex].Lines) + { + if (lineToCheck.Key == line) + { + lineToCheck.Value.Hits = 0; + } + } + } + _instrumentationHelper.DeleteHitsFile(result.HitsFilePath); _logger.LogVerbose($"Hit file '{result.HitsFilePath}' deleted"); } diff --git a/src/coverlet.core/Helpers/InstrumentationHelper.cs b/src/coverlet.core/Helpers/InstrumentationHelper.cs index 75cb4b019..e562cec02 100644 --- a/src/coverlet.core/Helpers/InstrumentationHelper.cs +++ b/src/coverlet.core/Helpers/InstrumentationHelper.cs @@ -198,7 +198,7 @@ public void BackupOriginalModule(string module, string identifier) } } - public void RestoreOriginalModule(string module, string identifier) + public virtual void RestoreOriginalModule(string module, string identifier) { var backupPath = GetBackupPath(module, identifier); var backupSymbolPath = Path.ChangeExtension(backupPath, ".pdb"); @@ -226,7 +226,7 @@ public void RestoreOriginalModule(string module, string identifier) }, retryStrategy, 10); } - public void RestoreOriginalModules() + public virtual void RestoreOriginalModules() { // Restore the original module - retry up to 10 times, since the destination file could be locked // See: https://github.com/tonerdo/coverlet/issues/25 diff --git a/src/coverlet.core/Instrumentation/Instrumenter.cs b/src/coverlet.core/Instrumentation/Instrumenter.cs index 9f8725a93..e9416da7a 100644 --- a/src/coverlet.core/Instrumentation/Instrumenter.cs +++ b/src/coverlet.core/Instrumentation/Instrumenter.cs @@ -4,6 +4,7 @@ using System.Diagnostics.CodeAnalysis; using System.IO; using System.Linq; +using System.Runtime.CompilerServices; using Coverlet.Core.Abstracts; using Coverlet.Core.Attributes; using Coverlet.Core.Logging; @@ -37,6 +38,7 @@ internal class Instrumenter private MethodReference _customTrackerRegisterUnloadEventsMethod; private MethodReference _customTrackerRecordHitMethod; private List _excludedSourceFiles; + private List _branchesInCompiledGeneratedClass; public Instrumenter( string module, @@ -130,6 +132,8 @@ public InstrumenterResult Instrument() } } + _result.BranchesInCompiledGeneratedClass = _branchesInCompiledGeneratedClass == null ? Array.Empty() : _branchesInCompiledGeneratedClass.ToArray(); + return _result; } @@ -454,7 +458,8 @@ private Instruction AddInstrumentationCode(MethodDefinition method, ILProcessor BranchKey key = new BranchKey(branchPoint.StartLine, (int)branchPoint.Ordinal); if (!document.Branches.ContainsKey(key)) { - document.Branches.Add(key, + document.Branches.Add( + key, new Branch { Number = branchPoint.StartLine, @@ -466,6 +471,19 @@ private Instruction AddInstrumentationCode(MethodDefinition method, ILProcessor Ordinal = branchPoint.Ordinal } ); + + if (method.DeclaringType.CustomAttributes.Any(x => x.AttributeType.FullName == typeof(CompilerGeneratedAttribute).FullName)) + { + if (_branchesInCompiledGeneratedClass == null) + { + _branchesInCompiledGeneratedClass = new List(); + } + + if (!_branchesInCompiledGeneratedClass.Contains(method.FullName)) + { + _branchesInCompiledGeneratedClass.Add(method.FullName); + } + } } _result.HitCandidates.Add(new HitCandidate(true, document.Index, branchPoint.StartLine, (int)branchPoint.Ordinal)); diff --git a/src/coverlet.core/Instrumentation/InstrumenterResult.cs b/src/coverlet.core/Instrumentation/InstrumenterResult.cs index 6c7823e32..1ac9c60a3 100644 --- a/src/coverlet.core/Instrumentation/InstrumenterResult.cs +++ b/src/coverlet.core/Instrumentation/InstrumenterResult.cs @@ -5,6 +5,7 @@ namespace Coverlet.Core.Instrumentation { + [DebuggerDisplay("Number = {Number} Hits = {Hits} Class = {Class} Method = {Method}")] [DataContract] public class Line { @@ -73,6 +74,7 @@ public Document() public Dictionary Branches { get; private set; } } + [DebuggerDisplay("isBranch = {isBranch} docIndex = {docIndex} start = {start} end = {end}")] [DataContract] public class HitCandidate { @@ -100,6 +102,8 @@ public InstrumenterResult() [DataMember] public string Module; [DataMember] + public string[] BranchesInCompiledGeneratedClass; + [DataMember] public string HitsFilePath; [DataMember] public string ModulePath; diff --git a/test/coverlet.core.tests/CoverageTests.cs b/test/coverlet.core.tests/CoverageTests.cs index 240d1fc88..03c0db900 100644 --- a/test/coverlet.core.tests/CoverageTests.cs +++ b/test/coverlet.core.tests/CoverageTests.cs @@ -92,15 +92,15 @@ public void SelectionStatements_If() // Similar to msbuild coverage result task CoverageResult result = TestInstrumentationHelper.GetCoverageResult(path); + // Generate html report to check + // TestInstrumentationHelper.GenerateHtmlReport(result); + // Asserts on doc/lines/branches result.Document("Instrumentation.SelectionStatements.cs") // (line, hits) .AssertLinesCovered((11, 1), (15, 0)) // (line,ordinal,hits) .AssertBranchesCovered((9, 0, 1), (9, 1, 0)); - - // if need to generate html report for debugging purpose - // TestInstrumentationHelper.GenerateHtmlReport(result); } finally { @@ -163,6 +163,7 @@ public void AsyncAwait() }, path).Dispose(); CoverageResult result = TestInstrumentationHelper.GetCoverageResult(path); + result.Document("Instrumentation.AsyncAwait.cs") .AssertLinesCovered(BuildConfiguration.Debug, // AsyncExecution(bool) @@ -192,5 +193,51 @@ public void AsyncAwait() File.Delete(path); } } + + [Fact] + public void Lambda_Issue343() + { + string path = Path.GetTempFileName(); + try + { + RemoteExecutor.Invoke(async pathSerialize => + { + CoveragePrepareResult coveragePrepareResult = await TestInstrumentationHelper.Run(instance => + { + instance.InvokeAnonymous_Test(); + ((Task)instance.InvokeAnonymousAsync_Test()).ConfigureAwait(false).GetAwaiter().GetResult(); + return Task.CompletedTask; + }, pathSerialize); + return 0; + }, path).Dispose(); + + CoverageResult result = TestInstrumentationHelper.GetCoverageResult(path); + + result.Document("Instrumentation.Lambda.cs") + .AssertLinesCoveredAllBut(BuildConfiguration.Debug, 23, 51) + .AssertBranchesCovered(BuildConfiguration.Debug, + // Expected branches + (22, 0, 0), + (22, 1, 1), + (50, 2, 0), + (50, 3, 1), + // Unexpected branches + (20, 0, 1), + (20, 1, 1), + (49, 0, 1), + (49, 1, 0), + (54, 4, 0), + (54, 5, 1), + (39, 0, 1), + (39, 1, 0), + (48, 0, 1), + (48, 1, 1) + ); + } + finally + { + File.Delete(path); + } + } } } \ No newline at end of file diff --git a/test/coverlet.core.tests/InstrumenterHelper.cs b/test/coverlet.core.tests/InstrumenterHelper.cs index 46e366095..23dae414d 100644 --- a/test/coverlet.core.tests/InstrumenterHelper.cs +++ b/test/coverlet.core.tests/InstrumenterHelper.cs @@ -4,6 +4,7 @@ using System.Linq; using System.Reflection; using System.Runtime.CompilerServices; +using System.Text; using System.Threading; using System.Threading.Tasks; @@ -78,6 +79,21 @@ public static Document ExpectedTotalNumberOfBranches(this Document document, Bui return document; } + public static string ToStringBranches(this Document document) + { + if (document is null) + { + throw new ArgumentNullException(nameof(document)); + } + + StringBuilder builder = new StringBuilder(); + foreach (KeyValuePair branch in document.Branches) + { + builder.AppendLine($"({branch.Value.Number}, {branch.Value.Ordinal}, {branch.Value.Hits}),"); + } + return builder.ToString(); + } + public static Document AssertBranchesCovered(this Document document, BuildConfiguration configuration, params (int line, int ordinal, int hits)[] lines) { if (document is null) @@ -125,6 +141,47 @@ public static Document AssertLinesCovered(this Document document, params (int li return AssertLinesCovered(document, BuildConfiguration.Debug | BuildConfiguration.Release, lines); } + public static Document AssertLinesCoveredAllBut(this Document document, BuildConfiguration configuration, params int[] linesNumber) + { + if (document is null) + { + throw new ArgumentNullException(nameof(document)); + } + + BuildConfiguration buildConfiguration = GetAssemblyBuildConfiguration(); + + if ((buildConfiguration & configuration) != buildConfiguration) + { + return document; + } + + foreach (KeyValuePair line in document.Lines) + { + bool skip = false; + foreach (int number in linesNumber) + { + if (line.Value.Number == number) + { + skip = true; + if (line.Value.Hits > 0) + { + throw new XunitException($"Hits not expected for line {line.Value.Number}"); + } + } + } + + if (skip) + continue; + + if (line.Value.Hits == 0) + { + throw new XunitException($"Hits expected for line: {line.Value.Number}"); + } + } + + return document; + } + public static Document AssertLinesCovered(this Document document, BuildConfiguration configuration, params (int line, int hits)[] lines) { if (document is null) @@ -189,11 +246,14 @@ public static class TestInstrumentationHelper /// public static void GenerateHtmlReport(CoverageResult coverageResult, IReporter reporter = null, string sourceFileFilter = "", [CallerMemberName]string directory = "") { + JsonReporter defaultReporter = new JsonReporter(); reporter ??= new CoberturaReporter(); DirectoryInfo dir = Directory.CreateDirectory(directory); dir.Delete(true); dir.Create(); - string reportFile = Path.Combine(dir.FullName, Path.ChangeExtension("report", reporter.Extension)); + string reportFile = Path.Combine(dir.FullName, Path.ChangeExtension("report", defaultReporter.Extension)); + File.WriteAllText(reportFile, defaultReporter.Report(coverageResult)); + reportFile = Path.Combine(dir.FullName, Path.ChangeExtension("report", reporter.Extension)); File.WriteAllText(reportFile, reporter.Report(coverageResult)); // i.e. reportgenerator -reports:"C:\git\coverlet\test\coverlet.core.tests\bin\Debug\netcoreapp2.0\Condition_If\report.cobertura.xml" -targetdir:"C:\git\coverlet\test\coverlet.core.tests\bin\Debug\netcoreapp2.0\Condition_If" -filefilters:+**\Samples\Instrumentation.cs new Generator().GenerateReport(new ReportConfiguration( @@ -215,20 +275,29 @@ public static CoverageResult GetCoverageResult(string filePath) using (var result = new FileStream(filePath, FileMode.Open)) { CoveragePrepareResult coveragePrepareResultLoaded = CoveragePrepareResult.Deserialize(result); - Coverage coverage = new Coverage(coveragePrepareResultLoaded, new Mock().Object, new InstrumentationHelper(new ProcessExitHandler(), new RetryHelper(), new FileSystem()), new FileSystem()); + Coverage coverage = new Coverage(coveragePrepareResultLoaded, new Mock().Object, DependencyInjection.Current.GetService(), new FileSystem()); return coverage.GetCoverageResult(); } } - async public static Task Run(Func callMethod, string persistPrepareResultToFile) + async public static Task Run(Func callMethod, string persistPrepareResultToFile, bool disableRestoreModules = false) { + var serviceCollection = new ServiceCollection(); + serviceCollection.AddTransient(); + serviceCollection.AddTransient(); + serviceCollection.AddTransient(); + if (disableRestoreModules) + { + serviceCollection.AddSingleton(); + } + else + { + serviceCollection.AddSingleton(); + } + // Setup correct retry helper to avoid exception in InstrumentationHelper.RestoreOriginalModules on remote process exit - DependencyInjection.Set(new ServiceCollection() - .AddTransient() - .AddTransient() - .AddTransient() - .AddSingleton() - .BuildServiceProvider()); + DependencyInjection.Set(serviceCollection.BuildServiceProvider()); + // Rename test file to avoid locks string location = typeof(T).Assembly.Location; @@ -243,7 +312,7 @@ async public static Task Run(Func callM Coverage coverage = new Coverage(newPath, new string[] { - $"[{Path.GetFileNameWithoutExtension(fileName)}*]{typeof(T).FullName}" + $"[{Path.GetFileNameWithoutExtension(fileName)}*]{typeof(T).FullName}*" }, Array.Empty(), new string[] @@ -363,4 +432,23 @@ public void LogWarning(string message) File.AppendAllText(_logFile, message + Environment.NewLine); } } + + class InstrumentationHelperForDebugging : InstrumentationHelper + { + public InstrumentationHelperForDebugging(IProcessExitHandler processExitHandler, IRetryHelper retryHelper, IFileSystem fileSystem) + : base(processExitHandler, retryHelper, fileSystem) + { + + } + + public override void RestoreOriginalModule(string module, string identifier) + { + // DO NOT RESTORE + } + + public override void RestoreOriginalModules() + { + // DO NOT RESTORE + } + } } diff --git a/test/coverlet.core.tests/Samples/Instrumentation.Lambda.cs b/test/coverlet.core.tests/Samples/Instrumentation.Lambda.cs new file mode 100644 index 000000000..010081bd0 --- /dev/null +++ b/test/coverlet.core.tests/Samples/Instrumentation.Lambda.cs @@ -0,0 +1,64 @@ +// Remember to use full name because adding new using directives change line numbers + +using System.Threading.Tasks; + +namespace Coverlet.Core.Samples.Tests +{ + public class Lambda_Issue343 + { + protected T WriteToStream(System.Func getResultFunction) + { + using (var stream = new System.IO.MemoryStream()) + { + var result = getResultFunction(stream, false); + return result; + } + } + + public bool InvokeAnonymous() + { + return WriteToStream((stream, condition) => + { + if (condition) + stream.WriteByte(1); + else + stream.WriteByte(0); + return condition; + } + ); + } + + public bool InvokeAnonymous_Test() + { + Lambda_Issue343 demoClass = new Lambda_Issue343(); + return demoClass.InvokeAnonymous(); + } + + protected async Task WriteToStreamAsync(System.Func> getResultFunction) + { + using (var stream = new System.IO.MemoryStream()) + { + var result = await getResultFunction(stream, false); + return result; + } + } + + async public Task InvokeAnonymousAsync() + { + return await WriteToStreamAsync(async (stream, condition) => + { + if (condition) + stream.WriteByte(1); + else + stream.WriteByte(0); + return await Task.FromResult(condition); + }); + } + + async public Task InvokeAnonymousAsync_Test() + { + Lambda_Issue343 demoClass = new Lambda_Issue343(); + return await demoClass.InvokeAnonymousAsync(); + } + } +}