Abstract VSTest discovery sink in PlatformServices (Phase 3 of platform-agnostic effort)

[Evangelink]

Why

Continues the platform-agnostic PlatformServices effort. Phase 1 (#9548), Phase 2 (#9567), and Phase 5 (#9550) are on main; this brings Phase 3 (discovery sink) to main.

Same change previously reviewed and merged as #9566 (into the integration branch dev/amauryleve/vstest-decoupling-base). Cherry-picked cleanly onto current main as a standalone, Phase-3-only PR now that its prerequisite (Phase 2) has landed.

What

The discovery engine (UnitTestDiscoverer) now emits the neutral UnitTestElement to a new platform-agnostic IUnitTestElementSink (SendTestElement(UnitTestElement)) instead of building VSTest TestCases and pushing to ITestCaseDiscoverySink. The single element.ToTestCase() translation moves behind the boundary via a Services/ bridge (UnitTestElementSinkExtensions.ToUnitTestElementSink(this ITestCaseDiscoverySink)), wrapped at the adapter boundary in MSTestDiscoverer. Mirrors the Phase 1/2/5 interface + bridge convention.

The internal execution-side collector TestCaseDiscoverySink now implements IUnitTestElementSink but still materializes TestCase into its ICollection<TestCase> Tests, so execution behaves identically.

No behavior change

Pure refactor — byte-for-byte identical discovered TestCases (fields, Id, properties, traits, ordering), filter point, and filterHasError bail-out.

Verification

Cherry-picked cleanly onto current main; PlatformServices + its unit-test project rebuild with 0 errors/0 warnings on the new base. (Originally verified green across all TFMs + full unit/integration suites as #9566.)

[Copilot]

Pull request overview

This PR delivers Phase 3 of the ongoing effort to make MSTestAdapter.PlatformServices platform-agnostic by removing its dependency on the VSTest object model. It abstracts the discovery output sink: the discovery engine (UnitTestDiscoverer) now emits the neutral UnitTestElement model to a new IUnitTestElementSink instead of building VSTest TestCases and pushing them to ITestCaseDiscoverySink. The single element.ToTestCase() translation moves behind a Services/ bridge, wrapped once at the adapter boundary in MSTestDiscoverer. This mirrors the already-merged Phase 1 (IAdapterMessageLogger), Phase 5 (ITestResultRecorder), and Phase 2 (ITestElementFilter) interface+bridge conventions. It is described as a pure, no-behavior-change refactor.

Changes:

• Introduces neutral IUnitTestElementSink (SendTestElement(UnitTestElement)) plus the UnitTestElementSinkExtensions.ToUnitTestElementSink() bridge that materializes TestCase at the VSTest boundary.
• Migrates UnitTestDiscoverer (DiscoverTests/DiscoverTestsInSource/SendTestCases) and the execution-side TestCaseDiscoverySink to the neutral sink; MSTestDiscoverer wraps the host sink at the boundary.
• Updates unit/integration tests to wrap mocked ITestCaseDiscoverySink via .ToUnitTestElementSink() and rewrites TestCaseDiscoverySinkTests for the new API.

Show a summary per file

File	Description
src/Adapter/MSTestAdapter.PlatformServices/Interfaces/IUnitTestElementSink.cs	New neutral discovery sink interface (mirrors Phase 1/2/5 interface placement/namespace).
src/Adapter/MSTestAdapter.PlatformServices/Services/UnitTestElementSinkExtensions.cs	New single VSTest bridge wrapping ITestCaseDiscoverySink, materializing TestCase via ToTestCase().
src/Adapter/MSTestAdapter.PlatformServices/Discovery/UnitTestDiscoverer.cs	Discovery engine now takes/emits IUnitTestElementSink; no longer references ITestCaseDiscoverySink or ToTestCase().
src/Adapter/MSTestAdapter.PlatformServices/Execution/TestCaseDiscoverySink.cs	Execution collector now implements IUnitTestElementSink, still materializing TestCase internally.
src/Adapter/MSTest.TestAdapter/VSTestAdapter/MSTestDiscoverer.cs	Adapter boundary wraps the host sink via .ToUnitTestElementSink().
test/.../Discovery/UnitTestDiscovererTests.cs	Call sites wrap the mocked sink; test override uses the new sink API.
test/.../Execution/TestCaseDiscoverySinkTests.cs	Rewritten to assert SendTestElement materialization and ordering.
test/.../Utilities/CLITestBase.discovery.cs	Integration helper wraps its internal sink before calling discovery.

Review details

• Files reviewed: 8/8 changed files
• Comments generated: 0
• Review effort level: Medium

[github-actions]

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.

Summary: Clean mechanical refactoring that introduces IUnitTestElementSink + UnitTestElementSinkExtensions following the exact same interface-plus-bridge pattern established in Phase 1 (IAdapterMessageLogger / AdapterMessageLoggerExtensions) and Phase 2 (ITestResultRecorder / TestResultRecorderExtensions). All new types are internal, no public API surface change, no behavioral change, tests properly updated to verify materialization through the new abstraction. The removal of the old null-accepting SendTestCase(TestCase?) in favor of non-nullable SendTestElement(UnitTestElement) is correct since the C# type system enforces the contract. Thread safety is maintained — TestCaseDiscoverySink is used sequentially per-source in the execution loop.

[github-actions]

🧪 Test quality grade — PR #9625

Grade	Test	Notes
B (80–89)	mod UnitTestDiscovererTests. DiscoverTestsInSourceShouldNotThrowWhenTreatDiscoveryWarningsAsErrorsIsFalse	Body slightly long (~33 lines) due to XML run-settings setup — consider a helper for repeated settings XML.
B (80–89)	mod UnitTestDiscovererTests. DiscoverTestsInSourceShouldSendBackTestCasesDiscovered	Loose Times.AtLeastOnce + It.IsAny(TestCase)() leaves discovered tests unverified — add FQN checks.
B (80–89)	mod UnitTestDiscovererTests. DiscoverTestsInSourceShouldThrowWhenTreatDiscoveryWarningsAsErrorsIsTrue	Strong exception + logger assertions; body slightly long (~38 lines) from XML settings — consider a settings helper.
A (90–100)	new TestCaseDiscoverySinkTests. SendTestElementShouldAddEachTestCaseInOrder	No issues found.
A (90–100)	new TestCaseDiscoverySinkTests. SendTestElementShouldAddTheMaterializedTestCaseToTests	No issues found.
A (90–100)	mod TestCaseDiscoverySinkTests. TestCaseDiscoverySinkConstructorShouldInitializeTests	No issues found.
A (90–100)	mod UnitTestDiscovererTests. DiscoverTestsInSourceShouldThrowOnFileNotFound	No issues found.
A (90–100)	mod UnitTestDiscovererTests. DiscoverTestsShouldThrowOnFileNotFound	No issues found.
A (90–100)	mod UnitTestDiscovererTests. SendTestCasesShouldNotSendAnyTestCasesIfFilterError	No issues found.
A (90–100)	mod UnitTestDiscovererTests. SendTestCasesShouldNotSendAnyTestCasesIfThereAreNoTestElements	No issues found.
A (90–100)	mod UnitTestDiscovererTests. SendTestCasesShouldSendAllTestCaseData	No issues found.
A (90–100)	mod UnitTestDiscovererTests. SendTestCasesShouldSendAllTestCasesIfGetTestCaseFilterNotPresent	No issues found.
A (90–100)	mod UnitTestDiscovererTests. SendTestCasesShouldSendAllTestCasesIfNullFilterExpression	No issues found.
A (90–100)	mod UnitTestDiscovererTests. SendTestCasesShouldSendFilteredTestCasesIfValidFilterExpression	No issues found.

_{This advisory comment was generated automatically. Grades are heuristic
and informational — they do not block merging. Re-run with
/grade-tests.}

🤖 Automated content 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 Grade Tests on PR (on open / sync) workflow. · 176.8 AIC · ⌖ 9.04 AIC · ⊞ 9.5K · [◷]( · ◷)