Replace SettingsMigrator tool with a Copilot skill and remove all code#15537
Draft
nohwnd wants to merge 3 commits intomicrosoft:mainfrom
Draft
Replace SettingsMigrator tool with a Copilot skill and remove all code#15537nohwnd wants to merge 3 commits intomicrosoft:mainfrom
nohwnd wants to merge 3 commits intomicrosoft:mainfrom
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR deprecates and removes the existing SettingsMigrator .NET tool (and its unit tests/localization assets) and replaces it with a Copilot “skill” document that describes the .testsettings → .runsettings migration steps.
Changes:
- Added a migration skill document that captures the XML node/attribute mapping rules and examples.
- Removed the SettingsMigrator tool project, resources (including localization), and unit tests.
- Removed SettingsMigrator packaging/build references (nuspec, csproj, solution, and nupkg verification).
Reviewed changes
Copilot reviewed 37 out of 38 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| .github/skills/settings-migration/SKILL.md | Adds the documented migration procedure intended to replace the removed executable tool. |
| TestPlatform.sln | Removes SettingsMigrator and its unit test projects from the solution. |
| eng/verify-nupkgs.ps1 | Stops expecting SettingsMigrator.exe in produced NuGet packages. |
| src/package/Microsoft.TestPlatform/Microsoft.TestPlatform.csproj | Drops the project reference to SettingsMigrator from packaging build. |
| src/package/Microsoft.TestPlatform/Microsoft.TestPlatform.nuspec | Removes SettingsMigrator binaries/resources from the packaged output. |
| src/package/Microsoft.VisualStudio.TestTools.TestPlatform.V2.CLI/Microsoft.VisualStudio.TestTools.TestPlatform.V2.CLI.csproj | Removes Vsix/source and project references related to SettingsMigrator. |
| src/SettingsMigrator/Migrator.cs | Removes the XML migration implementation (tool is decommissioned). |
| src/SettingsMigrator/PathResolver.cs | Removes CLI path resolution logic (tool is decommissioned). |
| src/SettingsMigrator/Program.cs | Removes the CLI entry point (tool is decommissioned). |
| src/SettingsMigrator/NullableHelpers.cs | Removes tool-specific generated nullable helpers. |
| src/SettingsMigrator/NullableHelpers.tt | Removes the tool’s T4 template used to generate nullable helpers. |
| src/SettingsMigrator/PublicAPI/PublicAPI.Shipped.txt | Removes public API tracking for the deleted tool assembly. |
| src/SettingsMigrator/PublicAPI/PublicAPI.Unshipped.txt | Removes unshipped API tracking for the deleted tool assembly. |
| src/SettingsMigrator/Resources/Resources.resx | Removes tool command-line/localized strings resources. |
| src/SettingsMigrator/Resources/xlf/Resources.xlf | Removes the XLIFF source file for localization. |
| src/SettingsMigrator/Resources/xlf/Resources.cs.xlf | Removes Czech localization for the tool. |
| src/SettingsMigrator/Resources/xlf/Resources.de.xlf | Removes German localization for the tool. |
| src/SettingsMigrator/Resources/xlf/Resources.es.xlf | Removes Spanish localization for the tool. |
| src/SettingsMigrator/Resources/xlf/Resources.fr.xlf | Removes French localization for the tool. |
| src/SettingsMigrator/Resources/xlf/Resources.it.xlf | Removes Italian localization for the tool. |
| src/SettingsMigrator/Resources/xlf/Resources.ja.xlf | Removes Japanese localization for the tool. |
| src/SettingsMigrator/Resources/xlf/Resources.ko.xlf | Removes Korean localization for the tool. |
| src/SettingsMigrator/Resources/xlf/Resources.pl.xlf | Removes Polish localization for the tool. |
| src/SettingsMigrator/Resources/xlf/Resources.pt-BR.xlf | Removes Portuguese (Brazil) localization for the tool. |
| src/SettingsMigrator/Resources/xlf/Resources.ru.xlf | Removes Russian localization for the tool. |
| src/SettingsMigrator/Resources/xlf/Resources.tr.xlf | Removes Turkish localization for the tool. |
| src/SettingsMigrator/Resources/xlf/Resources.zh-Hans.xlf | Removes Simplified Chinese localization for the tool. |
| src/SettingsMigrator/Resources/xlf/Resources.zh-Hant.xlf | Removes Traditional Chinese localization for the tool. |
| src/SettingsMigrator/SettingsMigrator.csproj | Removes the tool project definition from the repo. |
| src/SettingsMigrator/TestSettingsNodes.cs | Removes the helper model type used by the tool. |
| test/SettingsMigrator.UnitTests/MigratorTests.cs | Removes unit tests for migration behavior. |
| test/SettingsMigrator.UnitTests/PathResolverTests.cs | Removes unit tests for CLI/path resolution behavior. |
| test/SettingsMigrator.UnitTests/ProgramTests.cs | Removes unit tests for CLI argument handling. |
| test/SettingsMigrator.UnitTests/SettingsMigrator.UnitTests.csproj | Removes the unit test project definition. |
| test/SettingsMigrator.UnitTests/oldRunSettingsWithDataCollector.runsettings | Removes test data used by the deleted unit tests. |
| test/SettingsMigrator.UnitTests/oldRunSettingsWithEmbeddedSettings.runsettings | Removes test data used by the deleted unit tests. |
| test/SettingsMigrator.UnitTests/oldTestSettings.testsettings | Removes test data used by the deleted unit tests. |
Files not reviewed (1)
- src/SettingsMigrator/Resources/Resources.Designer.cs: Language not supported
|
|
||
| Visual Studio historically used `.testsettings` files for test configuration. The modern format is `.runsettings`. The two formats differ in structure, and some `.testsettings` nodes map to `<LegacySettings>` wrappers in `.runsettings` to preserve backward compatibility. | ||
|
|
||
| See also: [RFC 0023 - TestSettings Deprecation](../../docs/RFCs/0023-TestSettings-Deprecation.md) |
There was a problem hiding this comment.
The relative link likely points to .github/docs/... rather than the repo-level docs/... because this file lives under .github/skills/settings-migration/. Update the relative path (e.g., one more ..) so the RFC link resolves correctly on GitHub.
Suggested change
| See also: [RFC 0023 - TestSettings Deprecation](../../docs/RFCs/0023-TestSettings-Deprecation.md) | |
| See also: [RFC 0023 - TestSettings Deprecation](../../../docs/RFCs/0023-TestSettings-Deprecation.md) |
Comment on lines
+43
to
+52
| | XPath in .testsettings | Node Name | Purpose | | ||
| |---|---|---| | ||
| | `/TestSettings/Deployment` | Deployment | File deployment settings | | ||
| | `/TestSettings/Scripts` | Scripts | Setup/cleanup scripts | | ||
| | `/TestSettings/Execution/TestTypeSpecific/WebTestRunConfiguration` | WebSettings | Web test configuration | | ||
| | `/TestSettings/Execution/AgentRule/DataCollectors/DataCollector` | DataCollectors | Data collector definitions | | ||
| | `/TestSettings/Execution/Timeouts` | Timeouts | Timeout attributes | | ||
| | `/TestSettings/Execution/TestTypeSpecific/UnitTestRunConfig` | UnitTestConfig | Unit test run configuration | | ||
| | `/TestSettings/Execution/Hosts` | Hosts | Host configuration | | ||
| | `/TestSettings/Execution` | Execution | Execution attributes (parallelTestCount, hostProcessPlatform) | |
There was a problem hiding this comment.
This markdown table syntax is invalid due to the double leading pipes (||), which prevents GitHub from rendering it as a table. Replace each row with a single leading | so the mapping table renders correctly.
| ``` | ||
|
|
||
| **Rules for the Execution sub-node:** | ||
| - Only create `<Execution>` if at least one of `UnitTestConfig`, `parallelTestCount`, `testTimeout`, `Hosts` is present |
There was a problem hiding this comment.
The rules are internally inconsistent: the bullet list says hostProcessPlatform is an <Execution> attribute, but the condition for creating <Execution> doesn’t include hostProcessPlatform. Either include hostProcessPlatform in the creation condition, or explicitly document what to do when hostProcessPlatform is present but none of the other triggers are.
Suggested change
| - Only create `<Execution>` if at least one of `UnitTestConfig`, `parallelTestCount`, `testTimeout`, `Hosts` is present | |
| - Only create `<Execution>` if at least one of `UnitTestConfig`, `parallelTestCount`, `testTimeout`, `Hosts`, or `hostProcessPlatform` is present |
Member
Author
|
The guidance in the skill seems kinda useless, we don't support legacy mode anymore etc. so probably best course of action for the user is to delete tests settings and don't look back. |
The SettingsMigrator.exe tool converts legacy .testsettings files to .runsettings format. This replaces it with a Copilot skill that documents the complete migration process step-by-step, usable both by AI agents and humans following manual steps. Changes: - New .github/skills/settings-migration/SKILL.md with complete migration guide including XML mapping rules, examples, and edge cases - Removed src/SettingsMigrator/ (source, resources, 14 localization files) - Removed test/SettingsMigrator.UnitTests/ (16 unit tests, test data) - Removed from TestPlatform.sln, Microsoft.TestPlatform.nuspec, Microsoft.TestPlatform.csproj, V2.CLI.csproj, verify-nupkgs.ps1 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Documents how every code path, test scenario, and XML mapping from the removed SettingsMigrator tool is accounted for in the replacement skill. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Reports which .testsettings properties require legacy mode (most of them) and documents existing telemetry metrics for measuring feature usage. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
nohwnd
force-pushed
the
deprecate-settings-migrator
branch
from
407db97 to
e722f6b
Compare
|
|
||
| #### 3b. LegacySettings Node | ||
|
|
||
| If **any** of these are present: `Deployment`, `Scripts`, `UnitTestConfig`, `Hosts`, `parallelTestCount`, `testTimeout`, or `hostProcessPlatform`, create a `<LegacySettings>` node and also set `<MSTest><ForcedLegacyMode>true</ForcedLegacyMode></MSTest>`: |
There was a problem hiding this comment.
The rules are internally inconsistent for hostProcessPlatform: it is listed as a trigger for creating <LegacySettings>, and it is described as an attribute on <Execution>, but the condition for creating <Execution> does not include hostProcessPlatform. As written, a .testsettings with only hostProcessPlatform would have no documented destination for that value. Please update the <Execution> creation rule to include hostProcessPlatform (or explicitly document that hostProcessPlatform is only migrated when some other <Execution> trigger is present).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Replaces the \SettingsMigrator.exe\ tool with a Copilot skill that documents the complete .testsettings\ → .runsettings\ migration process, then removes all source code and tests.
Motivation
The SettingsMigrator is a .NET Framework-only console app (~420 lines of XML transformation code, 14 localization files, 16 unit tests). Its functionality is straightforward XML node mapping that can be documented as step-by-step instructions usable by both AI agents and humans.
Changes
Added
Removed (~2000 lines)
Verification