Skip to content

Removes the not instrumented/unused instrumenter results so coverage file doesn't get polluted with unused modules #225

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

Closed
Closed
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
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ Options:
--include Filter expressions to include specific modules and types.
--exclude-by-file Glob patterns specifying source files to exclude.
--merge-with Path to existing coverage result to merge.
--exclude-non-called-files Exclude non called/non instrumented files from coverage.
```

#### Code Coverage
Expand Down
3 changes: 2 additions & 1 deletion src/coverlet.console/Program.cs
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ static int Main(string[] args)
CommandOption includeFilters = app.Option("--include", "Filter expressions to include only specific modules and types.", CommandOptionType.MultipleValue);
CommandOption excludedSourceFiles = app.Option("--exclude-by-file", "Glob patterns specifying source files to exclude.", CommandOptionType.MultipleValue);
CommandOption mergeWith = app.Option("--merge-with", "Path to existing coverage result to merge.", CommandOptionType.SingleValue);
CommandOption excludeNonCalledFiles = app.Option("--exclude-non-called-files", "Exclude non called/non instrumented files from coverage report", CommandOptionType.NoValue);

app.OnExecute(() =>
{
Expand All @@ -44,7 +45,7 @@ static int Main(string[] args)
if (!target.HasValue())
throw new CommandParsingException(app, "Target must be specified.");

Coverage coverage = new Coverage(module.Value, excludeFilters.Values.ToArray(), includeFilters.Values.ToArray(), excludedSourceFiles.Values.ToArray(), mergeWith.Value());
Coverage coverage = new Coverage(module.Value, excludeFilters.Values.ToArray(), includeFilters.Values.ToArray(), excludedSourceFiles.Values.ToArray(), mergeWith.Value(), excludeNonCalledFiles.HasValue());
coverage.PrepareModules();

Process process = new Process();
Expand Down
68 changes: 41 additions & 27 deletions src/coverlet.core/Coverage.cs
Original file line number Diff line number Diff line change
Expand Up @@ -19,20 +19,22 @@ public class Coverage
private string[] _includeFilters;
private string[] _excludedSourceFiles;
private string _mergeWith;
private bool _excludeNonCalledFiles;
private List<InstrumenterResult> _results;

public string Identifier
{
get { return _identifier; }
}

public Coverage(string module, string[] excludeFilters, string[] includeFilters, string[] excludedSourceFiles, string mergeWith)
public Coverage(string module, string[] excludeFilters, string[] includeFilters, string[] excludedSourceFiles, string mergeWith, bool excludeNonCalledFiles)
{
_module = module;
_excludeFilters = excludeFilters;
_includeFilters = includeFilters;
_excludedSourceFiles = excludedSourceFiles;
_mergeWith = mergeWith;
_excludeNonCalledFiles = excludeNonCalledFiles;

_identifier = Guid.NewGuid().ToString();
_results = new List<InstrumenterResult>();
Expand Down Expand Up @@ -63,7 +65,7 @@ public void PrepareModules()

public CoverageResult GetCoverageResult()
{
CalculateCoverage();
CalculateCoverage(_excludeNonCalledFiles);

Modules modules = new Modules();
foreach (var result in _results)
Expand Down Expand Up @@ -160,13 +162,20 @@ public CoverageResult GetCoverageResult()
return coverageResult;
}

private void CalculateCoverage()
private void CalculateCoverage(bool excludeNonCalledFiles)
{
var resultsToRemove = new List<InstrumenterResult>();
foreach (var result in _results)
{
if (!File.Exists(result.HitsFilePath))
{
// File not instrumented, or nothing in it called. Warn about this?
// File not instrumented, or nothing in it called.
// Mark to be removed so no non used modules are included in coverage file?
if (excludeNonCalledFiles)
{
resultsToRemove.Add(result);
Copy link
Contributor Author

@pape77 pape77 Jul 2, 2019

Choose a reason for hiding this comment

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

@MarcoRossignoli this made the magic.
I just included a parameter for not doing this by default, because @tonerdo wanted to keep original behaviour (for some reason)
But this solved excluding any specific stuff that's not part of your code. Together with loop in line 245
You may want to try if it's still applicable. The referenced issue is probably now deprecated.
Current problem is more like coverlet trying to cover dlls that are not part of your code

Copy link
Collaborator

Choose a reason for hiding this comment

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

At the moment I don't have a strong opinion, but I think that if we support this feature should be throught switch(like in this PR) because could be also useful understand why some module are not "tested", in big project someone could lose "the problem" of "disabled" tests.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not sure if i can think an scenario in which you wouldn't see not tested modules which are part of your code.
Maybe if you don't test Api lvl ,then api.dll won't pop up and coverage will be higher. That's what you mean? but then you immediately don't see it covered in your coverage report or console output

Copy link
Collaborator

@MarcoRossignoli MarcoRossignoli Jul 2, 2019

Choose a reason for hiding this comment

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

but then you immediately don't see it covered in your coverage report or console output

Yes but I think a scenario where I have tens of modules...if I see module with 0% it's immediatly clear that something went wrong(for instance temporary commented tests and not re-enabled) if you trim that result I could lose the issue, make sense?

}

continue;
}

Expand Down Expand Up @@ -205,33 +214,38 @@ private void CalculateCoverage()
}
}

// 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)
{
List<KeyValuePair<(int, int), Branch>> branchesToRemove = new List<KeyValuePair<(int, int), Branch>>();
foreach (var branch in document.Value.Branches)
{
//if one branch is covered we search the other one only if it's not covered
if (CecilSymbolHelper.IsMoveNext(branch.Value.Method) && branch.Value.Hits > 0)
{
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);
}
}
}
}
foreach (var branchToRemove in branchesToRemove)
{
document.Value.Branches.Remove(branchToRemove.Key);
}
// 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)
{
List<KeyValuePair<(int, int), Branch>> branchesToRemove = new List<KeyValuePair<(int, int), Branch>>();
foreach (var branch in document.Value.Branches)
{
//if one branch is covered we search the other one only if it's not covered
if (CecilSymbolHelper.IsMoveNext(branch.Value.Method) && branch.Value.Hits > 0)
{
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);
}
}
}
}
foreach (var branchToRemove in branchesToRemove)
{
document.Value.Branches.Remove(branchToRemove.Key);
}
}

InstrumentationHelper.DeleteHitsFile(result.HitsFilePath);
}

foreach (var resultToRemove in resultsToRemove)
{
_results.Remove(resultToRemove);
}
}
}
}
10 changes: 8 additions & 2 deletions src/coverlet.msbuild.tasks/InstrumentationTask.cs
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ public class InstrumentationTask : Task
private string _include;
private string _excludeByFile;
private string _mergeWith;
private bool _excludeNonCalledFiles;

internal static Coverage Coverage
{
Expand Down Expand Up @@ -50,15 +51,20 @@ public string MergeWith
set { _mergeWith = value; }
}

public bool ExcludeNonCalledFiles
{
get { return _excludeNonCalledFiles; }
set { _excludeNonCalledFiles = value; }
}

public override bool Execute()
{
try
{
var excludedSourceFiles = _excludeByFile?.Split(',');
var excludeFilters = _exclude?.Split(',');
var includeFilters = _include?.Split(',');

_coverage = new Coverage(_path, excludeFilters, includeFilters, excludedSourceFiles, _mergeWith);
_coverage = new Coverage(_path, excludeFilters, includeFilters, excludedSourceFiles, _mergeWith, _excludeNonCalledFiles);
_coverage.PrepareModules();
}
catch (Exception ex)
Expand Down
1 change: 1 addition & 0 deletions src/coverlet.msbuild/coverlet.msbuild.props
Original file line number Diff line number Diff line change
Expand Up @@ -9,5 +9,6 @@
<MergeWith Condition="$(MergeWith) == ''"></MergeWith>
<Threshold Condition="$(Threshold) == ''">0</Threshold>
<ThresholdType Condition="$(ThresholdType) == ''">line,branch,method</ThresholdType>
<ExcludeNonCalledFiles Condition="$(ExcludeNonCalledFiles) == ''">false</ExcludeNonCalledFiles>
</PropertyGroup>
</Project>
2 changes: 2 additions & 0 deletions src/coverlet.msbuild/coverlet.msbuild.targets
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
Exclude="$(Exclude)"
ExcludeByFile="$(ExcludeByFile)"
MergeWith="$(MergeWith)"
ExcludeNonCalledFiles="$(ExcludeNonCalledFiles)"
Path="$(TargetPath)" />
</Target>

Expand All @@ -20,6 +21,7 @@
Exclude="$(Exclude)"
ExcludeByFile="$(ExcludeByFile)"
MergeWith="$(MergeWith)"
ExcludeNonCalledFiles="$(ExcludeNonCalledFiles)"
Path="$(TargetPath)" />
</Target>

Expand Down
26 changes: 15 additions & 11 deletions test/coverlet.core.tests/CoverageTests.cs
Original file line number Diff line number Diff line change
@@ -1,18 +1,15 @@
using System;
using System.IO;

using Xunit;
using Moq;

using Coverlet.Core;
using System.Collections.Generic;

namespace Coverlet.Core.Tests
{
public class CoverageTests
{
[Fact]
public void TestCoverage()
[Theory]
[InlineData(false)]
[InlineData(true)]
public void TestCoverage(bool excludeNonCalledFilesValue)
{
string module = GetType().Assembly.Location;
string pdb = Path.Combine(Path.GetDirectoryName(module), Path.GetFileNameWithoutExtension(module) + ".pdb");
Expand All @@ -22,17 +19,24 @@ 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);

// TODO: Find a way to mimick hits
// TODO: Find a way to mimic hits

// Since Coverage only instruments dependancies, we need a fake module here
// Since Coverage only instruments dependencies, we need a fake module here
var testModule = Path.Combine(directory.FullName, "test.module.dll");

var coverage = new Coverage(testModule, Array.Empty<string>(), Array.Empty<string>(), Array.Empty<string>(), string.Empty);
var coverage = new Coverage(testModule, Array.Empty<string>(), Array.Empty<string>(), Array.Empty<string>(), string.Empty, excludeNonCalledFilesValue);
coverage.PrepareModules();

var result = coverage.GetCoverageResult();

Assert.NotEmpty(result.Modules);
if (excludeNonCalledFilesValue)
{
Assert.Empty(result.Modules);
}
else
{
Assert.NotEmpty(result.Modules);
}

directory.Delete(true);
}
Expand Down