Neutralize the trait type on UnitTestElement (Phase 6e-3a) - #9624

Merged
Amaury Levé (Evangelink) merged 3 commits into
dev/amauryleve/vstest-decoupling-runcontextfrom
dev/amauryleve/vstest-decoupling-traits
Jul 5, 2026
Merged

Neutralize the trait type on UnitTestElement (Phase 6e-3a)#9624
Amaury Levé (Evangelink) merged 3 commits into
dev/amauryleve/vstest-decoupling-runcontextfrom
dev/amauryleve/vstest-decoupling-traits

Conversation

@Evangelink

Copy link
Copy Markdown
Member

Phase 6e-3a — Neutralize the trait type on UnitTestElement

Part of the initiative to make MSTestAdapter.PlatformServices platform-agnostic by removing its dependency on the VSTest object model (Microsoft.TestPlatform.ObjectModel). VSTest coupling moves up into MSTest.TestAdapter; the platform-services engine becomes neutral.

What this does

UnitTestElement.Traits was typed as the VSTest object-model Trait[], forcing every engine-side producer/consumer of traits to reference Microsoft.VisualStudio.TestPlatform.ObjectModel. This introduces a neutral TestTrait { Name, Value } struct (in the adapter's internal object model) and switches the neutral surface to it:

  • UnitTestElement.TraitsTestTrait[]?
  • ReflectHelper.GetTestPropertiesAsTraits / ReflectionHelper.GetTestPropertiesAsTraits now build TestTrait[]
  • TestExecutionManagerGetTestContextProperties, TestRunInfo, and the test-filter context read TestTrait

Result: 5 files stop referencing the VSTest object model (ReflectHelper, ReflectionHelper, TestExecutionManager.TestContext, TestRunInfo, UnitTestRunner.TestFilter), shrinking the remaining coupling from 18 → 13 files. The VSTest Trait now appears only at the adapter conversion boundary — TestCaseExtensions.ToUnitTestElementWithUpdatedSource (host TraitTestTrait) and UnitTestElement.ToTestCase (TestTrait → host Trait) — both of which already reference the object model and move to the adapter in a later phase.

Fidelity

  • TestTrait is [Serializable] on .NET Framework because UnitTestElement is serialized across app domains during isolated discovery/execution (verified by the net462 suite, incl. TypeEnumerator/discovery + trait tests, staying green).
  • Trait order and Name/Value are preserved end-to-end, so trait → TestContext property reporting and the materialized host TestCase.Traits are byte-for-byte identical.
  • TestTrait carries no VSTest/ObjectModel naming.

Verification

  • Full build.cmd -c Debug green 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 #95906d-1 #95916d-2 #96216e-1 #96226e-2 #96236e-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.

Amaury Levequeand others added 3 commits July 5, 2026 12:08
…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-runcontextJuly 5, 2026 19:23
@Evangelink
Amaury Levé (Evangelink) marked this pull request as ready for review July 5, 2026 19:23
@Evangelink
Amaury Levé (Evangelink) merged commit 60d363b into dev/amauryleve/vstest-decoupling-runcontextJul 5, 2026
27 of 29 checks passed
@Evangelink
Amaury Levé (Evangelink) deleted the dev/amauryleve/vstest-decoupling-traits branch July 5, 2026 19:24

@github-actionsgithub-actionsBot 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.

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:

  1. Trait.Name and Trait.Value survive 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&lt;Trait&gt;) overload with LINQ Select, that path would express intent more compactl…

Amaury Levé (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>
Amaury Levé (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>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant

@Evangelink
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Add copy buttons to all
 blocks\n(function() {\n function addCopyButtons() {\n document.querySelectorAll('pre code').forEach(function(codeBlock) {\n if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;\n codeBlock.parentElement.setAttribute('data-copy-added', 'true');\n \n var btn = document.createElement('button');\n btn.textContent = 'Copy';\n btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';\n btn.onmouseover = function() { this.style.opacity = '1'; };\n btn.onmouseout = function() { this.style.opacity = '0.7'; };\n btn.onclick = function() {\n navigator.clipboard.writeText(codeBlock.textContent).then(function() {\n btn.textContent = 'Copied!';\n setTimeout(function() { btn.textContent = 'Copy'; }, 1500);\n });\n };\n codeBlock.parentElement.style.position = 'relative';\n codeBlock.parentElement.appendChild(btn);\n });\n }\n \n addCopyButtons();\n \n // Re-run on dynamic content\n var observer = new MutationObserver(addCopyButtons);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Add Copy Buttons to Code Blocks");
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
Skip to content

Neutralize the trait type on UnitTestElement (Phase 6e-3a) - #9624

Merged
Amaury Levé (Evangelink) merged 3 commits into
dev/amauryleve/vstest-decoupling-runcontextfrom
dev/amauryleve/vstest-decoupling-traits
Jul 5, 2026
Merged

Neutralize the trait type on UnitTestElement (Phase 6e-3a)#9624
Amaury Levé (Evangelink) merged 3 commits into
dev/amauryleve/vstest-decoupling-runcontextfrom
dev/amauryleve/vstest-decoupling-traits

Conversation

@Evangelink

Copy link
Copy Markdown
Member

Phase 6e-3a — Neutralize the trait type on UnitTestElement

Part of the initiative to make MSTestAdapter.PlatformServices platform-agnostic by removing its dependency on the VSTest object model (Microsoft.TestPlatform.ObjectModel). VSTest coupling moves up into MSTest.TestAdapter; the platform-services engine becomes neutral.

What this does

UnitTestElement.Traits was typed as the VSTest object-model Trait[], forcing every engine-side producer/consumer of traits to reference Microsoft.VisualStudio.TestPlatform.ObjectModel. This introduces a neutral TestTrait { Name, Value } struct (in the adapter's internal object model) and switches the neutral surface to it:

  • UnitTestElement.TraitsTestTrait[]?
  • ReflectHelper.GetTestPropertiesAsTraits / ReflectionHelper.GetTestPropertiesAsTraits now build TestTrait[]
  • TestExecutionManagerGetTestContextProperties, TestRunInfo, and the test-filter context read TestTrait

Result: 5 files stop referencing the VSTest object model (ReflectHelper, ReflectionHelper, TestExecutionManager.TestContext, TestRunInfo, UnitTestRunner.TestFilter), shrinking the remaining coupling from 18 → 13 files. The VSTest Trait now appears only at the adapter conversion boundary — TestCaseExtensions.ToUnitTestElementWithUpdatedSource (host TraitTestTrait) and UnitTestElement.ToTestCase (TestTrait → host Trait) — both of which already reference the object model and move to the adapter in a later phase.

Fidelity

  • TestTrait is [Serializable] on .NET Framework because UnitTestElement is serialized across app domains during isolated discovery/execution (verified by the net462 suite, incl. TypeEnumerator/discovery + trait tests, staying green).
  • Trait order and Name/Value are preserved end-to-end, so trait → TestContext property reporting and the materialized host TestCase.Traits are byte-for-byte identical.
  • TestTrait carries no VSTest/ObjectModel naming.

Verification

  • Full build.cmd -c Debug green 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 #95906d-1 #95916d-2 #96216e-1 #96226e-2 #96236e-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.

Amaury Levequeand others added 3 commits July 5, 2026 12:08
…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-runcontextJuly 5, 2026 19:23
@Evangelink
Amaury Levé (Evangelink) marked this pull request as ready for review July 5, 2026 19:23
@Evangelink
Amaury Levé (Evangelink) merged commit 60d363b into dev/amauryleve/vstest-decoupling-runcontextJul 5, 2026
27 of 29 checks passed
@Evangelink
Amaury Levé (Evangelink) deleted the dev/amauryleve/vstest-decoupling-traits branch July 5, 2026 19:24

@github-actionsgithub-actionsBot 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.

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:

  1. Trait.Name and Trait.Value survive 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&lt;Trait&gt;) overload with LINQ Select, that path would express intent more compactl…

Amaury Levé (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>
Amaury Levé (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>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant

@Evangelink
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Force GitHub README to respect dark mode\n(function() {\n var style = document.createElement('style');\n style.textContent = '\n .markdown-body {\n color-scheme: dark light;\n }\n .markdown-body pre { background: #161b22 !important; }\n .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; }\n .markdown-body table th, .markdown-body table td { border-color: #30363d !important; }\n .markdown-body img { background: #0d1117; }\n .markdown-body blockquote { border-left-color: #8b949e; }\n .markdown-body hr { border-color: #30363d; }\n ';\n document.head.appendChild(style);\n})();", "GitHub Dark Mode README Fix"); } } catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Neutralize the trait type on UnitTestElement (Phase 6e-3a) - #9624

Merged
Amaury Levé (Evangelink) merged 3 commits into
dev/amauryleve/vstest-decoupling-runcontextfrom
dev/amauryleve/vstest-decoupling-traits
Jul 5, 2026
Merged

Neutralize the trait type on UnitTestElement (Phase 6e-3a)#9624
Amaury Levé (Evangelink) merged 3 commits into
dev/amauryleve/vstest-decoupling-runcontextfrom
dev/amauryleve/vstest-decoupling-traits

Conversation

@Evangelink

Copy link
Copy Markdown
Member

Phase 6e-3a — Neutralize the trait type on UnitTestElement

Part of the initiative to make MSTestAdapter.PlatformServices platform-agnostic by removing its dependency on the VSTest object model (Microsoft.TestPlatform.ObjectModel). VSTest coupling moves up into MSTest.TestAdapter; the platform-services engine becomes neutral.

What this does

UnitTestElement.Traits was typed as the VSTest object-model Trait[], forcing every engine-side producer/consumer of traits to reference Microsoft.VisualStudio.TestPlatform.ObjectModel. This introduces a neutral TestTrait { Name, Value } struct (in the adapter's internal object model) and switches the neutral surface to it:

  • UnitTestElement.TraitsTestTrait[]?
  • ReflectHelper.GetTestPropertiesAsTraits / ReflectionHelper.GetTestPropertiesAsTraits now build TestTrait[]
  • TestExecutionManagerGetTestContextProperties, TestRunInfo, and the test-filter context read TestTrait

Result: 5 files stop referencing the VSTest object model (ReflectHelper, ReflectionHelper, TestExecutionManager.TestContext, TestRunInfo, UnitTestRunner.TestFilter), shrinking the remaining coupling from 18 → 13 files. The VSTest Trait now appears only at the adapter conversion boundary — TestCaseExtensions.ToUnitTestElementWithUpdatedSource (host TraitTestTrait) and UnitTestElement.ToTestCase (TestTrait → host Trait) — both of which already reference the object model and move to the adapter in a later phase.

Fidelity

  • TestTrait is [Serializable] on .NET Framework because UnitTestElement is serialized across app domains during isolated discovery/execution (verified by the net462 suite, incl. TypeEnumerator/discovery + trait tests, staying green).
  • Trait order and Name/Value are preserved end-to-end, so trait → TestContext property reporting and the materialized host TestCase.Traits are byte-for-byte identical.
  • TestTrait carries no VSTest/ObjectModel naming.

Verification

  • Full build.cmd -c Debug green 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 #95906d-1 #95916d-2 #96216e-1 #96226e-2 #96236e-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.

Amaury Levequeand others added 3 commits July 5, 2026 12:08
…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-runcontextJuly 5, 2026 19:23
@Evangelink
Amaury Levé (Evangelink) marked this pull request as ready for review July 5, 2026 19:23
@Evangelink
Amaury Levé (Evangelink) merged commit 60d363b into dev/amauryleve/vstest-decoupling-runcontextJul 5, 2026
27 of 29 checks passed
@Evangelink
Amaury Levé (Evangelink) deleted the dev/amauryleve/vstest-decoupling-traits branch July 5, 2026 19:24

@github-actionsgithub-actionsBot 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.

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:

  1. Trait.Name and Trait.Value survive 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&lt;Trait&gt;) overload with LINQ Select, that path would express intent more compactl…

Amaury Levé (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>
Amaury Levé (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>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant

@Evangelink
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Highlight search terms from Google/DuckDuckGo/Bing referrer\n(function() {\n var ref = document.referrer;\n var terms = [];\n \n if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) {\n var url = new URL(ref);\n var q = url.searchParams.get('q') || url.searchParams.get('p');\n if (q) {\n terms = q.split(/\\s+/).filter(function(t) { return t.length > 2; });\n }\n }\n \n if (terms.length === 0) return;\n \n var style = document.createElement('style');\n style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }';\n document.head.appendChild(style);\n \n function highlight(node) {\n if (node.nodeType === 3) { // text node\n var text = node.textContent;\n var found = false;\n terms.forEach(function(term) {\n var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\\]\\\\]/g, '\\\\') + ')', 'gi');\n if (regex.test(text)) {\n found = true;\n var frag = document.createDocumentFragment();\n var parts = text.split(regex);\n parts.forEach(function(part, i) {\n if (i % 2 === 0) {\n frag.appendChild(document.createTextNode(part));\n } else {\n var span = document.createElement('span');\n span.className = 'userscript-highlight';\n span.textContent = part;\n frag.appendChild(span);\n }\n });\n node.parentNode.replaceChild(frag, node);\n }\n });\n } else if (node.nodeType === 1 && node.childNodes) { // element\n var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT'];\n if (!skipTags.includes(node.tagName)) {\n Array.from(node.childNodes).forEach(highlight);\n }\n }\n }\n \n highlight(document.body);\n \n // Re-highlight on dynamic content\n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1 || node.nodeType === 3) highlight(node);\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Highlight Search Terms"); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Neutralize the trait type on UnitTestElement (Phase 6e-3a) - #9624

Merged
Amaury Levé (Evangelink) merged 3 commits into
dev/amauryleve/vstest-decoupling-runcontextfrom
dev/amauryleve/vstest-decoupling-traits
Jul 5, 2026
Merged

Neutralize the trait type on UnitTestElement (Phase 6e-3a)#9624
Amaury Levé (Evangelink) merged 3 commits into
dev/amauryleve/vstest-decoupling-runcontextfrom
dev/amauryleve/vstest-decoupling-traits

Conversation

@Evangelink

Copy link
Copy Markdown
Member

Phase 6e-3a — Neutralize the trait type on UnitTestElement

Part of the initiative to make MSTestAdapter.PlatformServices platform-agnostic by removing its dependency on the VSTest object model (Microsoft.TestPlatform.ObjectModel). VSTest coupling moves up into MSTest.TestAdapter; the platform-services engine becomes neutral.

What this does

UnitTestElement.Traits was typed as the VSTest object-model Trait[], forcing every engine-side producer/consumer of traits to reference Microsoft.VisualStudio.TestPlatform.ObjectModel. This introduces a neutral TestTrait { Name, Value } struct (in the adapter's internal object model) and switches the neutral surface to it:

  • UnitTestElement.TraitsTestTrait[]?
  • ReflectHelper.GetTestPropertiesAsTraits / ReflectionHelper.GetTestPropertiesAsTraits now build TestTrait[]
  • TestExecutionManagerGetTestContextProperties, TestRunInfo, and the test-filter context read TestTrait

Result: 5 files stop referencing the VSTest object model (ReflectHelper, ReflectionHelper, TestExecutionManager.TestContext, TestRunInfo, UnitTestRunner.TestFilter), shrinking the remaining coupling from 18 → 13 files. The VSTest Trait now appears only at the adapter conversion boundary — TestCaseExtensions.ToUnitTestElementWithUpdatedSource (host TraitTestTrait) and UnitTestElement.ToTestCase (TestTrait → host Trait) — both of which already reference the object model and move to the adapter in a later phase.

Fidelity

  • TestTrait is [Serializable] on .NET Framework because UnitTestElement is serialized across app domains during isolated discovery/execution (verified by the net462 suite, incl. TypeEnumerator/discovery + trait tests, staying green).
  • Trait order and Name/Value are preserved end-to-end, so trait → TestContext property reporting and the materialized host TestCase.Traits are byte-for-byte identical.
  • TestTrait carries no VSTest/ObjectModel naming.

Verification

  • Full build.cmd -c Debug green 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 #95906d-1 #95916d-2 #96216e-1 #96226e-2 #96236e-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.

Amaury Levequeand others added 3 commits July 5, 2026 12:08
…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-runcontextJuly 5, 2026 19:23
@Evangelink
Amaury Levé (Evangelink) marked this pull request as ready for review July 5, 2026 19:23
@Evangelink
Amaury Levé (Evangelink) merged commit 60d363b into dev/amauryleve/vstest-decoupling-runcontextJul 5, 2026
27 of 29 checks passed
@Evangelink
Amaury Levé (Evangelink) deleted the dev/amauryleve/vstest-decoupling-traits branch July 5, 2026 19:24

@github-actionsgithub-actionsBot 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.

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:

  1. Trait.Name and Trait.Value survive 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&lt;Trait&gt;) overload with LINQ Select, that path would express intent more compactl…

Amaury Levé (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>
Amaury Levé (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>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant

@Evangelink
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Strip utm_, fbclid, gclid, etc. from all links on page\n(function() {\n var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content',\n 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid',\n 'ref', 'ref_src', 'source', 'medium', 'campaign'];\n \n function cleanUrl(url) {\n try {\n var u = new URL(url, window.location.origin);\n var changed = false;\n trackingParams.forEach(function(p) {\n if (u.searchParams.has(p)) {\n u.searchParams.delete(p);\n changed = true;\n }\n });\n return changed ? u.toString() : url;\n } catch (e) {\n return url;\n }\n }\n \n function cleanLinks() {\n document.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n \n cleanLinks();\n \n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1) {\n if (node.tagName === 'A') cleanLinks();\n node.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Remove Tracking Parameters from Links"); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + '
Skip to content

Neutralize the trait type on UnitTestElement (Phase 6e-3a) - #9624

Merged
Amaury Levé (Evangelink) merged 3 commits into
dev/amauryleve/vstest-decoupling-runcontextfrom
dev/amauryleve/vstest-decoupling-traits
Jul 5, 2026
Merged

Neutralize the trait type on UnitTestElement (Phase 6e-3a)#9624
Amaury Levé (Evangelink) merged 3 commits into
dev/amauryleve/vstest-decoupling-runcontextfrom
dev/amauryleve/vstest-decoupling-traits

Conversation

@Evangelink

Copy link
Copy Markdown
Member

Phase 6e-3a — Neutralize the trait type on UnitTestElement

Part of the initiative to make MSTestAdapter.PlatformServices platform-agnostic by removing its dependency on the VSTest object model (Microsoft.TestPlatform.ObjectModel). VSTest coupling moves up into MSTest.TestAdapter; the platform-services engine becomes neutral.

What this does

UnitTestElement.Traits was typed as the VSTest object-model Trait[], forcing every engine-side producer/consumer of traits to reference Microsoft.VisualStudio.TestPlatform.ObjectModel. This introduces a neutral TestTrait { Name, Value } struct (in the adapter's internal object model) and switches the neutral surface to it:

  • UnitTestElement.TraitsTestTrait[]?
  • ReflectHelper.GetTestPropertiesAsTraits / ReflectionHelper.GetTestPropertiesAsTraits now build TestTrait[]
  • TestExecutionManagerGetTestContextProperties, TestRunInfo, and the test-filter context read TestTrait

Result: 5 files stop referencing the VSTest object model (ReflectHelper, ReflectionHelper, TestExecutionManager.TestContext, TestRunInfo, UnitTestRunner.TestFilter), shrinking the remaining coupling from 18 → 13 files. The VSTest Trait now appears only at the adapter conversion boundary — TestCaseExtensions.ToUnitTestElementWithUpdatedSource (host TraitTestTrait) and UnitTestElement.ToTestCase (TestTrait → host Trait) — both of which already reference the object model and move to the adapter in a later phase.

Fidelity

  • TestTrait is [Serializable] on .NET Framework because UnitTestElement is serialized across app domains during isolated discovery/execution (verified by the net462 suite, incl. TypeEnumerator/discovery + trait tests, staying green).
  • Trait order and Name/Value are preserved end-to-end, so trait → TestContext property reporting and the materialized host TestCase.Traits are byte-for-byte identical.
  • TestTrait carries no VSTest/ObjectModel naming.

Verification

  • Full build.cmd -c Debug green 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 #95906d-1 #95916d-2 #96216e-1 #96226e-2 #96236e-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.

Amaury Levequeand others added 3 commits July 5, 2026 12:08
…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-runcontextJuly 5, 2026 19:23
@Evangelink
Amaury Levé (Evangelink) marked this pull request as ready for review July 5, 2026 19:23
@Evangelink
Amaury Levé (Evangelink) merged commit 60d363b into dev/amauryleve/vstest-decoupling-runcontextJul 5, 2026
27 of 29 checks passed
@Evangelink
Amaury Levé (Evangelink) deleted the dev/amauryleve/vstest-decoupling-traits branch July 5, 2026 19:24

@github-actionsgithub-actionsBot 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.

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:

  1. Trait.Name and Trait.Value survive 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&lt;Trait&gt;) overload with LINQ Select, that path would express intent more compactl…

Amaury Levé (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>
Amaury Levé (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>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant

@Evangelink
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Auto-enable theater mode on YouTube\n(function() {\n function tryTheater() {\n var btn = document.querySelector('button[aria-label=\"Theater mode\"], ytd-player #player button[title=\"Theater mode\"]');\n if (btn && !btn.classList.contains('activated')) {\n btn.click();\n }\n }\n \n // Try immediately\n tryTheater();\n \n // Try after navigation (SPA)\n var lastUrl = location.href;\n setInterval(function() {\n if (location.href !== lastUrl) {\n lastUrl = location.href;\n setTimeout(tryTheater, 500);\n }\n }, 1000);\n \n // Also try on player load\n var observer = new MutationObserver(tryTheater);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "YouTube Theater Mode Default"); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Neutralize the trait type on UnitTestElement (Phase 6e-3a) - #9624

Merged
Amaury Levé (Evangelink) merged 3 commits into
dev/amauryleve/vstest-decoupling-runcontextfrom
dev/amauryleve/vstest-decoupling-traits
Jul 5, 2026
Merged

Neutralize the trait type on UnitTestElement (Phase 6e-3a)#9624
Amaury Levé (Evangelink) merged 3 commits into
dev/amauryleve/vstest-decoupling-runcontextfrom
dev/amauryleve/vstest-decoupling-traits

Conversation

@Evangelink

Copy link
Copy Markdown
Member

Phase 6e-3a — Neutralize the trait type on UnitTestElement

Part of the initiative to make MSTestAdapter.PlatformServices platform-agnostic by removing its dependency on the VSTest object model (Microsoft.TestPlatform.ObjectModel). VSTest coupling moves up into MSTest.TestAdapter; the platform-services engine becomes neutral.

What this does

UnitTestElement.Traits was typed as the VSTest object-model Trait[], forcing every engine-side producer/consumer of traits to reference Microsoft.VisualStudio.TestPlatform.ObjectModel. This introduces a neutral TestTrait { Name, Value } struct (in the adapter's internal object model) and switches the neutral surface to it:

  • UnitTestElement.TraitsTestTrait[]?
  • ReflectHelper.GetTestPropertiesAsTraits / ReflectionHelper.GetTestPropertiesAsTraits now build TestTrait[]
  • TestExecutionManagerGetTestContextProperties, TestRunInfo, and the test-filter context read TestTrait

Result: 5 files stop referencing the VSTest object model (ReflectHelper, ReflectionHelper, TestExecutionManager.TestContext, TestRunInfo, UnitTestRunner.TestFilter), shrinking the remaining coupling from 18 → 13 files. The VSTest Trait now appears only at the adapter conversion boundary — TestCaseExtensions.ToUnitTestElementWithUpdatedSource (host TraitTestTrait) and UnitTestElement.ToTestCase (TestTrait → host Trait) — both of which already reference the object model and move to the adapter in a later phase.

Fidelity

  • TestTrait is [Serializable] on .NET Framework because UnitTestElement is serialized across app domains during isolated discovery/execution (verified by the net462 suite, incl. TypeEnumerator/discovery + trait tests, staying green).
  • Trait order and Name/Value are preserved end-to-end, so trait → TestContext property reporting and the materialized host TestCase.Traits are byte-for-byte identical.
  • TestTrait carries no VSTest/ObjectModel naming.

Verification

  • Full build.cmd -c Debug green 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 #95906d-1 #95916d-2 #96216e-1 #96226e-2 #96236e-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.

Amaury Levequeand others added 3 commits July 5, 2026 12:08
…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-runcontextJuly 5, 2026 19:23
@Evangelink
Amaury Levé (Evangelink) marked this pull request as ready for review July 5, 2026 19:23
@Evangelink
Amaury Levé (Evangelink) merged commit 60d363b into dev/amauryleve/vstest-decoupling-runcontextJul 5, 2026
27 of 29 checks passed
@Evangelink
Amaury Levé (Evangelink) deleted the dev/amauryleve/vstest-decoupling-traits branch July 5, 2026 19:24

@github-actionsgithub-actionsBot 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.

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:

  1. Trait.Name and Trait.Value survive 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&lt;Trait&gt;) overload with LINQ Select, that path would express intent more compactl…

Amaury Levé (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>
Amaury Levé (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>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant

@Evangelink
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Remove or un-stick sticky/fixed headers that block content\n(function() {\n function unstick() {\n document.querySelectorAll('header, nav, [role=\"banner\"], .header, .navbar, .sticky, .fixed-top, [style*=\"position: fixed\"], [style*=\"position:sticky\"]').forEach(function(el) {\n if (el.style.position === 'fixed' || el.style.position === 'sticky' || \n getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') {\n el.style.position = 'static';\n el.style.top = 'auto';\n el.style.zIndex = 'auto';\n }\n });\n }\n \n unstick();\n \n var observer = new MutationObserver(unstick);\n observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] });\n})();", "Kill Sticky Headers"); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Neutralize the trait type on UnitTestElement (Phase 6e-3a) - #9624

Merged
Amaury Levé (Evangelink) merged 3 commits into
dev/amauryleve/vstest-decoupling-runcontextfrom
dev/amauryleve/vstest-decoupling-traits
Jul 5, 2026
Merged

Neutralize the trait type on UnitTestElement (Phase 6e-3a)#9624
Amaury Levé (Evangelink) merged 3 commits into
dev/amauryleve/vstest-decoupling-runcontextfrom
dev/amauryleve/vstest-decoupling-traits

Conversation

@Evangelink

Copy link
Copy Markdown
Member

Phase 6e-3a — Neutralize the trait type on UnitTestElement

Part of the initiative to make MSTestAdapter.PlatformServices platform-agnostic by removing its dependency on the VSTest object model (Microsoft.TestPlatform.ObjectModel). VSTest coupling moves up into MSTest.TestAdapter; the platform-services engine becomes neutral.

What this does

UnitTestElement.Traits was typed as the VSTest object-model Trait[], forcing every engine-side producer/consumer of traits to reference Microsoft.VisualStudio.TestPlatform.ObjectModel. This introduces a neutral TestTrait { Name, Value } struct (in the adapter's internal object model) and switches the neutral surface to it:

  • UnitTestElement.TraitsTestTrait[]?
  • ReflectHelper.GetTestPropertiesAsTraits / ReflectionHelper.GetTestPropertiesAsTraits now build TestTrait[]
  • TestExecutionManagerGetTestContextProperties, TestRunInfo, and the test-filter context read TestTrait

Result: 5 files stop referencing the VSTest object model (ReflectHelper, ReflectionHelper, TestExecutionManager.TestContext, TestRunInfo, UnitTestRunner.TestFilter), shrinking the remaining coupling from 18 → 13 files. The VSTest Trait now appears only at the adapter conversion boundary — TestCaseExtensions.ToUnitTestElementWithUpdatedSource (host TraitTestTrait) and UnitTestElement.ToTestCase (TestTrait → host Trait) — both of which already reference the object model and move to the adapter in a later phase.

Fidelity

  • TestTrait is [Serializable] on .NET Framework because UnitTestElement is serialized across app domains during isolated discovery/execution (verified by the net462 suite, incl. TypeEnumerator/discovery + trait tests, staying green).
  • Trait order and Name/Value are preserved end-to-end, so trait → TestContext property reporting and the materialized host TestCase.Traits are byte-for-byte identical.
  • TestTrait carries no VSTest/ObjectModel naming.

Verification

  • Full build.cmd -c Debug green 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 #95906d-1 #95916d-2 #96216e-1 #96226e-2 #96236e-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.

Amaury Levequeand others added 3 commits July 5, 2026 12:08
…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-runcontextJuly 5, 2026 19:23
@Evangelink
Amaury Levé (Evangelink) marked this pull request as ready for review July 5, 2026 19:23
@Evangelink
Amaury Levé (Evangelink) merged commit 60d363b into dev/amauryleve/vstest-decoupling-runcontextJul 5, 2026
27 of 29 checks passed
@Evangelink
Amaury Levé (Evangelink) deleted the dev/amauryleve/vstest-decoupling-traits branch July 5, 2026 19:24

@github-actionsgithub-actionsBot 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.

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:

  1. Trait.Name and Trait.Value survive 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&lt;Trait&gt;) overload with LINQ Select, that path would express intent more compactl…

Amaury Levé (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>
Amaury Levé (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>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant

@Evangelink
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Universal Dark Mode - works on any site\n(function() {\n var enabled = true;\n \n function applyDarkMode() {\n if (!enabled) return;\n \n // Create style element if it doesn't exist\n var style = document.getElementById('universal-dark-mode-style');\n if (!style) {\n style = document.createElement('style');\n style.id = 'universal-dark-mode-style';\n document.head.appendChild(style);\n }\n \n // Dark mode CSS - inverts colors but preserves images/video\n style.textContent = '\n /* Invert everything except media */\n html {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #1a1a2e !important;\n }\n \n /* Restore images, videos, iframes, canvas */\n img, video, iframe, canvas, svg, picture, [style*=\"background-image\"] {\n filter: invert(1) hue-rotate(180deg) !important;\n }\n \n /* Preserve specific elements that should not be inverted */\n .no-dark-mode, .no-dark-mode *,\n [data-theme=\"light\"], [data-theme=\"light\"],\n .ace_editor, .ace_editor *,\n .CodeMirror, .CodeMirror *,\n .monaco-editor, .monaco-editor *,\n .markdown-body pre, .markdown-body pre *,\n .highlight, .highlight *,\n pre code, pre code * {\n filter: none !important;\n }\n \n /* Fix common UI elements */\n .modal, .popup, .dropdown-menu, .tooltip, .popover {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #2d2d44 !important;\n border-color: #444 !important;\n }\n \n /* Scrollbars */\n ::-webkit-scrollbar { background: #1a1a2e !important; }\n ::-webkit-scrollbar-thumb { background: #444 !important; }\n ::-webkit-scrollbar-thumb:hover { background: #555 !important; }\n \n /* Selection */\n ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ';\n }\n \n function removeDarkMode() {\n var style = document.getElementById('universal-dark-mode-style');\n if (style) style.remove();\n }\n \n // Toggle with Alt+Shift+D\n document.addEventListener('keydown', function(e) {\n if (e.altKey && e.shiftKey && e.key === 'D') {\n e.preventDefault();\n enabled = !enabled;\n if (enabled) {\n applyDarkMode();\n console.log('[Universal Dark Mode] Enabled');\n } else {\n removeDarkMode();\n console.log('[Universal Dark Mode] Disabled');\n }\n }\n });\n \n // Apply on load\n applyDarkMode();\n \n // Re-apply on dynamic content\n var observer = new MutationObserver(function(mutations) {\n if (enabled && !document.getElementById('universal-dark-mode-style')) {\n applyDarkMode();\n }\n });\n observer.observe(document.head, { childList: true });\n \n console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle');\n})();", "Universal Dark Mode"); } } catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); } })(); })();
Skip to content

Neutralize the trait type on UnitTestElement (Phase 6e-3a) - #9624

Merged
Amaury Levé (Evangelink) merged 3 commits into
dev/amauryleve/vstest-decoupling-runcontextfrom
dev/amauryleve/vstest-decoupling-traits
Jul 5, 2026
Merged

Neutralize the trait type on UnitTestElement (Phase 6e-3a)#9624
Amaury Levé (Evangelink) merged 3 commits into
dev/amauryleve/vstest-decoupling-runcontextfrom
dev/amauryleve/vstest-decoupling-traits

Conversation

@Evangelink

Copy link
Copy Markdown
Member

Phase 6e-3a — Neutralize the trait type on UnitTestElement

Part of the initiative to make MSTestAdapter.PlatformServices platform-agnostic by removing its dependency on the VSTest object model (Microsoft.TestPlatform.ObjectModel). VSTest coupling moves up into MSTest.TestAdapter; the platform-services engine becomes neutral.

What this does

UnitTestElement.Traits was typed as the VSTest object-model Trait[], forcing every engine-side producer/consumer of traits to reference Microsoft.VisualStudio.TestPlatform.ObjectModel. This introduces a neutral TestTrait { Name, Value } struct (in the adapter's internal object model) and switches the neutral surface to it:

  • UnitTestElement.TraitsTestTrait[]?
  • ReflectHelper.GetTestPropertiesAsTraits / ReflectionHelper.GetTestPropertiesAsTraits now build TestTrait[]
  • TestExecutionManagerGetTestContextProperties, TestRunInfo, and the test-filter context read TestTrait

Result: 5 files stop referencing the VSTest object model (ReflectHelper, ReflectionHelper, TestExecutionManager.TestContext, TestRunInfo, UnitTestRunner.TestFilter), shrinking the remaining coupling from 18 → 13 files. The VSTest Trait now appears only at the adapter conversion boundary — TestCaseExtensions.ToUnitTestElementWithUpdatedSource (host TraitTestTrait) and UnitTestElement.ToTestCase (TestTrait → host Trait) — both of which already reference the object model and move to the adapter in a later phase.

Fidelity

  • TestTrait is [Serializable] on .NET Framework because UnitTestElement is serialized across app domains during isolated discovery/execution (verified by the net462 suite, incl. TypeEnumerator/discovery + trait tests, staying green).
  • Trait order and Name/Value are preserved end-to-end, so trait → TestContext property reporting and the materialized host TestCase.Traits are byte-for-byte identical.
  • TestTrait carries no VSTest/ObjectModel naming.

Verification

  • Full build.cmd -c Debug green 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 #95906d-1 #95916d-2 #96216e-1 #96226e-2 #96236e-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.

Amaury Levequeand others added 3 commits July 5, 2026 12:08
…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-runcontextJuly 5, 2026 19:23
@Evangelink
Amaury Levé (Evangelink) marked this pull request as ready for review July 5, 2026 19:23
@Evangelink
Amaury Levé (Evangelink) merged commit 60d363b into dev/amauryleve/vstest-decoupling-runcontextJul 5, 2026
27 of 29 checks passed
@Evangelink
Amaury Levé (Evangelink) deleted the dev/amauryleve/vstest-decoupling-traits branch July 5, 2026 19:24

@github-actionsgithub-actionsBot 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.

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:

  1. Trait.Name and Trait.Value survive 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&lt;Trait&gt;) overload with LINQ Select, that path would express intent more compactl…

Amaury Levé (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>
Amaury Levé (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>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant

@Evangelink