Skip to content

Add [xunit*]* to default excluded modules filter if not specified #462

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
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
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,13 @@ private string[] ParseExcludeFilters(XmlElement configurationElement)
}
}

// if we've only one element mean that we only added CoverletConstants.DefaultExcludeFilter
Copy link
Contributor

@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.

wouldn't make that assumption, but more like excludeFilters.Contains(CoverletConstants.DefaultExcludeFilter) && excludeFilters.Count==1
is way safer.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

CoverletConstants.DefaultExcludeFilter will be always present

// so add default exclusions
if (excludeFilters.Count == 1)
{
excludeFilters.Add("[xunit*]*");
}

return excludeFilters.ToArray();
}

Expand Down
6 changes: 6 additions & 0 deletions src/coverlet.console/Program.cs
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,12 @@ static int Main(string[] args)
logger.Level = verbosity.ParsedValue;
}

// We add default exclusion filter if no specified
if (excludeFilters.Values.Count == 0)
Copy link
Contributor

@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.

Just curious, why did we chose to include if Count == 0 instead of excludeFilters.Values.Contains("[xunit*]*")?
Since i cannot think an scenario in which we want to include this dlls, and maybe we forgot to include it in our custom exclude filters. Or we missincluded it using the wrong wildcards or whatever
@MarcoRossignoli

Copy link
Contributor

Choose a reason for hiding this comment

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

Comment says "We add default exclusion filter if no specified" but you are not adding the CoverletConstants. DefaultExcludeFilter here, but the xunit one, which i find confusing

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Since i cannot think an scenario in which we want to include this dlls

The idea is to remove only in default config...after first include users win.

Copy link
Contributor

@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.

When are you adding the CoverletConstants. DefaultExcludeFilter btw?
Not sure about the code, but if this later goes into CoverletSettingsParser then it will never reach the code you added in that file (the if statement), since you add this before reaching that part, and then all it does is add the constants, so count is always 2
If you pass some custom exclude filters as argument, here you don't add xunit, and later count will be >1 , then your if statement is also not excecuted...

Copy link
Collaborator Author

@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.

CoverletConstants.DefaultExcludeFilter is needed only by collector.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Today we've 3 entry point/driver for coverlet console/msbuild/collector they behave in a different way on defaults.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

check manual tests FYI
#462 (comment)
#462 (comment)

{
excludeFilters.Values.Add("[xunit*]*");
}

Coverage coverage = new Coverage(module.Value,
includeFilters.Values.ToArray(),
includeDirectories.Values.ToArray(),
Expand Down
6 changes: 6 additions & 0 deletions src/coverlet.msbuild.tasks/InstrumentationTask.cs
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,12 @@ public override bool Execute()
var excludedSourceFiles = _excludeByFile?.Split(',');
var excludeAttributes = _excludeByAttribute?.Split(',');

// We add default exclusion filter if no specified
if (excludeFilters is null || excludeFilters.Length == 0)
{
excludeFilters = new string[] { "[xunit*]*" };
}

Coverage coverage = new Coverage(_path, includeFilters, includeDirectories, excludeFilters, excludedSourceFiles, excludeAttributes, _includeTestAssembly, _singleHit, _mergeWith, _useSourceLink, _logger);
CoveragePrepareResult prepareResult = coverage.PrepareModules();
InstrumenterState = new TaskItem(System.IO.Path.GetTempFileName());
Expand Down
28 changes: 28 additions & 0 deletions test/coverlet.collector.tests/CoverletSettingsParserTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -78,5 +78,33 @@ private void CreateCoverletNodes(XmlDocument doc, XmlElement configElement, stri
node.InnerText = nodeValue;
configElement.AppendChild(node);
}

[Fact]
public void ParseShouldSkipXunitModulesIfEmptyExclude()
{
var testModules = new List<string> { "abc.dll" };

CoverletSettings coverletSettings = _coverletSettingsParser.Parse(null, testModules);

Assert.Equal("[coverlet.*]*", coverletSettings.ExcludeFilters[0]);
Assert.Equal("[xunit*]*", coverletSettings.ExcludeFilters[1]);
Assert.Equal(2, coverletSettings.ExcludeFilters.Length);
}

[Fact]
public void ParseShouldNotSkipXunitModulesIfNotEmptyExclude()
{
var testModules = new List<string> { "abc.dll" };
var doc = new XmlDocument();
var configElement = doc.CreateElement("Configuration");
this.CreateCoverletNodes(doc, configElement, CoverletConstants.ExcludeFiltersElementName, "[coverlet.*.tests?]*");

CoverletSettings coverletSettings = _coverletSettingsParser.Parse(configElement, testModules);

Assert.Equal("[coverlet.*]*", coverletSettings.ExcludeFilters[0]);
Assert.Equal("[coverlet.*.tests?]*", coverletSettings.ExcludeFilters[1]);
Assert.Equal(2, coverletSettings.ExcludeFilters.Length);
Assert.DoesNotContain("[xunit*]*", coverletSettings.ExcludeFilters);
}
}
}