Add ITestFilter / [TestFilterProvider] for programmatic test filtering (#8894)#8896
Draft
Evangelink wants to merge 4 commits into
Draft
Add ITestFilter / [TestFilterProvider] for programmatic test filtering (#8894)#8896Evangelink wants to merge 4 commits into
Evangelink wants to merge 4 commits into
Conversation
#8894) Introduces a new opt-in extension point so users can plug in their own filtering logic without paying for [AssemblyInitialize] / [ClassInitialize] on tests that will be dropped or skipped: - New attribute Microsoft.VisualStudio.TestTools.UnitTesting.TestFilterProviderAttribute (assembly-level, AllowMultiple = true) declares an ITestFilter implementation. - New interface ITestFilter with single method Filter(TestFilterContext) returning a readonly struct TestFilterResult (Run / Drop / Skip(reason)). - TestFilterContext exposes the data available before any type is loaded: FullyQualifiedName, DisplayName, TestClassName, TestMethodName, Categories, Traits and Priority. Discovery reuses the assembly-graph BFS already powering [AssemblyFixtureProvider] (TypeCache.ProviderDiscovery.cs). Filters are cached per source assembly via TypeCache.GetOrLoadTestFilters and only instantiated once per run. Invocation happens at the very top of UnitTestRunner.RunSingleTestAsync, BEFORE GetTestMethodInfo, so a Drop pays zero AssemblyInit/ClassInit/type-load cost. A Skip(reason) is surfaced as a regular Ignored test result. Filters are composed with AND (first non-Run wins). Filter exceptions become an Error test result so they can't silently affect test selection. Also adds an _assemblyInitializeWasExecuted flag on UnitTestRunner so end-of- assembly cleanup still runs when the last test of an assembly is filtered out. Refs #8894 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Adds an opt-in, programmatic filtering extension point to MSTest via a new [assembly: TestFilterProvider] attribute and ITestFilter interface, allowing tests to be dropped or skipped before any test type is loaded or any assembly/class initialization runs (reducing upfront cost for filtered-out tests).
Changes:
- Introduces new public filtering API surface in
Microsoft.VisualStudio.TestTools.UnitTesting(ITestFilter,TestFilterContext,TestFilterResult,TestFilterAction,TestFilterProviderAttribute) and registers it inPublicAPI.Unshipped.txt. - Implements adapter-side discovery/caching of filter providers (BFS over referenced assemblies) and applies filters at the start of
UnitTestRunner.RunSingleTestAsync. - Adds localized diagnostics/resources for filter-provider discovery/instantiation/invocation failures (UTA073–UTA078).
Show a summary per file
| File | Description |
|---|---|
| src/TestFramework/TestFramework/PublicAPI/PublicAPI.Unshipped.txt | Declares newly introduced public APIs for the filtering feature. |
| src/TestFramework/TestFramework/Filtering/ITestFilter.cs | New interface that users implement to decide Run/Drop/Skip per test. |
| src/TestFramework/TestFramework/Filtering/TestFilterContext.cs | New metadata-only context passed to filters (no type load). |
| src/TestFramework/TestFramework/Filtering/TestFilterAction.cs | New enum representing filter decision actions. |
| src/TestFramework/TestFramework/Filtering/TestFilterResult.cs | New result type returned by filters (Run/Drop/Skip). |
| src/TestFramework/TestFramework/Attributes/Lifecycle/TestFilterProviderAttribute.cs | New assembly-level attribute used to register filters. |
| src/Adapter/MSTestAdapter.PlatformServices/Execution/UnitTestRunner.cs | Applies filters before type-load/init and adds bookkeeping for filtered-out tests + assembly cleanup edge case. |
| src/Adapter/MSTestAdapter.PlatformServices/Execution/TypeCache.FilterDiscovery.cs | Discovers and instantiates registered filters (BFS over reference graph), caches per test assembly source. |
| src/Adapter/MSTestAdapter.PlatformServices/Resources/Resource.resx | Adds new UTA messages for filter provider diagnostics. |
| src/Adapter/MSTestAdapter.PlatformServices/Resources/xlf/Resource.cs.xlf | Localization entry updates for new diagnostics. |
| src/Adapter/MSTestAdapter.PlatformServices/Resources/xlf/Resource.de.xlf | Localization entry updates for new diagnostics. |
| src/Adapter/MSTestAdapter.PlatformServices/Resources/xlf/Resource.es.xlf | Localization entry updates for new diagnostics. |
| src/Adapter/MSTestAdapter.PlatformServices/Resources/xlf/Resource.fr.xlf | Localization entry updates for new diagnostics. |
| src/Adapter/MSTestAdapter.PlatformServices/Resources/xlf/Resource.it.xlf | Localization entry updates for new diagnostics. |
| src/Adapter/MSTestAdapter.PlatformServices/Resources/xlf/Resource.ja.xlf | Localization entry updates for new diagnostics. |
| src/Adapter/MSTestAdapter.PlatformServices/Resources/xlf/Resource.ko.xlf | Localization entry updates for new diagnostics. |
| src/Adapter/MSTestAdapter.PlatformServices/Resources/xlf/Resource.pl.xlf | Localization entry updates for new diagnostics. |
| src/Adapter/MSTestAdapter.PlatformServices/Resources/xlf/Resource.pt-BR.xlf | Localization entry updates for new diagnostics. |
| src/Adapter/MSTestAdapter.PlatformServices/Resources/xlf/Resource.ru.xlf | Localization entry updates for new diagnostics. |
| src/Adapter/MSTestAdapter.PlatformServices/Resources/xlf/Resource.tr.xlf | Localization entry updates for new diagnostics. |
| src/Adapter/MSTestAdapter.PlatformServices/Resources/xlf/Resource.zh-Hans.xlf | Localization entry updates for new diagnostics. |
| src/Adapter/MSTestAdapter.PlatformServices/Resources/xlf/Resource.zh-Hant.xlf | Localization entry updates for new diagnostics. |
Copilot's findings
- Files reviewed: 22/22 changed files
- Comments generated: 3
Comment on lines
+463
to
+470
| switch (result.Action) | ||
| { | ||
| case TestFilterAction.Run: | ||
| continue; | ||
| case TestFilterAction.Drop: | ||
| return []; | ||
| case TestFilterAction.Skip: | ||
| return [TestResult.CreateIgnoredResult(result.SkipReason)]; |
Comment on lines
+10
to
+13
| /// Designed as a <see langword="readonly"/> <see langword="struct"/> so the filter hot path stays | ||
| /// allocation-free. The static <see cref="Run"/> / <see cref="Drop"/> properties (one shared value | ||
| /// each) and the parameterized <see cref="Skip(string)"/> factory are the only ways to construct | ||
| /// a result; this keeps the surface evolvable. |
Comment on lines
+54
to
+58
| /// <param name="reason">A non-empty human-readable explanation surfaced in TRX / console / IDE output.</param> | ||
| /// <returns>A <see cref="TestFilterResult"/> with <see cref="Action"/> equal to <see cref="TestFilterAction.Skip"/>.</returns> | ||
| /// <exception cref="ArgumentNullException">Thrown when <paramref name="reason"/> is <see langword="null"/>.</exception> | ||
| public static TestFilterResult Skip(string reason) | ||
| => new(TestFilterAction.Skip, reason ?? throw new ArgumentNullException(nameof(reason))); |
Removes multi-filter discovery (no BFS across referenced assemblies, no AND composition, no ordering). Each test assembly may declare at most one [assembly: TestFilterProvider(typeof(T))]; users compose multiple strategies inside their own ITestFilter implementation. This sidesteps ordering questions and dedup logic. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- TestFilterResult.Skip: reject null/empty/whitespace reasons (matches XML doc). - TypeCache: enforce single [TestFilterProvider] per assembly (UTA079) instead of silently first-wins. - ITestFilter: document UTA078 Error contract when Filter throws, and clarify single-instance caching semantics. - Add unit tests for TestFilterResult validation and InstantiateTestFilter error paths (UTA074-077). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Comment on lines
+10
to
+13
| /// Designed as a <see langword="readonly"/> <see langword="struct"/> so the filter hot path stays | ||
| /// allocation-free. The static <see cref="Run"/> / <see cref="Drop"/> properties (one shared value | ||
| /// each) and the parameterized <see cref="Skip(string)"/> factory are the only ways to construct | ||
| /// a result; this keeps the surface evolvable. |
Comment on lines
+15
to
+33
| private readonly ConcurrentDictionary<string, TestFilterBox> _testFilterBySource = | ||
| new(StringComparer.Ordinal); | ||
|
|
||
| /// <summary> | ||
| /// Returns the cached <see cref="ITestFilter"/> instance registered via | ||
| /// <see cref="TestFilterProviderAttribute"/> on the given test assembly, or | ||
| /// <see langword="null"/> if the assembly does not register one. | ||
| /// </summary> | ||
| /// <param name="assemblySource">The test assembly source path (typically <c>TestMethod.AssemblyName</c>).</param> | ||
| /// <remarks> | ||
| /// Discovery is metadata-only for the probe step and never forces the test types of the | ||
| /// assembly to load. The filter <em>type</em> is loaded the first time the filter for a | ||
| /// given source is requested. Only the test assembly itself is inspected — registering a | ||
| /// <see cref="TestFilterProviderAttribute"/> in a referenced library has no effect. | ||
| /// </remarks> | ||
| internal ITestFilter? GetOrLoadTestFilter(string assemblySource) | ||
| => _testFilterBySource | ||
| .GetOrAdd(assemblySource, static src => new TestFilterBox(LoadTestFilterForSource(src))) | ||
| .Filter; |
Comment on lines
+525
to
+527
| testContextForAssemblyCleanup = PlatformServiceProvider.Instance.GetTestContext(testMethod: null, null, testContextProperties, messageLogger, testContextForTestExecution.Context.CurrentTestOutcome); | ||
|
|
||
| TestResult? assemblyCleanupResult = await RunAssemblyCleanupAsync(testContextForAssemblyCleanup, _typeCache, filterResult).ConfigureAwait(false); |
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
|
|
||
| public void Equality_RunAndDropAreNotEqual() | ||
| { | ||
| (TestFilterResult.Run == TestFilterResult.Drop).Should().BeFalse(); |
Pivot TestFilterContext from a 7-arg positional ctor to a parameterless ctor plus public mutable properties so new fields can be added in future releases without breaking source or binary callers. `init` is banned for new public API in this repo, so the properties use plain `set`. Surface structured identification in the same shape as `TestMethodIdentifier` (Namespace, ClassName, ManagedTypeName, ManagedMethodName, MethodArity, ParameterTypeFullNames) alongside the existing flat fields. The structured fields come from the Hierarchy slots that discovery already populates and from a cheap `ManagedNameParser.ParseManagedMethodName` call, so the test type is still not loaded. Rename `AssemblyName` to `Source`: the underlying TestMethod field is actually a file path (matches VSTest TestCase.Source). Document on ITestFilter.Filter that the filter runs before `[Ignore]` is evaluated; returning `Run` does not override a later `[Ignore]`. Decisions explicitly out of scope: - No bypass for built-in adapter filtering (--filter, [Ignore]): keeps the contracts CI scripts and IDEs depend on. - No "already-included / already-excluded" info: --filter runs before ITestFilter so "already-included" would be tautological. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Comment on lines
+514
to
+518
| catch | ||
| { | ||
| // Defensive: if the managed name is malformed for any reason, surface what we | ||
| // can via the flat strings rather than failing the filter. | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Implements the design from #8894: a small, opt-in extension point that lets users plug in their own filtering logic in MSTest before any test-class type is loaded or any
[AssemblyInitialize]/[ClassInitialize]runs. This makes it cheap to drop or skip tests programmatically — the original motivation for the issue.What's exposed
All new public API lives in
Microsoft.VisualStudio.TestTools.UnitTesting:[assembly: TestFilterProvider(typeof(MyFilter))]— assembly-level attribute,AllowMultiple = false(at most one per test assembly), points at anITestFilterimplementation. Same shape as the existing[AssemblyFixtureProvider].interface ITestFilter— a single method:TestFilterResult Filter(TestFilterContext context).readonly struct TestFilterResultwithstatic TestFilterResult Run,static TestFilterResult Drop, andstatic TestFilterResult Skip(string reason).enum TestFilterAction { Run = 0, Drop = 1, Skip = 2 }.sealed class TestFilterContext— surfaces only what we know without loading the test type. Designed as a parameterless-ctor + public mutable properties bag (noinit, norequired, no positional ctor) so we can add fields in future releases without breaking source or binary callers.FullyQualifiedName,DisplayName,MethodName,Source(test assembly file path, matches VSTestTestCase.Source).TestMethodIdentifiershape, populated when discovery provides them; otherwisenull):Namespace,ClassName,ManagedTypeName,ManagedMethodName,MethodArity,ParameterTypeFullNames. Parsed cheaply from the managed name — noMethodInforeflection, so the type is still not loaded.Categories,Traits,Priority.Example
Why a single filter per assembly
The original design allowed multiple
[TestFilterProvider]attributes with AND composition. We simplified to a single filter per test assembly because:ITestFilterand call them in the order they choose. Library authors exposeITestFiltertypes; the test author opts in explicitly.How it works
TypeCache.FilterDiscovery.cs. It inspects only the test assembly itself (no BFS over references) via a fastCustomAttributeDataprobe, then materializes the attribute and validates theFilterType(non-generic, not abstract/interface, implementsITestFilter, has a public parameterless ctor). The resolved filter is cached per source assembly viaTypeCache.GetOrLoadTestFilterso the cost is paid at most once per run. A smallTestFilterBoxwrapper lets the cache distinguish "not computed yet" from "computed and no filter declared".UnitTestRunner.RunSingleTestAsync, before_typeCache.GetTestMethodInfo. That means aDropliterally pays zero[AssemblyInitialize]/[ClassInitialize]/ type-load cost — which is the whole point of the issue.[Ignore]).UTA078) so they can't silently drop tests.[TestFilterProvider]on the same assembly are rejected at discovery time withUTA079instead of silently using the first one.Validation hardening (post code-review pass)
TestFilterResult.Skip(reason)now throws onnull/ empty / whitespace reasons, matching the XML doc contract.ITestFilterXML docs now state the UTA078 contract onFilterexceptions and clarify that the resolved filter is cached per source assembly and reused for every test in that assembly.TestFilterResultvalidation and equality plus everyInstantiateTestFiltererror branch (UTA074 generic, UTA075 abstract/interface, UTA076 wrong base type, UTA077 ctor failure / non-public ctor).API redesign (second review pass)
Pivoted
TestFilterContextfrom a 7-arg positional ctor +initproperties to a parameterless ctor + plain mutable properties. Rationale:initis banned for new public API in this repo (see.github/copilot-instructions.md). Mutablesetaccessors are required.TestMethodIdentifier(Namespace,ClassName,ManagedTypeName,ManagedMethodName,MethodArity,ParameterTypeFullNames), populated from theHierarchy[Namespace/Class]slots that discovery already produces and from a cheapManagedNameParser.ParseManagedMethodNamecall (noMethodInfo, no reflection, no type-load). Flat fields stay too for callers that just want a string match.Sourcerenamed fromAssemblyName. The underlyingTestMethod.AssemblyNameis actually a file PATH (matches VSTestTestCase.Source); calling itAssemblyNamewould mislead users.[Ignore]ordering is now documented onITestFilter.Filter. The filter runs before[Ignore]is evaluated ([Ignore]requires loading the type, which the filter is specifically designed to avoid). ReturningRuntherefore does NOT override a later[Ignore].Explicitly out of scope
Two reviewer-raised questions were considered and intentionally not addressed:
--filter,[Ignore]). NO — preserves the contracts CI scripts and IDEs rely on.ITestFilteris layered on top of, not in place of, those gates.--filterruns beforeITestFilter, so the filter only ever sees survivors of that gate. "Already-included" would be tautological, and tracking "already-excluded" across the run would force materializing state we currently don't need.Assembly cleanup edge case
The cleanup tail of
RunSingleTestAsyncpreviously gated end-of-assembly cleanup ontestMethodInfo?.Parent.Parent.IsAssemblyInitializeExecuted == true. If the last test of an assembly is filtered out,testMethodInfois null and that guard never fires. Fixed by introducing a runner-level_assemblyInitializeWasExecutedflag that survives across filtered-out tests and is OR'd into the guard inside the newFinishFilteredOutTestAsync.Out of scope (deferred follow-ups)
MSTESTNNNanalyzer to validate[TestFilterProvider]targets at compile time (public + parameterless, implementsITestFilter, etc.). Diagnostics are currently runtime-only (UTA073–UTA078).Validation
build.cmd -projects src\Adapter\MSTestAdapter.PlatformServices\MSTestAdapter.PlatformServices.csproj— green.build.cmd -projects src\TestFramework\TestFramework\TestFramework.csproj— green.build.cmd -test -projects test\UnitTests\MSTestAdapter.PlatformServices.UnitTests\MSTestAdapter.PlatformServices.UnitTests.csproj— all TFMs pass (net462, net48, net8.0, net9.0, net8.0-windows10.0.18362.0).build.cmd -test -projects test\UnitTests\TestFramework.UnitTests\TestFramework.UnitTests.csproj— all TFMs pass (net48, net8.0, net9.0, net8.0-windows10.0.18362.0).Closes #8894 once design questions in the issue are settled.