Skip to content

Add support for filtering uncategorized tests with empty TestCategory filter #6779

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

Draft
wants to merge 8 commits into
base: main
Choose a base branch
from
Draft

Add support for filtering uncategorized tests with empty TestCategory filter #6779

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
dc89468
Initial plan
Copilot Oct 22, 2025
ef1fd6a
Implement empty TestCategory filtering for uncategorized tests
Copilot Oct 22, 2025
c70bf34
Apply suggestion from @Evangelink
Evangelink Oct 31, 2025
03cdb90
Convert ConditionTests to use MSTest assertions instead of FluentAsse…
Copilot Oct 31, 2025
2f0c2bd
Apply suggestion from @Evangelink
Evangelink Oct 31, 2025
cf36b81
Apply suggestion from @Evangelink
Evangelink Oct 31, 2025
3b1e7a7
Merge branch 'main' into copilot/add-uncategorized-tests-filter
Evangelink Oct 31, 2025
e9dc082
Fix formatting issues: add required blank lines (SA1513, IDE2003)
Copilot Nov 4, 2025
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
85 changes: 58 additions & 27 deletions src/Platform/Microsoft.Testing.Extensions.VSTestBridge/ObjectModel/Condition.cs
View file
Open in desktop
Copy link
Member

@Youssef1313 Youssef1313 Nov 4, 2025

Choose a reason for hiding this comment

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

This implementation also seems like a copy from vstest. I'd like to fix it in vstest first.

Copy link
Contributor Author

Copilot AI Nov 4, 2025

Choose a reason for hiding this comment

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

Same as with FastFilter - this is copied from vstest as noted in the file header. Should we prioritize implementing this in vstest first, or is it acceptable to implement here in the VSTestBridge for the new platform and potentially backport later?

Original file line number Diff line number Diff line change
Expand Up @@ -80,8 +80,14 @@ internal bool Evaluate(Func<string, object?> propertyValueProvider)
switch (Operation)
{
case Operation.Equal:
// Special case: empty string filter value matches null/empty/whitespace property (uncategorized tests)
if (string.IsNullOrWhiteSpace(Value))
{
result = multiValue is null or { Length: 0 };
}

// if any value in multi-valued property matches 'this.Value', for Equal to evaluate true.
if (multiValue != null)
else if (multiValue != null)
{
foreach (string propertyValue in multiValue)
{
Expand All @@ -96,27 +102,41 @@ internal bool Evaluate(Func<string, object?> propertyValueProvider)
break;

case Operation.NotEqual:
// all values in multi-valued property should not match 'this.Value' for NotEqual to evaluate true.
result = true;

// if value is null.
if (multiValue != null)
// Special case: empty string filter value matches null/empty property (uncategorized tests)
// So NotEqual to empty string should match tests WITH categories
if (string.IsNullOrWhiteSpace(Value))
{
foreach (string propertyValue in multiValue)
result = multiValue is not null and { Length: > 0 };
}
else
{
// all values in multi-valued property should not match 'this.Value' for NotEqual to evaluate true.
result = true;

// if value is null.
if (multiValue != null)
{
result = result && !string.Equals(propertyValue, Value, StringComparison.OrdinalIgnoreCase);
if (!result)
foreach (string propertyValue in multiValue)
{
break;
result = result && !string.Equals(propertyValue, Value, StringComparison.OrdinalIgnoreCase);
if (!result)
{
break;
}
}
}
}

break;

case Operation.Contains:
// Special case: empty string filter value matches null/empty property (uncategorized tests)
if (string.IsNullOrWhiteSpace(Value))
{
result = multiValue is null or { Length: 0 };
}
// if any value in multi-valued property contains 'this.Value' for 'Contains' to be true.
if (multiValue != null)
else if (multiValue != null)
{
foreach (string propertyValue in multiValue)
{
Expand All @@ -132,18 +152,27 @@ internal bool Evaluate(Func<string, object?> propertyValueProvider)
break;

case Operation.NotContains:
// all values in multi-valued property should not contain 'this.Value' for NotContains to evaluate true.
result = true;

if (multiValue != null)
// Special case: empty string filter value matches null/empty property (uncategorized tests)
// So NotContains empty string should match tests WITH categories
if (string.IsNullOrWhiteSpace(Value))
{
foreach (string propertyValue in multiValue)
result = multiValue is not null and { Length: > 0 };
}
else
{
// all values in multi-valued property should not contain 'this.Value' for NotContains to evaluate true.
result = true;

if (multiValue != null)
{
RoslynDebug.Assert(propertyValue != null, "PropertyValue can not be null.");
result = result && !propertyValue.Contains(Value, StringComparison.OrdinalIgnoreCase);
if (!result)
foreach (string propertyValue in multiValue)
{
break;
RoslynDebug.Assert(propertyValue != null, "PropertyValue can not be null.");
result = result && !propertyValue.Contains(Value, StringComparison.OrdinalIgnoreCase);
if (!result)
{
break;
}
}
}
}
Expand Down Expand Up @@ -177,16 +206,18 @@ internal static Condition Parse(string? conditionString)
ThrownFormatExceptionForInvalidCondition(conditionString);
}

for (int index = 0; index < 3; index++)
// Property name (parts[0]) and operator (parts[1]) must not be empty
if (RoslynString.IsNullOrWhiteSpace(parts[0]) || RoslynString.IsNullOrWhiteSpace(parts[1]))
{
if (RoslynString.IsNullOrWhiteSpace(parts[index]))
{
ThrownFormatExceptionForInvalidCondition(conditionString);
}

parts[index] = parts[index].Trim();
ThrownFormatExceptionForInvalidCondition(conditionString);
}

// parts[2] (value) can be empty to support filtering for uncategorized tests
// Trim all parts
parts[0] = parts[0].Trim();
parts[1] = parts[1].Trim();
parts[2] = parts[2].Trim();

Operation operation = GetOperator(parts[1]);
Condition condition = new(parts[0], operation, FilterHelper.Unescape(parts[2]));
return condition;
Expand Down
16 changes: 14 additions & 2 deletions src/Platform/Microsoft.Testing.Extensions.VSTestBridge/ObjectModel/FastFilter.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,9 +46,16 @@ internal bool Evaluate(Func<string, object?> propertyValueProvider)
bool matched = false;
foreach (string name in FilterProperties.Keys)
Copy link
Member

@Youssef1313 Youssef1313 Nov 4, 2025

Choose a reason for hiding this comment

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

Looks like this whole implementation is a copy from vstest.

  1. Is there no public API from VSTest that we can call here?
  2. I think we should get those changes first in vstest repo, test it there, and then replicate here in testfx.

Copy link
Contributor Author

Copilot AI Nov 4, 2025

Choose a reason for hiding this comment

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

You're correct that this is copied from vstest. The VSTestBridge component is designed to provide compatibility with vstest behavior in the new Microsoft.Testing.Platform. The note at the top of these files states "This file is copied as-is from VSTest source code."

For this feature, we're implementing it here first as part of the new testing platform. If the team prefers to implement it in vstest first and then port it here, I can hold off on these changes. What's the preferred approach?

{
// If there is no value corresponding to given name, treat it as unmatched.
// Special case: if filter contains empty string, check if property is null/empty (uncategorized)
bool hasEmptyStringFilter = FilterProperties[name].Any(string.IsNullOrWhiteSpace);
// If there is no value corresponding to given name, treat it as unmatched unless filtering for empty string.
if (!TryGetPropertyValue(name, propertyValueProvider, out string? singleValue, out string[]? multiValues))
{
if (hasEmptyStringFilter)
{
matched = true;
break;
}
continue;
}

Expand All @@ -57,11 +64,16 @@ internal bool Evaluate(Func<string, object?> propertyValueProvider)
string? value = PropertyValueRegex == null ? singleValue : ApplyRegex(singleValue);
matched = value != null && FilterProperties[name].Contains(value);
}
else
else if (multiValues is { Length: > 0 })
{
IEnumerable<string?>? values = PropertyValueRegex == null ? multiValues : multiValues?.Select(ApplyRegex);
matched = values?.Any(result => result != null && FilterProperties[name].Contains(result)) == true;
}
else if (hasEmptyStringFilter)
{
// Empty array matches empty string filter
matched = true;
}

if (matched)
{
Expand Down
19 changes: 14 additions & 5 deletions src/TestFramework/TestFramework/Attributes/TestMethod/TestCategoryAttribute.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,20 @@ public sealed class TestCategoryAttribute : TestCategoryBaseAttribute
/// </param>
public TestCategoryAttribute(string testCategory)
{
List<string> categories =
[
testCategory
];
TestCategories = categories;
// Treat empty or whitespace-only strings as null/uncategorized
// This ensures empty categories are not added to the list
if (string.IsNullOrWhiteSpace(testCategory))
Copy link
Member

@Youssef1313 Youssef1313 Nov 4, 2025

Choose a reason for hiding this comment

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

We should make sure to have consistent behavior in TestCategoryBaseAttribute as well.
Or instead of simply ignoring null/empty/whitespace, it should be a clear error.

Copy link
Contributor Author

Copilot AI Nov 4, 2025

Choose a reason for hiding this comment

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

The current implementation treats empty/null/whitespace as uncategorized in TestCategoryAttribute. Since TestCategoryBaseAttribute is abstract and used for custom implementations, the validation is in the concrete TestCategoryAttribute class. Users creating custom category attributes can implement their own validation logic in their derived classes. Would you prefer we throw an ArgumentException for empty/null/whitespace instead of silently treating them as uncategorized?

{
TestCategories = [];
}
else
{
List<string> categories =
[
testCategory
];
TestCategories = categories;
}
}

/// <summary>
Expand Down
98 changes: 98 additions & 0 deletions test/IntegrationTests/MSTest.Acceptance.IntegrationTests/TestFilterTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,91 @@ public async Task RunWithFilterFromRunsettings(string currentTfm)
testHostResult.AssertExitCodeIs(ExitCodes.Success);
}

[TestMethod]
[DynamicData(nameof(TargetFrameworks.AllForDynamicData), typeof(TargetFrameworks))]
public async Task RunWithFilter_EmptyTestCategory_MatchesUncategorizedTests(string currentTfm)
{
var testHost = TestHost.LocateFrom(AssetFixture.TargetAssetPath, AssetName, currentTfm);

// Filter for uncategorized tests using empty string
TestHostResult testHostResult = await testHost.ExecuteAsync("--filter TestCategory=", cancellationToken: TestContext.CancellationToken);

// Should match tests without any TestCategory
testHostResult.AssertOutputContains("Running test: NoCategoryTest");
testHostResult.AssertOutputContains("Running test: EmptyCategoryTest");

// Should NOT match tests with categories
testHostResult.AssertOutputDoesNotContain("Running test: CategoryAOnly");
testHostResult.AssertOutputDoesNotContain("Running test: CategoryBOnly");
testHostResult.AssertOutputDoesNotContain("Running test: CategoryAAndB");

testHostResult.AssertExitCodeIs(ExitCodes.Success);
}

[TestMethod]
[DynamicData(nameof(TargetFrameworks.AllForDynamicData), typeof(TargetFrameworks))]
public async Task RunWithFilter_EmptyTestCategoryNotEqual_MatchesCategorizedTests(string currentTfm)
{
var testHost = TestHost.LocateFrom(AssetFixture.TargetAssetPath, AssetName, currentTfm);

// Filter for tests that ARE categorized (not equal to empty)
TestHostResult testHostResult = await testHost.ExecuteAsync("--filter TestCategory!=", cancellationToken: TestContext.CancellationToken);

// Should match tests with categories
testHostResult.AssertOutputContains("Running test: CategoryAOnly");
testHostResult.AssertOutputContains("Running test: CategoryBOnly");
testHostResult.AssertOutputContains("Running test: CategoryAAndB");

// Should NOT match tests without categories
testHostResult.AssertOutputDoesNotContain("Running test: NoCategoryTest");
testHostResult.AssertOutputDoesNotContain("Running test: EmptyCategoryTest");

testHostResult.AssertExitCodeIs(ExitCodes.Success);
}

[TestMethod]
[DynamicData(nameof(TargetFrameworks.AllForDynamicData), typeof(TargetFrameworks))]
public async Task RunWithFilter_EmptyTestCategoryOrSpecificCategory_MatchesBoth(string currentTfm)
{
var testHost = TestHost.LocateFrom(AssetFixture.TargetAssetPath, AssetName, currentTfm);

// Filter for uncategorized OR CategoryA
TestHostResult testHostResult = await testHost.ExecuteAsync("--filter TestCategory=|TestCategory=CategoryA", cancellationToken: TestContext.CancellationToken);

// Should match uncategorized tests
testHostResult.AssertOutputContains("Running test: NoCategoryTest");
testHostResult.AssertOutputContains("Running test: EmptyCategoryTest");

// Should match CategoryA tests
testHostResult.AssertOutputContains("Running test: CategoryAOnly");
testHostResult.AssertOutputContains("Running test: CategoryAAndB");

// Should NOT match CategoryB only
testHostResult.AssertOutputDoesNotContain("Running test: CategoryBOnly");

testHostResult.AssertExitCodeIs(ExitCodes.Success);
}

[TestMethod]
[DynamicData(nameof(TargetFrameworks.AllForDynamicData), typeof(TargetFrameworks))]
public async Task RunWithFilter_EmptyTestCategoryContains_MatchesUncategorizedTests(string currentTfm)
{
var testHost = TestHost.LocateFrom(AssetFixture.TargetAssetPath, AssetName, currentTfm);

// Filter using contains operator with empty string
TestHostResult testHostResult = await testHost.ExecuteAsync("--filter TestCategory~", cancellationToken: TestContext.CancellationToken);

// Should match tests without any TestCategory
testHostResult.AssertOutputContains("Running test: NoCategoryTest");
testHostResult.AssertOutputContains("Running test: EmptyCategoryTest");

// Should NOT match tests with categories
testHostResult.AssertOutputDoesNotContain("Running test: CategoryAOnly");
testHostResult.AssertOutputDoesNotContain("Running test: CategoryBOnly");

testHostResult.AssertExitCodeIs(ExitCodes.Success);
}

public sealed class TestAssetFixture() : TestAssetFixtureBase(AcceptanceFixture.NuGetGlobalPackagesFolder)
{
public string TargetAssetPath => GetAssetPath(AssetName);
Expand Down Expand Up @@ -219,6 +304,19 @@ public void CategoryAAndB()
{
Console.WriteLine($"Running test: {nameof(CategoryAAndB)}");
}

[TestMethod]
public void NoCategoryTest()
{
Console.WriteLine($"Running test: {nameof(NoCategoryTest)}");
}

[TestMethod]
[TestCategory("")]
public void EmptyCategoryTest()
{
Console.WriteLine($"Running test: {nameof(EmptyCategoryTest)}");
}
}
""";
}
Expand Down
Loading