-
Notifications
You must be signed in to change notification settings - Fork 262
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
Add ability to ignore specific test cases #4457
base: main
Are you sure you want to change the base?
Changes from 3 commits
158cba5
1bae8e7
4617364
e115201
b85a392
4fa10cb
bbae399
dd75de4
e9a277e
a494d2b
ba23fad
bc9adae
e9ca889
a10d9c3
95fbe37
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 | ||||||||
---|---|---|---|---|---|---|---|---|---|---|
|
@@ -9,7 +9,7 @@ namespace Microsoft.VisualStudio.TestTools.UnitTesting; | |||||||||
/// Attribute to define in-line data for a test method. | ||||||||||
/// </summary> | ||||||||||
[AttributeUsage(AttributeTargets.Method, AllowMultiple = true)] | ||||||||||
public class DataRowAttribute : Attribute, ITestDataSource, ITestDataSourceUnfoldingCapability | ||||||||||
public class DataRowAttribute : Attribute, ITestDataSource, ITestDataSourceUnfoldingCapability, ITestDataSourceIgnoreCapability | ||||||||||
{ | ||||||||||
/// <summary> | ||||||||||
/// Initializes a new instance of the <see cref="DataRowAttribute"/> class. | ||||||||||
|
@@ -53,6 +53,11 @@ public DataRowAttribute(string?[]? stringArrayData) | |||||||||
/// </summary> | ||||||||||
public string? DisplayName { get; set; } | ||||||||||
|
||||||||||
/// <summary> | ||||||||||
/// Gets or sets a reason to ignore the specific test case. Setting the property to non-null value will ignore the test case. | ||||||||||
/// </summary> | ||||||||||
Comment on lines
+56
to
+58
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.
Suggested change
|
||||||||||
public string? Ignore { get; set; } | ||||||||||
|
||||||||||
/// <inheritdoc /> | ||||||||||
public TestDataSourceUnfoldingStrategy UnfoldingStrategy { get; set; } = TestDataSourceUnfoldingStrategy.Auto; | ||||||||||
|
||||||||||
|
Original file line number | Diff line number | Diff line change | ||||||||
---|---|---|---|---|---|---|---|---|---|---|
|
@@ -32,7 +32,7 @@ public enum DynamicDataSourceType | |||||||||
/// Attribute to define dynamic data for a test method. | ||||||||||
/// </summary> | ||||||||||
[AttributeUsage(AttributeTargets.Method, AllowMultiple = true)] | ||||||||||
public sealed class DynamicDataAttribute : Attribute, ITestDataSource, ITestDataSourceEmptyDataSourceExceptionInfo, ITestDataSourceUnfoldingCapability | ||||||||||
public sealed class DynamicDataAttribute : Attribute, ITestDataSource, ITestDataSourceEmptyDataSourceExceptionInfo, ITestDataSourceUnfoldingCapability, ITestDataSourceIgnoreCapability | ||||||||||
{ | ||||||||||
private readonly string _dynamicDataSourceName; | ||||||||||
private readonly DynamicDataSourceType _dynamicDataSourceType; | ||||||||||
|
@@ -114,6 +114,11 @@ public DynamicDataAttribute(string dynamicDataSourceName, Type dynamicDataDeclar | |||||||||
/// <inheritdoc /> | ||||||||||
public TestDataSourceUnfoldingStrategy UnfoldingStrategy { get; set; } = TestDataSourceUnfoldingStrategy.Auto; | ||||||||||
|
||||||||||
/// <summary> | ||||||||||
/// Gets or sets a reason to ignore this dynamic data source. Setting the property to non-null value will ignore the dynamic data source. | ||||||||||
/// </summary> | ||||||||||
Comment on lines
+117
to
+119
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.
Suggested change
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. I was intentionally not inheriting as I'm adjusting the wording a little bit for the specific concrete implementation. It's not super important though as the wording from the interface should still be clear enough for the concrete implementations. |
||||||||||
public string? Ignore { get; set; } | ||||||||||
|
||||||||||
/// <inheritdoc /> | ||||||||||
public IEnumerable<object[]> GetData(MethodInfo methodInfo) | ||||||||||
=> DynamicDataProvider.Instance.GetData(_dynamicDataDeclaringType, _dynamicDataSourceType, _dynamicDataSourceName, methodInfo); | ||||||||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,15 @@ | ||
// 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.TestTools.UnitTesting; | ||
|
||
/// <summary> | ||
/// Specifies the capability of a test data source to be ignored and define the ignore reason. | ||
/// </summary> | ||
public interface ITestDataSourceIgnoreCapability | ||
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. Could it be worth making this one more generic (not linked to data sources) so we can apply it to test methods and test classes too? 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. @Evangelink The impl is quite different, at least currently. For example, for specific test cases, we need to have a VSTest property for that to restore the associated ignore message. But we don't need that for when it's the whole test method or test class being ignored. |
||
{ | ||
/// <summary> | ||
/// Gets or sets a reason to ignore the test data source. Setting the property to non-null value will ignore the test data source. | ||
/// </summary> | ||
string? Ignore { get; set; } | ||
} |
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.
It's probably good to have more states but I will have a closer look later.
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.
Sure. Let's discuss later