Skip to content

Move test-case filter parsing to adapter boundary (Phase 6d-1)#9591

Merged
Evangelink merged 1 commit into
dev/amauryleve/vstest-decoupling-settingsfrom
dev/amauryleve/vstest-decoupling-filter
Jul 5, 2026
Merged

Move test-case filter parsing to adapter boundary (Phase 6d-1)#9591
Evangelink merged 1 commit into
dev/amauryleve/vstest-decoupling-settingsfrom
dev/amauryleve/vstest-decoupling-filter

Conversation

@Evangelink

Copy link
Copy Markdown
Member

Phase 6d-1 of the PlatformServices platform-agnostic initiative

Part of removing the VSTest object model (Microsoft.TestPlatform.ObjectModel) dependency from MSTestAdapter.PlatformServices.

This phase moves the test-case filter parsing to the adapter boundary. TestMethodFilter (which parses the VSTest ITestCaseFilterExpression from IRunContext/IDiscoveryContext.GetTestCaseFilter) moves out of PlatformServices into MSTest.TestAdapter, and the engine/discoverer receive an injected neutral ITestElementFilter via a new ITestElementFilterProvider abstraction.

What changed

  • New ITestElementFilterProvider (PlatformServices.Interface): ITestElementFilter? GetTestElementFilter(IAdapterMessageLogger, out bool filterHasError).
  • Moved TestMethodFilter (+ nested TestElementFilter) → MSTest.TestAdapter (class body byte-identical); added a boundary TestElementFilterProvider that closes over the VSTest context.
  • The boundary (MSTestExecutor, MSTestDiscoverer — which the MTP MSTestBridgedTestFramework also routes through) builds new TestElementFilterProvider(context) and injects it into TestExecutionManager.RunTestsAsync/ExecuteTestsAsync and UnitTestDiscoverer.DiscoverTests.
  • The engine/discoverer invoke the provider at the exact same call sites they previously built the filter (per source), so parse-error reporting keeps the same timing and per-source count.

No behavior change

TestElementFilter.Matches still does element.ToTestCase() (byte-for-byte; the #9568 no-round-trip optimization is deferred as its own follow-up). This removes ITestCaseFilterExpression / GetTestCaseFilter / MatchTestCase / the VSTest filter TestProperty set from PlatformServices code. IRunContext/IDiscoveryContext remain only for deployment + settings extraction (removed in 6d-2).

Verification

  • Full build green across all TFMs (net462, net8.0, net9.0, UWP, WinUI).
  • MSTestAdapter.PlatformServices.UnitTests: 897 (net8.0) / 935 (net462); MSTestAdapter.UnitTests: 21; MSTest.IntegrationTests: 47 pass / 1 skip — including TestCaseFilteringTests, the out-of-proc filter regression net.
  • Expert MSTest/MTP reviewer: no material findings; definitive byte-for-byte verdict on the per-source parse-error timing/count and the null-provider equivalence.

Base / stacking

Remaining work

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>
@Evangelink Evangelink changed the base branch from dev/amauryleve/vstest-decoupling-base to dev/amauryleve/vstest-decoupling-settings July 3, 2026 17:20
@Evangelink Evangelink marked this pull request as ready for review July 5, 2026 09:07
@Evangelink Evangelink added the state/needs-review Awaiting review from the team. label Jul 5, 2026

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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 in ExecuteTestsAsync before the sequential foreach/await loop; the parallel workers spawned inside ExecuteTestsInSourceAsync capture a local ITestElementFilter? variable (the result of GetTestElementFilter) and never touch the field directly. No unsynchronized cross-thread access.

  • ?. + out bool initialization pattern: bool filterHasError = false; ITestElementFilter? filter = filterProvider?.GetTestElementFilter(logger, out filterHasError); — when filterProvider is null the pre-init false is the correct value (no filter = no error), and the if (filterHasError) return; guard is not triggered. Correct for the MTP no-filter path.

  • Per-source call count: GetTestElementFilter is called once per source in discovery and once per source in execution (same as the old code with separate TestMethodFilter instances). Error-message count is unchanged.

  • TestMethodFilter state: _supportedProperties is populated once in the constructor and only read thereafter; concurrent TryGetValue reads from parallel test-filter matching are safe on an unmodified Dictionary<K,V>.

  • Public API surface: ITestElementFilterProvider and TestElementFilterProvider are both internal; no public API is added, removed, or changed.

  • Assertion library: MSTestAdapter.PlatformServices.UnitTests bans MSTest Assert/StringAssert/CollectionAssert and requires AwesomeAssertions — all updated test call sites use .Should() correctly.

  • Project references: MSTestAdapter.PlatformServices.UnitTests.csproj already references MSTest.TestAdapter.csproj, so TestElementFilterProvider is accessible from the updated tests.

@Evangelink Evangelink merged commit 012c8c9 into dev/amauryleve/vstest-decoupling-settings Jul 5, 2026
46 of 47 checks passed
@Evangelink Evangelink deleted the dev/amauryleve/vstest-decoupling-filter branch July 5, 2026 18:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

state/needs-review Awaiting review from the team.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants