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

Filter during discovery #205

Merged
merged 2 commits into from
May 2, 2020
Merged

Filter during discovery #205

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
9c651a8
Filter during discovery
nohwnd Mar 27, 2020
3092634
Remove spacing
nohwnd Mar 27, 2020
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
5 changes: 4 additions & 1 deletion src/xunit.runner.visualstudio/Sinks/VsDiscoverySink.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,13 +40,15 @@ public class VsDiscoverySink : IMessageSinkWithTypes, IVsDiscoverySink, IDisposa
readonly string source;
readonly List<ITestCase> testCaseBatch = new List<ITestCase>();
readonly TestPlatformContext testPlatformContext;
readonly TestCaseFilter testCaseFilter;

public VsDiscoverySink(string source,
ITestFrameworkDiscoverer discoverer,
LoggerHelper logger,
ITestCaseDiscoverySink discoverySink,
ITestFrameworkDiscoveryOptions discoveryOptions,
TestPlatformContext testPlatformContext,
TestCaseFilter testCaseFilter,
Func<bool> cancelThunk)
{
this.source = source;
Expand All @@ -55,6 +57,7 @@ public class VsDiscoverySink : IMessageSinkWithTypes, IVsDiscoverySink, IDisposa
this.discoverySink = discoverySink;
this.discoveryOptions = discoveryOptions;
this.testPlatformContext = testPlatformContext;
this.testCaseFilter = testCaseFilter;
this.cancelThunk = cancelThunk;

descriptorProvider = (discoverer as ITestCaseDescriptorProvider) ?? new DefaultTestCaseDescriptorProvider(discoverer);
Expand Down Expand Up @@ -205,7 +208,7 @@ private void SendExistingTestCases()
foreach (var descriptor in descriptors)
{
var vsTestCase = CreateVsTestCase(source, descriptor, logger, testPlatformContext);
if (vsTestCase != null)
if (vsTestCase != null && testCaseFilter.MatchTestCase(vsTestCase))
{
if (discoveryOptions.GetInternalDiagnosticMessagesOrDefault())
logger.LogWithSource(source, "Discovered test case '{0}' (ID = '{1}', VS FQN = '{2}')", descriptor.DisplayName, descriptor.UniqueID, vsTestCase.FullyQualifiedName);
Expand Down
59 changes: 57 additions & 2 deletions src/xunit.runner.visualstudio/Utility/TestCaseFilter.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
using System.Collections.Generic;
using System.IO;
using System.Linq;
using System.Reflection;
using Microsoft.VisualStudio.TestPlatform.ObjectModel;
using Microsoft.VisualStudio.TestPlatform.ObjectModel.Adapter;

Expand All @@ -14,8 +15,9 @@ public class TestCaseFilter

readonly HashSet<string> knownTraits;
List<string> supportedPropertyNames;
ITestCaseFilterExpression filterExpression;
readonly ITestCaseFilterExpression filterExpression;
readonly bool successfullyGotFilter;
readonly bool isDiscovery;

public TestCaseFilter(IRunContext runContext, LoggerHelper logger, string assemblyFileName, HashSet<string> knownTraits)
{
Expand All @@ -25,6 +27,16 @@ public TestCaseFilter(IRunContext runContext, LoggerHelper logger, string assemb
successfullyGotFilter = GetTestCaseFilterExpression(runContext, logger, assemblyFileName, out filterExpression);
}

public TestCaseFilter(IDiscoveryContext discoveryContext, LoggerHelper logger)
{
// Traits are not known at discovery time because we load them from tests
isDiscovery = true;
knownTraits = new HashSet<string>();
supportedPropertyNames = GetSupportedPropertyNames();

successfullyGotFilter = GetTestCaseFilterExpressionFromDiscoveryContext(discoveryContext, logger, out filterExpression);
}

public bool MatchTestCase(TestCase testCase)
{
if (!successfullyGotFilter)
Expand All @@ -44,7 +56,7 @@ public bool MatchTestCase(TestCase testCase)
public object PropertyProvider(TestCase testCase, string name)
{
// Traits filtering
if (knownTraits.Contains(name))
if (isDiscovery || knownTraits.Contains(name))
Copy link
Contributor Author

@nohwnd nohwnd Mar 28, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

@bradwilson I probably did not explain myself well with the trait filter. What I meant is that in the line above the filter checks for known traits. If this hashset is empty it will not consider any traits. I can either skip the Contains check as I do now, and make the filtering a bit slower, OR add to the known traits all traits from the current batch, before processing filtering that batch.

The latter solution seems to be optimal, but I would rather add to the known traits via method, than by passing a live reference to a hashset, because that seems more obvious and less fragile. Do you agree?

Copy link
Member

Choose a reason for hiding this comment

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

I don't obsess too much with optimization like this unless it's demonstrably poor. I usually rely on the .NET or Roslyn team to come back and tell me where they've hit performance issues. 😂

{
var result = new List<string>();

Expand Down Expand Up @@ -83,6 +95,49 @@ bool GetTestCaseFilterExpression(IRunContext runContext, LoggerHelper logger, st
}
}

bool GetTestCaseFilterExpressionFromDiscoveryContext(IDiscoveryContext discoveryContext, LoggerHelper logger, out ITestCaseFilterExpression filter)
{
filter = null;

if (discoveryContext is IRunContext runContext)
{
try
{
filter = runContext.GetTestCaseFilter(supportedPropertyNames, null);
return true;
}
catch (TestPlatformException e)
{
logger.LogWarning("Exception filtering tests: {0}", e.Message);
return false;
}
}
else
{
try
{
#if WINDOWS_UAP
// on UWP .Net Native Tool Chain we are not able to run methods via invoke, act like no filter was specified for UWP
#else
// GetTestCaseFilter is present on DiscoveryContext but not in IDiscoveryContext interface
var method = discoveryContext.GetType().GetRuntimeMethod("GetTestCaseFilter", new[] { typeof(IEnumerable<string>), typeof(Func<string, TestProperty>) });
filter = (ITestCaseFilterExpression)method?.Invoke(discoveryContext, new object[] { supportedPropertyNames, null });
#endif
return true;
}
catch (TargetInvocationException e)
{
if (e?.InnerException is TestPlatformException ex)
{
logger.LogWarning("Exception filtering tests: {0}", ex.InnerException?.Message);
return false;
}

throw e.InnerException;
}
}
}

List<string> GetSupportedPropertyNames()
{
// Returns the set of well-known property names usually used with the Test Plugins (Used Test Traits + DisplayName + FullyQualifiedName)
Expand Down
3 changes: 2 additions & 1 deletion src/xunit.runner.visualstudio/VsTestRunner.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,9 +101,10 @@ void ITestDiscoverer.DiscoverTests(IEnumerable<string> sources, IDiscoveryContex
RequireXunitTestProperty = true
};

var testCaseFilter = new TestCaseFilter(discoveryContext, loggerHelper);
DiscoverTests(
sources, loggerHelper, testPlatformContext, runSettings,
(source, discoverer, discoveryOptions) => new VsDiscoverySink(source, discoverer, loggerHelper, discoverySink, discoveryOptions, testPlatformContext, () => cancelled)
(source, discoverer, discoveryOptions) => new VsDiscoverySink(source, discoverer, loggerHelper, discoverySink, discoveryOptions, testPlatformContext, testCaseFilter, () => cancelled)
);
}

Expand Down