Neutralize the trait type on UnitTestElement (Phase 6e-3a)#9624
Merged
Evangelink merged 3 commits intoJul 5, 2026
Merged
Conversation
…hase 6e-1) The execution engine used the VSTest IFrameworkHandle exclusively to obtain an IAdapterMessageLogger via ToAdapterMessageLogger(). Replace the IFrameworkHandle parameter with the neutral IAdapterMessageLogger throughout TestExecutionManager (RunTestsAsync both overloads, ExecuteTestsAsync, ExecuteTestsInSourceAsync, Deploy); the adapter boundary (MSTestExecutor) now calls frameworkHandle.ToAdapterMessageLogger() once and injects the result. This removes the last VSTest ObjectModel.Adapter reference from the execution engine. No behavior change: the logger wrapper is stateless, so injecting one instance is identical to building one per call site. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Move the three remaining VSTest-object-model bridge helpers out of MSTestAdapter.PlatformServices and into MSTest.TestAdapter: AdapterMessageLoggerExtensions, MessageLevel (ToTestMessageLevel), and UnitTestElementSinkExtensions. These are the last code references to Microsoft.VisualStudio.TestPlatform.ObjectModel.Logging / ITestCaseDiscoverySink in PlatformServices; only doc comments now mention the VSTest types. The logical namespace is unchanged so callers at the adapter boundary and the integration harness are unaffected. PlatformServices.UnitTests calls the ToAdapterMessageLogger bridge, which now lives in MSTest.TestAdapter; touching that module runs its [ModuleInitializer] (MSTestExecutor.SetPlatformLogger), which assigns PlatformServiceProvider.Instance.AdapterTraceLogger. Make the test double's setter tolerate the assignment (as the real PlatformServiceProvider does) instead of throwing. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Replace the VSTest object-model Trait type carried on UnitTestElement.Traits
with a neutral, platform-agnostic TestTrait { Name, Value } struct. The
engine-side producers and consumers (ReflectHelper/ReflectionHelper
GetTestPropertiesAsTraits, TypeEnumerator, TestExecutionManager TestContext
building, TestRunInfo, the test-filter context) now operate on TestTrait, so
five files stop referencing Microsoft.VisualStudio.TestPlatform.ObjectModel.
The VSTest Trait only survives at the adapter conversion boundary
(TestCaseExtensions and UnitTestElement.ToTestCase), which convert between
TestTrait and the host trait type.
TestTrait is [Serializable] on .NET Framework because UnitTestElement is
serialized across app domains during isolated discovery/execution; order and
Name/Value are preserved, so trait -> TestContext reporting and the produced
host test case are byte-for-byte identical.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Base automatically changed from
dev/amauryleve/vstest-decoupling-bridges2
to
dev/amauryleve/vstest-decoupling-runcontext
July 5, 2026 19:23
Evangelink
merged commit 60d363b
into
dev/amauryleve/vstest-decoupling-runcontext
27 of 29 checks passed
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Comments that could not be inline-anchored
src/Adapter/MSTestAdapter.PlatformServices/ObjectModel/TestTrait.cs:5
[NIT — Naming/Conventions] public accessibility modifiers on an internal readonly struct are consistent with how UnitTestElement and UnitTestResult are authored in this codebase, but they conflict with the repo guideline "Default to internal; every public member is a long-term commitment." Since TestTrait is internal, the public keyword can never leak this type or its members outside the assembly — so there is zero API-surface impact — but future readers may wonder why the…
src/Adapter/MSTestAdapter.PlatformServices/ObjectModel/TestTrait.cs:10
[NIT — Defensive Coding] The constructor accepts name and value as string (non-nullable), but there are no null guards. TestPropertyAttribute validates its arguments, so null should not flow here in practice. However, as a standalone, reusable struct the lack of guards makes future misuse silent (a null Name would reach ValidateAndAssignTestProperty and be silently swallowed by its StringEx.IsNullOrEmpty guard — which is tolerable but may hide bugs).
Consider either adding `A…
src/Adapter/MSTestAdapter.PlatformServices/ObjectModel/UnitTestElement.cs:375
[MINOR — Test Completeness / IPC Wire Compatibility] The net462 BinaryFormatter app-domain serialization path is tested indirectly through the MSTest.IntegrationTests suite (which the PR description says passes). However, there is no dedicated unit test that explicitly round-trips a UnitTestElement with non-empty Traits through BinaryFormatter on NETFRAMEWORK to assert:
Trait.NameandTrait.Valuesurvive the crossing (not silently zeroed due to missing field-name mapping).
2.…
src/Adapter/MSTestAdapter.PlatformServices/ObjectModel/UnitTestElement.cs:387
[INFORMATIONAL — Performance/Allocations] The old testCase.Traits.AddRange(Traits) was replaced with a foreach+Add loop, which is the correct approach here (per-item conversion to new Trait(...) is unavoidable). This is not a regression — TraitCollection.Add is O(1) amortized, and the total cost is the same O(n) as AddRange. Just noting that if TraitCollection ever exposes an AddRange(IEnumerable<Trait>) overload with LINQ Select, that path would express intent more compactl…
Evangelink
added a commit
that referenced
this pull request
Jul 5, 2026
…ervices Squashed rebase of the vstest-decoupling PlatformServices stack onto main. Phases 1, 2 and 5 already landed on main via #9548/#9567/#9550; this commit carries the remaining net-new work: - Phase 3 (#9566): abstract the VSTest discovery sink (IUnitTestElementSink). - Phase 4 (#9572): abstract VSTest execution input. - Phase 6a (#9576): neutralize deployment input (DeploymentContext). - Phase 6b (#9579): neutralize test result recording (ITestResultRecorder). - Phase 6c (#9585): neutralize test message logging. - Phase 6c2: neutralize run-settings input in the host layer (settingsXml). - Phase 6d-1: move test-case filter parsing to the adapter boundary (ITestElementFilterProvider / TestElementFilterProvider). - Phase 6d-2: remove IRunContext/IDiscoveryContext from PlatformServices. - Phase 6e-1 (#9622): remove IFrameworkHandle from the execution engine. - Phase 6e-2 (#9623): relocate VSTest logger/sink bridges to the adapter. - Phase 6e-3a (#9624): neutralize the trait type on UnitTestElement. Result: MSTestAdapter.PlatformServices no longer references the VSTest run/discovery context or result object model; those types live only at the MSTest.TestAdapter boundary. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Evangelink
added a commit
that referenced
this pull request
Jul 5, 2026
…ervices Squashed rebase of the vstest-decoupling PlatformServices stack onto main. Phases 1, 2 and 5 already landed on main via #9548/#9567/#9550; this commit carries the remaining net-new work: - Phase 3 (#9566): abstract the VSTest discovery sink (IUnitTestElementSink). - Phase 4 (#9572): abstract VSTest execution input. - Phase 6a (#9576): neutralize deployment input (DeploymentContext). - Phase 6b (#9579): neutralize test result recording (ITestResultRecorder). - Phase 6c (#9585): neutralize test message logging. - Phase 6c2: neutralize run-settings input in the host layer (settingsXml). - Phase 6d-1: move test-case filter parsing to the adapter boundary (ITestElementFilterProvider / TestElementFilterProvider). - Phase 6d-2: remove IRunContext/IDiscoveryContext from PlatformServices. - Phase 6e-1 (#9622): remove IFrameworkHandle from the execution engine. - Phase 6e-2 (#9623): relocate VSTest logger/sink bridges to the adapter. - Phase 6e-3a (#9624): neutralize the trait type on UnitTestElement. Result: MSTestAdapter.PlatformServices no longer references the VSTest run/discovery context or result object model; those types live only at the MSTest.TestAdapter boundary. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
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
1 participant
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.
Phase 6e-3a — Neutralize the trait type on
UnitTestElementPart of the initiative to make
MSTestAdapter.PlatformServicesplatform-agnostic by removing its dependency on the VSTest object model (Microsoft.TestPlatform.ObjectModel). VSTest coupling moves up intoMSTest.TestAdapter; the platform-services engine becomes neutral.What this does
UnitTestElement.Traitswas typed as the VSTest object-modelTrait[], forcing every engine-side producer/consumer of traits to referenceMicrosoft.VisualStudio.TestPlatform.ObjectModel. This introduces a neutralTestTrait { Name, Value }struct (in the adapter's internal object model) and switches the neutral surface to it:UnitTestElement.Traits→TestTrait[]?ReflectHelper.GetTestPropertiesAsTraits/ReflectionHelper.GetTestPropertiesAsTraitsnow buildTestTrait[]TestExecutionManagerGetTestContextProperties,TestRunInfo, and the test-filter context readTestTraitResult: 5 files stop referencing the VSTest object model (
ReflectHelper,ReflectionHelper,TestExecutionManager.TestContext,TestRunInfo,UnitTestRunner.TestFilter), shrinking the remaining coupling from 18 → 13 files. The VSTestTraitnow appears only at the adapter conversion boundary —TestCaseExtensions.ToUnitTestElementWithUpdatedSource(hostTrait→TestTrait) andUnitTestElement.ToTestCase(TestTrait→ hostTrait) — both of which already reference the object model and move to the adapter in a later phase.Fidelity
TestTraitis[Serializable]on .NET Framework becauseUnitTestElementis serialized across app domains during isolated discovery/execution (verified by the net462 suite, incl.TypeEnumerator/discovery + trait tests, staying green).Name/Valueare preserved end-to-end, so trait →TestContextproperty reporting and the materialized hostTestCase.Traitsare byte-for-byte identical.TestTraitcarries no VSTest/ObjectModelnaming.Verification
build.cmd -c Debuggreen across all TFMs (net462/net8.0/net9.0/UWP/WinUI), IDE0005 clean.MSTestAdapter.PlatformServices.UnitTests: 897 (net8.0) / 935 (net462), 0 failed.MSTestAdapter.UnitTests: 21, 0 failed.MSTest.IntegrationTests(cross-proc, net462): 48 total, 0 failed, 1 skipped (OutputIsNotMixedWhenTestsRunInParallel, pre-existing known flaky).Stacking
Stacked chain onto
dev/amauryleve/vstest-decoupling-base:6c2 #9590→6d-1 #9591→6d-2 #9621→6e-1 #9622→6e-2 #9623→ 6e-3a (this).Base for this PR is the 6e-2 branch (
dev/amauryleve/vstest-decoupling-bridges2). Review/merge after the predecessors reach the base. Do not rebase/reset the base.