Native MTP integration for MSTest (Phase 6a): native providers, config, resources#9748
Conversation
…g, resources Make the experimental native MTP path fully self-contained on the request/registration side, without changing shipping behavior (the VSTest bridge remains the default and stays referenced). The risky default-flip + bridge-reference removal becomes a trivial follow-up. - Register MSTest's own command-line option providers, runsettings configuration source and environment-variable provider natively (identical option names/descriptions to the bridge), replacing the bridge Helpers. Both frameworks read options by name, so the swap is safe for the default bridged path too. - Add a native IMSTestTrxReportCapability; MSTestCapabilities implements both it and the bridge's IInternalVSTestBridgeTrxReportCapability so both frameworks observe TRX enablement. - Own the localized strings natively via PlatformAdapterResources (resx + xlf copied from the bridge's ExtensionResources), instead of reading the bridge's resources. - Fix native localization at runtime: MSTest.TestAdapter slims localized satellites to the build-machine culture (fine for VSTest, where the UI language is fixed at build time). For MTP the UI language is chosen at run time, so copy every culture's satellite to the output when EnableMSTestRunner is true; keep the slim behavior otherwise. - Bump the MSTest.TestAdapter expected package file count for the added satellites. Validated: full build clean; unit tests (45 + 898) pass; native-path acceptance sweep green (runsettings incl. localization, filter, help, TRX, threading); default bridged path unaffected. The only failures are 3 pre-existing NativeAOT smoke tests that also fail on the untouched default path (VSTest ObjectModel trim warnings-as-errors in this environment). Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
| foreach (string argument in arguments) | ||
| { | ||
| if (!argument.Contains('=')) | ||
| { | ||
| return ValidationResult.InvalidTask(string.Format(CultureInfo.CurrentCulture, PlatformAdapterResources.TestRunParameterOptionArgumentIsNotParameter, argument)); | ||
| } | ||
| } |
There was a problem hiding this comment.
Addressed in the follow-up PR #9749 — replaced the implicitly-filtering oreach with an explicit FirstOrDefault filter (same behavior and localized message).
There was a problem hiding this comment.
Pull request overview
This PR is Phase 6a of removing the Microsoft.Testing.Extensions.VSTestBridge dependency from MSTest's Microsoft.Testing.Platform (MTP) path (RFC 018). It makes the experimental native MTP path self-contained on the request/registration side without changing shipping behavior: the VSTest bridge remains the default (behind the MSTEST_EXPERIMENTAL_NATIVE_MTP opt-in) and stays referenced. AddMSTest now registers MSTest's own command-line option providers, runsettings configuration source, and environment-variable provider (with option names/descriptions identical to the bridge), so both the native and default-bridged frameworks read options by name. It also introduces a native IMSTestTrxReportCapability, native localized resources (PlatformAdapterResources resx + 13 xlf copied from the bridge), and a runtime-localization fix so MTP consumers get every culture's satellite in their output.
Changes:
- Replace bridge
Helpers.Add*Serviceregistrations inAddMSTestwith native providers (MSTestRunSettings*,MSTestTest*CommandLineOptionsProvider), a native config source, and env-var provider; addIMSTestTrxReportCapabilityimplemented alongside the bridge's TRX capability. - Own user-facing strings via a new
PlatformAdapterResourcesresx + 13 localized xlf, replacing reads from the bridge'sExtensionResources. - MSBuild targets: when
EnableMSTestRunner=true, copy all localized satellites (runtime UI-language selection); keep the slim build-machine-culture behavior for VSTest; bump the expected package file count (+13) inverify-nupkgs.ps1.
Show a summary per file
| File | Description |
|---|---|
TestApplicationBuilderExtensions.cs |
Swaps bridge Add*Service calls for native provider registrations; MSTestCapabilities now implements both TRX interfaces. |
MSTestTestFramework.cs |
Reads TRX enablement via native IMSTestTrxReportCapability; error string sourced from PlatformAdapterResources. |
MSTestRunSettingsCommandLineOptionsProvider.cs |
New native --settings provider mirroring the bridge. |
MSTestTestCaseFilterCommandLineOptionsProvider.cs |
New native --filter provider mirroring the bridge. |
MSTestTestRunParametersCommandLineOptionsProvider.cs |
New native --test-parameter provider mirroring the bridge. |
MSTestRunSettingsConfigurationProvider.cs |
New native runsettings config source (ResultsDirectory → PlatformResultDirectory). |
MSTestRunSettingsEnvironmentVariableProvider.cs |
New native env-var provider applying runsettings <EnvironmentVariables>. |
MSTestRunSettings.cs |
ReadRunSettings now takes file names; option-name constants point at native providers; strings from PlatformAdapterResources. |
MSTestFilterContext.cs |
Filter option name references the native provider constant. |
IMSTestTrxReportCapability.cs |
New native TRX capability interface. |
Resources/PlatformAdapterResources.resx + 13 xlf |
New native localized strings copied from the bridge. |
buildTransitive/common/MSTest.TestAdapter.targets |
Copies all culture satellites for MTP; keeps slim behavior for VSTest (UICulture carries trailing separator). |
eng/verify-nupkgs.ps1 |
Expected MSTest.TestAdapter file count 53 → 66 for the added satellites. |
Review details
- Files reviewed: 26/26 changed files
- Comments generated: 0
- Review effort level: Medium
There was a problem hiding this comment.
Note
🤖 Automated review by GitHub Copilot. Generated by the Expert Code Review workflow. To request a follow-up action, reply by tagging @copilot directly.
Expert Review Summary
Verdict: ✅ All evaluated dimensions clean — no findings.
| # | Dimension | Severity | Result |
|---|---|---|---|
| 1 | Algorithmic Correctness | MAJOR | ✅ LGTM |
| 2 | Threading & Concurrency | BLOCKING | ✅ LGTM |
| 4 | Public API & Binary Compat | BLOCKING | ✅ LGTM |
| 5 | Performance & Allocations | MAJOR | ✅ LGTM |
| 6 | Cross-TFM Compatibility | MAJOR | ✅ LGTM |
| 7 | Resource & IDisposable | MAJOR | ✅ LGTM |
| 8 | Defensive Coding | MAJOR | ✅ LGTM |
| 9 | Localization & Resources | MAJOR | ✅ LGTM |
| 13 | Test Completeness | MAJOR | ✅ LGTM |
| 15 | Code Structure | MODERATE | ✅ LGTM |
| 20 | Build Infrastructure | MODERATE | ✅ LGTM |
| 21 | Scope & PR Discipline | MODERATE | ✅ LGTM |
Skipped (not applicable): dims 3 (no security-sensitive IPC), 10-12 (no test files changed), 14 (no data-driven tests), 16-17 (clean naming/docs), 18 (no analyzer changes), 19 (no IPC serialization), 22 (trivial PS1 number bump).
Notable observations (informational, not blocking)
MSTEST_EXPERIMENTAL_NATIVE_MTPcasing: The checkis "1" or "true" or "True"won't match"TRUE"(all-caps). This is fine for an experimental opt-in flag but worth harmonizing when the flag is promoted in Phase 6b.MSTestRunSettingsConfigurationProvider.TryGet()re-parses the XDocument on each call. Since this is a cold path (config lookup, not per-test), it's acceptable but could be cached if the key set grows.- Localization: The xlf files contain pre-filled
state="translated"translations, consistent with the PR's approach of copying established strings from the VSTest bridge'sExtensionResources. This is the correct workflow for strings that already have approved translations. CountdownEvent.WaitAsyncis a repo extension (CountDownEventExtensions.cs), not BCL — confirmed available on all target TFMs.
Well-structured PR with clear phase separation. The registration-swap approach (both bridge and native frameworks read options by name) is sound and low-risk.
…llow-up) (#9749) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
- 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>
- 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>
What
Continues the removal of the
Microsoft.Testing.Extensions.VSTestBridgedependency from MSTest's Microsoft.Testing.Platform (MTP) path (RFC 018). This phase makes the experimental native MTP path fully self-contained on the request/registration side, without changing shipping behavior: the VSTest bridge remains the default (behind theMSTEST_EXPERIMENTAL_NATIVE_MTPopt-in) and stays referenced. The risky default-flip + bridge-reference removal becomes a trivial, CI-gated follow-up (Phase 6b).Changes
Helpers. Both frameworks read options by name, so the registration swap is safe for the default bridged path too.IMSTestTrxReportCapability.MSTestCapabilitiesimplements both it and the bridge'sIInternalVSTestBridgeTrxReportCapability, so both frameworks observe the same TRX enablement.PlatformAdapterResources(resx + 13 xlf, copied from the bridge'sExtensionResources) instead of reading the bridge's resources.MSTest.TestAdapterslims localized satellites to the build-machine culture (correct for VSTest, where the UI language is fixed at build time). For MTP the UI language is chosen at run time (e.g.TESTINGPLATFORM_UI_LANGUAGE), so copy every culture's satellite to the output whenEnableMSTestRunneristrue; keep the slim behavior otherwise. This keeps VSTest consumers' output unchanged.MSTest.TestAdapterexpected package file count inverify-nupkgs.ps1for the added satellites.Validation
MSTestAdapter.UnitTests(45) andMSTestAdapter.PlatformServices.UnitTests(898) pass._Localizationcases), filter, help/info, TRX, threading; fullMSTest.Acceptance= 784 passed / 3 skipped.NativeAotsmoke tests, which also fail on the untouched default path (VSTest ObjectModel trim warnings-as-errors in this environment) — not a regression.Follow-up (Phase 6b, separate PR)
Flip the default to native, remove the
MSTEST_EXPERIMENTAL_NATIVE_MTPflag andMSTestBridgedTestFramework/BridgedConfiguration, drop theMicrosoft.Testing.Extensions.VSTestBridgeProjectReference, and add direct refs toTrxReport.Abstractions+Microsoft.TestPlatform.ObjectModel.Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>