Skip to content

Native MTP integration for MSTest — RFC 018 + Phases 1–5 (opt-in native path)#9706

Merged
Evangelink merged 4 commits into
mainfrom
dev/amauryleve/remove-vstest-bridge-mstest
Jul 8, 2026
Merged

Native MTP integration for MSTest — RFC 018 + Phases 1–5 (opt-in native path)#9706
Evangelink merged 4 commits into
mainfrom
dev/amauryleve/remove-vstest-bridge-mstest

Conversation

@Evangelink

@Evangelink Evangelink commented Jul 7, 2026

Copy link
Copy Markdown
Member

What

Works toward removing the Microsoft.Testing.Extensions.VSTestBridge dependency from MSTest's Microsoft.Testing.Platform (MTP) code path, so MSTest plugs into MTP natively. Delivered as a reviewable, phased series — all behind the opt-in MSTEST_EXPERIMENTAL_NATIVE_MTP switch, so the shipping default (the bridge) is unchanged.

  • RFC 018 — the roadmap/design (docs/RFCs/018-...md).
  • Phase 1 — native TestNode production seam (MSTestTestNodeConverter, MtpUnitTestElementSink, MtpTestResultRecorder).
  • Phase 2 — proved the seams by wiring them behind the flag (now superseded by the native framework).
  • Phases 3–5 — a native MSTestTestFramework that handles the MTP request directly: native filtering, runsettings/config, message logging, and session lifecycle. No VSTest bridge object-model adapters on the native request path.
  • Review feedback addressed (async neutral seams, UTF-8 BOM, doc style).

Why

On the MTP path, MSTest round-trips through the VSTest object model (context/filter/runsettings/handle/sink adapters) even though its engine already runs on neutral models. Going native removes that plumbing.

Phases 3–5 in the latest commit

A native MSTestTestFramework : ITestFramework, IDataProducer replaces the bridge framework on the flag path and reuses MSTest's existing MSTestDiscoverer/MSTestExecutor engine. New (all #if !WINDOWS_UWP):

  • MSTestFilterContext (MSTestRunContext/MSTestDiscoveryContext) — native IRunContext/IDiscoveryContext that builds the VSTest ITestCaseFilterExpression from the MTP ITestExecutionFilter + --filter + runsettings <TestCaseFilter> (reusing Microsoft.TestPlatform.Filter.Source, now referenced directly). MSTest's existing TestMethodFilter matching is unchanged.
  • MSTestRunSettings — native IRunSettings: reads runsettings, patches MTP defaults (DesignMode/ResultsDirectory/--test-parameter), and warns on unsupported entries (reusing the bridge's already-localized ExtensionResources).
  • MSTestFrameworkHandle — an IFrameworkHandle that only forwards messages to IOutputDevice (results flow through the native recorder).
  • AddMSTest branches the framework factory on the flag; the bridge's shared command-line/config/env-var registrations are kept for now (retired with the dependency in the final step).

Validation

  • Full .\build.cmd — 0 warnings, 0 errors across all TFMs.
  • Unit tests: 45 (MSTestAdapter.UnitTests) + 898 (MSTestAdapter.PlatformServices.UnitTests), all green.
  • Native path (flag on, acceptance): 185 tests green — FilterTests, RunsettingsTests (incl. localization), OutputTests, TrxReportTests, ThreadingTests (STA), ServerModeTests, DeploymentItem, DataSourceTests, Inconclusive/Ignore, TestRunParameters, TimeoutTests.
  • Default bridge path (flag off, acceptance): FilterTests + RunsettingsTests (45) — unchanged.

Remaining — Phase 6 (separate, gated)

Flip the default to native and drop the VSTestBridge dependency from MSTest. This is gated on:

  1. Full MSTest acceptance-suite parity on the native path (hundreds of tests — needs CI).
  2. Native replacements for the bridge's remaining shared registrations (the --filter/--settings/--test-parameter option providers and the runsettings config/env-var providers), so the package reference can be removed.

The bridge package itself is not removed — NUnit/Expecto/3rd-party adapters still use it. Only MSTest stops depending on it.

…bridge)

Proposes a staged roadmap to plug MSTest directly into Microsoft.Testing.Platform as a native ITestFramework, replacing the Microsoft.Testing.Extensions.VSTestBridge indirection on the MTP code path. Documents the existing neutral seams (IUnitTestElementSink, ITestResultRecorder, IAdapterMessageLogger, ITestElementFilterProvider, DeploymentContext), a 6-phase plan, capability parity checklist, testing strategy, and risks.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings July 7, 2026 12:48

Copilot AI 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.

Pull request overview

This PR adds RFC 018, a design/roadmap document proposing that MSTest plug directly into Microsoft.Testing.Platform (MTP) as a first-class ITestFramework, retiring the Microsoft.Testing.Extensions.VSTestBridge indirection on the MTP code path. It is a documentation-only artifact (status "Under discussion") with no production code changes, intended to let maintainers steer the approach before implementation. The RFC documents the current double-conversion architecture, the existing neutral seams the engine already uses, a 6-phase independently-shippable roadmap, a capability/feature-parity checklist, and the testing strategy, risks, and open questions.

Changes:

  • Documents current vs. target MTP architecture, highlighting the wasteful neutral model → VSTest TestCase/TestResult → TestNode double conversion and the resulting information loss (empty AssemblyFullName/ReturnTypeFullName).
  • Lays out a 6-phase roadmap that keeps the bridge as default until the native path reaches parity, with a feature-parity checklist and dual-run testing strategy.
  • Follows the established docs/RFCs/ convention; the bridge package is not removed (NUnit/Expecto/third-party adapters still use it) — only MSTest stops depending on it.
Show a summary per file
File Description
docs/RFCs/018-Native-MTP-Integration-For-MSTest.md New RFC describing the native MTP integration design, phasing, parity checklist, testing strategy, risks, and open questions. All referenced types, seams, file paths, and the quoted in-code comment were verified as accurate against the codebase.

Review details

  • Files reviewed: 1/1 changed files
  • Comments generated: 0
  • Review effort level: Medium

@github-actions github-actions Bot 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. Generated by the Expert Code Review workflow. To request a follow-up action, reply by tagging @copilot directly.

# Dimension Verdict
17 Documentation Accuracy 🟢 2 NIT
21 Scope & PR Discipline 🟢 1 NIT

✅ 20/22 dimensions N/A (no code changes). 2 dimensions reviewed — only minor nits found.

Summary: This is a well-structured, thoroughly researched RFC. The claims about existing neutral seams (IUnitTestElementSink, ITestResultRecorder, IAdapterMessageLogger, ITestElementFilterProvider) are verified against the codebase, the MSTestBridgedTestFramework information-loss quote is accurate (line 100–106 in that file), and the 6-phase roadmap with the capability parity checklist covers all bridge features I can identify. The design principle of "boundary replacement, not engine rewrite" is well-supported by the architecture diagrams.

  • Path convention: use forward slashes for cross-platform consistency (line 84)
  • Consider linking a tracking issue for the 6-phase roadmap

Comment thread docs/RFCs/018-Native-MTP-Integration-For-MSTest.md Outdated
Comment thread docs/RFCs/018-Native-MTP-Integration-For-MSTest.md
Adds the first, unwired building block for plugging MSTest directly into Microsoft.Testing.Platform instead of routing through the VSTest bridge:

- MSTestTestNodeConverter: builds MTP TestNodes directly from MSTest's neutral models (UnitTestElement + framework TestResult), faithfully porting the mapping the bridge does today via ObjectModelConverters/TestResultExtensions/UnitTestElementExtensions (outcome, timing, std out/err, TRX categories/messages/exception/type-name, traits, file location, TestMethodIdentifierProperty).
- MtpUnitTestElementSink / MtpTestResultRecorder: MTP-native IUnitTestElementSink / ITestResultRecorder that publish TestNodeUpdateMessage on the message bus.
- MSTestTestNodeException: native message+stacktrace carrier (replaces the bridge's VSTestException on the MSTest path).
- UnitTestElementExtensions.GetTestId: exposes the stable test id neutrally, without materializing a VSTest TestCase.

The bridge remains the default; these seams are covered by 24 unit tests and are wired in a later phase. No behavior change.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
@Evangelink Evangelink changed the title docs: RFC 018 — native MTP integration for MSTest (retire the VSTest bridge) Native MTP integration for MSTest — RFC 018 + Phase 1 (TestNode production seam) Jul 7, 2026
@github-actions

This comment has been minimized.

When the MSTEST_EXPERIMENTAL_NATIVE_MTP opt-in is set, MSTest publishes discovery and result TestNodes directly via the Phase 1 native converter+seams, bypassing the bridge's ObjectModelConverters / FrameworkHandlerAdapter / TestCaseDiscoverySinkAdapter. The bridge remains the default; only context/filter/config/command-line parsing still flows through it.

Changes:
- VSTestBridge base: expose the session UID as an internal auto-property (bridge already has InternalsVisibleTo MSTest.TestAdapter; no public API change).
- MSTestDiscoverer: add an IUnitTestElementSink overload and share a discovery core so the native sink is used directly instead of discoverySink.ToUnitTestElementSink().
- MSTestExecutor: add a Func<MSTestSettings,ITestResultRecorder> overload and share a run core so a native recorder is used instead of frameworkHandle.ToTestResultRecorder(); the framework handle is still used for message logging and apartment-state handling.
- MSTestBridgedTestFramework: flag-gated native wiring.

Validated: full build 0/0; MSTestAdapter.UnitTests 45/45; native-path acceptance OutputTests(6)+TrxReportTests(3)+Inconclusive/Ignore/DataSource(38) all green; default bridge-path OutputTests(6) unchanged.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings July 7, 2026 14:04
@Evangelink Evangelink changed the title Native MTP integration for MSTest — RFC 018 + Phase 1 (TestNode production seam) Native MTP integration for MSTest — RFC 018 + Phase 1 & 2 (opt-in native TestNode path) Jul 7, 2026

Copilot AI 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.

Review details

  • Files reviewed: 11/11 changed files
  • Comments generated: 5
  • Review effort level: Medium

Comment thread src/Adapter/MSTest.TestAdapter/TestingPlatformAdapter/MSTestTestNodeConverter.cs Outdated
Comment thread src/Adapter/MSTest.TestAdapter/TestingPlatformAdapter/MtpTestResultRecorder.cs Outdated
Comment thread src/Adapter/MSTest.TestAdapter/TestingPlatformAdapter/MtpUnitTestElementSink.cs Outdated
Comment thread src/Adapter/MSTest.TestAdapter/TestingPlatformAdapter/MSTestTestNodeException.cs Outdated
Comment thread test/UnitTests/MSTestAdapter.UnitTests/MSTestTestNodeConverterTests.cs Outdated
@github-actions

github-actions Bot commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

🧪 Test quality grade — PR #9706

24 new test methods in MSTestTestNodeConverterTests cover MSTestTestNodeConverter, MtpUnitTestElementSink, and MtpTestResultRecorder. All 24 earn A (90–100): clear Arrange-Act-Assert structure, focused single-behavior design, and rich AwesomeAssertions throughout. Two minor opportunities: some tests pass DateTimeOffset.Now as a non-timing parameter (a named constant would be cleaner), and one negative test omits an absence assertion for symmetry with its enabled counterpart.

GradeTestNotes
A (90–100) new MSTestTestNodeConverterTests.
MtpTestResultRecorder_
RecordEmptyResult_
PublishesNothing
No issues found.
A (90–100) new MSTestTestNodeConverterTests.
MtpTestResultRecorder_
RecordResult_
PublishesResultNodeAndReturnsFailedFlag
Tests both return value and published state; consider splitting into two focused tests for sharper isolation.
A (90–100) new MSTestTestNodeConverterTests.
MtpTestResultRecorder_
RecordResult_
ReturnsFalse_
ForPassedTest
Return value correctly checked; consider also asserting the published node carries PassedTestNodeStateProperty for symmetry.
A (90–100) new MSTestTestNodeConverterTests.
MtpTestResultRecorder_
RecordStart_
PublishesInProgressNode
No issues found.
A (90–100) new MSTestTestNodeConverterTests.
MtpUnitTestElementSink_
PublishesDiscoveredTestNode
No issues found.
A (90–100) new MSTestTestNodeConverterTests.
ToDiscoveredTestNode_
AddsCategoriesAndTraitsAsMetadata
No issues found.
A (90–100) new MSTestTestNodeConverterTests.
ToDiscoveredTestNode_
AddsFileLocation_
WhenDeclaringFileKnown
No issues found.
A (90–100) new MSTestTestNodeConverterTests.
ToDiscoveredTestNode_
AddsTestMethodIdentifier_
FromManagedNames
No issues found.
A (90–100) new MSTestTestNodeConverterTests.
ToDiscoveredTestNode_
AddsTrxCategories_
OnlyWhenTrxEnabled
No issues found.
A (90–100) new MSTestTestNodeConverterTests.
ToDiscoveredTestNode_
DoesNotAddFileLocation_
WhenDeclaringFileUnknown
No issues found.
A (90–100) new MSTestTestNodeConverterTests.
ToDiscoveredTestNode_
DoesNotAddTestMethodIdentifier_
WhenNoManagedMethodName
No issues found.
A (90–100) new MSTestTestNodeConverterTests.
ToDiscoveredTestNode_
SetsUidDisplayNameAndDiscoveredState
No issues found.
A (90–100) new MSTestTestNodeConverterTests.
ToDiscoveredTestNode_
UsesExplicitDisplayName_
WhenProvided
No issues found.
A (90–100) new MSTestTestNodeConverterTests.
ToInProgressTestNode_
AddsInProgressState
No issues found.
A (90–100) new MSTestTestNodeConverterTests.
ToResultTestNode_
AddsTimingProperty
Uses explicit deterministic timestamps; duration equality verified — no issues found.
A (90–100) new MSTestTestNodeConverterTests.
ToResultTestNode_
AddsTrxProperties_
WhenTrxEnabled
No issues found.
A (90–100) new MSTestTestNodeConverterTests.
ToResultTestNode_
DoesNotAddTrxProperties_
WhenTrxDisabled
Consider also asserting TrxFullyQualifiedTypeNameProperty absence for symmetry with the enabled test.
A (90–100) new MSTestTestNodeConverterTests.
ToResultTestNode_
IncludesDebugTraceAndTestContextBannersInStandardOutput
No issues found.
A (90–100) new MSTestTestNodeConverterTests.
ToResultTestNode_
MapsFailedOutcome_
WithMessageAndStackTrace
Both exception fields verified; replace DateTimeOffset.Now in non-timing args with a fixed constant for clarity.
A (90–100) new MSTestTestNodeConverterTests.
ToResultTestNode_
MapsIgnoredOutcomeToSkipped
No issues found.
A (90–100) new MSTestTestNodeConverterTests.
ToResultTestNode_
MapsInconclusiveToSkipped_
ByDefault
No issues found.
A (90–100) new MSTestTestNodeConverterTests.
ToResultTestNode_
MapsLogOutputAndLogErrorToStandardStreams
No issues found.
A (90–100) new MSTestTestNodeConverterTests.
ToResultTestNode_
MapsNotFoundOutcomeToError
No issues found.
A (90–100) new MSTestTestNodeConverterTests.
ToResultTestNode_
MapsPassedOutcome
No issues found.

This advisory comment was generated automatically. Grades are heuristic
and informational — they do not block merging. Re-run with
/grade-tests.

🤖 Automated content by GitHub Copilot. Generated by the Grade Tests on PR (on open / sync) workflow. · 145.8 AIC · ⌖ 10.8 AIC · ⊞ 9.5K · [◷]( · )

@Evangelink Evangelink added the state/needs-review Awaiting review from the team. label Jul 7, 2026
- Make the neutral discovery/result seams asynchronous end-to-end (per review): ITestResultRecorder (RecordStartAsync/RecordEmptyResultAsync/RecordResultAsync) and IUnitTestElementSink (SendTestElementAsync). Thread async through UnitTestDiscoverer (DiscoverTestsAsync/DiscoverTestsInSourceAsync/SendTestCasesAsync) and TestExecutionManager.SendTestResultsAsync. The MTP implementations now await the message-bus publish instead of blocking with GetAwaiter().GetResult(); the VSTest implementations return completed tasks (sync fast-path preserves STA behavior).
- Add the UTF-8 BOM required by .editorconfig to the five new .cs files.
- Use forward slashes for the seam path in RFC 018.

Validated: full build 0/0; unit tests MSTestAdapter.UnitTests 45/45 and MSTestAdapter.PlatformServices.UnitTests 898/898; acceptance default-path threading/output/trx 41 green; native-path threading/output/trx/outcome/data-driven 75 green.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
@Evangelink
Evangelink enabled auto-merge (squash) July 8, 2026 10:16
@Evangelink
Evangelink merged commit 641c192 into main Jul 8, 2026
101 of 102 checks passed
@Evangelink
Evangelink deleted the dev/amauryleve/remove-vstest-bridge-mstest branch July 8, 2026 12:02
github-actions Bot added a commit that referenced this pull request Jul 8, 2026
Apply two project-convention improvements to files added in recent PRs:

- MSTestTestNodeConverter.cs (added in #9706): replace Substring calls
  with range operators per csharp_style_prefer_range_operator = true
- TestMethodRunner.DataSource.cs (added in #9729): replace == null with
  is null per the project's 'always use is null / is not null' rule

No behavioral change; builds verified to succeed with 0 errors.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@Evangelink Evangelink changed the title Native MTP integration for MSTest — RFC 018 + Phase 1 & 2 (opt-in native TestNode path) Native MTP integration for MSTest — RFC 018 + Phases 1–5 (opt-in native path) Jul 8, 2026
github-actions Bot added a commit that referenced this pull request Jul 9, 2026
- AzureFoundry: add DefaultAzureCredential/managed identity auth details
  and required environment variables (PR #9707)
- MSTestTestFramework: new entry documenting the native MTP ITestFramework
  for MSTest introduced by RFC 018 (PRs #9706, #9743, #9748, #9755)
- VSTestBridge: note MSTest no longer depends on it on the MTP path
  as of MSTest 4.3

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Evangelink pushed a commit that referenced this pull request Jul 9, 2026
- AzureFoundry: add DefaultAzureCredential/managed identity auth details
  and required environment variables (PR #9707)
- MSTestTestFramework: new entry documenting the native MTP ITestFramework
  for MSTest introduced by RFC 018 (PRs #9706, #9743, #9748, #9755)
- VSTestBridge: note MSTest no longer depends on it on the MTP path
  as of MSTest 4.3

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

state/needs-review Awaiting review from the team.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants