Abstract VSTest result reporting in PlatformServices (Phase 5 of platform-agnostic effort)#9550
Open
Evangelink wants to merge 1 commit into
Open
Conversation
Introduce a platform-agnostic `ITestResultRecorder` so the execution result path in PlatformServices no longer constructs VSTest result-side types (`TestResult`, `TestOutcome`, `TestResultMessage`, `AttachmentSet`, `UriDataAttachment`). The single VSTest translation point now lives in the adapter-facing bridge `HostTestResultRecorder` (`Services/TestResultRecorderExtensions.ToTestResultRecorder`), mirroring the Phase 1 `IAdapterMessageLogger` + `AdapterMessageLoggerExtensions` pattern. `TestExecutionManager.Runner.cs` routes start/empty/result reporting through the neutral recorder. `TestResultExtensions.ToTestResult` and `UnitTestOutcomeHelper.ToTestOutcome` are unchanged and are now called from the bridge. This is a pure refactor with no behavior change: the outcome mapping, assembled `TestResult`, and the trace / `_hasAnyTestFailed` / NotFound+HotReload branches are preserved. Independent of and parallel to PR #9548 (Phase 1). Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR is Phase 5 of the effort to make MSTestAdapter.PlatformServices platform-agnostic by concentrating VSTest result-object-model usage into a single bridge and having the execution pipeline report results through a neutral abstraction.
Changes:
- Introduces a new internal result-recording abstraction:
ITestResultRecorder. - Adds a single VSTest bridge (
TestResultRecorderExtensions/HostTestResultRecorder) that translates framework results into VSTestTestResultand forwards toITestExecutionRecorder. - Refactors
TestExecutionManager.Runnerto record start/empty/result viaITestResultRecorder, and updates the relevant unit test call site.
Show a summary per file
| File | Description |
|---|---|
| test/UnitTests/MSTestAdapter.PlatformServices.UnitTests/Execution/TestExecutionManagerTests.cs | Updates a direct SendTestResults call to pass an ITestResultRecorder wrapper instead of a raw VSTest recorder. |
| src/Adapter/MSTestAdapter.PlatformServices/Services/TestResultRecorderExtensions.cs | Adds the single translation point from VSTest ITestExecutionRecorder to the platform-agnostic ITestResultRecorder. |
| src/Adapter/MSTestAdapter.PlatformServices/Interfaces/ITestResultRecorder.cs | Introduces the new internal recorder interface used by PlatformServices to report execution lifecycle and results. |
| src/Adapter/MSTestAdapter.PlatformServices/Execution/TestExecutionManager.Runner.cs | Switches result reporting from direct VSTest result-type construction to the new recorder abstraction. |
Review details
- Files reviewed: 4/4 changed files
- Comments generated: 1
- Review effort level: Low
Comment on lines
+15
to
+18
| /// This abstraction lets the platform services layer report test start/end and results without taking a | ||
| /// dependency on a specific test platform's result object model (for example the VSTest <c>TestResult</c>, | ||
| /// <c>TestOutcome</c> and attachment types). The mapping to a concrete host recorder (for example the | ||
| /// VSTest <c>ITestExecutionRecorder</c>) is provided by the adapter layer. |
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.
Why
Part of the multi-PR effort to make
MSTestAdapter.PlatformServicesplatform-agnostic by removing its dependency on the VSTest object model (Microsoft.VisualStudio.TestPlatform.ObjectModel.*). The VSTest coupling is being concentrated into small adapter-facing bridges so it can later be lifted entirely out of PlatformServices.Phase 5 targets the execution result / output path: the VSTest result-side types (
TestResult,TestOutcome,TestResultMessage,AttachmentSet,UriDataAttachment) and the recorder role used to report results.What
ITestResultRecorder(Interfaces/ITestResultRecorder.cs, namespace…PlatformServices.Interface): a functional, platform-agnostic sink withRecordStart/RecordEmptyResult/RecordResult(TestCase, framework TestResult, start, end). It carries no VSTest result-type naming and reuses the frameworkTestResult.HostTestResultRecorderbehindTestResultRecorderExtensions.ToTestResultRecorder(this ITestExecutionRecorder, computerName, settings)(Services/TestResultRecorderExtensions.cs, namespace…PlatformServices). This bridge is the only place that builds a VSTestTestResult/TestOutcome, records attachments/messages, and calls the VSTest recorder — mirroring the Phase 1IAdapterMessageLogger+AdapterMessageLoggerExtensionspattern.Execution/TestExecutionManager.Runner.csnow reports start/empty/result throughITestResultRecorder, so it no longer constructs any VSTest result-side type. The VSTest recorder is wrapped once at the boundary (insideExecuteTestsWithTestRunnerAsync) to avoid touching Phase 1 files.TestResultExtensions.ToTestResultandUnitTestOutcomeHelper.ToTestOutcomeare unchanged (still directly unit-tested) and are now invoked from the bridge.No behavior change
This is a pure refactor. The
UnitTestOutcome→ VSTestTestOutcomemapping and the assembledTestResult(messages, attachments, error info, timings) are byte-for-byte equivalent, and every branch of the originalSendTestResultsflow is preserved: empty-results →RecordEnd(None); per result →RecordEnd(mapped), trace onFailed,_hasAnyTestFailed(under#if !WINDOWS_UWP && !WIN_UI), and the guardedRecordResultinsidetry/catch(TestCanceledException)with theNotFound+ hot-reload skip.Verification
build.cmd -c Debuggreen across all TFMs (net462, net8.0, net9.0, UWP, WinUI); analyzers-as-errors clean.MSTestAdapter.PlatformServices.UnitTests(net8.0): 889 passed.MSTestAdapter.UnitTests(net8.0): 21 passed.expert-reviewerconfirmed byte-for-byte equivalence of the refactored flow.Relationship to other PRs
Independent of and parallel to #9548 (Phase 1). The diff is disjoint from Phase 1's production files. One Phase-1 test file (
TestExecutionManagerTests.cs) is touched by a single line — the directSendTestResultscall now wraps_frameworkHandle.ToTestResultRecorder(...); this hunk is far from Phase 1's edits and should not conflict.