Improve VSTest CLI migration guidance - #10864
Conversation
Provide value-aware replacements for unsupported VSTest logger and collector options while preserving MTP's canonical option model. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: a86293a1-7ea6-4869-9596-b97d6b079cb2
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Review Summary
Clean, well-structured change that adds value-aware migration diagnostics for VSTest CLI options (--logger, --collect) instead of introducing a permanent mapping API. The implementation is sound across all 22 review dimensions.
What looks good
- Design: Correctly preserves MTP's single-owner command-line model. The early-return pattern in
TryAppendVSTestOptionGuidanceprevents the VSTest guidance from mixing with the "did you mean?" / extension-package suggestions. - Correctness: The
arguments.Count == 1guard gracefully degrades to generic guidance when no argument or multiple arguments are present. The three accepted--outputverbosity values (minimal,normal,detailed) exactly match the registeredTerminalTestReporterCommandLineOptionsProviderconstants. - Localization: 新建 resource strings follow the repo conventions —
{Locked}markers correctly protect option names and product terms. XLF files are properly regenerated. - Test coverage: Tests cover all major branches (TRX with/without filename, console verbosity, Code Coverage, XPlat Coverage, blame, generic fallback) plus the
testconfig.jsonpath. Tests verify both the VSTest diagnostic banner and the extension-package suggestions. - RFC / glossary updates: RFC 015 is marked rejected with a clear decision rationale, and the glossary entries are updated to match.
Minor observations (non-blocking)
- Unrecognized console verbosity values (e.g.
quiet,diagnostic) fall through to the generic--loggerguidance rather than mentioning that those specific levels have no MTP equivalent. This is an acceptable design choice but could be made slightly more actionable in a follow-up. - Multi-argument edge case:
--logger trx results.trx(two separate arguments instead oftrx;LogFileName=results.trx) would bypass the TRX-specific guidance becausearguments.Count != 1. This matches how VSTest parses it (as a single semicolon-delimited value), so users are unlikely to encounter this.
No bugs, no security issues, no public API changes, no breaking behavior. LGTM.
There was a problem hiding this comment.
Copilot review overview
Review tier: Balanced
Findings: 3
新建议题s introduced by this change (3)
| Severity | Finding |
|---|---|
src/Platform/Microsoft.Testing.Platform/CommandLine/CommandLineOptionsValidator.UnknownAndBootstrapValidation.cs — Pass validOptionNames into the VSTest-guidance path and gate each package hint on the replacement… |
|
src/Platform/Microsoft.Testing.Platform/CommandLine/CommandLineOptionsValidator.UnknownAndBootstrapValidation.cs — Recognize configured Microsoft collector values rather than only the bare friendly name. A common… |
|
src/Platform/Microsoft.Testing.Platform/CommandLine/CommandLineOptionsValidator.UnknownAndBootstrapValidation.cs — Treat XPlat Code Coverage;... as the same recognized collector shape. Coverlet values can include… |
What changed in this PR
Improves diagnostics for unsupported VSTest CLI options while preserving MTP’s command-line model.
Changes:
- Adds value-aware migration guidance for
--loggerand--collect. - Adds unit coverage for CLI and
testconfig.jsondiagnostics. - Rejects RFC 015 and updates documentation and localization resources.
| File | Description |
|---|---|
CommandLineHandlerTests.cs |
Tests migration diagnostics. |
PlatformResources.cs.xlf |
Adds Czech localization entries. |
PlatformResources.de.xlf |
Adds German localization entries. |
PlatformResources.es.xlf |
Adds Spanish localization entries. |
PlatformResources.fr.xlf |
Adds French localization entries. |
PlatformResources.it.xlf |
Adds Italian localization entries. |
PlatformResources.ja.xlf |
Adds Japanese localization entries. |
PlatformResources.ko.xlf |
Adds Korean localization entries. |
PlatformResources.pl.xlf |
Adds Polish localization entries. |
PlatformResources.pt-BR.xlf |
Adds Brazilian Portuguese entries. |
PlatformResources.ru.xlf |
Adds Russian localization entries. |
PlatformResources.tr.xlf |
Adds Turkish localization entries. |
PlatformResources.zh-Hans.xlf |
Adds Simplified Chinese entries. |
PlatformResources.zh-Hant.xlf |
Adds Traditional Chinese entries. |
PlatformResources.resx |
Defines new diagnostic messages. |
CommandLineOptionsValidator.UnknownAndBootstrapValidation.cs |
Generates value-aware migration guidance. |
015-Command-Line-Option-Mappings.md |
Records RFC rejection and rationale. |
glossary.md |
Updates mapping and coverage terminology. |
💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Avoid redundant package hints and recognize configured coverage collector values in VSTest migration diagnostics. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: a86293a1-7ea6-4869-9596-b97d6b079cb2
This comment has been minimized.
This comment has been minimized.
🧪 Expert test review — PR #10864Only one test file changed:
This advisory comment was generated automatically. Grades are heuristic
|
There was a problem hiding this comment.
Copilot review overview
Review tier: Balanced
Findings: 1
新建议题s introduced by this change (1)
| Severity | Finding |
|---|---|
src/Platform/Microsoft.Testing.Platform/Resources/PlatformResources.resx — The lock covers only the opening fragment ('--output) instead of the complete command invocation.… |
问题 resolved since last review (3)
| Severity | Finding |
|---|---|
src/Platform/Microsoft.Testing.Platform/CommandLine/CommandLineOptionsValidator.UnknownAndBootstrapValidation.cs — Treat XPlat Code Coverage;... as the same recognized collector shape. Coverlet values can include… View resolved comment |
|
src/Platform/Microsoft.Testing.Platform/CommandLine/CommandLineOptionsValidator.UnknownAndBootstrapValidation.cs — Recognize configured Microsoft collector values rather than only the bare friendly name. A common… View resolved comment |
|
src/Platform/Microsoft.Testing.Platform/CommandLine/CommandLineOptionsValidator.UnknownAndBootstrapValidation.cs — Pass validOptionNames into the VSTest-guidance path and gate each package hint on the replacement… View resolved comment |
Lock the complete formatted output option and simplify the console verbosity guard. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: a86293a1-7ea6-4869-9596-b97d6b079cb2
This comment has been minimized.
This comment has been minimized.
🧪 Expert test review — PR #10864
This advisory comment was generated automatically. Grades are heuristic
|
There was a problem hiding this comment.
🤖 Automated content by GitHub Copilot. Generated by the Test Reviewer on PR (on open / sync) workflow. · auto · 67.5 AIC · ⌖ 1.16 AIC · ⊞ 16.9K · ◷
There was a problem hiding this comment.
Copilot review overview
Review tier: Balanced
Findings: None
问题 resolved since last review (1)
| Severity | Finding |
|---|---|
src/Platform/Microsoft.Testing.Platform/Resources/PlatformResources.resx — The lock covers only the opening fragment ('--output) instead of the complete command invocation.… View resolved comment |
Verify blame migration guidance still suggests the missing sibling extension when one replacement is already registered. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: a86293a1-7ea6-4869-9596-b97d6b079cb2
This comment has been minimized.
This comment has been minimized.
🧪 Expert test review — PR #10864
This advisory comment was generated automatically. Grades are heuristic
|
There was a problem hiding this comment.
Copilot review overview
Review tier: Balanced
Findings: None
Suppressed comments (1)
Previously missed (1) — in code that hasn't changed since the last review.
test/UnitTests/Microsoft.Testing.Platform.UnitTests/CommandLine/CommandLineHandlerTests.cs:662
- The value-aware console branch accepts
minimal,normal, anddetailed, but this table only exercisesdetailed. Removing either of the other two accepted values would leave the suite green while those users silently fall back to generic logger guidance. Add rows for both mappings.
[DataRow("logger", "console;verbosity=detailed", "For comparable console verbosity, use '--output detailed'.")]
Cover every supported console verbosity and assert replacement guidance remains present when package hints are suppressed. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: a86293a1-7ea6-4869-9596-b97d6b079cb2
🧵 Parallel-safety audit — PR #10864Nothing audited here touches process-global state, shared filesystem paths, or Scope reviewed: the only changed test file is
Audited Re-run with
|
🧪 Expert test review — PR #10864Reviewed the 6 new test methods added in
This advisory comment was generated automatically. Grades are heuristic and informational — they do not block merging. Suggestions on the Files changed tab can be applied with one click. Re-run with
|

MTP currently reports VSTest-specific
--loggerand--collectarguments as generic unknown options, leaving users without an actionable migration path. This change keeps those arguments invalid while explaining the canonical MTP replacement instead of introducing a permanent compatibility mapping API.Summary
testconfig.jsondiagnostics with unit tests.Testing
Microsoft.Testing.Platform.UnitTestsproject buildCommandLineHandlerTests: 85 passed--logger trxreturning the replacement guidance and invalid-command-line exit codeFixes: #7249