Skip to content

Inject InstrumentationHelper #531

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 7 commits into from
Sep 10, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion src/coverlet.collector/DataCollection/CoverageWrapper.cs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
using Coverlet.Collector.Utilities.Interfaces;
using Coverlet.Core;
using Coverlet.Core.Abstracts;
using Coverlet.Core.Logging;

namespace Coverlet.Collector.DataCollection
Expand Down Expand Up @@ -28,7 +29,8 @@ public Coverage CreateCoverage(CoverletSettings settings, ILogger coverletLogger
settings.SingleHit,
settings.MergeWith,
settings.UseSourceLink,
coverletLogger);
coverletLogger,
(IInstrumentationHelper)DependencyInjection.Current.GetService(typeof(IInstrumentationHelper)));
}

/// <summary>
Expand Down
4 changes: 3 additions & 1 deletion src/coverlet.console/Program.cs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
using ConsoleTables;
using Coverlet.Console.Logging;
using Coverlet.Core;
using Coverlet.Core.Abstracts;
using Coverlet.Core.Enums;
using Coverlet.Core.Reporters;
using McMaster.Extensions.CommandLineUtils;
Expand Down Expand Up @@ -69,7 +70,8 @@ static int Main(string[] args)
singleHit.HasValue(),
mergeWith.Value(),
useSourceLink.HasValue(),
logger);
logger,
(IInstrumentationHelper)DependencyInjection.Current.GetService(typeof(IInstrumentationHelper)));
coverage.PrepareModules();

Process process = new Process();
Expand Down
18 changes: 18 additions & 0 deletions src/coverlet.core/Abstracts/InstrumentationHelper.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
namespace Coverlet.Core.Abstracts
{
public interface IInstrumentationHelper
{
void BackupOriginalModule(string module, string identifier);
void DeleteHitsFile(string path);
string[] GetCoverableModules(string module, string[] directories, bool includeTestAssembly);
bool HasPdb(string module, out bool embedded);
bool IsModuleExcluded(string module, string[] excludeFilters);
bool IsModuleIncluded(string module, string[] includeFilters);
bool IsValidFilterExpression(string filter);
bool IsTypeExcluded(string module, string type, string[] excludeFilters);
bool IsTypeIncluded(string module, string type, string[] includeFilters);
void RestoreOriginalModule(string module, string identifier);
bool EmbeddedPortablePdbHasLocalSource(string module);
bool IsLocalMethod(string method);
}
}
30 changes: 17 additions & 13 deletions src/coverlet.core/Coverage.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
using System.Collections.Generic;
using System.IO;
using System.Linq;

using Coverlet.Core.Abstracts;
using Coverlet.Core.Helpers;
using Coverlet.Core.Instrumentation;
using Coverlet.Core.Logging;
Expand All @@ -26,6 +26,7 @@ public class Coverage
private string _mergeWith;
private bool _useSourceLink;
private ILogger _logger;
private IInstrumentationHelper _instrumentationHelper;
private List<InstrumenterResult> _results;

public string Identifier
Expand All @@ -43,7 +44,8 @@ public Coverage(string module,
bool singleHit,
string mergeWith,
bool useSourceLink,
ILogger logger)
ILogger logger,
IInstrumentationHelper instrumentationHelper)
{
_module = module;
_includeFilters = includeFilters;
Expand All @@ -56,44 +58,46 @@ public Coverage(string module,
_mergeWith = mergeWith;
_useSourceLink = useSourceLink;
_logger = logger;
_instrumentationHelper = instrumentationHelper;

_identifier = Guid.NewGuid().ToString();
_results = new List<InstrumenterResult>();
}

public Coverage(CoveragePrepareResult prepareResult, ILogger logger)
public Coverage(CoveragePrepareResult prepareResult, ILogger logger, IInstrumentationHelper instrumentationHelper)
{
_identifier = prepareResult.Identifier;
_module = prepareResult.Module;
_mergeWith = prepareResult.MergeWith;
_useSourceLink = prepareResult.UseSourceLink;
_results = new List<InstrumenterResult>(prepareResult.Results);
_logger = logger;
_instrumentationHelper = instrumentationHelper;
}

public CoveragePrepareResult PrepareModules()
{
string[] modules = InstrumentationHelper.GetCoverableModules(_module, _includeDirectories, _includeTestAssembly);
string[] modules = _instrumentationHelper.GetCoverableModules(_module, _includeDirectories, _includeTestAssembly);

Array.ForEach(_excludeFilters ?? Array.Empty<string>(), filter => _logger.LogVerbose($"Excluded module filter '{filter}'"));
Array.ForEach(_includeFilters ?? Array.Empty<string>(), filter => _logger.LogVerbose($"Included module filter '{filter}'"));

_excludeFilters = _excludeFilters?.Where(f => InstrumentationHelper.IsValidFilterExpression(f)).ToArray();
_includeFilters = _includeFilters?.Where(f => InstrumentationHelper.IsValidFilterExpression(f)).ToArray();
_excludeFilters = _excludeFilters?.Where(f => _instrumentationHelper.IsValidFilterExpression(f)).ToArray();
_includeFilters = _includeFilters?.Where(f => _instrumentationHelper.IsValidFilterExpression(f)).ToArray();

foreach (var module in modules)
{
if (InstrumentationHelper.IsModuleExcluded(module, _excludeFilters) ||
!InstrumentationHelper.IsModuleIncluded(module, _includeFilters))
if (_instrumentationHelper.IsModuleExcluded(module, _excludeFilters) ||
!_instrumentationHelper.IsModuleIncluded(module, _includeFilters))
{
_logger.LogVerbose($"Excluded module: '{module}'");
continue;
}

var instrumenter = new Instrumenter(module, _identifier, _excludeFilters, _includeFilters, _excludedSourceFiles, _excludeAttributes, _singleHit, _logger);
var instrumenter = new Instrumenter(module, _identifier, _excludeFilters, _includeFilters, _excludedSourceFiles, _excludeAttributes, _singleHit, _logger, _instrumentationHelper);
if (instrumenter.CanInstrument())
{
InstrumentationHelper.BackupOriginalModule(module, _identifier);
_instrumentationHelper.BackupOriginalModule(module, _identifier);

// Guard code path and restore if instrumentation fails.
try
Expand All @@ -105,7 +109,7 @@ public CoveragePrepareResult PrepareModules()
catch (Exception ex)
{
_logger.LogWarning($"Unable to instrument module: {module} because : {ex.Message}");
InstrumentationHelper.RestoreOriginalModule(module, _identifier);
_instrumentationHelper.RestoreOriginalModule(module, _identifier);
}
}
}
Expand Down Expand Up @@ -206,7 +210,7 @@ public CoverageResult GetCoverageResult()
}

modules.Add(Path.GetFileName(result.ModulePath), documents);
InstrumentationHelper.RestoreOriginalModule(result.ModulePath, _identifier);
_instrumentationHelper.RestoreOriginalModule(result.ModulePath, _identifier);
}

var coverageResult = new CoverageResult { Identifier = _identifier, Modules = modules, InstrumentedResults = _results };
Expand Down Expand Up @@ -301,7 +305,7 @@ private void CalculateCoverage()
}
}

InstrumentationHelper.DeleteHitsFile(result.HitsFilePath);
_instrumentationHelper.DeleteHitsFile(result.HitsFilePath);
}
}

Expand Down
6 changes: 5 additions & 1 deletion src/coverlet.core/DependencyInjection.cs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@

namespace Coverlet.Core
{
internal static class DependencyInjection
public static class DependencyInjection
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The only thing I need to do is to expose container to be used by console/task/collector without expose internal implementation.

{
private static Lazy<IServiceProvider> _serviceProvider = new Lazy<IServiceProvider>(() => InitDefaultServices(), true);

Expand All @@ -28,6 +28,10 @@ private static IServiceProvider InitDefaultServices()
IServiceCollection serviceCollection = new ServiceCollection();
serviceCollection.AddTransient<IRetryHelper, RetryHelper>();
serviceCollection.AddTransient<IProcessExitHandler, ProcessExitHandler>();

// We need to keep singleton/static semantics
serviceCollection.AddSingleton<IInstrumentationHelper, InstrumentationHelper>();

return serviceCollection.BuildServiceProvider();
}

Expand Down
60 changes: 29 additions & 31 deletions src/coverlet.core/Helpers/InstrumentationHelper.cs
Original file line number Diff line number Diff line change
Expand Up @@ -9,22 +9,21 @@
using System.Reflection.PortableExecutable;
using System.Text.RegularExpressions;
using Coverlet.Core.Abstracts;
using Microsoft.Extensions.FileSystemGlobbing;
using Microsoft.Extensions.DependencyInjection;
using Microsoft.Extensions.FileSystemGlobbing.Abstractions;

namespace Coverlet.Core.Helpers
{
internal static class InstrumentationHelper
internal class InstrumentationHelper : IInstrumentationHelper
{
private static readonly ConcurrentDictionary<string, string> _backupList = new ConcurrentDictionary<string, string>();
private readonly ConcurrentDictionary<string, string> _backupList = new ConcurrentDictionary<string, string>();
private readonly IRetryHelper _retryHelper;

static InstrumentationHelper()
public InstrumentationHelper(IProcessExitHandler processExitHandler, IRetryHelper retryHelper)
{
DependencyInjection.Current.GetService<IProcessExitHandler>().Add((s, e) => RestoreOriginalModules());
processExitHandler.Add((s, e) => RestoreOriginalModules());
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ViktorHofer you did this change, now a single instance of InstrumentationHelper will be returned by "static" container(singleton) so the delegate will keep object alive for "restore".
Is it ok for you or do you see some issue with it?

_retryHelper = retryHelper;
}

public static string[] GetCoverableModules(string module, string[] directories, bool includeTestAssembly)
public string[] GetCoverableModules(string module, string[] directories, bool includeTestAssembly)
{
Debug.Assert(directories != null);

Expand Down Expand Up @@ -68,7 +67,7 @@ public static string[] GetCoverableModules(string module, string[] directories,
.ToArray();
}

public static bool HasPdb(string module, out bool embedded)
public bool HasPdb(string module, out bool embedded)
{
embedded = false;
using (var moduleStream = File.OpenRead(module))
Expand All @@ -94,7 +93,7 @@ public static bool HasPdb(string module, out bool embedded)
}
}

public static bool EmbeddedPortablePdbHasLocalSource(string module)
public bool EmbeddedPortablePdbHasLocalSource(string module)
{
using (FileStream moduleStream = File.OpenRead(module))
using (var peReader = new PEReader(moduleStream))
Expand Down Expand Up @@ -129,7 +128,7 @@ public static bool EmbeddedPortablePdbHasLocalSource(string module)
return true;
}

public static void BackupOriginalModule(string module, string identifier)
public void BackupOriginalModule(string module, string identifier)
{
var backupPath = GetBackupPath(module, identifier);
var backupSymbolPath = Path.ChangeExtension(backupPath, ".pdb");
Expand All @@ -150,7 +149,7 @@ public static void BackupOriginalModule(string module, string identifier)
}
}

public static void RestoreOriginalModule(string module, string identifier)
public void RestoreOriginalModule(string module, string identifier)
{
var backupPath = GetBackupPath(module, identifier);
var backupSymbolPath = Path.ChangeExtension(backupPath, ".pdb");
Expand All @@ -159,14 +158,14 @@ public static void RestoreOriginalModule(string module, string identifier)
// See: https://github.com/tonerdo/coverlet/issues/25
var retryStrategy = CreateRetryStrategy();

DependencyInjection.Current.GetService<IRetryHelper>().Retry(() =>
_retryHelper.Retry(() =>
{
File.Copy(backupPath, module, true);
File.Delete(backupPath);
_backupList.TryRemove(module, out string _);
}, retryStrategy, 10);

DependencyInjection.Current.GetService<IRetryHelper>().Retry(() =>
_retryHelper.Retry(() =>
{
if (File.Exists(backupSymbolPath))
{
Expand All @@ -178,7 +177,7 @@ public static void RestoreOriginalModule(string module, string identifier)
}, retryStrategy, 10);
}

public static void RestoreOriginalModules()
public 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
Expand All @@ -187,7 +186,7 @@ public static void RestoreOriginalModules()
foreach (string key in _backupList.Keys.ToList())
{
string backupPath = _backupList[key];
DependencyInjection.Current.GetService<IRetryHelper>().Retry(() =>
_retryHelper.Retry(() =>
{
File.Copy(backupPath, key, true);
File.Delete(backupPath);
Expand All @@ -196,15 +195,15 @@ public static void RestoreOriginalModules()
}
}

public static void DeleteHitsFile(string path)
public void DeleteHitsFile(string path)
{
// Retry hitting the hits file - retry up to 10 times, since the file could be locked
// See: https://github.com/tonerdo/coverlet/issues/25
var retryStrategy = CreateRetryStrategy();
DependencyInjection.Current.GetService<IRetryHelper>().Retry(() => File.Delete(path), retryStrategy, 10);
_retryHelper.Retry(() => File.Delete(path), retryStrategy, 10);
}

public static bool IsValidFilterExpression(string filter)
public bool IsValidFilterExpression(string filter)
{
if (filter == null)
return false;
Expand Down Expand Up @@ -236,7 +235,7 @@ public static bool IsValidFilterExpression(string filter)
return true;
}

public static bool IsModuleExcluded(string module, string[] excludeFilters)
public bool IsModuleExcluded(string module, string[] excludeFilters)
{
if (excludeFilters == null || excludeFilters.Length == 0)
return false;
Expand Down Expand Up @@ -264,7 +263,7 @@ public static bool IsModuleExcluded(string module, string[] excludeFilters)
return false;
}

public static bool IsModuleIncluded(string module, string[] includeFilters)
public bool IsModuleIncluded(string module, string[] includeFilters)
{
if (includeFilters == null || includeFilters.Length == 0)
return true;
Expand All @@ -291,7 +290,7 @@ public static bool IsModuleIncluded(string module, string[] includeFilters)
return false;
}

public static bool IsTypeExcluded(string module, string type, string[] excludeFilters)
public bool IsTypeExcluded(string module, string type, string[] excludeFilters)
{
if (excludeFilters == null || excludeFilters.Length == 0)
return false;
Expand All @@ -303,7 +302,7 @@ public static bool IsTypeExcluded(string module, string type, string[] excludeFi
return IsTypeFilterMatch(module, type, excludeFilters);
}

public static bool IsTypeIncluded(string module, string type, string[] includeFilters)
public bool IsTypeIncluded(string module, string type, string[] includeFilters)
{
if (includeFilters == null || includeFilters.Length == 0)
return true;
Expand All @@ -315,10 +314,10 @@ public static bool IsTypeIncluded(string module, string type, string[] includeFi
return IsTypeFilterMatch(module, type, includeFilters);
}

public static bool IsLocalMethod(string method)
public bool IsLocalMethod(string method)
=> new Regex(WildcardToRegex("<*>*__*|*")).IsMatch(method);

private static bool IsTypeFilterMatch(string module, string type, string[] filters)
private bool IsTypeFilterMatch(string module, string type, string[] filters)
{
Debug.Assert(module != null);
Debug.Assert(filters != null);
Expand All @@ -338,15 +337,15 @@ private static bool IsTypeFilterMatch(string module, string type, string[] filte
return false;
}

private static string GetBackupPath(string module, string identifier)
private string GetBackupPath(string module, string identifier)
{
return Path.Combine(
Path.GetTempPath(),
Path.GetFileNameWithoutExtension(module) + "_" + identifier + ".dll"
);
}

private static Func<TimeSpan> CreateRetryStrategy(int initialSleepSeconds = 6)
private Func<TimeSpan> CreateRetryStrategy(int initialSleepSeconds = 6)
{
TimeSpan retryStrategy()
{
Expand All @@ -358,14 +357,14 @@ TimeSpan retryStrategy()
return retryStrategy;
}

private static string WildcardToRegex(string pattern)
private string WildcardToRegex(string pattern)
{
return "^" + Regex.Escape(pattern).
Replace("\\*", ".*").
Replace("\\?", "?") + "$";
}

private static bool IsAssembly(string filePath)
private bool IsAssembly(string filePath)
{
Debug.Assert(filePath != null);

Expand All @@ -383,5 +382,4 @@ private static bool IsAssembly(string filePath)
}
}
}
}

}
Loading