Move test-case filter parsing to adapter boundary (Phase 6d-1)#9591
Conversation
Move TestMethodFilter (and its nested TestElementFilter) out of MSTestAdapter.PlatformServices up into MSTest.TestAdapter, and inject the neutral ITestElementFilter into the engine and discoverer via a new ITestElementFilterProvider abstraction. - New neutral ITestElementFilterProvider (PlatformServices.Interface): the boundary builds it (TestElementFilterProvider, closing over the VSTest IRunContext/IDiscoveryContext) and passes it into TestExecutionManager.RunTestsAsync/ExecuteTestsAsync and UnitTestDiscoverer.DiscoverTests. - The engine/discoverer invoke the provider at the EXACT points they previously built the filter (per source), so filter parse-error reporting keeps the same timing and per-source semantics; TestElementFilter.Matches still does element.ToTestCase() (byte-for-byte; #9568 deferred). - This removes ITestCaseFilterExpression / GetTestCaseFilter / MatchTestCase / the VSTest TestProperty filter set from PlatformServices code. IRunContext/IDiscoveryContext remain only for deployment + settings extraction (removed in a follow-up). No behavior change: filtered set/order and the discovery/execution filterHasError bail-out are identical; TestCaseFilteringTests (out-of-proc filter regression net) stays green. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Note
🤖 Automated review by GitHub Copilot. Posted via a maintainer's GitHub token, so it appears under their account — the account owner did not write or approve this content personally. Generated by the Expert Code Review workflow. To request a follow-up action, reply by tagging @copilot directly.
✅ 22/22 dimensions clean — no findings.
Folder hotspot coverage: All changed files fall under src/Adapter/ (correctness, threading, API) and test/ (assertions, isolation, flakiness) — both clusters received priority analysis.
Spot-checks verified:
-
Threading safety of
_testElementFilterProvider: The new non-readonly field is written once inExecuteTestsAsyncbefore the sequentialforeach/awaitloop; the parallel workers spawned insideExecuteTestsInSourceAsynccapture a localITestElementFilter?variable (the result ofGetTestElementFilter) and never touch the field directly. No unsynchronized cross-thread access. -
?.+out boolinitialization pattern:bool filterHasError = false; ITestElementFilter? filter = filterProvider?.GetTestElementFilter(logger, out filterHasError);— whenfilterProvideris null the pre-initfalseis the correct value (no filter = no error), and theif (filterHasError) return;guard is not triggered. Correct for the MTP no-filter path. -
Per-source call count:
GetTestElementFilteris called once per source in discovery and once per source in execution (same as the old code with separateTestMethodFilterinstances). Error-message count is unchanged. -
TestMethodFilterstate:_supportedPropertiesis populated once in the constructor and only read thereafter; concurrentTryGetValuereads from parallel test-filter matching are safe on an unmodifiedDictionary<K,V>. -
Public API surface:
ITestElementFilterProviderandTestElementFilterProviderare bothinternal; no public API is added, removed, or changed. -
Assertion library:
MSTestAdapter.PlatformServices.UnitTestsbans MSTestAssert/StringAssert/CollectionAssertand requiresAwesomeAssertions— all updated test call sites use.Should()correctly. -
Project references:
MSTestAdapter.PlatformServices.UnitTests.csprojalready referencesMSTest.TestAdapter.csproj, soTestElementFilterProvideris accessible from the updated tests.
012c8c9
into
dev/amauryleve/vstest-decoupling-settings
Phase 6d-1 of the PlatformServices platform-agnostic initiative
Part of removing the VSTest object model (
Microsoft.TestPlatform.ObjectModel) dependency fromMSTestAdapter.PlatformServices.This phase moves the test-case filter parsing to the adapter boundary.
TestMethodFilter(which parses the VSTestITestCaseFilterExpressionfromIRunContext/IDiscoveryContext.GetTestCaseFilter) moves out of PlatformServices intoMSTest.TestAdapter, and the engine/discoverer receive an injected neutralITestElementFiltervia a newITestElementFilterProviderabstraction.What changed
ITestElementFilterProvider(PlatformServices.Interface):ITestElementFilter? GetTestElementFilter(IAdapterMessageLogger, out bool filterHasError).TestMethodFilter(+ nestedTestElementFilter) →MSTest.TestAdapter(class body byte-identical); added a boundaryTestElementFilterProviderthat closes over the VSTest context.MSTestExecutor,MSTestDiscoverer— which the MTPMSTestBridgedTestFrameworkalso routes through) buildsnew TestElementFilterProvider(context)and injects it intoTestExecutionManager.RunTestsAsync/ExecuteTestsAsyncandUnitTestDiscoverer.DiscoverTests.No behavior change
TestElementFilter.Matchesstill doeselement.ToTestCase()(byte-for-byte; the #9568 no-round-trip optimization is deferred as its own follow-up). This removesITestCaseFilterExpression/GetTestCaseFilter/MatchTestCase/ the VSTest filterTestPropertyset from PlatformServices code.IRunContext/IDiscoveryContextremain only for deployment + settings extraction (removed in 6d-2).Verification
MSTestAdapter.PlatformServices.UnitTests: 897 (net8.0) / 935 (net462);MSTestAdapter.UnitTests: 21;MSTest.IntegrationTests: 47 pass / 1 skip — includingTestCaseFilteringTests, the out-of-proc filter regression net.Base / stacking
dev/amauryleve/vstest-decoupling-settings; review/merge after Neutralize run-settings input in PlatformServices host layer (Phase 6c2) #9590. When 6c2 merges intovstest-decoupling-base, this PR is rebased onto the updated base (diff is 6d-1 only, 10 files).Remaining work
IRunContext/IDiscoveryContextfrom PlatformServices entirely (deploy-context + settings extraction to the boundary).UnitTestElement↔TestCaseconversion +EngineConstants.Microsoft.TestPlatform.ObjectModelPackageReference + guard test.