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.

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

Trim whitespace between values when reading from configuration from runsettings #679

Merged
merged 4 commits into from
Jan 15, 2020
Merged
Show file tree
Hide file tree
Changes from 3 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
23 changes: 16 additions & 7 deletions src/coverlet.collector/DataCollection/CoverletSettingsParser.cs
Original file line number Diff line number Diff line change
Expand Up @@ -86,8 +86,7 @@ private string[] ParseReportFormats(XmlElement configurationElement)
if (configurationElement != null)
{
XmlElement reportFormatElement = configurationElement[CoverletConstants.ReportFormatElementName];
formats = reportFormatElement?.InnerText?.Split(',').Select(format => format.Trim())
.Where(format => !string.IsNullOrEmpty(format)).ToArray();
formats = this.SplitElement(reportFormatElement);
}

return formats is null || formats.Length == 0 ? new[] { CoverletConstants.DefaultReportFormat } : formats;
Expand All @@ -101,7 +100,7 @@ private string[] ParseReportFormats(XmlElement configurationElement)
private string[] ParseIncludeFilters(XmlElement configurationElement)
{
XmlElement includeFiltersElement = configurationElement[CoverletConstants.IncludeFiltersElementName];
return includeFiltersElement?.InnerText?.Split(',');
return this.SplitElement(includeFiltersElement);
}

/// <summary>
Expand All @@ -112,7 +111,7 @@ private string[] ParseIncludeFilters(XmlElement configurationElement)
private string[] ParseIncludeDirectories(XmlElement configurationElement)
{
XmlElement includeDirectoriesElement = configurationElement[CoverletConstants.IncludeDirectoriesElementName];
return includeDirectoriesElement?.InnerText?.Split(',');
return this.SplitElement(includeDirectoriesElement);
}

/// <summary>
Expand All @@ -127,7 +126,7 @@ private string[] ParseExcludeFilters(XmlElement configurationElement)
if (configurationElement != null)
{
XmlElement excludeFiltersElement = configurationElement[CoverletConstants.ExcludeFiltersElementName];
string[] filters = excludeFiltersElement?.InnerText?.Split(',');
string[] filters = this.SplitElement(excludeFiltersElement);
if (filters != null)
{
excludeFilters.AddRange(filters);
Expand All @@ -145,7 +144,7 @@ private string[] ParseExcludeFilters(XmlElement configurationElement)
private string[] ParseExcludeSourceFiles(XmlElement configurationElement)
{
XmlElement excludeSourceFilesElement = configurationElement[CoverletConstants.ExcludeSourceFilesElementName];
return excludeSourceFilesElement?.InnerText?.Split(',');
return this.SplitElement(excludeSourceFilesElement);
}

/// <summary>
Expand All @@ -156,7 +155,7 @@ private string[] ParseExcludeSourceFiles(XmlElement configurationElement)
private string[] ParseExcludeAttributes(XmlElement configurationElement)
{
XmlElement excludeAttributesElement = configurationElement[CoverletConstants.ExcludeAttributesElementName];
return excludeAttributesElement?.InnerText?.Split(',');
return this.SplitElement(excludeAttributesElement);
}

/// <summary>
Expand Down Expand Up @@ -205,5 +204,15 @@ private bool ParseIncludeTestAssembly(XmlElement configurationElement)
bool.TryParse(includeTestAssemblyElement?.InnerText, out bool includeTestAssembly);
return includeTestAssembly;
}

/// <summary>
/// Splits a comma separated elements into an array
/// </summary>
/// <param name="element">The element to split</param>
/// <returns>An array of the values in the element</returns>
private string[] SplitElement(XmlElement element)
{
return element?.InnerText?.Split(',', StringSplitOptions.RemoveEmptyEntries).Where(value => !string.IsNullOrWhiteSpace(value)).Select(value => value.Trim()).ToArray();
}
}
}
83 changes: 76 additions & 7 deletions test/coverlet.collector.tests/CoverletSettingsParserTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -42,17 +42,33 @@ public void ParseShouldSelectFirstTestModuleFromTestModulesList()
Assert.Equal("module1.dll", coverletSettings.TestModule);
}

[Fact]
public void ParseShouldCorrectlyParseConfigurationElement()
[Theory]
[InlineData("[*]*,[coverlet]*", "[coverlet.*.tests?]*,[coverlet.*.tests.*]*", @"E:\temp,/var/tmp", "module1.cs,module2.cs", "Obsolete,GeneratedCodeAttribute,CompilerGeneratedAttribute")]
[InlineData("[*]*,,[coverlet]*", "[coverlet.*.tests?]*,,[coverlet.*.tests.*]*", @"E:\temp,,/var/tmp", "module1.cs,,module2.cs", "Obsolete,,GeneratedCodeAttribute,,CompilerGeneratedAttribute")]
[InlineData("[*]*, ,[coverlet]*", "[coverlet.*.tests?]*, ,[coverlet.*.tests.*]*", @"E:\temp, ,/var/tmp", "module1.cs, ,module2.cs", "Obsolete, ,GeneratedCodeAttribute, ,CompilerGeneratedAttribute")]
[InlineData("[*]*,\t,[coverlet]*", "[coverlet.*.tests?]*,\t,[coverlet.*.tests.*]*", @"E:\temp,\t,/var/tmp", "module1.cs,\t,module2.cs", "Obsolete,\t,GeneratedCodeAttribute,\t,CompilerGeneratedAttribute")]
[InlineData("[*]*, [coverlet]*", "[coverlet.*.tests?]*, [coverlet.*.tests.*]*", @"E:\temp, /var/tmp", "module1.cs, module2.cs", "Obsolete, GeneratedCodeAttribute, CompilerGeneratedAttribute")]
[InlineData("[*]*,\t[coverlet]*", "[coverlet.*.tests?]*,\t[coverlet.*.tests.*]*", "E:\\temp,\t/var/tmp", "module1.cs,\tmodule2.cs", "Obsolete,\tGeneratedCodeAttribute,\tCompilerGeneratedAttribute")]
[InlineData("[*]*, \t[coverlet]*", "[coverlet.*.tests?]*, \t[coverlet.*.tests.*]*", "E:\\temp, \t/var/tmp", "module1.cs, \tmodule2.cs", "Obsolete, \tGeneratedCodeAttribute, \tCompilerGeneratedAttribute")]
[InlineData("[*]*,\r\n[coverlet]*", "[coverlet.*.tests?]*,\r\n[coverlet.*.tests.*]*", "E:\\temp,\r\n/var/tmp", "module1.cs,\r\nmodule2.cs", "Obsolete,\r\nGeneratedCodeAttribute,\r\nCompilerGeneratedAttribute")]
MarcoRossignoli marked this conversation as resolved.
Show resolved Hide resolved
[InlineData("[*]*, \r\n [coverlet]*", "[coverlet.*.tests?]*, \r\n [coverlet.*.tests.*]*", "E:\\temp, \r\n /var/tmp", "module1.cs, \r\n module2.cs", "Obsolete, \r\n GeneratedCodeAttribute, \r\n CompilerGeneratedAttribute")]
[InlineData("[*]*,\t\r\n\t[coverlet]*", "[coverlet.*.tests?]*,\t\r\n\t[coverlet.*.tests.*]*", "E:\\temp,\t\r\n\t/var/tmp", "module1.cs,\t\r\n\tmodule2.cs", "Obsolete,\t\r\n\tGeneratedCodeAttribute,\t\r\n\tCompilerGeneratedAttribute")]
[InlineData("[*]*, \t \r\n \t [coverlet]*", "[coverlet.*.tests?]*, \t \r\n \t [coverlet.*.tests.*]*", "E:\\temp, \t \r\n \t /var/tmp", "module1.cs, \t \r\n \t module2.cs", "Obsolete, \t \r\n \t GeneratedCodeAttribute, \t \r\n \t CompilerGeneratedAttribute")]
[InlineData(" [*]* , [coverlet]* ", " [coverlet.*.tests?]* , [coverlet.*.tests.*]* ", " E:\\temp , /var/tmp ", " module1.cs , module2.cs ", " Obsolete , GeneratedCodeAttribute , CompilerGeneratedAttribute ")]
public void ParseShouldCorrectlyParseConfigurationElement(string includeFilters,
MarcoRossignoli marked this conversation as resolved.
Show resolved Hide resolved
string excludeFilters,
string includeDirectories,
string excludeSourceFiles,
string excludeAttributes)
{
var testModules = new List<string> { "abc.dll" };
var doc = new XmlDocument();
var configElement = doc.CreateElement("Configuration");
this.CreateCoverletNodes(doc, configElement, CoverletConstants.IncludeFiltersElementName, "[*]*");
this.CreateCoverletNodes(doc, configElement, CoverletConstants.ExcludeFiltersElementName, "[coverlet.*.tests?]*");
this.CreateCoverletNodes(doc, configElement, CoverletConstants.IncludeDirectoriesElementName, @"E:\temp");
this.CreateCoverletNodes(doc, configElement, CoverletConstants.ExcludeSourceFilesElementName, "module1.cs,module2.cs");
this.CreateCoverletNodes(doc, configElement, CoverletConstants.ExcludeAttributesElementName, "Obsolete,GeneratedCodeAttribute,CompilerGeneratedAttribute");
this.CreateCoverletNodes(doc, configElement, CoverletConstants.IncludeFiltersElementName, includeFilters);
this.CreateCoverletNodes(doc, configElement, CoverletConstants.ExcludeFiltersElementName, excludeFilters);
this.CreateCoverletNodes(doc, configElement, CoverletConstants.IncludeDirectoriesElementName, includeDirectories);
this.CreateCoverletNodes(doc, configElement, CoverletConstants.ExcludeSourceFilesElementName, excludeSourceFiles);
this.CreateCoverletNodes(doc, configElement, CoverletConstants.ExcludeAttributesElementName, excludeAttributes);
this.CreateCoverletNodes(doc, configElement, CoverletConstants.MergeWithElementName, "/path/to/result.json");
this.CreateCoverletNodes(doc, configElement, CoverletConstants.UseSourceLinkElementName, "false");
this.CreateCoverletNodes(doc, configElement, CoverletConstants.SingleHitElementName, "true");
Expand All @@ -62,22 +78,68 @@ public void ParseShouldCorrectlyParseConfigurationElement()

Assert.Equal("abc.dll", coverletSettings.TestModule);
Assert.Equal("[*]*", coverletSettings.IncludeFilters[0]);
Assert.Equal("[coverlet]*", coverletSettings.IncludeFilters[1]);
Assert.Equal(@"E:\temp", coverletSettings.IncludeDirectories[0]);
Assert.Equal("/var/tmp", coverletSettings.IncludeDirectories[1]);
Assert.Equal("module1.cs", coverletSettings.ExcludeSourceFiles[0]);
Assert.Equal("module2.cs", coverletSettings.ExcludeSourceFiles[1]);
Assert.Equal("Obsolete", coverletSettings.ExcludeAttributes[0]);
Assert.Equal("GeneratedCodeAttribute", coverletSettings.ExcludeAttributes[1]);
Assert.Equal("CompilerGeneratedAttribute", coverletSettings.ExcludeAttributes[2]);
Assert.Equal("/path/to/result.json", coverletSettings.MergeWith);
Assert.Equal("[coverlet.*]*", coverletSettings.ExcludeFilters[0]);
Assert.Equal("[coverlet.*.tests?]*", coverletSettings.ExcludeFilters[1]);
Assert.Equal("[coverlet.*.tests.*]*", coverletSettings.ExcludeFilters[2]);

Assert.False(coverletSettings.UseSourceLink);
Assert.True(coverletSettings.SingleHit);
Assert.True(coverletSettings.IncludeTestAssembly);
}

[Fact]
public void ParseShouldCorrectlyParseConfigurationElementWithNullInnerText()
{
var testModules = new List<string> { "abc.dll" };
var doc = new XmlDocument();
var configElement = doc.CreateElement("Configuration");
this.CreateCoverleteNullInnerTextNodes(doc, configElement, CoverletConstants.IncludeFiltersElementName);
this.CreateCoverleteNullInnerTextNodes(doc, configElement, CoverletConstants.ExcludeFiltersElementName);
this.CreateCoverleteNullInnerTextNodes(doc, configElement, CoverletConstants.IncludeDirectoriesElementName);
this.CreateCoverleteNullInnerTextNodes(doc, configElement, CoverletConstants.ExcludeSourceFilesElementName);
this.CreateCoverleteNullInnerTextNodes(doc, configElement, CoverletConstants.ExcludeAttributesElementName);

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

Assert.Equal("abc.dll", coverletSettings.TestModule);
Assert.Empty(coverletSettings.IncludeFilters);
Assert.Empty(coverletSettings.IncludeDirectories);
Assert.Empty(coverletSettings.ExcludeSourceFiles);
Assert.Empty(coverletSettings.ExcludeAttributes);
Assert.Single(coverletSettings.ExcludeFilters, "[coverlet.*]*");
}

[Fact]
public void ParseShouldCorrectlyParseConfigurationElementWithNullElements()
{
var testModules = new List<string> { "abc.dll" };
var doc = new XmlDocument();
var configElement = doc.CreateElement("Configuration");

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

Assert.Equal("abc.dll", coverletSettings.TestModule);
Assert.Null(coverletSettings.IncludeFilters);
Assert.Null(coverletSettings.IncludeDirectories);
Assert.Null(coverletSettings.ExcludeSourceFiles);
Assert.Null(coverletSettings.ExcludeAttributes);
Assert.Single(coverletSettings.ExcludeFilters, "[coverlet.*]*");
}

[Theory]
[InlineData(" , json", 1, new[] { "json" })]
[InlineData(" , json, ", 1, new[] { "json" })]
[InlineData("json,cobertura", 2, new[] { "json", "cobertura" })]
[InlineData("json,\r\ncobertura", 2, new[] { "json", "cobertura" })]
MarcoRossignoli marked this conversation as resolved.
Show resolved Hide resolved
[InlineData(" , json,, cobertura ", 2, new[] { "json", "cobertura" })]
[InlineData(" , json, , cobertura ", 2, new[] { "json", "cobertura" })]
public void ParseShouldCorrectlyParseMultipleFormats(string formats, int formatsCount, string[] expectedReportFormats)
Expand Down Expand Up @@ -115,5 +177,12 @@ private void CreateCoverletNodes(XmlDocument doc, XmlElement configElement, stri
node.InnerText = nodeValue;
configElement.AppendChild(node);
}

private void CreateCoverleteNullInnerTextNodes(XmlDocument doc, XmlElement configElement, string nodeSetting)
{
var node = doc.CreateNode("element", nodeSetting, string.Empty);
node.InnerText = null;
configElement.AppendChild(node);
}
}
}