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

Improve lambda scenario coverage #583

Merged
merged 2 commits into from
Oct 9, 2019
Merged

Improve lambda scenario coverage #583

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a1579c1
improve a bit lambda corner case coverage
MarcoRossignoli Oct 8, 2019
ed7f2ff
updates
MarcoRossignoli Oct 8, 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
115 changes: 113 additions & 2 deletions src/coverlet.core/Coverage.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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<string> 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<string>();
}

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))
Expand All @@ -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)
Expand All @@ -253,15 +345,15 @@ 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))
{
int hitCandidatesCount = br.ReadInt32();

// 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];
Expand All @@ -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");
}
Expand Down
4 changes: 2 additions & 2 deletions src/coverlet.core/Helpers/InstrumentationHelper.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand Down Expand Up @@ -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
Expand Down
20 changes: 19 additions & 1 deletion src/coverlet.core/Instrumentation/Instrumenter.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -37,6 +38,7 @@ internal class Instrumenter
private MethodReference _customTrackerRegisterUnloadEventsMethod;
private MethodReference _customTrackerRecordHitMethod;
private List<string> _excludedSourceFiles;
private List<string> _branchesInCompiledGeneratedClass;

public Instrumenter(
string module,
Expand Down Expand Up @@ -130,6 +132,8 @@ public InstrumenterResult Instrument()
}
}

_result.BranchesInCompiledGeneratedClass = _branchesInCompiledGeneratedClass == null ? Array.Empty<string>() : _branchesInCompiledGeneratedClass.ToArray();

return _result;
}

Expand Down Expand Up @@ -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,
Expand All @@ -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<string>();
}

if (!_branchesInCompiledGeneratedClass.Contains(method.FullName))
{
_branchesInCompiledGeneratedClass.Add(method.FullName);
}
}
}

_result.HitCandidates.Add(new HitCandidate(true, document.Index, branchPoint.StartLine, (int)branchPoint.Ordinal));
Expand Down
4 changes: 4 additions & 0 deletions src/coverlet.core/Instrumentation/InstrumenterResult.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@

namespace Coverlet.Core.Instrumentation
{
[DebuggerDisplay("Number = {Number} Hits = {Hits} Class = {Class} Method = {Method}")]
[DataContract]
public class Line
{
Expand Down Expand Up @@ -73,6 +74,7 @@ public Document()
public Dictionary<BranchKey, Branch> Branches { get; private set; }
}

[DebuggerDisplay("isBranch = {isBranch} docIndex = {docIndex} start = {start} end = {end}")]
[DataContract]
public class HitCandidate
{
Expand Down Expand Up @@ -100,6 +102,8 @@ public InstrumenterResult()
[DataMember]
public string Module;
[DataMember]
public string[] BranchesInCompiledGeneratedClass;
[DataMember]
public string HitsFilePath;
[DataMember]
public string ModulePath;
Expand Down
53 changes: 50 additions & 3 deletions test/coverlet.core.tests/CoverageTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
{
Expand Down Expand Up @@ -163,6 +163,7 @@ public void AsyncAwait()
}, path).Dispose();

CoverageResult result = TestInstrumentationHelper.GetCoverageResult(path);

result.Document("Instrumentation.AsyncAwait.cs")
.AssertLinesCovered(BuildConfiguration.Debug,
// AsyncExecution(bool)
Expand Down Expand Up @@ -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<Lambda_Issue343>(instance =>
{
instance.InvokeAnonymous_Test();
((Task<bool>)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);
}
}
}
}
Loading