Merge pull request #341 from derhally/master

Added logging to Coverage to help with debugging
coverlet-coverage · Feb 27, 2019 · f258333 · f258333
2 parents 60c4f9e + 5d0670e
commit f258333
Show file tree

Hide file tree

Showing 8 changed files with 110 additions and 46 deletions.
diff --git a/src/coverlet.console/Logging/ConsoleLogger.cs b/src/coverlet.console/Logging/ConsoleLogger.cs
@@ -1,39 +1,39 @@
 using System;
+using Coverlet.Core.Logging;
 using static System.Console;
 
 namespace Coverlet.Console.Logging
 {
     class ConsoleLogger : ILogger
     {
-        public void LogError(string message)
-        {
-            ForegroundColor = ConsoleColor.Red;
-            WriteLine(message);
-            ForegroundColor = ConsoleColor.White;
-        }
+        private static readonly object _sync = new object();
 
-        public void LogInformation(string message)
-        {
-            WriteLine(message);
-        }
+        public void LogError(string message) => Log(message, ConsoleColor.Red);
 
-        public void LogSuccess(string message)
-        {
-            ForegroundColor = ConsoleColor.Green;
-            WriteLine(message);
-            ForegroundColor = ConsoleColor.White;
-        }
+        public void LogError(Exception exception) => LogError(exception.ToString());
 
-        public void LogVerbose(string message)
-        {
-            throw new System.NotImplementedException();
-        }
+        public void LogInformation(string message) => Log(message, ForegroundColor);
+
+        public void LogVerbose(string message) => throw new NotImplementedException();
+
+        public void LogWarning(string message) => Log(message, ConsoleColor.Yellow);
 
-        public void LogWarning(string message)
+        private void Log(string message, ConsoleColor color)
         {
-            ForegroundColor = ConsoleColor.Yellow;
-            WriteLine(message);
-            ForegroundColor = ConsoleColor.White;
+            lock (_sync)
+            {
+                ConsoleColor currentForegroundColor;
+                if (color != (currentForegroundColor = ForegroundColor))
+                {
+                    ForegroundColor = color;
+                    WriteLine(message);
+                    ForegroundColor = currentForegroundColor;
+                }
+                else
+                {
+                    WriteLine(message);
+                }
+            }
         }
     }
 }
diff --git a/src/coverlet.console/Program.cs b/src/coverlet.console/Program.cs
@@ -3,6 +3,7 @@
 using System.Diagnostics;
 using System.IO;
 using System.Text;
+using System.Threading.Tasks;
 using ConsoleTables;
 using Coverlet.Console.Logging;
 using Coverlet.Core;
@@ -49,7 +50,7 @@ static int Main(string[] args)
                 if (!target.HasValue())
                     throw new CommandParsingException(app, "Target must be specified.");
 
-                Coverage coverage = new Coverage(module.Value, includeFilters.Values.ToArray(), includeDirectories.Values.ToArray(), excludeFilters.Values.ToArray(), excludedSourceFiles.Values.ToArray(), excludeAttributes.Values.ToArray(), singleHit.HasValue(), mergeWith.Value(), useSourceLink.HasValue());
+                Coverage coverage = new Coverage(module.Value, includeFilters.Values.ToArray(), includeDirectories.Values.ToArray(), excludeFilters.Values.ToArray(), excludedSourceFiles.Values.ToArray(), excludeAttributes.Values.ToArray(), singleHit.HasValue(), mergeWith.Value(), useSourceLink.HasValue(), logger);
                 coverage.PrepareModules();
 
                 Process process = new Process();
@@ -58,9 +59,23 @@ static int Main(string[] args)
                 process.StartInfo.CreateNoWindow = true;
                 process.StartInfo.RedirectStandardOutput = true;
                 process.StartInfo.RedirectStandardError = true;
+                process.OutputDataReceived += (sender, eventArgs) =>
+                {
+                    if(!string.IsNullOrEmpty(eventArgs.Data))
+                        logger.LogInformation(eventArgs.Data);
+                };
+
+                process.ErrorDataReceived += (sender, eventArgs) =>
+                {
+                    if (!string.IsNullOrEmpty(eventArgs.Data))
+                        logger.LogError(eventArgs.Data);
+                };
+
                 process.Start();
-                logger.LogInformation(process.StandardOutput.ReadToEnd());
-                logger.LogError(process.StandardError.ReadToEnd());
+
+                process.BeginErrorReadLine();
+                process.BeginOutputReadLine();
+
                 process.WaitForExit();
 
                 var dOutput = output.HasValue() ? output.Value() : Directory.GetCurrentDirectory() + Path.DirectorySeparatorChar.ToString();

diff --git a/src/coverlet.core/Coverage.cs b/src/coverlet.core/Coverage.cs
@@ -3,10 +3,9 @@
 using System.IO;
 using System.Linq;
 
-using Coverlet.Core.Enums;
 using Coverlet.Core.Helpers;
 using Coverlet.Core.Instrumentation;
-using Coverlet.Core.Symbols;
+using Coverlet.Core.Logging;
 
 using Newtonsoft.Json;
 using Newtonsoft.Json.Linq;
@@ -25,14 +24,24 @@ public class Coverage
         private bool _singleHit;
         private string _mergeWith;
         private bool _useSourceLink;
+        private ILogger _logger;
         private List<InstrumenterResult> _results;
 
         public string Identifier
         {
             get { return _identifier; }
         }
 
-        public Coverage(string module, string[] includeFilters, string[] includeDirectories, string[] excludeFilters, string[] excludedSourceFiles, string[] excludeAttributes, bool singleHit, string mergeWith, bool useSourceLink)
+        public Coverage(string module, 
+            string[] includeFilters, 
+            string[] includeDirectories, 
+            string[] excludeFilters, 
+            string[] excludedSourceFiles, 
+            string[] excludeAttributes, 
+            bool singleHit, 
+            string mergeWith, 
+            bool useSourceLink,
+            ILogger logger)
         {
             _module = module;
             _includeFilters = includeFilters;
@@ -43,6 +52,7 @@ public Coverage(string module, string[] includeFilters, string[] includeDirector
             _singleHit = singleHit;
             _mergeWith = mergeWith;
             _useSourceLink = useSourceLink;
+            _logger = logger;
 
             _identifier = Guid.NewGuid().ToString();
             _results = new List<InstrumenterResult>();
@@ -72,9 +82,9 @@ public void PrepareModules()
                         var result = instrumenter.Instrument();
                         _results.Add(result);
                     }
-                    catch (Exception)
+                    catch (Exception ex)
                     {
-                        // TODO: With verbose logging we should note that instrumentation failed.
+                        _logger.LogWarning($"Unable to instrument module: {module} because : {ex.Message}");
                         InstrumentationHelper.RestoreOriginalModule(module, _identifier);
                     }
                 }

diff --git a/src/coverlet.console/Logging/ILogger.cs → src/coverlet.core/Logging/ILogger.cs b/src/coverlet.console/Logging/ILogger.cs → src/coverlet.core/Logging/ILogger.cs
@@ -1,11 +1,13 @@
-namespace Coverlet.Console.Logging
+using System;
+
+namespace Coverlet.Core.Logging
 {
-    interface ILogger
+    public interface ILogger
     {
-        void LogSuccess(string message);
         void LogVerbose(string message);
         void LogInformation(string message);
         void LogWarning(string message);
         void LogError(string message);
+        void LogError(Exception exception);
     }
 }
diff --git a/src/coverlet.msbuild.tasks/CoverageResultTask.cs b/src/coverlet.msbuild.tasks/CoverageResultTask.cs
@@ -18,6 +18,7 @@ public class CoverageResultTask : Task
         private double _threshold;
         private string _thresholdType;
         private string _thresholdStat;
+        private MSBuildLogger _logger;
 
         [Required]
         public string Output
@@ -54,6 +55,11 @@ public string ThresholdStat
             set { _thresholdStat = value; }
         }
 
+        public CoverageResultTask()
+        {
+            _logger = new MSBuildLogger(Log);
+        }
+
         public override bool Execute()
         {
             try
@@ -103,7 +109,7 @@ public override bool Execute()
 
                 var thresholdTypeFlags = ThresholdTypeFlags.None;
                 var thresholdStat = ThresholdStatistic.Minimum;
-                
+
                 foreach (var thresholdType in _thresholdType.Split(',').Select(t => t.Trim()))
                 {
                     if (thresholdType.Equals("line", StringComparison.OrdinalIgnoreCase))
@@ -132,7 +138,7 @@ public override bool Execute()
                 var coverageTable = new ConsoleTable("Module", "Line", "Branch", "Method");
                 var summary = new CoverageSummary();
                 int numModules = result.Modules.Count;
-                
+
                 var totalLinePercent = summary.CalculateLineCoverage(result.Modules).Percent * 100;
                 var totalBranchPercent = summary.CalculateBranchCoverage(result.Modules).Percent * 100;
                 var totalMethodPercent = summary.CalculateMethodCoverage(result.Modules).Percent * 100;
@@ -152,10 +158,10 @@ public override bool Execute()
                 coverageTable.Columns.Clear();
                 coverageTable.Rows.Clear();
 
-                coverageTable.AddColumn(new [] { "", "Line", "Branch", "Method"});
+                coverageTable.AddColumn(new[] { "", "Line", "Branch", "Method" });
                 coverageTable.AddRow("Total", $"{totalLinePercent}%", $"{totalBranchPercent}%", $"{totalMethodPercent}%");
                 coverageTable.AddRow("Average", $"{totalLinePercent / numModules}%", $"{totalBranchPercent / numModules}%", $"{totalMethodPercent / numModules}%");
-                
+
                 Console.WriteLine(coverageTable.ToStringAlternative());
 
                 thresholdTypeFlags = result.GetThresholdTypesBelowThreshold(summary, _threshold, thresholdTypeFlags, thresholdStat);
@@ -182,7 +188,7 @@ public override bool Execute()
             }
             catch (Exception ex)
             {
-                Log.LogErrorFromException(ex);
+                _logger.LogError(ex);
                 return false;
             }
 

diff --git a/src/coverlet.msbuild.tasks/InstrumentationTask.cs b/src/coverlet.msbuild.tasks/InstrumentationTask.cs
@@ -17,6 +17,7 @@ public class InstrumentationTask : Task
         private bool _singleHit;
         private string _mergeWith;
         private bool _useSourceLink;
+        private readonly MSBuildLogger _logger;
 
         internal static Coverage Coverage
         {
@@ -78,6 +79,11 @@ public bool UseSourceLink
             set { _useSourceLink = value; }
         }
 
+        public InstrumentationTask()
+        {
+            _logger = new MSBuildLogger(Log);
+        }
+
         public override bool Execute()
         {
             try
@@ -88,12 +94,12 @@ public override bool Execute()
                 var excludedSourceFiles = _excludeByFile?.Split(',');
                 var excludeAttributes = _excludeByAttribute?.Split(',');
 
-                _coverage = new Coverage(_path, includeFilters, includeDirectories, excludeFilters, excludedSourceFiles, excludeAttributes, _singleHit, _mergeWith, _useSourceLink);
+                _coverage = new Coverage(_path, includeFilters, includeDirectories, excludeFilters, excludedSourceFiles, excludeAttributes, _singleHit, _mergeWith, _useSourceLink, _logger);
                 _coverage.PrepareModules();
             }
             catch (Exception ex)
             {
-                Log.LogErrorFromException(ex);
+                _logger.LogError(ex);
                 return false;
             }
 

diff --git a/src/coverlet.msbuild.tasks/MSBuildLogger.cs b/src/coverlet.msbuild.tasks/MSBuildLogger.cs
@@ -0,0 +1,25 @@
+using System;
+using Microsoft.Build.Framework;
+using Microsoft.Build.Utilities;
+using ILogger = Coverlet.Core.Logging.ILogger;
+
+namespace Coverlet.MSbuild.Tasks
+{
+    class MSBuildLogger : ILogger
+    {
+        private readonly TaskLoggingHelper _log;
+
+        public MSBuildLogger(TaskLoggingHelper log) => _log = log;
+
+        public void LogVerbose(string message) => _log.LogMessage(MessageImportance.Low, message);
+
+        // We use `MessageImportance.High` because with `MessageImportance.Normal` doesn't show anything
+        public void LogInformation(string message)=> _log.LogMessage(MessageImportance.High, message);
+
+        public void LogWarning(string message) => _log.LogWarning(message);
+
+        public void LogError(string message) => _log.LogError(message);
+
+        public void LogError(Exception exception) => _log.LogErrorFromException(exception);
+    }
+}
diff --git a/test/coverlet.core.tests/CoverageTests.cs b/test/coverlet.core.tests/CoverageTests.cs
@@ -1,11 +1,9 @@
 using System;
 using System.IO;
-
+using Coverlet.Core.Logging;
 using Xunit;
 using Moq;
 
-using Coverlet.Core;
-using System.Collections.Generic;
 
 namespace Coverlet.Core.Tests
 {
@@ -22,9 +20,11 @@ public void TestCoverage()
             File.Copy(module, Path.Combine(directory.FullName, Path.GetFileName(module)), true);
             File.Copy(pdb, Path.Combine(directory.FullName, Path.GetFileName(pdb)), true);
 
+            var logger = Mock.Of<ILogger>();
+
             // TODO: Find a way to mimick hits
 
-            var coverage = new Coverage(Path.Combine(directory.FullName, Path.GetFileName(module)), Array.Empty<string>(), Array.Empty<string>(), Array.Empty<string>(), Array.Empty<string>(), Array.Empty<string>(), false, string.Empty, false);
+            var coverage = new Coverage(Path.Combine(directory.FullName, Path.GetFileName(module)), Array.Empty<string>(), Array.Empty<string>(), Array.Empty<string>(), Array.Empty<string>(), Array.Empty<string>(), false, string.Empty, false, logger);
             coverage.PrepareModules();
 
             var result = coverage.GetCoverageResult();