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

Jump to bottom

[Bug fix]Incorrect coverage for statement lambda function #369

Closed
wants to merge 1 commit into from
Closed

[Bug fix]Incorrect coverage for statement lambda function #369

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
144 changes: 107 additions & 37 deletions src/coverlet.core/Coverage.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -189,6 +189,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) && File.Exists(_mergeWith))
Expand All @@ -200,6 +257,41 @@ public CoverageResult GetCoverageResult()
return coverageResult;
}

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 bool BranchInCompilerGeneratedClass(string methodName)
{
foreach (var instrumentedResult in _results)
{
if (instrumentedResult.BranchesInCompiledGeneratedClass.Contains(methodName))
{
return true;
}
}
return false;
}

private void CalculateCoverage()
{
foreach (var result in _results)
Expand All @@ -221,15 +313,17 @@ private void CalculateCoverage()
}
}

List<(int docIndex, int line)> zeroHitsLines = new List<(int docIndex, int line)>();

var documentsList = result.Documents.Values.ToList();

using (var fs = new FileStream(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 @@ -247,57 +341,33 @@ private void CalculateCoverage()
{
var line = document.Lines[j];
line.Hits += hits;

// We register 0 hit lines for later cleanup false positive
if (hits == 0)
{
zeroHitsLines.Add((hitLocation.docIndex, line.Number));
}
}
}
}
}

// for MoveNext() compiler autogenerated method we need to patch false positive (IAsyncStateMachine for instance)
// we'll remove all MoveNext() not covered branch
foreach (var document in result.Documents)
// Cleanup nested state machine false positive hits
foreach (var (docIndex, line) in zeroHitsLines)
{
List<KeyValuePair<(int, int), Branch>> branchesToRemove = new List<KeyValuePair<(int, int), Branch>>();
foreach (var branch in document.Value.Branches)
foreach (var lineToCheck in documentsList[docIndex].Lines)
{
//if one branch is covered we search the other one only if it's not covered
if (IsAsyncStateMachineMethod(branch.Value.Method) && branch.Value.Hits > 0)
if (lineToCheck.Key == line)
{
foreach (var moveNextBranch in document.Value.Branches)
{
if (moveNextBranch.Value.Method == branch.Value.Method && moveNextBranch.Value != branch.Value && moveNextBranch.Value.Hits == 0)
{
branchesToRemove.Add(moveNextBranch);
}
}
lineToCheck.Value.Hits = 0;
}
}
foreach (var branchToRemove in branchesToRemove)
{
document.Value.Branches.Remove(branchToRemove.Key);
}
}

InstrumentationHelper.DeleteHitsFile(result.HitsFilePath);
}
}

private bool IsAsyncStateMachineMethod(string method)
{
if (!method.EndsWith("::MoveNext()"))
{
return false;
}

foreach (var instrumentationResult in _results)
{
if (instrumentationResult.AsyncMachineStateMethod.Contains(method))
{
return true;
}
}
return false;
}

private string GetSourceLinkUrl(Dictionary<string, string> sourceLinkDocuments, string document)
{
if (sourceLinkDocuments.TryGetValue(document, out string url))
Expand Down
51 changes: 0 additions & 51 deletions src/coverlet.core/CoverageResult.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -108,57 +108,6 @@ internal void Merge(Modules modules)
}
}
}

// for MoveNext() compiler autogenerated method we need to patch false positive (IAsyncStateMachine for instance)
// we'll remove all MoveNext() not covered branch
List<BranchInfo> branchesToRemove = new List<BranchInfo>();
foreach (var module in this.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 one branch is covered we search the other one only if it's not covered
if (IsAsyncStateMachineMethod(method.Key) && branch.Hits > 0)
{
foreach (var moveNextBranch in method.Value.Branches)
{
if (moveNextBranch != branch && moveNextBranch.Hits == 0)
{
branchesToRemove.Add(moveNextBranch);
}
}
}
}
foreach (var branchToRemove in branchesToRemove)
{
method.Value.Branches.Remove(branchToRemove);
}
}
}
}
}
}

private bool IsAsyncStateMachineMethod(string method)
{
if (!method.EndsWith("::MoveNext()"))
{
return false;
}

foreach (var instrumentedResult in InstrumentedResults)
{
if (instrumentedResult.AsyncMachineStateMethod.Contains(method))
{
return true;
}
}
return false;
}

public ThresholdTypeFlags GetThresholdTypesBelowThreshold(CoverageSummary summary, double threshold, ThresholdTypeFlags thresholdTypes, ThresholdStatistic thresholdStat)
Expand Down
Loading