Neutralize test message logging in PlatformServices execution engine (Phase 6c) - #9585

Merged
Amaury Levé (Evangelink) merged 1 commit into
dev/amauryleve/vstest-decoupling-basefrom
dev/amauryleve/vstest-decoupling-contexts
Jul 3, 2026
Merged

Neutralize test message logging in PlatformServices execution engine (Phase 6c)#9585
Amaury Levé (Evangelink) merged 1 commit into
dev/amauryleve/vstest-decoupling-basefrom
dev/amauryleve/vstest-decoupling-contexts

Conversation

@Evangelink

@EvangelinkAmaury Levé (Evangelink) commented Jul 3, 2026

Copy link
Copy Markdown
Member

Phase 6c of the PlatformServices platform-agnostic initiative

Part of the effort to remove the VSTest object model (Microsoft.TestPlatform.ObjectModel) dependency from MSTestAdapter.PlatformServices, moving VSTest coupling up into MSTest.TestAdapter.

This phase neutralizes the test message logger: it replaces the VSTest IMessageLogger with the platform-agnostic IAdapterMessageLogger (introduced in Phase 1) throughout the execution engine, test-context, and class-cleanup paths — including the logger marshaled into the isolation host (child AppDomain) for RunSingleTest.

What changed

  • RemotingMessageLogger now implements IAdapterMessageLogger instead of VSTest IMessageLogger (still MarshalByRefObject on netfx). It wraps a parent-domain IAdapterMessageLogger and forwards SendMessage(MessageLevel, string). Marshaling stays by reference — the wrapped logger never crosses into the child domain, and MessageLevel is a serializable enum, so no VSTest type is needed child-side.
  • IPlatformServiceProvider.GetTestContext, UnitTestRunner.RunSingleTest(Async)/ForceCleanup, ClassCleanupManager.ForceCleanup, TestContextImplementation (field + ctor), and InitializeRandomTestOrder now take IAdapterMessageLogger.
  • TestContext.DisplayMessage forwards MessageLevel directly to the neutral logger (the MessageLevelTestMessageLevel conversion now happens once, at the boundary bridge).
  • The engine obtains the neutral logger via frameworkHandle.ToAdapterMessageLogger() at the same points it already used for discovery.

No behavior change

The same messages cross the same AppDomain boundary to the same host logger; MessageLevel maps 1:1 to TestMessageLevel at the bridge. The non-AppDomain path is unchanged. The single remaining VSTest logging reference in PlatformServices is the AdapterMessageLoggerExtensions bridge, to be relocated with the other bridges in a later phase.

Verification

  • Full build green across all TFMs (net462, net8.0, net9.0, UWP, WinUI).
  • MSTestAdapter.PlatformServices.UnitTests: 897 (net8.0) / 935 (net462); MSTestAdapter.UnitTests: 21 — all pass.
  • MSTest.IntegrationTests (net462, exercises AppDomain-isolated execution → child-domain logging via the marshaled logger): 47 passed / 1 skipped / 0 failed.
  • Expert MSTest/MTP reviewer: no correctness/marshaling/fidelity/public-API findings.

Base

Remaining work (later phases)

…(Phase 6c)
Replace the VSTest IMessageLogger with the platform-agnostic IAdapterMessageLogger
throughout the execution engine, test-context and class-cleanup paths, including the
message logger marshaled into the isolation (child app-domain) host.
- RemotingMessageLogger now implements IAdapterMessageLogger (still MarshalByRefObject
on netfx); marshaling stays by-reference so no VSTest object-model type is needed in
the child domain.
- IPlatformServiceProvider.GetTestContext, UnitTestRunner.RunSingleTest(Async)/ForceCleanup,
ClassCleanupManager.ForceCleanup, TestContextImplementation and InitializeRandomTestOrder
now take IAdapterMessageLogger; TestContext.DisplayMessage forwards MessageLevel directly.
- The engine obtains the neutral logger via frameworkHandle.ToAdapterMessageLogger() at the
same points it already did for discovery. The single remaining VSTest logging reference is
the AdapterMessageLoggerExtensions bridge, relocated with the other bridges in a later phase.
No behavior change: the same messages cross the same app-domain boundary to the same host
logger; MessageLevel maps 1:1 to TestMessageLevel at the bridge.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
@Evangelink
Amaury Levé (Evangelink)force-pushed the dev/amauryleve/vstest-decoupling-contexts branch from d8c1a52 to a1392bbCompareJuly 3, 2026 14:49
@Evangelink
Amaury Levé (Evangelink) marked this pull request as ready for review July 3, 2026 15:05
@Evangelink
Amaury Levé (Evangelink) merged commit 274e479 into dev/amauryleve/vstest-decoupling-baseJul 3, 2026
19 of 24 checks passed
@Evangelink
Amaury Levé (Evangelink) deleted the dev/amauryleve/vstest-decoupling-contexts branch July 3, 2026 15:05

@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.

Note

🤖 Automated review by GitHub Copilot. Posted via a maintainer's GitHub token, so it appears under their account — the account owner did not write or approve this content personally. Generated by the Expert Code Review workflow. To request a follow-up action, reply by tagging @copilot directly.

✅ 22/22 dimensions clean — no findings.


Reviewed against all 22 dimensions. This is a clean, mechanical decoupling of IMessageLogger (VSTest) in favour of IAdapterMessageLogger (platform-agnostic) across 13 files. Key observations that were verified:

Correctness — The MessageLevelTestMessageLevel conversion is now centralised in exactly one place (AdapterMessageLoggerExtensions.HostMessageLogger.SendMessage). All call sites pass MessageLevel directly. The DisplayMessage simplification to _messageLogger?.SendMessage(messageLevel, message) is semantically equivalent to the six-line version it replaces.

AppDomain marshaling (netfx)RemotingMessageLogger is MarshalByRefObject on .NET Framework; the wrapped IAdapterMessageLogger stays in the parent domain. Only MessageLevel (a serializable enum) and string cross the AppDomain boundary. Correct.

Threading_adapterMessageLogger in the override partial class is written once (synchronously) before base.RunTestsAsync starts any parallel workers, then read in the downstream async continuation. Write-before-read in a single async chain — no race. Parallel test workers capture adapterMessageLogger (a local) by closure; SendMessage calls are concurrent but the VSTest host logger they ultimately reach is thread-safe.

! null-forgiving_adapterMessageLogger! in ExecuteTestsAsync override is safe because RunTestsAsync always initialises it before calling base.RunTestsAsync, which is the only path that invokes the override.

Cross-TFM#if NETFRAMEWORK guard on MarshalByRefObject is correct. Non-netfx stub provided.

Test assertionsMSTestAdapter.PlatformServices.UnitTests bans MSTest Assert; the updated tests correctly use AwesomeAssertions + Moq. Mocks set up with It.IsAny<MessageLevel>() and verified with specific MessageLevel values — correct.

Diff artefact in UnitTestRunner.cs (cosmetic) — Hunk 3 (ForceCleanup) shows the IMessageLogger signature as an unchanged context line alongside identical -/+ lines for the IAdapterMessageLogger version. This is a git diff alignment artefact from the stacked-PR base; the compiled result has only one ForceCleanup signature taking IAdapterMessageLogger. No action needed.

Removed using System.Diagnostics.CodeAnalysis — Safe: [NotNullWhen] was already removed from TestContextImplementation in a prior phase, making the directive dead code. Correct cleanup.

Amaury Levé (Evangelink) added a commit that referenced this pull request Jul 5, 2026
…(Phase 6c) (#9585)
Co-authored-by: Amaury Leveque <amauryleve@users.noreply.github.com>
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>
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 test message logging in PlatformServices execution engine (Phase 6c) - #9585

Merged
Amaury Levé (Evangelink) merged 1 commit into
dev/amauryleve/vstest-decoupling-basefrom
dev/amauryleve/vstest-decoupling-contexts
Jul 3, 2026
Merged

Neutralize test message logging in PlatformServices execution engine (Phase 6c)#9585
Amaury Levé (Evangelink) merged 1 commit into
dev/amauryleve/vstest-decoupling-basefrom
dev/amauryleve/vstest-decoupling-contexts

Conversation

@Evangelink

@EvangelinkAmaury Levé (Evangelink) commented Jul 3, 2026

Copy link
Copy Markdown
Member

Phase 6c of the PlatformServices platform-agnostic initiative

Part of the effort to remove the VSTest object model (Microsoft.TestPlatform.ObjectModel) dependency from MSTestAdapter.PlatformServices, moving VSTest coupling up into MSTest.TestAdapter.

This phase neutralizes the test message logger: it replaces the VSTest IMessageLogger with the platform-agnostic IAdapterMessageLogger (introduced in Phase 1) throughout the execution engine, test-context, and class-cleanup paths — including the logger marshaled into the isolation host (child AppDomain) for RunSingleTest.

What changed

  • RemotingMessageLogger now implements IAdapterMessageLogger instead of VSTest IMessageLogger (still MarshalByRefObject on netfx). It wraps a parent-domain IAdapterMessageLogger and forwards SendMessage(MessageLevel, string). Marshaling stays by reference — the wrapped logger never crosses into the child domain, and MessageLevel is a serializable enum, so no VSTest type is needed child-side.
  • IPlatformServiceProvider.GetTestContext, UnitTestRunner.RunSingleTest(Async)/ForceCleanup, ClassCleanupManager.ForceCleanup, TestContextImplementation (field + ctor), and InitializeRandomTestOrder now take IAdapterMessageLogger.
  • TestContext.DisplayMessage forwards MessageLevel directly to the neutral logger (the MessageLevelTestMessageLevel conversion now happens once, at the boundary bridge).
  • The engine obtains the neutral logger via frameworkHandle.ToAdapterMessageLogger() at the same points it already used for discovery.

No behavior change

The same messages cross the same AppDomain boundary to the same host logger; MessageLevel maps 1:1 to TestMessageLevel at the bridge. The non-AppDomain path is unchanged. The single remaining VSTest logging reference in PlatformServices is the AdapterMessageLoggerExtensions bridge, to be relocated with the other bridges in a later phase.

Verification

  • Full build green across all TFMs (net462, net8.0, net9.0, UWP, WinUI).
  • MSTestAdapter.PlatformServices.UnitTests: 897 (net8.0) / 935 (net462); MSTestAdapter.UnitTests: 21 — all pass.
  • MSTest.IntegrationTests (net462, exercises AppDomain-isolated execution → child-domain logging via the marshaled logger): 47 passed / 1 skipped / 0 failed.
  • Expert MSTest/MTP reviewer: no correctness/marshaling/fidelity/public-API findings.

Base

Remaining work (later phases)

…(Phase 6c)
Replace the VSTest IMessageLogger with the platform-agnostic IAdapterMessageLogger
throughout the execution engine, test-context and class-cleanup paths, including the
message logger marshaled into the isolation (child app-domain) host.
- RemotingMessageLogger now implements IAdapterMessageLogger (still MarshalByRefObject
on netfx); marshaling stays by-reference so no VSTest object-model type is needed in
the child domain.
- IPlatformServiceProvider.GetTestContext, UnitTestRunner.RunSingleTest(Async)/ForceCleanup,
ClassCleanupManager.ForceCleanup, TestContextImplementation and InitializeRandomTestOrder
now take IAdapterMessageLogger; TestContext.DisplayMessage forwards MessageLevel directly.
- The engine obtains the neutral logger via frameworkHandle.ToAdapterMessageLogger() at the
same points it already did for discovery. The single remaining VSTest logging reference is
the AdapterMessageLoggerExtensions bridge, relocated with the other bridges in a later phase.
No behavior change: the same messages cross the same app-domain boundary to the same host
logger; MessageLevel maps 1:1 to TestMessageLevel at the bridge.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
@Evangelink
Amaury Levé (Evangelink)force-pushed the dev/amauryleve/vstest-decoupling-contexts branch from d8c1a52 to a1392bbCompareJuly 3, 2026 14:49
@Evangelink
Amaury Levé (Evangelink) marked this pull request as ready for review July 3, 2026 15:05
@Evangelink
Amaury Levé (Evangelink) merged commit 274e479 into dev/amauryleve/vstest-decoupling-baseJul 3, 2026
19 of 24 checks passed
@Evangelink
Amaury Levé (Evangelink) deleted the dev/amauryleve/vstest-decoupling-contexts branch July 3, 2026 15:05

@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.

Note

🤖 Automated review by GitHub Copilot. Posted via a maintainer's GitHub token, so it appears under their account — the account owner did not write or approve this content personally. Generated by the Expert Code Review workflow. To request a follow-up action, reply by tagging @copilot directly.

✅ 22/22 dimensions clean — no findings.


Reviewed against all 22 dimensions. This is a clean, mechanical decoupling of IMessageLogger (VSTest) in favour of IAdapterMessageLogger (platform-agnostic) across 13 files. Key observations that were verified:

Correctness — The MessageLevelTestMessageLevel conversion is now centralised in exactly one place (AdapterMessageLoggerExtensions.HostMessageLogger.SendMessage). All call sites pass MessageLevel directly. The DisplayMessage simplification to _messageLogger?.SendMessage(messageLevel, message) is semantically equivalent to the six-line version it replaces.

AppDomain marshaling (netfx)RemotingMessageLogger is MarshalByRefObject on .NET Framework; the wrapped IAdapterMessageLogger stays in the parent domain. Only MessageLevel (a serializable enum) and string cross the AppDomain boundary. Correct.

Threading_adapterMessageLogger in the override partial class is written once (synchronously) before base.RunTestsAsync starts any parallel workers, then read in the downstream async continuation. Write-before-read in a single async chain — no race. Parallel test workers capture adapterMessageLogger (a local) by closure; SendMessage calls are concurrent but the VSTest host logger they ultimately reach is thread-safe.

! null-forgiving_adapterMessageLogger! in ExecuteTestsAsync override is safe because RunTestsAsync always initialises it before calling base.RunTestsAsync, which is the only path that invokes the override.

Cross-TFM#if NETFRAMEWORK guard on MarshalByRefObject is correct. Non-netfx stub provided.

Test assertionsMSTestAdapter.PlatformServices.UnitTests bans MSTest Assert; the updated tests correctly use AwesomeAssertions + Moq. Mocks set up with It.IsAny<MessageLevel>() and verified with specific MessageLevel values — correct.

Diff artefact in UnitTestRunner.cs (cosmetic) — Hunk 3 (ForceCleanup) shows the IMessageLogger signature as an unchanged context line alongside identical -/+ lines for the IAdapterMessageLogger version. This is a git diff alignment artefact from the stacked-PR base; the compiled result has only one ForceCleanup signature taking IAdapterMessageLogger. No action needed.

Removed using System.Diagnostics.CodeAnalysis — Safe: [NotNullWhen] was already removed from TestContextImplementation in a prior phase, making the directive dead code. Correct cleanup.

Amaury Levé (Evangelink) added a commit that referenced this pull request Jul 5, 2026
…(Phase 6c) (#9585)
Co-authored-by: Amaury Leveque <amauryleve@users.noreply.github.com>
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>
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 test message logging in PlatformServices execution engine (Phase 6c) - #9585

Merged
Amaury Levé (Evangelink) merged 1 commit into
dev/amauryleve/vstest-decoupling-basefrom
dev/amauryleve/vstest-decoupling-contexts
Jul 3, 2026
Merged

Neutralize test message logging in PlatformServices execution engine (Phase 6c)#9585
Amaury Levé (Evangelink) merged 1 commit into
dev/amauryleve/vstest-decoupling-basefrom
dev/amauryleve/vstest-decoupling-contexts

Conversation

@Evangelink

@EvangelinkAmaury Levé (Evangelink) commented Jul 3, 2026

Copy link
Copy Markdown
Member

Phase 6c of the PlatformServices platform-agnostic initiative

Part of the effort to remove the VSTest object model (Microsoft.TestPlatform.ObjectModel) dependency from MSTestAdapter.PlatformServices, moving VSTest coupling up into MSTest.TestAdapter.

This phase neutralizes the test message logger: it replaces the VSTest IMessageLogger with the platform-agnostic IAdapterMessageLogger (introduced in Phase 1) throughout the execution engine, test-context, and class-cleanup paths — including the logger marshaled into the isolation host (child AppDomain) for RunSingleTest.

What changed

  • RemotingMessageLogger now implements IAdapterMessageLogger instead of VSTest IMessageLogger (still MarshalByRefObject on netfx). It wraps a parent-domain IAdapterMessageLogger and forwards SendMessage(MessageLevel, string). Marshaling stays by reference — the wrapped logger never crosses into the child domain, and MessageLevel is a serializable enum, so no VSTest type is needed child-side.
  • IPlatformServiceProvider.GetTestContext, UnitTestRunner.RunSingleTest(Async)/ForceCleanup, ClassCleanupManager.ForceCleanup, TestContextImplementation (field + ctor), and InitializeRandomTestOrder now take IAdapterMessageLogger.
  • TestContext.DisplayMessage forwards MessageLevel directly to the neutral logger (the MessageLevelTestMessageLevel conversion now happens once, at the boundary bridge).
  • The engine obtains the neutral logger via frameworkHandle.ToAdapterMessageLogger() at the same points it already used for discovery.

No behavior change

The same messages cross the same AppDomain boundary to the same host logger; MessageLevel maps 1:1 to TestMessageLevel at the bridge. The non-AppDomain path is unchanged. The single remaining VSTest logging reference in PlatformServices is the AdapterMessageLoggerExtensions bridge, to be relocated with the other bridges in a later phase.

Verification

  • Full build green across all TFMs (net462, net8.0, net9.0, UWP, WinUI).
  • MSTestAdapter.PlatformServices.UnitTests: 897 (net8.0) / 935 (net462); MSTestAdapter.UnitTests: 21 — all pass.
  • MSTest.IntegrationTests (net462, exercises AppDomain-isolated execution → child-domain logging via the marshaled logger): 47 passed / 1 skipped / 0 failed.
  • Expert MSTest/MTP reviewer: no correctness/marshaling/fidelity/public-API findings.

Base

Remaining work (later phases)

…(Phase 6c)
Replace the VSTest IMessageLogger with the platform-agnostic IAdapterMessageLogger
throughout the execution engine, test-context and class-cleanup paths, including the
message logger marshaled into the isolation (child app-domain) host.
- RemotingMessageLogger now implements IAdapterMessageLogger (still MarshalByRefObject
on netfx); marshaling stays by-reference so no VSTest object-model type is needed in
the child domain.
- IPlatformServiceProvider.GetTestContext, UnitTestRunner.RunSingleTest(Async)/ForceCleanup,
ClassCleanupManager.ForceCleanup, TestContextImplementation and InitializeRandomTestOrder
now take IAdapterMessageLogger; TestContext.DisplayMessage forwards MessageLevel directly.
- The engine obtains the neutral logger via frameworkHandle.ToAdapterMessageLogger() at the
same points it already did for discovery. The single remaining VSTest logging reference is
the AdapterMessageLoggerExtensions bridge, relocated with the other bridges in a later phase.
No behavior change: the same messages cross the same app-domain boundary to the same host
logger; MessageLevel maps 1:1 to TestMessageLevel at the bridge.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
@Evangelink
Amaury Levé (Evangelink)force-pushed the dev/amauryleve/vstest-decoupling-contexts branch from d8c1a52 to a1392bbCompareJuly 3, 2026 14:49
@Evangelink
Amaury Levé (Evangelink) marked this pull request as ready for review July 3, 2026 15:05
@Evangelink
Amaury Levé (Evangelink) merged commit 274e479 into dev/amauryleve/vstest-decoupling-baseJul 3, 2026
19 of 24 checks passed
@Evangelink
Amaury Levé (Evangelink) deleted the dev/amauryleve/vstest-decoupling-contexts branch July 3, 2026 15:05

@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.

Note

🤖 Automated review by GitHub Copilot. Posted via a maintainer's GitHub token, so it appears under their account — the account owner did not write or approve this content personally. Generated by the Expert Code Review workflow. To request a follow-up action, reply by tagging @copilot directly.

✅ 22/22 dimensions clean — no findings.


Reviewed against all 22 dimensions. This is a clean, mechanical decoupling of IMessageLogger (VSTest) in favour of IAdapterMessageLogger (platform-agnostic) across 13 files. Key observations that were verified:

Correctness — The MessageLevelTestMessageLevel conversion is now centralised in exactly one place (AdapterMessageLoggerExtensions.HostMessageLogger.SendMessage). All call sites pass MessageLevel directly. The DisplayMessage simplification to _messageLogger?.SendMessage(messageLevel, message) is semantically equivalent to the six-line version it replaces.

AppDomain marshaling (netfx)RemotingMessageLogger is MarshalByRefObject on .NET Framework; the wrapped IAdapterMessageLogger stays in the parent domain. Only MessageLevel (a serializable enum) and string cross the AppDomain boundary. Correct.

Threading_adapterMessageLogger in the override partial class is written once (synchronously) before base.RunTestsAsync starts any parallel workers, then read in the downstream async continuation. Write-before-read in a single async chain — no race. Parallel test workers capture adapterMessageLogger (a local) by closure; SendMessage calls are concurrent but the VSTest host logger they ultimately reach is thread-safe.

! null-forgiving_adapterMessageLogger! in ExecuteTestsAsync override is safe because RunTestsAsync always initialises it before calling base.RunTestsAsync, which is the only path that invokes the override.

Cross-TFM#if NETFRAMEWORK guard on MarshalByRefObject is correct. Non-netfx stub provided.

Test assertionsMSTestAdapter.PlatformServices.UnitTests bans MSTest Assert; the updated tests correctly use AwesomeAssertions + Moq. Mocks set up with It.IsAny<MessageLevel>() and verified with specific MessageLevel values — correct.

Diff artefact in UnitTestRunner.cs (cosmetic) — Hunk 3 (ForceCleanup) shows the IMessageLogger signature as an unchanged context line alongside identical -/+ lines for the IAdapterMessageLogger version. This is a git diff alignment artefact from the stacked-PR base; the compiled result has only one ForceCleanup signature taking IAdapterMessageLogger. No action needed.

Removed using System.Diagnostics.CodeAnalysis — Safe: [NotNullWhen] was already removed from TestContextImplementation in a prior phase, making the directive dead code. Correct cleanup.

Amaury Levé (Evangelink) added a commit that referenced this pull request Jul 5, 2026
…(Phase 6c) (#9585)
Co-authored-by: Amaury Leveque <amauryleve@users.noreply.github.com>
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>
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 test message logging in PlatformServices execution engine (Phase 6c) - #9585

Merged
Amaury Levé (Evangelink) merged 1 commit into
dev/amauryleve/vstest-decoupling-basefrom
dev/amauryleve/vstest-decoupling-contexts
Jul 3, 2026
Merged

Neutralize test message logging in PlatformServices execution engine (Phase 6c)#9585
Amaury Levé (Evangelink) merged 1 commit into
dev/amauryleve/vstest-decoupling-basefrom
dev/amauryleve/vstest-decoupling-contexts

Conversation

@Evangelink

@EvangelinkAmaury Levé (Evangelink) commented Jul 3, 2026

Copy link
Copy Markdown
Member

Phase 6c of the PlatformServices platform-agnostic initiative

Part of the effort to remove the VSTest object model (Microsoft.TestPlatform.ObjectModel) dependency from MSTestAdapter.PlatformServices, moving VSTest coupling up into MSTest.TestAdapter.

This phase neutralizes the test message logger: it replaces the VSTest IMessageLogger with the platform-agnostic IAdapterMessageLogger (introduced in Phase 1) throughout the execution engine, test-context, and class-cleanup paths — including the logger marshaled into the isolation host (child AppDomain) for RunSingleTest.

What changed

  • RemotingMessageLogger now implements IAdapterMessageLogger instead of VSTest IMessageLogger (still MarshalByRefObject on netfx). It wraps a parent-domain IAdapterMessageLogger and forwards SendMessage(MessageLevel, string). Marshaling stays by reference — the wrapped logger never crosses into the child domain, and MessageLevel is a serializable enum, so no VSTest type is needed child-side.
  • IPlatformServiceProvider.GetTestContext, UnitTestRunner.RunSingleTest(Async)/ForceCleanup, ClassCleanupManager.ForceCleanup, TestContextImplementation (field + ctor), and InitializeRandomTestOrder now take IAdapterMessageLogger.
  • TestContext.DisplayMessage forwards MessageLevel directly to the neutral logger (the MessageLevelTestMessageLevel conversion now happens once, at the boundary bridge).
  • The engine obtains the neutral logger via frameworkHandle.ToAdapterMessageLogger() at the same points it already used for discovery.

No behavior change

The same messages cross the same AppDomain boundary to the same host logger; MessageLevel maps 1:1 to TestMessageLevel at the bridge. The non-AppDomain path is unchanged. The single remaining VSTest logging reference in PlatformServices is the AdapterMessageLoggerExtensions bridge, to be relocated with the other bridges in a later phase.

Verification

  • Full build green across all TFMs (net462, net8.0, net9.0, UWP, WinUI).
  • MSTestAdapter.PlatformServices.UnitTests: 897 (net8.0) / 935 (net462); MSTestAdapter.UnitTests: 21 — all pass.
  • MSTest.IntegrationTests (net462, exercises AppDomain-isolated execution → child-domain logging via the marshaled logger): 47 passed / 1 skipped / 0 failed.
  • Expert MSTest/MTP reviewer: no correctness/marshaling/fidelity/public-API findings.

Base

Remaining work (later phases)

…(Phase 6c)
Replace the VSTest IMessageLogger with the platform-agnostic IAdapterMessageLogger
throughout the execution engine, test-context and class-cleanup paths, including the
message logger marshaled into the isolation (child app-domain) host.
- RemotingMessageLogger now implements IAdapterMessageLogger (still MarshalByRefObject
on netfx); marshaling stays by-reference so no VSTest object-model type is needed in
the child domain.
- IPlatformServiceProvider.GetTestContext, UnitTestRunner.RunSingleTest(Async)/ForceCleanup,
ClassCleanupManager.ForceCleanup, TestContextImplementation and InitializeRandomTestOrder
now take IAdapterMessageLogger; TestContext.DisplayMessage forwards MessageLevel directly.
- The engine obtains the neutral logger via frameworkHandle.ToAdapterMessageLogger() at the
same points it already did for discovery. The single remaining VSTest logging reference is
the AdapterMessageLoggerExtensions bridge, relocated with the other bridges in a later phase.
No behavior change: the same messages cross the same app-domain boundary to the same host
logger; MessageLevel maps 1:1 to TestMessageLevel at the bridge.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
@Evangelink
Amaury Levé (Evangelink)force-pushed the dev/amauryleve/vstest-decoupling-contexts branch from d8c1a52 to a1392bbCompareJuly 3, 2026 14:49
@Evangelink
Amaury Levé (Evangelink) marked this pull request as ready for review July 3, 2026 15:05
@Evangelink
Amaury Levé (Evangelink) merged commit 274e479 into dev/amauryleve/vstest-decoupling-baseJul 3, 2026
19 of 24 checks passed
@Evangelink
Amaury Levé (Evangelink) deleted the dev/amauryleve/vstest-decoupling-contexts branch July 3, 2026 15:05

@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.

Note

🤖 Automated review by GitHub Copilot. Posted via a maintainer's GitHub token, so it appears under their account — the account owner did not write or approve this content personally. Generated by the Expert Code Review workflow. To request a follow-up action, reply by tagging @copilot directly.

✅ 22/22 dimensions clean — no findings.


Reviewed against all 22 dimensions. This is a clean, mechanical decoupling of IMessageLogger (VSTest) in favour of IAdapterMessageLogger (platform-agnostic) across 13 files. Key observations that were verified:

Correctness — The MessageLevelTestMessageLevel conversion is now centralised in exactly one place (AdapterMessageLoggerExtensions.HostMessageLogger.SendMessage). All call sites pass MessageLevel directly. The DisplayMessage simplification to _messageLogger?.SendMessage(messageLevel, message) is semantically equivalent to the six-line version it replaces.

AppDomain marshaling (netfx)RemotingMessageLogger is MarshalByRefObject on .NET Framework; the wrapped IAdapterMessageLogger stays in the parent domain. Only MessageLevel (a serializable enum) and string cross the AppDomain boundary. Correct.

Threading_adapterMessageLogger in the override partial class is written once (synchronously) before base.RunTestsAsync starts any parallel workers, then read in the downstream async continuation. Write-before-read in a single async chain — no race. Parallel test workers capture adapterMessageLogger (a local) by closure; SendMessage calls are concurrent but the VSTest host logger they ultimately reach is thread-safe.

! null-forgiving_adapterMessageLogger! in ExecuteTestsAsync override is safe because RunTestsAsync always initialises it before calling base.RunTestsAsync, which is the only path that invokes the override.

Cross-TFM#if NETFRAMEWORK guard on MarshalByRefObject is correct. Non-netfx stub provided.

Test assertionsMSTestAdapter.PlatformServices.UnitTests bans MSTest Assert; the updated tests correctly use AwesomeAssertions + Moq. Mocks set up with It.IsAny<MessageLevel>() and verified with specific MessageLevel values — correct.

Diff artefact in UnitTestRunner.cs (cosmetic) — Hunk 3 (ForceCleanup) shows the IMessageLogger signature as an unchanged context line alongside identical -/+ lines for the IAdapterMessageLogger version. This is a git diff alignment artefact from the stacked-PR base; the compiled result has only one ForceCleanup signature taking IAdapterMessageLogger. No action needed.

Removed using System.Diagnostics.CodeAnalysis — Safe: [NotNullWhen] was already removed from TestContextImplementation in a prior phase, making the directive dead code. Correct cleanup.

Amaury Levé (Evangelink) added a commit that referenced this pull request Jul 5, 2026
…(Phase 6c) (#9585)
Co-authored-by: Amaury Leveque <amauryleve@users.noreply.github.com>
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>
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 test message logging in PlatformServices execution engine (Phase 6c) - #9585

Merged
Amaury Levé (Evangelink) merged 1 commit into
dev/amauryleve/vstest-decoupling-basefrom
dev/amauryleve/vstest-decoupling-contexts
Jul 3, 2026
Merged

Neutralize test message logging in PlatformServices execution engine (Phase 6c)#9585
Amaury Levé (Evangelink) merged 1 commit into
dev/amauryleve/vstest-decoupling-basefrom
dev/amauryleve/vstest-decoupling-contexts

Conversation

@Evangelink

@EvangelinkAmaury Levé (Evangelink) commented Jul 3, 2026

Copy link
Copy Markdown
Member

Phase 6c of the PlatformServices platform-agnostic initiative

Part of the effort to remove the VSTest object model (Microsoft.TestPlatform.ObjectModel) dependency from MSTestAdapter.PlatformServices, moving VSTest coupling up into MSTest.TestAdapter.

This phase neutralizes the test message logger: it replaces the VSTest IMessageLogger with the platform-agnostic IAdapterMessageLogger (introduced in Phase 1) throughout the execution engine, test-context, and class-cleanup paths — including the logger marshaled into the isolation host (child AppDomain) for RunSingleTest.

What changed

  • RemotingMessageLogger now implements IAdapterMessageLogger instead of VSTest IMessageLogger (still MarshalByRefObject on netfx). It wraps a parent-domain IAdapterMessageLogger and forwards SendMessage(MessageLevel, string). Marshaling stays by reference — the wrapped logger never crosses into the child domain, and MessageLevel is a serializable enum, so no VSTest type is needed child-side.
  • IPlatformServiceProvider.GetTestContext, UnitTestRunner.RunSingleTest(Async)/ForceCleanup, ClassCleanupManager.ForceCleanup, TestContextImplementation (field + ctor), and InitializeRandomTestOrder now take IAdapterMessageLogger.
  • TestContext.DisplayMessage forwards MessageLevel directly to the neutral logger (the MessageLevelTestMessageLevel conversion now happens once, at the boundary bridge).
  • The engine obtains the neutral logger via frameworkHandle.ToAdapterMessageLogger() at the same points it already used for discovery.

No behavior change

The same messages cross the same AppDomain boundary to the same host logger; MessageLevel maps 1:1 to TestMessageLevel at the bridge. The non-AppDomain path is unchanged. The single remaining VSTest logging reference in PlatformServices is the AdapterMessageLoggerExtensions bridge, to be relocated with the other bridges in a later phase.

Verification

  • Full build green across all TFMs (net462, net8.0, net9.0, UWP, WinUI).
  • MSTestAdapter.PlatformServices.UnitTests: 897 (net8.0) / 935 (net462); MSTestAdapter.UnitTests: 21 — all pass.
  • MSTest.IntegrationTests (net462, exercises AppDomain-isolated execution → child-domain logging via the marshaled logger): 47 passed / 1 skipped / 0 failed.
  • Expert MSTest/MTP reviewer: no correctness/marshaling/fidelity/public-API findings.

Base

Remaining work (later phases)

…(Phase 6c)
Replace the VSTest IMessageLogger with the platform-agnostic IAdapterMessageLogger
throughout the execution engine, test-context and class-cleanup paths, including the
message logger marshaled into the isolation (child app-domain) host.
- RemotingMessageLogger now implements IAdapterMessageLogger (still MarshalByRefObject
on netfx); marshaling stays by-reference so no VSTest object-model type is needed in
the child domain.
- IPlatformServiceProvider.GetTestContext, UnitTestRunner.RunSingleTest(Async)/ForceCleanup,
ClassCleanupManager.ForceCleanup, TestContextImplementation and InitializeRandomTestOrder
now take IAdapterMessageLogger; TestContext.DisplayMessage forwards MessageLevel directly.
- The engine obtains the neutral logger via frameworkHandle.ToAdapterMessageLogger() at the
same points it already did for discovery. The single remaining VSTest logging reference is
the AdapterMessageLoggerExtensions bridge, relocated with the other bridges in a later phase.
No behavior change: the same messages cross the same app-domain boundary to the same host
logger; MessageLevel maps 1:1 to TestMessageLevel at the bridge.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
@Evangelink
Amaury Levé (Evangelink)force-pushed the dev/amauryleve/vstest-decoupling-contexts branch from d8c1a52 to a1392bbCompareJuly 3, 2026 14:49
@Evangelink
Amaury Levé (Evangelink) marked this pull request as ready for review July 3, 2026 15:05
@Evangelink
Amaury Levé (Evangelink) merged commit 274e479 into dev/amauryleve/vstest-decoupling-baseJul 3, 2026
19 of 24 checks passed
@Evangelink
Amaury Levé (Evangelink) deleted the dev/amauryleve/vstest-decoupling-contexts branch July 3, 2026 15:05

@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.

Note

🤖 Automated review by GitHub Copilot. Posted via a maintainer's GitHub token, so it appears under their account — the account owner did not write or approve this content personally. Generated by the Expert Code Review workflow. To request a follow-up action, reply by tagging @copilot directly.

✅ 22/22 dimensions clean — no findings.


Reviewed against all 22 dimensions. This is a clean, mechanical decoupling of IMessageLogger (VSTest) in favour of IAdapterMessageLogger (platform-agnostic) across 13 files. Key observations that were verified:

Correctness — The MessageLevelTestMessageLevel conversion is now centralised in exactly one place (AdapterMessageLoggerExtensions.HostMessageLogger.SendMessage). All call sites pass MessageLevel directly. The DisplayMessage simplification to _messageLogger?.SendMessage(messageLevel, message) is semantically equivalent to the six-line version it replaces.

AppDomain marshaling (netfx)RemotingMessageLogger is MarshalByRefObject on .NET Framework; the wrapped IAdapterMessageLogger stays in the parent domain. Only MessageLevel (a serializable enum) and string cross the AppDomain boundary. Correct.

Threading_adapterMessageLogger in the override partial class is written once (synchronously) before base.RunTestsAsync starts any parallel workers, then read in the downstream async continuation. Write-before-read in a single async chain — no race. Parallel test workers capture adapterMessageLogger (a local) by closure; SendMessage calls are concurrent but the VSTest host logger they ultimately reach is thread-safe.

! null-forgiving_adapterMessageLogger! in ExecuteTestsAsync override is safe because RunTestsAsync always initialises it before calling base.RunTestsAsync, which is the only path that invokes the override.

Cross-TFM#if NETFRAMEWORK guard on MarshalByRefObject is correct. Non-netfx stub provided.

Test assertionsMSTestAdapter.PlatformServices.UnitTests bans MSTest Assert; the updated tests correctly use AwesomeAssertions + Moq. Mocks set up with It.IsAny<MessageLevel>() and verified with specific MessageLevel values — correct.

Diff artefact in UnitTestRunner.cs (cosmetic) — Hunk 3 (ForceCleanup) shows the IMessageLogger signature as an unchanged context line alongside identical -/+ lines for the IAdapterMessageLogger version. This is a git diff alignment artefact from the stacked-PR base; the compiled result has only one ForceCleanup signature taking IAdapterMessageLogger. No action needed.

Removed using System.Diagnostics.CodeAnalysis — Safe: [NotNullWhen] was already removed from TestContextImplementation in a prior phase, making the directive dead code. Correct cleanup.

Amaury Levé (Evangelink) added a commit that referenced this pull request Jul 5, 2026
…(Phase 6c) (#9585)
Co-authored-by: Amaury Leveque <amauryleve@users.noreply.github.com>
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>
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 test message logging in PlatformServices execution engine (Phase 6c) - #9585

Merged
Amaury Levé (Evangelink) merged 1 commit into
dev/amauryleve/vstest-decoupling-basefrom
dev/amauryleve/vstest-decoupling-contexts
Jul 3, 2026
Merged

Neutralize test message logging in PlatformServices execution engine (Phase 6c)#9585
Amaury Levé (Evangelink) merged 1 commit into
dev/amauryleve/vstest-decoupling-basefrom
dev/amauryleve/vstest-decoupling-contexts

Conversation

@Evangelink

@EvangelinkAmaury Levé (Evangelink) commented Jul 3, 2026

Copy link
Copy Markdown
Member

Phase 6c of the PlatformServices platform-agnostic initiative

Part of the effort to remove the VSTest object model (Microsoft.TestPlatform.ObjectModel) dependency from MSTestAdapter.PlatformServices, moving VSTest coupling up into MSTest.TestAdapter.

This phase neutralizes the test message logger: it replaces the VSTest IMessageLogger with the platform-agnostic IAdapterMessageLogger (introduced in Phase 1) throughout the execution engine, test-context, and class-cleanup paths — including the logger marshaled into the isolation host (child AppDomain) for RunSingleTest.

What changed

  • RemotingMessageLogger now implements IAdapterMessageLogger instead of VSTest IMessageLogger (still MarshalByRefObject on netfx). It wraps a parent-domain IAdapterMessageLogger and forwards SendMessage(MessageLevel, string). Marshaling stays by reference — the wrapped logger never crosses into the child domain, and MessageLevel is a serializable enum, so no VSTest type is needed child-side.
  • IPlatformServiceProvider.GetTestContext, UnitTestRunner.RunSingleTest(Async)/ForceCleanup, ClassCleanupManager.ForceCleanup, TestContextImplementation (field + ctor), and InitializeRandomTestOrder now take IAdapterMessageLogger.
  • TestContext.DisplayMessage forwards MessageLevel directly to the neutral logger (the MessageLevelTestMessageLevel conversion now happens once, at the boundary bridge).
  • The engine obtains the neutral logger via frameworkHandle.ToAdapterMessageLogger() at the same points it already used for discovery.

No behavior change

The same messages cross the same AppDomain boundary to the same host logger; MessageLevel maps 1:1 to TestMessageLevel at the bridge. The non-AppDomain path is unchanged. The single remaining VSTest logging reference in PlatformServices is the AdapterMessageLoggerExtensions bridge, to be relocated with the other bridges in a later phase.

Verification

  • Full build green across all TFMs (net462, net8.0, net9.0, UWP, WinUI).
  • MSTestAdapter.PlatformServices.UnitTests: 897 (net8.0) / 935 (net462); MSTestAdapter.UnitTests: 21 — all pass.
  • MSTest.IntegrationTests (net462, exercises AppDomain-isolated execution → child-domain logging via the marshaled logger): 47 passed / 1 skipped / 0 failed.
  • Expert MSTest/MTP reviewer: no correctness/marshaling/fidelity/public-API findings.

Base

Remaining work (later phases)

…(Phase 6c)
Replace the VSTest IMessageLogger with the platform-agnostic IAdapterMessageLogger
throughout the execution engine, test-context and class-cleanup paths, including the
message logger marshaled into the isolation (child app-domain) host.
- RemotingMessageLogger now implements IAdapterMessageLogger (still MarshalByRefObject
on netfx); marshaling stays by-reference so no VSTest object-model type is needed in
the child domain.
- IPlatformServiceProvider.GetTestContext, UnitTestRunner.RunSingleTest(Async)/ForceCleanup,
ClassCleanupManager.ForceCleanup, TestContextImplementation and InitializeRandomTestOrder
now take IAdapterMessageLogger; TestContext.DisplayMessage forwards MessageLevel directly.
- The engine obtains the neutral logger via frameworkHandle.ToAdapterMessageLogger() at the
same points it already did for discovery. The single remaining VSTest logging reference is
the AdapterMessageLoggerExtensions bridge, relocated with the other bridges in a later phase.
No behavior change: the same messages cross the same app-domain boundary to the same host
logger; MessageLevel maps 1:1 to TestMessageLevel at the bridge.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
@Evangelink
Amaury Levé (Evangelink)force-pushed the dev/amauryleve/vstest-decoupling-contexts branch from d8c1a52 to a1392bbCompareJuly 3, 2026 14:49
@Evangelink
Amaury Levé (Evangelink) marked this pull request as ready for review July 3, 2026 15:05
@Evangelink
Amaury Levé (Evangelink) merged commit 274e479 into dev/amauryleve/vstest-decoupling-baseJul 3, 2026
19 of 24 checks passed
@Evangelink
Amaury Levé (Evangelink) deleted the dev/amauryleve/vstest-decoupling-contexts branch July 3, 2026 15:05

@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.

Note

🤖 Automated review by GitHub Copilot. Posted via a maintainer's GitHub token, so it appears under their account — the account owner did not write or approve this content personally. Generated by the Expert Code Review workflow. To request a follow-up action, reply by tagging @copilot directly.

✅ 22/22 dimensions clean — no findings.


Reviewed against all 22 dimensions. This is a clean, mechanical decoupling of IMessageLogger (VSTest) in favour of IAdapterMessageLogger (platform-agnostic) across 13 files. Key observations that were verified:

Correctness — The MessageLevelTestMessageLevel conversion is now centralised in exactly one place (AdapterMessageLoggerExtensions.HostMessageLogger.SendMessage). All call sites pass MessageLevel directly. The DisplayMessage simplification to _messageLogger?.SendMessage(messageLevel, message) is semantically equivalent to the six-line version it replaces.

AppDomain marshaling (netfx)RemotingMessageLogger is MarshalByRefObject on .NET Framework; the wrapped IAdapterMessageLogger stays in the parent domain. Only MessageLevel (a serializable enum) and string cross the AppDomain boundary. Correct.

Threading_adapterMessageLogger in the override partial class is written once (synchronously) before base.RunTestsAsync starts any parallel workers, then read in the downstream async continuation. Write-before-read in a single async chain — no race. Parallel test workers capture adapterMessageLogger (a local) by closure; SendMessage calls are concurrent but the VSTest host logger they ultimately reach is thread-safe.

! null-forgiving_adapterMessageLogger! in ExecuteTestsAsync override is safe because RunTestsAsync always initialises it before calling base.RunTestsAsync, which is the only path that invokes the override.

Cross-TFM#if NETFRAMEWORK guard on MarshalByRefObject is correct. Non-netfx stub provided.

Test assertionsMSTestAdapter.PlatformServices.UnitTests bans MSTest Assert; the updated tests correctly use AwesomeAssertions + Moq. Mocks set up with It.IsAny<MessageLevel>() and verified with specific MessageLevel values — correct.

Diff artefact in UnitTestRunner.cs (cosmetic) — Hunk 3 (ForceCleanup) shows the IMessageLogger signature as an unchanged context line alongside identical -/+ lines for the IAdapterMessageLogger version. This is a git diff alignment artefact from the stacked-PR base; the compiled result has only one ForceCleanup signature taking IAdapterMessageLogger. No action needed.

Removed using System.Diagnostics.CodeAnalysis — Safe: [NotNullWhen] was already removed from TestContextImplementation in a prior phase, making the directive dead code. Correct cleanup.

Amaury Levé (Evangelink) added a commit that referenced this pull request Jul 5, 2026
…(Phase 6c) (#9585)
Co-authored-by: Amaury Leveque <amauryleve@users.noreply.github.com>
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>
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 test message logging in PlatformServices execution engine (Phase 6c) - #9585

Merged
Amaury Levé (Evangelink) merged 1 commit into
dev/amauryleve/vstest-decoupling-basefrom
dev/amauryleve/vstest-decoupling-contexts
Jul 3, 2026
Merged

Neutralize test message logging in PlatformServices execution engine (Phase 6c)#9585
Amaury Levé (Evangelink) merged 1 commit into
dev/amauryleve/vstest-decoupling-basefrom
dev/amauryleve/vstest-decoupling-contexts

Conversation

@Evangelink

@EvangelinkAmaury Levé (Evangelink) commented Jul 3, 2026

Copy link
Copy Markdown
Member

Phase 6c of the PlatformServices platform-agnostic initiative

Part of the effort to remove the VSTest object model (Microsoft.TestPlatform.ObjectModel) dependency from MSTestAdapter.PlatformServices, moving VSTest coupling up into MSTest.TestAdapter.

This phase neutralizes the test message logger: it replaces the VSTest IMessageLogger with the platform-agnostic IAdapterMessageLogger (introduced in Phase 1) throughout the execution engine, test-context, and class-cleanup paths — including the logger marshaled into the isolation host (child AppDomain) for RunSingleTest.

What changed

  • RemotingMessageLogger now implements IAdapterMessageLogger instead of VSTest IMessageLogger (still MarshalByRefObject on netfx). It wraps a parent-domain IAdapterMessageLogger and forwards SendMessage(MessageLevel, string). Marshaling stays by reference — the wrapped logger never crosses into the child domain, and MessageLevel is a serializable enum, so no VSTest type is needed child-side.
  • IPlatformServiceProvider.GetTestContext, UnitTestRunner.RunSingleTest(Async)/ForceCleanup, ClassCleanupManager.ForceCleanup, TestContextImplementation (field + ctor), and InitializeRandomTestOrder now take IAdapterMessageLogger.
  • TestContext.DisplayMessage forwards MessageLevel directly to the neutral logger (the MessageLevelTestMessageLevel conversion now happens once, at the boundary bridge).
  • The engine obtains the neutral logger via frameworkHandle.ToAdapterMessageLogger() at the same points it already used for discovery.

No behavior change

The same messages cross the same AppDomain boundary to the same host logger; MessageLevel maps 1:1 to TestMessageLevel at the bridge. The non-AppDomain path is unchanged. The single remaining VSTest logging reference in PlatformServices is the AdapterMessageLoggerExtensions bridge, to be relocated with the other bridges in a later phase.

Verification

  • Full build green across all TFMs (net462, net8.0, net9.0, UWP, WinUI).
  • MSTestAdapter.PlatformServices.UnitTests: 897 (net8.0) / 935 (net462); MSTestAdapter.UnitTests: 21 — all pass.
  • MSTest.IntegrationTests (net462, exercises AppDomain-isolated execution → child-domain logging via the marshaled logger): 47 passed / 1 skipped / 0 failed.
  • Expert MSTest/MTP reviewer: no correctness/marshaling/fidelity/public-API findings.

Base

Remaining work (later phases)

…(Phase 6c)
Replace the VSTest IMessageLogger with the platform-agnostic IAdapterMessageLogger
throughout the execution engine, test-context and class-cleanup paths, including the
message logger marshaled into the isolation (child app-domain) host.
- RemotingMessageLogger now implements IAdapterMessageLogger (still MarshalByRefObject
on netfx); marshaling stays by-reference so no VSTest object-model type is needed in
the child domain.
- IPlatformServiceProvider.GetTestContext, UnitTestRunner.RunSingleTest(Async)/ForceCleanup,
ClassCleanupManager.ForceCleanup, TestContextImplementation and InitializeRandomTestOrder
now take IAdapterMessageLogger; TestContext.DisplayMessage forwards MessageLevel directly.
- The engine obtains the neutral logger via frameworkHandle.ToAdapterMessageLogger() at the
same points it already did for discovery. The single remaining VSTest logging reference is
the AdapterMessageLoggerExtensions bridge, relocated with the other bridges in a later phase.
No behavior change: the same messages cross the same app-domain boundary to the same host
logger; MessageLevel maps 1:1 to TestMessageLevel at the bridge.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
@Evangelink
Amaury Levé (Evangelink)force-pushed the dev/amauryleve/vstest-decoupling-contexts branch from d8c1a52 to a1392bbCompareJuly 3, 2026 14:49
@Evangelink
Amaury Levé (Evangelink) marked this pull request as ready for review July 3, 2026 15:05
@Evangelink
Amaury Levé (Evangelink) merged commit 274e479 into dev/amauryleve/vstest-decoupling-baseJul 3, 2026
19 of 24 checks passed
@Evangelink
Amaury Levé (Evangelink) deleted the dev/amauryleve/vstest-decoupling-contexts branch July 3, 2026 15:05

@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.

Note

🤖 Automated review by GitHub Copilot. Posted via a maintainer's GitHub token, so it appears under their account — the account owner did not write or approve this content personally. Generated by the Expert Code Review workflow. To request a follow-up action, reply by tagging @copilot directly.

✅ 22/22 dimensions clean — no findings.


Reviewed against all 22 dimensions. This is a clean, mechanical decoupling of IMessageLogger (VSTest) in favour of IAdapterMessageLogger (platform-agnostic) across 13 files. Key observations that were verified:

Correctness — The MessageLevelTestMessageLevel conversion is now centralised in exactly one place (AdapterMessageLoggerExtensions.HostMessageLogger.SendMessage). All call sites pass MessageLevel directly. The DisplayMessage simplification to _messageLogger?.SendMessage(messageLevel, message) is semantically equivalent to the six-line version it replaces.

AppDomain marshaling (netfx)RemotingMessageLogger is MarshalByRefObject on .NET Framework; the wrapped IAdapterMessageLogger stays in the parent domain. Only MessageLevel (a serializable enum) and string cross the AppDomain boundary. Correct.

Threading_adapterMessageLogger in the override partial class is written once (synchronously) before base.RunTestsAsync starts any parallel workers, then read in the downstream async continuation. Write-before-read in a single async chain — no race. Parallel test workers capture adapterMessageLogger (a local) by closure; SendMessage calls are concurrent but the VSTest host logger they ultimately reach is thread-safe.

! null-forgiving_adapterMessageLogger! in ExecuteTestsAsync override is safe because RunTestsAsync always initialises it before calling base.RunTestsAsync, which is the only path that invokes the override.

Cross-TFM#if NETFRAMEWORK guard on MarshalByRefObject is correct. Non-netfx stub provided.

Test assertionsMSTestAdapter.PlatformServices.UnitTests bans MSTest Assert; the updated tests correctly use AwesomeAssertions + Moq. Mocks set up with It.IsAny<MessageLevel>() and verified with specific MessageLevel values — correct.

Diff artefact in UnitTestRunner.cs (cosmetic) — Hunk 3 (ForceCleanup) shows the IMessageLogger signature as an unchanged context line alongside identical -/+ lines for the IAdapterMessageLogger version. This is a git diff alignment artefact from the stacked-PR base; the compiled result has only one ForceCleanup signature taking IAdapterMessageLogger. No action needed.

Removed using System.Diagnostics.CodeAnalysis — Safe: [NotNullWhen] was already removed from TestContextImplementation in a prior phase, making the directive dead code. Correct cleanup.

Amaury Levé (Evangelink) added a commit that referenced this pull request Jul 5, 2026
…(Phase 6c) (#9585)
Co-authored-by: Amaury Leveque <amauryleve@users.noreply.github.com>
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>
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 test message logging in PlatformServices execution engine (Phase 6c) - #9585

Merged
Amaury Levé (Evangelink) merged 1 commit into
dev/amauryleve/vstest-decoupling-basefrom
dev/amauryleve/vstest-decoupling-contexts
Jul 3, 2026
Merged

Neutralize test message logging in PlatformServices execution engine (Phase 6c)#9585
Amaury Levé (Evangelink) merged 1 commit into
dev/amauryleve/vstest-decoupling-basefrom
dev/amauryleve/vstest-decoupling-contexts

Conversation

@Evangelink

@EvangelinkAmaury Levé (Evangelink) commented Jul 3, 2026

Copy link
Copy Markdown
Member

Phase 6c of the PlatformServices platform-agnostic initiative

Part of the effort to remove the VSTest object model (Microsoft.TestPlatform.ObjectModel) dependency from MSTestAdapter.PlatformServices, moving VSTest coupling up into MSTest.TestAdapter.

This phase neutralizes the test message logger: it replaces the VSTest IMessageLogger with the platform-agnostic IAdapterMessageLogger (introduced in Phase 1) throughout the execution engine, test-context, and class-cleanup paths — including the logger marshaled into the isolation host (child AppDomain) for RunSingleTest.

What changed

  • RemotingMessageLogger now implements IAdapterMessageLogger instead of VSTest IMessageLogger (still MarshalByRefObject on netfx). It wraps a parent-domain IAdapterMessageLogger and forwards SendMessage(MessageLevel, string). Marshaling stays by reference — the wrapped logger never crosses into the child domain, and MessageLevel is a serializable enum, so no VSTest type is needed child-side.
  • IPlatformServiceProvider.GetTestContext, UnitTestRunner.RunSingleTest(Async)/ForceCleanup, ClassCleanupManager.ForceCleanup, TestContextImplementation (field + ctor), and InitializeRandomTestOrder now take IAdapterMessageLogger.
  • TestContext.DisplayMessage forwards MessageLevel directly to the neutral logger (the MessageLevelTestMessageLevel conversion now happens once, at the boundary bridge).
  • The engine obtains the neutral logger via frameworkHandle.ToAdapterMessageLogger() at the same points it already used for discovery.

No behavior change

The same messages cross the same AppDomain boundary to the same host logger; MessageLevel maps 1:1 to TestMessageLevel at the bridge. The non-AppDomain path is unchanged. The single remaining VSTest logging reference in PlatformServices is the AdapterMessageLoggerExtensions bridge, to be relocated with the other bridges in a later phase.

Verification

  • Full build green across all TFMs (net462, net8.0, net9.0, UWP, WinUI).
  • MSTestAdapter.PlatformServices.UnitTests: 897 (net8.0) / 935 (net462); MSTestAdapter.UnitTests: 21 — all pass.
  • MSTest.IntegrationTests (net462, exercises AppDomain-isolated execution → child-domain logging via the marshaled logger): 47 passed / 1 skipped / 0 failed.
  • Expert MSTest/MTP reviewer: no correctness/marshaling/fidelity/public-API findings.

Base

Remaining work (later phases)

…(Phase 6c)
Replace the VSTest IMessageLogger with the platform-agnostic IAdapterMessageLogger
throughout the execution engine, test-context and class-cleanup paths, including the
message logger marshaled into the isolation (child app-domain) host.
- RemotingMessageLogger now implements IAdapterMessageLogger (still MarshalByRefObject
on netfx); marshaling stays by-reference so no VSTest object-model type is needed in
the child domain.
- IPlatformServiceProvider.GetTestContext, UnitTestRunner.RunSingleTest(Async)/ForceCleanup,
ClassCleanupManager.ForceCleanup, TestContextImplementation and InitializeRandomTestOrder
now take IAdapterMessageLogger; TestContext.DisplayMessage forwards MessageLevel directly.
- The engine obtains the neutral logger via frameworkHandle.ToAdapterMessageLogger() at the
same points it already did for discovery. The single remaining VSTest logging reference is
the AdapterMessageLoggerExtensions bridge, relocated with the other bridges in a later phase.
No behavior change: the same messages cross the same app-domain boundary to the same host
logger; MessageLevel maps 1:1 to TestMessageLevel at the bridge.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
@Evangelink
Amaury Levé (Evangelink)force-pushed the dev/amauryleve/vstest-decoupling-contexts branch from d8c1a52 to a1392bbCompareJuly 3, 2026 14:49
@Evangelink
Amaury Levé (Evangelink) marked this pull request as ready for review July 3, 2026 15:05
@Evangelink
Amaury Levé (Evangelink) merged commit 274e479 into dev/amauryleve/vstest-decoupling-baseJul 3, 2026
19 of 24 checks passed
@Evangelink
Amaury Levé (Evangelink) deleted the dev/amauryleve/vstest-decoupling-contexts branch July 3, 2026 15:05

@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.

Note

🤖 Automated review by GitHub Copilot. Posted via a maintainer's GitHub token, so it appears under their account — the account owner did not write or approve this content personally. Generated by the Expert Code Review workflow. To request a follow-up action, reply by tagging @copilot directly.

✅ 22/22 dimensions clean — no findings.


Reviewed against all 22 dimensions. This is a clean, mechanical decoupling of IMessageLogger (VSTest) in favour of IAdapterMessageLogger (platform-agnostic) across 13 files. Key observations that were verified:

Correctness — The MessageLevelTestMessageLevel conversion is now centralised in exactly one place (AdapterMessageLoggerExtensions.HostMessageLogger.SendMessage). All call sites pass MessageLevel directly. The DisplayMessage simplification to _messageLogger?.SendMessage(messageLevel, message) is semantically equivalent to the six-line version it replaces.

AppDomain marshaling (netfx)RemotingMessageLogger is MarshalByRefObject on .NET Framework; the wrapped IAdapterMessageLogger stays in the parent domain. Only MessageLevel (a serializable enum) and string cross the AppDomain boundary. Correct.

Threading_adapterMessageLogger in the override partial class is written once (synchronously) before base.RunTestsAsync starts any parallel workers, then read in the downstream async continuation. Write-before-read in a single async chain — no race. Parallel test workers capture adapterMessageLogger (a local) by closure; SendMessage calls are concurrent but the VSTest host logger they ultimately reach is thread-safe.

! null-forgiving_adapterMessageLogger! in ExecuteTestsAsync override is safe because RunTestsAsync always initialises it before calling base.RunTestsAsync, which is the only path that invokes the override.

Cross-TFM#if NETFRAMEWORK guard on MarshalByRefObject is correct. Non-netfx stub provided.

Test assertionsMSTestAdapter.PlatformServices.UnitTests bans MSTest Assert; the updated tests correctly use AwesomeAssertions + Moq. Mocks set up with It.IsAny<MessageLevel>() and verified with specific MessageLevel values — correct.

Diff artefact in UnitTestRunner.cs (cosmetic) — Hunk 3 (ForceCleanup) shows the IMessageLogger signature as an unchanged context line alongside identical -/+ lines for the IAdapterMessageLogger version. This is a git diff alignment artefact from the stacked-PR base; the compiled result has only one ForceCleanup signature taking IAdapterMessageLogger. No action needed.

Removed using System.Diagnostics.CodeAnalysis — Safe: [NotNullWhen] was already removed from TestContextImplementation in a prior phase, making the directive dead code. Correct cleanup.

Amaury Levé (Evangelink) added a commit that referenced this pull request Jul 5, 2026
…(Phase 6c) (#9585)
Co-authored-by: Amaury Leveque <amauryleve@users.noreply.github.com>
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>
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