Abstract VSTest message logging in PlatformServices (Phase 1 of platform-agnostic effort)#9548
Open
Evangelink wants to merge 1 commit into
Open
Abstract VSTest message logging in PlatformServices (Phase 1 of platform-agnostic effort)#9548Evangelink wants to merge 1 commit into
Evangelink wants to merge 1 commit into
Conversation
Introduce a platform-agnostic IAdapterMessageLogger abstraction (reusing the existing MessageLevel enum) so the platform services layer no longer depends on the VSTest IMessageLogger/TestMessageLevel for the standalone message-logger role. The VSTest bridge (ToAdapterMessageLogger) lives in the adapter-facing extension and is applied at the MSTestDiscoverer/MSTestExecutor boundary and at the two execution sites that reuse the framework handle as a logger. The recorder's dual logger role (IFrameworkHandle/ITestExecutionRecorder) is intentionally left for the later recorder phase, since logger and recorder are the same object there. Tests keep their Mock<IMessageLogger> and wrap with .ToAdapterMessageLogger() at migrated call sites, so TestMessageLevel Verify assertions are unchanged. 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 1 of the effort to make MSTestAdapter.PlatformServices platform-agnostic by abstracting adapter-to-host message logging away from VSTest’s object model and shifting the translation to the adapter boundary.
Changes:
- Introduces a neutral message-logging contract (
IAdapterMessageLogger) based on the existingMessageLevelenum. - Adds a single VSTest→neutral bridge (
ToAdapterMessageLogger(this IMessageLogger)) and migrates PlatformServices call sites fromIMessageLogger/TestMessageLeveltoIAdapterMessageLogger/MessageLevel. - Updates adapter and test call sites to wrap existing
IMessageLoggermocks/handles with.ToAdapterMessageLogger().
Show a summary per file
| File | Description |
|---|---|
| test/UnitTests/MSTestAdapter.PlatformServices.UnitTests/RunConfigurationSettingsTests.cs | Updates settings population tests to pass the new adapter logger wrapper. |
| test/UnitTests/MSTestAdapter.PlatformServices.UnitTests/ObjectModel/UnitTestResultTests.cs | Wraps logger when retrieving settings to keep outcome mapping tests exercising the bridge. |
| test/UnitTests/MSTestAdapter.PlatformServices.UnitTests/MSTestSettingsTests.cs | Migrates settings/config tests to the new logger abstraction while retaining TestMessageLevel verifications. |
| test/UnitTests/MSTestAdapter.PlatformServices.UnitTests/Helpers/UnitTestOutcomeHelperTests.cs | Updates settings initialization to use .ToAdapterMessageLogger(). |
| test/UnitTests/MSTestAdapter.PlatformServices.UnitTests/Execution/UnitTestRunnerTests.cs | Wraps logger in runner-related settings reads. |
| test/UnitTests/MSTestAdapter.PlatformServices.UnitTests/Execution/TypeCacheTests.cs | Updates settings read/population paths to use the adapter logger wrapper. |
| test/UnitTests/MSTestAdapter.PlatformServices.UnitTests/Execution/TestMethodFilterTests.cs | Passes an IAdapterMessageLogger into filter expression paths via wrapper. |
| test/UnitTests/MSTestAdapter.PlatformServices.UnitTests/Execution/TestExecutionManagerTests.cs | Updates settings population during execution tests to use the adapter logger wrapper. |
| test/UnitTests/MSTestAdapter.PlatformServices.UnitTests/Discovery/UnitTestDiscovererTests.cs | Migrates discovery tests and test doubles to IAdapterMessageLogger. |
| test/UnitTests/MSTestAdapter.PlatformServices.UnitTests/Discovery/TypeEnumeratorTests.cs | Updates discovery settings initialization to use .ToAdapterMessageLogger(). |
| test/UnitTests/MSTestAdapter.PlatformServices.UnitTests/Discovery/AssemblyEnumeratorTests.cs | Wraps logger when constructing settings for discovery enumerator tests. |
| test/IntegrationTests/MSTest.IntegrationTests/Utilities/CLITestBase.discovery.cs | Updates integration discovery helper to pass the adapter logger wrapper. |
| src/Adapter/MSTestAdapter.PlatformServices/TestMethodFilter.cs | Switches filter logging from IMessageLogger/TestMessageLevel to IAdapterMessageLogger/MessageLevel. |
| src/Adapter/MSTestAdapter.PlatformServices/Services/AdapterMessageLoggerExtensions.cs | Adds the VSTest IMessageLogger → IAdapterMessageLogger bridge. |
| src/Adapter/MSTestAdapter.PlatformServices/MSTestSettings.RunSettingsXml.cs | Migrates runsettings parsing logging to IAdapterMessageLogger/MessageLevel. |
| src/Adapter/MSTestAdapter.PlatformServices/MSTestSettings.Configuration.cs | Migrates configuration/runsettings warning logging to IAdapterMessageLogger/MessageLevel. |
| src/Adapter/MSTestAdapter.PlatformServices/Interfaces/IAdapterMessageLogger.cs | Introduces the platform-agnostic adapter message logger contract. |
| src/Adapter/MSTestAdapter.PlatformServices/Helpers/MSTestDiscovererHelpers.cs | Updates discovery initialization logging to use the neutral logger. |
| src/Adapter/MSTestAdapter.PlatformServices/Execution/TestExecutionManager.Parallelization.cs | Updates filter-expression acquisition to pass an adapter logger wrapper. |
| src/Adapter/MSTestAdapter.PlatformServices/Execution/TestExecutionManager.cs | Caches an IAdapterMessageLogger wrapper for framework-handle logging during discovery in execution. |
| src/Adapter/MSTest.TestAdapter/VSTestAdapter/MSTestExecutor.cs | Wraps frameworkHandle at the adapter boundary before calling into PlatformServices. |
| src/Adapter/MSTest.TestAdapter/VSTestAdapter/MSTestDiscoverer.cs | Wraps the VSTest logger once and threads the neutral logger through discovery. |
Review details
- Files reviewed: 23/23 changed files
- Comments generated: 2
- Review effort level: Low
Comment on lines
+26
to
+27
| public static IAdapterMessageLogger ToAdapterMessageLogger(this IMessageLogger messageLogger) | ||
| => new HostMessageLogger(messageLogger); |
Comment on lines
74
to
76
| // Default test set is filtered tests based on user provided filter criteria | ||
| ITestCaseFilterExpression? filterExpression = _testMethodFilter.GetFilterExpression(runContext, frameworkHandle, out bool filterHasError); | ||
| ITestCaseFilterExpression? filterExpression = _testMethodFilter.GetFilterExpression(runContext, frameworkHandle.ToAdapterMessageLogger(), out bool filterHasError); | ||
| if (filterHasError) |
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
| """; | ||
|
|
||
| var adapterSettings = MSTestSettings.GetSettings(runSettingsXml, MSTestSettings.SettingsNameAlias, _mockMessageLogger.Object); | ||
| var adapterSettings = MSTestSettings.GetSettings(runSettingsXml, MSTestSettings.SettingsNameAlias, _mockMessageLogger.Object.ToAdapterMessageLogger()); |
| }); | ||
|
|
||
| var adapterSettings = MSTestSettings.GetSettings(runSettingsXml, MSTestSettings.SettingsName, _mockMessageLogger.Object); | ||
| var adapterSettings = MSTestSettings.GetSettings(runSettingsXml, MSTestSettings.SettingsName, _mockMessageLogger.Object.ToAdapterMessageLogger()); |
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
This is the first, self-contained slice of a larger effort to make
MSTestAdapter.PlatformServicesplatform-agnostic by removing its dependency on the VSTest object model (Microsoft.TestPlatform.ObjectModel). The end goal is for the VSTest coupling to live only in the adapter layer (MSTest.TestAdapter), so the platform-services engine can eventually be driven by an MTP-native adapter without round-tripping through the VSTest bridge.The full phased plan (0–8) is tracked separately; this PR delivers Phase 0 + Phase 1 (message logging) only, to keep the change reviewable.
What
Introduces a platform-agnostic message-logging abstraction and migrates the standalone logger role in PlatformServices off VSTest types. The VSTest connection is now made at the adapter boundary.
IAdapterMessageLogger—SendMessage(MessageLevel, string), reusing the existing neutralMessageLevelenum fromTestFramework.Extensions.AdapterMessageLoggerExtensions.ToAdapterMessageLogger(this IMessageLogger)— the single VSTest → neutral bridge (internalHostMessageLogger, maps via the pre-existingToTestMessageLevel()).IMessageLogger/TestMessageLevel→IAdapterMessageLogger/MessageLevel):MSTestDiscovererHelpers,MSTestSettings.Configuration,MSTestSettings.RunSettingsXml,UnitTestDiscoverer,TestMethodFilter.frameworkHandle.ToAdapterMessageLogger()inMSTestDiscoverer/MSTestExecutor, plus the two execution sites that reuse the framework handle as a logger.Deliberately out of scope
The recorder's dual logger role (
IFrameworkHandle/ITestExecutionRecorder.SendMessage) is intentionally left for the later recorder phase, since there the logger and the recorder are the same object. TheToAdapterMessageLoggerbridge stays in PlatformServices as a temporary translation point until that phase.No behavior change
This is a pure refactor. The
MessageLevel↔TestMessageLevelmapping is 1:1, and no message levels or paths were dropped. Tests keep theirMock<IMessageLogger>and wrap with.ToAdapterMessageLogger()at migrated call sites, so the existingVerify(... TestMessageLevel ...)assertions still exercise the mapping end-to-end.Verification
srcbuild green across all TFMs (net462, net8.0, net9.0, UWP, WinUI).MSTestAdapter.PlatformServices.UnitTests: 889 passed.MSTestAdapter.UnitTests: 21 passed.