Skip to content
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.

Sign up for GitHub

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

Fix exclude by files #524

Merged
merged 5 commits into from
Sep 6, 2019
Merged

Fix exclude by files #524

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
758c5f1
fix exclude by files
MarcoRossignoli Aug 20, 2019
b64c8d3
merge master
MarcoRossignoli Aug 20, 2019
c2584f4
fix InstrumentMethod
MarcoRossignoli Aug 20, 2019
7d70560
updates
Aug 20, 2019
52e5901
improve logging
MarcoRossignoli Aug 20, 2019
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: 1 addition & 3 deletions src/coverlet.core/Coverage.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,11 +74,9 @@ public Coverage(CoveragePrepareResult prepareResult, ILogger logger)
public CoveragePrepareResult PrepareModules()
{
string[] modules = InstrumentationHelper.GetCoverableModules(_module, _includeDirectories, _includeTestAssembly);
string[] excludes = InstrumentationHelper.GetExcludedFiles(_excludedSourceFiles);

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}'"));
Array.ForEach(excludes ?? Array.Empty<string>(), filter => _logger.LogVerbose($"Excluded source files '{filter}'"));

_excludeFilters = _excludeFilters?.Where(f => InstrumentationHelper.IsValidFilterExpression(f)).ToArray();
_includeFilters = _includeFilters?.Where(f => InstrumentationHelper.IsValidFilterExpression(f)).ToArray();
Expand All @@ -92,7 +90,7 @@ public CoveragePrepareResult PrepareModules()
continue;
}

var instrumenter = new Instrumenter(module, _identifier, _excludeFilters, _includeFilters, excludes, _excludeAttributes, _singleHit, _logger);
var instrumenter = new Instrumenter(module, _identifier, _excludeFilters, _includeFilters, _excludedSourceFiles, _excludeAttributes, _singleHit, _logger);
if (instrumenter.CanInstrument())
{
InstrumentationHelper.BackupOriginalModule(module, _identifier);
Expand Down
40 changes: 0 additions & 40 deletions src/coverlet.core/Helpers/InstrumentationHelper.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -318,46 +318,6 @@ public static bool IsTypeIncluded(string module, string type, string[] includeFi
public static bool IsLocalMethod(string method)
=> new Regex(WildcardToRegex("<*>*__*|*")).IsMatch(method);

public static string[] GetExcludedFiles(string[] excludes)
{
const string RELATIVE_KEY = nameof(RELATIVE_KEY);
string parentDir = Directory.GetCurrentDirectory();

if (excludes == null || !excludes.Any()) return Array.Empty<string>();

var matcherDict = new Dictionary<string, Matcher>() { { RELATIVE_KEY, new Matcher() } };
foreach (var excludeRule in excludes)
{
if (Path.IsPathRooted(excludeRule))
{
var root = Path.GetPathRoot(excludeRule);
if (!matcherDict.ContainsKey(root))
{
matcherDict.Add(root, new Matcher());
}
matcherDict[root].AddInclude(Path.GetFullPath(excludeRule).Substring(root.Length));
}
else
{
matcherDict[RELATIVE_KEY].AddInclude(excludeRule);
}
}

var files = new List<string>();
foreach (var entry in matcherDict)
{
var root = entry.Key;
var matcher = entry.Value;
var directoryInfo = new DirectoryInfo(root.Equals(RELATIVE_KEY) ? parentDir : root);
var fileMatchResult = matcher.Execute(new DirectoryInfoWrapper(directoryInfo));
var currentFiles = fileMatchResult.Files
.Select(f => Path.GetFullPath(Path.Combine(directoryInfo.ToString(), f.Path)));
files.AddRange(currentFiles);
}

return files.Distinct().ToArray();
}

private static bool IsTypeFilterMatch(string module, string type, string[] filters)
{
Debug.Assert(module != null);
Expand Down
56 changes: 50 additions & 6 deletions src/coverlet.core/Instrumentation/Instrumenter.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
using Coverlet.Core.Helpers;
using Coverlet.Core.Logging;
using Coverlet.Core.Symbols;

using Microsoft.Extensions.FileSystemGlobbing;
using Mono.Cecil;
using Mono.Cecil.Cil;
using Mono.Cecil.Rocks;
Expand All @@ -22,7 +22,7 @@ internal class Instrumenter
private readonly string _identifier;
private readonly string[] _excludeFilters;
private readonly string[] _includeFilters;
private readonly string[] _excludedFiles;
private readonly ExcludedFilesHelper _excludedFilesHelper;
private readonly string[] _excludedAttributes;
private readonly bool _singleHit;
private readonly bool _isCoreLibrary;
Expand All @@ -36,14 +36,15 @@ internal class Instrumenter
private MethodReference _customTrackerRegisterUnloadEventsMethod;
private MethodReference _customTrackerRecordHitMethod;
private List<string> _asyncMachineStateMethod;
private List<string> _excludedSourceFiles;

public Instrumenter(string module, string identifier, string[] excludeFilters, string[] includeFilters, string[] excludedFiles, string[] excludedAttributes, bool singleHit, ILogger logger)
{
_module = module;
_identifier = identifier;
_excludeFilters = excludeFilters;
_includeFilters = includeFilters;
_excludedFiles = excludedFiles ?? Array.Empty<string>();
_excludedFilesHelper = new ExcludedFilesHelper(excludedFiles, logger);
_excludedAttributes = excludedAttributes;
_singleHit = singleHit;
_isCoreLibrary = Path.GetFileNameWithoutExtension(_module) == "System.Private.CoreLib";
Expand Down Expand Up @@ -103,6 +104,14 @@ public InstrumenterResult Instrument()

_result.AsyncMachineStateMethod = _asyncMachineStateMethod == null ? Array.Empty<string>() : _asyncMachineStateMethod.ToArray();

if (_excludedSourceFiles != null)
{
foreach (string sourceFile in _excludedSourceFiles)
{
_logger.LogVerbose($"Excluded source file: '{sourceFile}'");
}
}

return _result;
}

Expand Down Expand Up @@ -321,9 +330,12 @@ private void InstrumentType(TypeDefinition type)
private void InstrumentMethod(MethodDefinition method)
{
var sourceFile = method.DebugInformation.SequencePoints.Select(s => s.Document.Url).FirstOrDefault();
if (!string.IsNullOrEmpty(sourceFile) && _excludedFiles.Contains(sourceFile))
if (!string.IsNullOrEmpty(sourceFile) && _excludedFilesHelper.Exclude(sourceFile))
{
_logger.LogVerbose($"Excluded source file: '{sourceFile}'");
if (!(_excludedSourceFiles ??= new List<string>()).Contains(sourceFile))
{
_excludedSourceFiles.Add(sourceFile);
}
return;
}

Expand Down Expand Up @@ -562,7 +574,7 @@ private bool IsExcludeAttribute(CustomAttribute customAttribute)
customAttribute.AttributeType.Name.Equals(a.EndsWith("Attribute") ? a : $"{a}Attribute"));
}

private static Mono.Cecil.Cil.MethodBody GetMethodBody(MethodDefinition method)
private static MethodBody GetMethodBody(MethodDefinition method)
{
try
{
Expand Down Expand Up @@ -731,4 +743,36 @@ public override AssemblyDefinition Resolve(AssemblyNameReference name)
}
}
}

// Exclude files helper https://docs.microsoft.com/en-us/dotnet/api/microsoft.extensions.filesystemglobbing.matcher?view=aspnetcore-2.2
internal class ExcludedFilesHelper
{
Matcher _matcher;

public ExcludedFilesHelper(string[] excludes, ILogger logger)
{
if (excludes != null && excludes.Length > 0)
{
_matcher = new Matcher();
foreach (var excludeRule in excludes)
{
if (excludeRule is null)
{
continue;
}
logger.LogVerbose($"Excluded source file rule '{excludeRule}'");
_matcher.AddInclude(Path.IsPathRooted(excludeRule) ? excludeRule.Substring(Path.GetPathRoot(excludeRule).Length) : excludeRule);
}
}
}

public bool Exclude(string sourceFile)
{
if (_matcher is null || sourceFile is null)
return false;

// We strip out drive because it doesn't work with globbing
return _matcher.Match(Path.IsPathRooted(sourceFile) ? sourceFile.Substring(Path.GetPathRoot(sourceFile).Length) : sourceFile).HasMatches;
}
}
}
1 change: 1 addition & 0 deletions src/coverlet.core/coverlet.core.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
<TargetFramework>netstandard2.0</TargetFramework>
<AssemblyVersion>5.1.1</AssemblyVersion>
<IsPackable>false</IsPackable>
<LangVersion>preview</LangVersion>
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should master use preview version of C#?

Copy link
Collaborator Author

@MarcoRossignoli MarcoRossignoli Aug 21, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

Good question!
I did it for test project https://github.com/tonerdo/coverlet/blob/master/test/coverlet.core.tests/coverlet.core.tests.csproj
At the moment the only feature I used is null coalesce ??= because other features(i.e. IAsyncEnumerable) needs new netcoreapp3.0/standard2.1 object.
Also corefx seem allow preview https://github.com/dotnet/corefx/blob/master/Directory.Build.props#L248
@ViktorHofer do you see any issue with this config related to emitted IL?
cc: @tonerdo

Copy link
Contributor

Choose a reason for hiding this comment

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

No concerns from my side.

</PropertyGroup>

<ItemGroup>
Expand Down
55 changes: 0 additions & 55 deletions test/coverlet.core.tests/Helpers/InstrumentationHelperTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,61 +72,6 @@ public void TestDeleteHitsFile()
Assert.False(File.Exists(tempFile));
}

public static IEnumerable<object[]> GetExcludedFilesReturnsEmptyArgs =>
new[]
{
new object[]{null},
new object[]{new string[0]},
new object[]{new string[]{ Path.GetRandomFileName() }},
new object[]{new string[]{Path.GetRandomFileName(),
Path.Combine(Directory.GetCurrentDirectory(), Path.GetRandomFileName())}
}
};

[Theory]
[MemberData(nameof(GetExcludedFilesReturnsEmptyArgs))]
public void TestGetExcludedFilesReturnsEmpty(string[] excludedFiles)
{
Assert.False(InstrumentationHelper.GetExcludedFiles(excludedFiles)?.Any());
}

[Fact]
public void TestGetExcludedFilesUsingAbsFile()
{
var file = Path.GetRandomFileName();
File.Create(file).Dispose();
var excludedFiles = InstrumentationHelper.GetExcludedFiles(
new string[] { Path.Combine(Directory.GetCurrentDirectory(), file) }
);
File.Delete(file);
Assert.Single(excludedFiles);
}

[Fact]
public void TestGetExcludedFilesUsingGlobbing()
{
var fileExtension = Path.GetRandomFileName();
var paths = new string[]{
$"{Path.GetRandomFileName()}.{fileExtension}",
$"{Path.GetRandomFileName()}.{fileExtension}"
};

foreach (var path in paths)
{
File.Create(path).Dispose();
}

var excludedFiles = InstrumentationHelper.GetExcludedFiles(
new string[] { $"*.{fileExtension}" });

foreach (var path in paths)
{
File.Delete(path);
}

Assert.Equal(paths.Length, excludedFiles.Count());
}

[Fact]
public void TestIsModuleExcludedWithoutFilter()
{
Expand Down
103 changes: 103 additions & 0 deletions test/coverlet.core.tests/Instrumentation/InstrumenterTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
using System;
using System.Collections.Generic;
using System.IO;
using System.Linq;
using System.Reflection;
using System.Runtime.InteropServices;

using Coverlet.Core.Helpers;
using Coverlet.Core.Logging;
Expand Down Expand Up @@ -233,6 +235,107 @@ public void TestInstrument_NetStandardAwareAssemblyResolver_FromFolder()
Assert.Equal(Path.Combine(Path.GetDirectoryName(Assembly.GetExecutingAssembly().Location), "netstandard.dll"), Path.GetFullPath(resolved.MainModule.FileName));
}

public static IEnumerable<object[]> TestInstrument_ExcludedFilesHelper_Data()
{
yield return new object[] { new string[]{ @"one.txt" }, new ValueTuple<string, bool, bool>[]
{
(@"one.txt", true, false),
(@"c:\dir\one.txt", false, true),
(@"dir/one.txt", false, false)
}};
yield return new object[] { new string[]{ @"*one.txt" }, new ValueTuple<string, bool, bool>[]
{
(@"one.txt", true , false),
(@"c:\dir\one.txt", false, true),
(@"dir/one.txt", false, false)
}};
yield return new object[] { new string[]{ @"*.txt" }, new ValueTuple<string, bool, bool>[]
{
(@"one.txt", true, false),
(@"c:\dir\one.txt", false, true),
(@"dir/one.txt", false, false)
}};
yield return new object[] { new string[]{ @"*.*" }, new ValueTuple<string, bool, bool>[]
{
(@"one.txt", true, false),
(@"c:\dir\one.txt", false, true),
(@"dir/one.txt", false, false)
}};
yield return new object[] { new string[]{ @"one.*" }, new ValueTuple<string, bool, bool>[]
{
(@"one.txt", true, false),
(@"c:\dir\one.txt", false, true),
(@"dir/one.txt", false, false)
}};
yield return new object[] { new string[]{ @"dir/*.txt" }, new ValueTuple<string, bool, bool>[]
{
(@"one.txt", false, false),
(@"c:\dir\one.txt", true, true),
(@"dir/one.txt", true, false)
}};
yield return new object[] { new string[]{ @"dir\*.txt" }, new ValueTuple<string, bool, bool>[]
{
(@"one.txt", false, false),
(@"c:\dir\one.txt", true, true),
(@"dir/one.txt", true, false)
}};
yield return new object[] { new string[]{ @"**/*" }, new ValueTuple<string, bool, bool>[]
{
(@"one.txt", true, false),
(@"c:\dir\one.txt", true, true),
(@"dir/one.txt", true, false)
}};
yield return new object[] { new string[]{ @"dir/**/*" }, new ValueTuple<string, bool, bool>[]
{
(@"one.txt", false, false),
(@"c:\dir\one.txt", true, true),
(@"dir/one.txt", true, false),
(@"c:\dir\dir2\one.txt", true, true),
(@"dir/dir2/one.txt", true, false)
}};
yield return new object[] { new string[]{ @"one.txt", @"dir\*two.txt" }, new ValueTuple<string, bool, bool>[]
{
(@"one.txt", true, false),
(@"c:\dir\imtwo.txt", true, true),
(@"dir/one.txt", false, false)
}};

// This is a special case test different drive same path
// We strip out drive from path to check for globbing
// BTW I don't know if makes sense add a filter with full path maybe we should forbid
yield return new object[] { new string[]{ @"c:\dir\one.txt" }, new ValueTuple<string, bool, bool>[]
{
(@"c:\dir\one.txt", true, true),
(@"d:\dir\one.txt", true, true) // maybe should be false?
}};

yield return new object[] { new string[]{ null }, new ValueTuple<string, bool, bool>[]
{
(null, false, false),
}};
}

[Theory]
[MemberData(nameof(TestInstrument_ExcludedFilesHelper_Data))]
public void TestInstrument_ExcludedFilesHelper(string[] excludeFilterHelper, ValueTuple<string, bool, bool>[] result)
{
var exludeFilterHelper = new ExcludedFilesHelper(excludeFilterHelper, new Mock<ILogger>().Object);
foreach (ValueTuple<string, bool, bool> checkFile in result)
{
if (checkFile.Item3) // run test only on windows platform
{
if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows))
{
Assert.Equal(checkFile.Item2, exludeFilterHelper.Exclude(checkFile.Item1));
}
}
else
{
Assert.Equal(checkFile.Item2, exludeFilterHelper.Exclude(checkFile.Item1));
}
}
}

[Fact]
public void SkipEmbeddedPpdbWithoutLocalSource()
{
Expand Down