Skip to content

Commit

Permalink
review fixes
Browse files Browse the repository at this point in the history
  • Loading branch information
lahma committed Apr 23, 2023
1 parent 15289ef commit ba694d5
Show file tree
Hide file tree
Showing 10 changed files with 30 additions and 8 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -344,6 +344,7 @@ private static int CountTestCases(ITest test, ITestFilter filter)
if (!test.IsSuite)
return filter.Pass(test) ? 1: 0;

// Use for-loop to avoid allocating the enumerator
int count = 0;
var tests = test.Tests;
for (var i = 0; i < tests.Count; i++)
Expand Down
4 changes: 4 additions & 0 deletions src/NUnitFramework/framework/Internal/Filters/AndFilter.cs
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,8 @@ internal sealed class AndFilter : CompositeFilter
/// <returns>True if all the component filters pass, otherwise false</returns>
public override bool Pass( ITest test, bool negated )
{
// Use foreach-loop against array instead of LINQ for best performance

if (negated)
{
foreach (var filter in Filters)
Expand Down Expand Up @@ -61,6 +63,7 @@ public override bool Pass( ITest test, bool negated )
/// <returns>True if all the component filters match, otherwise false</returns>
public override bool Match( ITest test )
{
// Use foreach-loop against array instead of LINQ for best performance
foreach (var filter in Filters)
{
if (!filter.Match(test))
Expand All @@ -79,6 +82,7 @@ public override bool Match( ITest test )
/// <returns>True if all the component filters explicit match, otherwise false</returns>
public override bool IsExplicitMatch( ITest test )
{
// Use foreach-loop against array instead of LINQ for best performance
foreach (var filter in Filters)
{
if (!filter.IsExplicitMatch(test))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ public override bool Match(ITest test)
{
if (test.Properties.TryGet(PropertyNames.Category, out var testCategories))
{
// Use for-loop to avoid allocating the enumerator
for (var i = 0; i < testCategories.Count; ++i)
{
if (Match((string) testCategories[i]))
Expand Down
4 changes: 2 additions & 2 deletions src/NUnitFramework/framework/Internal/Filters/InFilter.cs
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ public static bool TryOptimize(OrFilter orFilter, [NotNullWhen(true)] out InFilt
// eagerly build expected value collection
var expectedValues = new List<string?>(orFilter.Filters.Length);

// use for for faster traversal without boxed enumerator
// Use foreach-loop against array instead of LINQ for best performance
foreach (var filter in orFilter.Filters)
{
// make sure invariants are valid
Expand Down Expand Up @@ -79,7 +79,7 @@ public static bool TryOptimize(OrFilter orFilter, [NotNullWhen(true)] out InFilt
NamespaceFilter _ => test => test.TypeInfo?.Namespace,
TestNameFilter _ => test => test.Name,
_ => throw new InvalidOperationException("Invalid filter, logic wrong")
};
};
string xmlElementName = orFilter.Filters[0] switch
{
ClassNameFilter _ => ClassNameFilter.XmlElementName,
Expand Down
4 changes: 4 additions & 0 deletions src/NUnitFramework/framework/Internal/Filters/OrFilter.cs
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,8 @@ public OrFilter(params TestFilter[] filters) : base(filters)
/// <returns>True if any of the component filters pass, otherwise false</returns>
public override bool Pass( ITest test, bool negated )
{
// Use foreach-loop against array instead of LINQ for best performance

if (negated)
{
foreach (var filter in Filters)
Expand Down Expand Up @@ -65,6 +67,7 @@ public override bool Pass( ITest test, bool negated )
/// <returns>True if any of the component filters match, otherwise false</returns>
public override bool Match( ITest test )
{
// Use foreach-loop against array instead of LINQ for best performance
foreach (var filter in Filters)
{
if (filter.Match(test))
Expand All @@ -83,6 +86,7 @@ public override bool Match( ITest test )
/// <returns>True if any of the component filters explicit match, otherwise false</returns>
public override bool IsExplicitMatch( ITest test )
{
// Use foreach-loop against array instead of LINQ for best performance
foreach (var filter in Filters)
{
if (filter.IsExplicitMatch(test))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ public override bool Match(ITest test)
{
if (test.Properties.TryGet(_propertyName, out var values))
{
// Use for-loop to avoid allocating the enumerator
for (var i = 0; i < values.Count; ++i)
{
if (Match((string) values[i]))
Expand Down
1 change: 1 addition & 0 deletions src/NUnitFramework/framework/Internal/PreFilter.cs
Original file line number Diff line number Diff line change
Expand Up @@ -198,6 +198,7 @@ private bool MatchMethodElement(Type type)

private bool MatchSetUpFixture(Type type)
{
// checking length instead of for example LINQ .Any(), which would need to box array into IEnumerable
return IsSubNamespace(type.Namespace) &&
type.GetCustomAttributes(typeof(SetUpFixtureAttribute), true).Length > 0;
}
Expand Down
1 change: 1 addition & 0 deletions src/NUnitFramework/framework/Internal/PropertyBag.cs
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,7 @@ public TNode AddToXml(TNode parentNode, bool recursive)
{
TNode properties = parentNode.AddElement("properties");

// enumerating dictionary directly with struct enumerator which is fastest
foreach (var pair in inner)
{
foreach (var value in pair.Value)
Expand Down
6 changes: 3 additions & 3 deletions src/NUnitFramework/framework/Internal/TestFilter.cs
Original file line number Diff line number Diff line change
Expand Up @@ -111,11 +111,11 @@ protected virtual bool MatchDescendant(ITest test)
/// </summary>
public static TestFilter FromXml(string xmlText)
{
const string emptyFilterXml1 = "<filter />";
const string emptyFilterXml2 = "<filter/>";
const string emptyFilterXmlWithSpace = "<filter />";
const string emptyFilterWithoutSpace = "<filter/>";

// check for fast cases
if (string.IsNullOrEmpty(xmlText) || xmlText.Length < 11 && xmlText is emptyFilterXml1 or emptyFilterXml2)
if (string.IsNullOrEmpty(xmlText) || xmlText.Length < 11 && xmlText is emptyFilterXmlWithSpace or emptyFilterWithoutSpace)
{
return Empty;
}
Expand Down
15 changes: 12 additions & 3 deletions src/NUnitFramework/framework/Internal/Tests/TestSuite.cs
Original file line number Diff line number Diff line change
Expand Up @@ -222,9 +222,11 @@ public override int TestCaseCount
{
int count = 0;

for (var i = 0; i < Tests.Count; i++)
// Use for-loop to avoid allocating the enumerator
var testsToCheck = Tests;
for (var i = 0; i < testsToCheck.Count; i++)
{
count += Tests[i].TestCaseCount;
count += testsToCheck[i].TestCaseCount;
}

return count;
Expand Down Expand Up @@ -293,8 +295,15 @@ public override TNode AddToXml(TNode parentNode, bool recursive)


if (recursive)
foreach (Test test in this.Tests)
{
// Use for-loop to avoid allocating the enumerator
var testsToAdd = Tests;
for (var i = 0; i < testsToAdd.Count; i++)
{
var test = testsToAdd[i];
test.AddToXml(thisNode, recursive);
}
}

return thisNode;
}
Expand Down

0 comments on commit ba694d5

Please sign in to comment.