-
Notifications
You must be signed in to change notification settings - Fork 255
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
Adding filtering support at discovery #271
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,16 +1,16 @@ | ||
// Copyright (c) Microsoft Corporation. All rights reserved. | ||
// Licensed under the MIT license. See LICENSE file in the project root for full license information. | ||
|
||
namespace Microsoft.VisualStudio.TestPlatform.MSTest.TestAdapter.Execution | ||
namespace Microsoft.VisualStudio.TestPlatform.MSTest.TestAdapter | ||
{ | ||
using System; | ||
using System.Collections.Generic; | ||
using System.Diagnostics; | ||
using System.Linq; | ||
using System.Reflection; | ||
using Microsoft.VisualStudio.TestPlatform.ObjectModel; | ||
using Microsoft.VisualStudio.TestPlatform.ObjectModel.Adapter; | ||
using Microsoft.VisualStudio.TestPlatform.ObjectModel.Logging; | ||
using Constants = Microsoft.VisualStudio.TestPlatform.MSTest.TestAdapter.Constants; | ||
|
||
internal class TestMethodFilter | ||
{ | ||
|
@@ -34,24 +34,24 @@ internal TestMethodFilter() | |
/// <summary> | ||
/// Returns ITestCaseFilterExpression for TestProperties supported by adapter. | ||
/// </summary> | ||
/// <param name="runContext">The current context of the run.</param> | ||
/// <param name="testExecutionRecorder">Handler to report test messages/start/end and results.</param> | ||
/// <param name="context">The current context of the run.</param> | ||
/// <param name="logger">Handler to report test messages/start/end and results.</param> | ||
/// <param name="filterHasError">Indicates that the filter is unsupported/has an error.</param> | ||
/// <returns>A filter expression.</returns> | ||
internal ITestCaseFilterExpression GetFilterExpression(IRunContext runContext, IMessageLogger testExecutionRecorder, out bool filterHasError) | ||
internal ITestCaseFilterExpression GetFilterExpression(IDiscoveryContext context, IMessageLogger logger, out bool filterHasError) | ||
{ | ||
filterHasError = false; | ||
ITestCaseFilterExpression filter = null; | ||
if (runContext != null) | ||
if (context != null) | ||
{ | ||
try | ||
{ | ||
filter = runContext.GetTestCaseFilter(this.supportedProperties.Keys, this.PropertyProvider); | ||
filter = (context is IRunContext) ? this.GetTestCaseFilterFromRunContext(context as IRunContext) : this.GetTestCaseFilterFromDiscoveryContext(context); | ||
} | ||
catch (TestPlatformFormatException ex) | ||
{ | ||
filterHasError = true; | ||
testExecutionRecorder.SendMessage(TestMessageLevel.Error, ex.Message); | ||
logger.SendMessage(TestMessageLevel.Error, ex.Message); | ||
} | ||
} | ||
|
||
|
@@ -95,5 +95,34 @@ internal object PropertyValueProvider(TestCase currentTest, string propertyName) | |
|
||
return null; | ||
} | ||
|
||
/// <summary> | ||
/// Gets filter expression from run context. | ||
/// </summary> | ||
/// <param name="context">Run context</param> | ||
/// <returns>Filter expression.</returns> | ||
private ITestCaseFilterExpression GetTestCaseFilterFromRunContext(IRunContext context) | ||
{ | ||
return context.GetTestCaseFilter(this.supportedProperties.Keys, this.PropertyProvider); | ||
} | ||
|
||
/// <summary> | ||
/// Gets filter expression from discovery context. | ||
/// </summary> | ||
/// <param name="context">Discovery context</param> | ||
/// <returns>Filter expression.</returns> | ||
private ITestCaseFilterExpression GetTestCaseFilterFromDiscoveryContext(IDiscoveryContext context) | ||
{ | ||
try | ||
{ | ||
// GetTestCaseFilter is present in DiscoveryContext but not in IDiscoveryContext interface. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Was this present in Dev11 as well? Did we validate this scenario in Dev11/Dev12? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Validated in dev 12 |
||
MethodInfo methodGetTestCaseFilter = context.GetType().GetRuntimeMethod("GetTestCaseFilter", new[] { typeof(IEnumerable<string>), typeof(Func<string, TestProperty>) }); | ||
return (ITestCaseFilterExpression)methodGetTestCaseFilter?.Invoke(context, new object[] { this.supportedProperties.Keys, (Func<string, TestProperty>)this.PropertyProvider }); | ||
} | ||
catch (TargetInvocationException ex) | ||
{ | ||
throw ex.InnerException; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. and who would handle this if it isn't a TestPlatformException or for that matter a non-TargetInvocationException is thrown? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. platform handles it automatically. message is logged in case of exception by platform. |
||
} | ||
} | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This would not return anything. Is this the spec?
Should we log in that case?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I discussed with Pratap. Discovery should not happen in case filter has error