Unify placeholder support across MTP --*-filename CLI options#8574
Merged
Conversation
Co-authored-by: Evangelink <11340282+Evangelink@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…-hangdump-filename
All four MTP *-filename CLI options that route through user code are now consistent:
- --report-trx-filename (already used ArtifactNamingHelper)
- --hangdump-filename (already used ArtifactNamingHelper + legacy %p)
- --report-html-filename: now resolves {pname}/{pid}/{asm}/{tfm}/{time} via ArtifactNamingHelper before combining with the output directory and sanitizing any invalid filename chars introduced by substitution.
- --crashdump-filename: intentionally left unchanged - the value is forwarded verbatim to the .NET runtime via DbgMiniDumpName and is expanded by createdump's own placeholders (%p/%e/%t/%h).
Option descriptions for --report-html-filename and --hangdump-filename now advertise the supported placeholders. Localized XLF files regenerated and the HelpInfoAllExtensionsTests --help/--info expectations updated accordingly. Added a unit test for HtmlReportEngine covering placeholder resolution.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR makes MTP *-filename CLI options behave more consistently by routing placeholder expansion through the existing ArtifactNamingHelper (where applicable) and updating help/option descriptions to document the supported placeholders (while keeping crash dump filename handling runtime-owned).
Changes:
- Added placeholder resolution support for
--report-html-filename(including sanitization of the resolved filename leaf). - Documented placeholder support for
--hangdump-filename(including the legacy%ptoken) and updated acceptance help/info expectations accordingly. - Introduced a shared
CommandLineOptionsProviderBaseand refactored several providers to use it, with new unit tests covering default behaviors.
Show a summary per file
| File | Description |
|---|---|
| test/UnitTests/Microsoft.Testing.Platform.UnitTests/CommandLine/CommandLineOptionsProviderBaseTests.cs | Adds unit coverage for new shared command line provider base defaults/validation behavior. |
| test/UnitTests/Microsoft.Testing.Extensions.UnitTests/HtmlReportEngineTests.cs | Adds a unit test verifying placeholder resolution for user-supplied HTML report filenames. |
| test/IntegrationTests/Microsoft.Testing.Platform.Acceptance.IntegrationTests/HelpInfoAllExtensionsTests.cs | Updates --help/--info expectations to include new/expanded filename placeholder documentation. |
| src/Platform/Microsoft.Testing.Platform/CommandLine/CommandLineOptionsProviderBase.cs | Introduces a shared base implementation for ICommandLineOptionsProvider. |
| src/Platform/Microsoft.Testing.Extensions.MSBuild/MSBuildCommandLineProvider.cs | Refactors provider to inherit from the new base class. |
| src/Platform/Microsoft.Testing.Extensions.HtmlReport/Resources/xlf/ExtensionResources.zh-Hant.xlf | Regenerated localization file reflecting updated HTML filename option description source. |
| src/Platform/Microsoft.Testing.Extensions.HtmlReport/Resources/xlf/ExtensionResources.zh-Hans.xlf | Regenerated localization file reflecting updated HTML filename option description source. |
| src/Platform/Microsoft.Testing.Extensions.HtmlReport/Resources/xlf/ExtensionResources.tr.xlf | Regenerated localization file reflecting updated HTML filename option description source. |
| src/Platform/Microsoft.Testing.Extensions.HtmlReport/Resources/xlf/ExtensionResources.ru.xlf | Regenerated localization file reflecting updated HTML filename option description source. |
| src/Platform/Microsoft.Testing.Extensions.HtmlReport/Resources/xlf/ExtensionResources.pt-BR.xlf | Regenerated localization file reflecting updated HTML filename option description source. |
| src/Platform/Microsoft.Testing.Extensions.HtmlReport/Resources/xlf/ExtensionResources.pl.xlf | Regenerated localization file reflecting updated HTML filename option description source. |
| src/Platform/Microsoft.Testing.Extensions.HtmlReport/Resources/xlf/ExtensionResources.ko.xlf | Regenerated localization file reflecting updated HTML filename option description source. |
| src/Platform/Microsoft.Testing.Extensions.HtmlReport/Resources/xlf/ExtensionResources.ja.xlf | Regenerated localization file reflecting updated HTML filename option description source. |
| src/Platform/Microsoft.Testing.Extensions.HtmlReport/Resources/xlf/ExtensionResources.it.xlf | Regenerated localization file reflecting updated HTML filename option description source. |
| src/Platform/Microsoft.Testing.Extensions.HtmlReport/Resources/xlf/ExtensionResources.fr.xlf | Regenerated localization file reflecting updated HTML filename option description source. |
| src/Platform/Microsoft.Testing.Extensions.HtmlReport/Resources/xlf/ExtensionResources.es.xlf | Regenerated localization file reflecting updated HTML filename option description source. |
| src/Platform/Microsoft.Testing.Extensions.HtmlReport/Resources/xlf/ExtensionResources.de.xlf | Regenerated localization file reflecting updated HTML filename option description source. |
| src/Platform/Microsoft.Testing.Extensions.HtmlReport/Resources/xlf/ExtensionResources.cs.xlf | Regenerated localization file reflecting updated HTML filename option description source. |
| src/Platform/Microsoft.Testing.Extensions.HtmlReport/Resources/ExtensionResources.resx | Updates the HTML filename option description to document placeholders and provide an example. |
| src/Platform/Microsoft.Testing.Extensions.HtmlReport/Microsoft.Testing.Extensions.HtmlReport.csproj | Links ArtifactNamingHelper.cs into the HtmlReport extension for placeholder resolution. |
| src/Platform/Microsoft.Testing.Extensions.HtmlReport/HtmlReportEngine.cs | Implements user filename template resolution + sanitization for HTML reports. |
| src/Platform/Microsoft.Testing.Extensions.HangDump/Resources/xlf/ExtensionResources.zh-Hant.xlf | Regenerated localization file reflecting updated hang dump filename option description source. |
| src/Platform/Microsoft.Testing.Extensions.HangDump/Resources/xlf/ExtensionResources.zh-Hans.xlf | Regenerated localization file reflecting updated hang dump filename option description source. |
| src/Platform/Microsoft.Testing.Extensions.HangDump/Resources/xlf/ExtensionResources.tr.xlf | Regenerated localization file reflecting updated hang dump filename option description source. |
| src/Platform/Microsoft.Testing.Extensions.HangDump/Resources/xlf/ExtensionResources.ru.xlf | Regenerated localization file reflecting updated hang dump filename option description source. |
| src/Platform/Microsoft.Testing.Extensions.HangDump/Resources/xlf/ExtensionResources.pt-BR.xlf | Regenerated localization file reflecting updated hang dump filename option description source. |
| src/Platform/Microsoft.Testing.Extensions.HangDump/Resources/xlf/ExtensionResources.pl.xlf | Regenerated localization file reflecting updated hang dump filename option description source. |
| src/Platform/Microsoft.Testing.Extensions.HangDump/Resources/xlf/ExtensionResources.ko.xlf | Regenerated localization file reflecting updated hang dump filename option description source. |
| src/Platform/Microsoft.Testing.Extensions.HangDump/Resources/xlf/ExtensionResources.ja.xlf | Regenerated localization file reflecting updated hang dump filename option description source. |
| src/Platform/Microsoft.Testing.Extensions.HangDump/Resources/xlf/ExtensionResources.it.xlf | Regenerated localization file reflecting updated hang dump filename option description source. |
| src/Platform/Microsoft.Testing.Extensions.HangDump/Resources/xlf/ExtensionResources.fr.xlf | Regenerated localization file reflecting updated hang dump filename option description source. |
| src/Platform/Microsoft.Testing.Extensions.HangDump/Resources/xlf/ExtensionResources.es.xlf | Regenerated localization file reflecting updated hang dump filename option description source. |
| src/Platform/Microsoft.Testing.Extensions.HangDump/Resources/xlf/ExtensionResources.de.xlf | Regenerated localization file reflecting updated hang dump filename option description source. |
| src/Platform/Microsoft.Testing.Extensions.HangDump/Resources/xlf/ExtensionResources.cs.xlf | Regenerated localization file reflecting updated hang dump filename option description source. |
| src/Platform/Microsoft.Testing.Extensions.HangDump/Resources/ExtensionResources.resx | Updates hang dump filename option description to document placeholders and legacy %p. |
| src/Platform/Microsoft.Testing.Extensions.HangDump/HangDumpCommandLineProvider.cs | Refactors provider to inherit from the new base class and override validation methods. |
| src/Platform/Microsoft.Testing.Extensions.CrashDump/CrashDumpCommandLineProvider.cs | Refactors provider to inherit from the new base class and override validation methods. |
Copilot's findings
- Files reviewed: 37/37 changed files
- Comments generated: 0
Resolve conflicts: - CrashDumpCommandLineProvider.cs: keep main's single primary-constructor body. - HtmlReportEngine.cs: adopt main's ResolveHtmlFileName / GetProvidedFileName helpers (functionally equivalent to our ResolveUserFileName); main also uses _clock.UtcNow directly. - HtmlReport ExtensionResources.resx: keep main's longer description that documents relative/absolute path support; placeholder list and example remain. - HtmlReport xlf files: regenerate from clean main baseline so the trans-units carry a single <source>/<target>. - HelpInfoAllExtensionsTests.cs: adopt main's longer description for the --report-html-filename --help and --info expectation blocks. - HtmlReportEngineTests.cs: drop the placeholder unit test we added, since main introduced a more thorough GenerateReportAsync_ExplicitFileName_ ResolvesPlaceholdersAndSanitizesLeafName test. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
Comment on lines
+121
to
+128
| foreach (AttributeData attribute in attributes) | ||
| { | ||
| AttributeApplicationModel? model = BuildAttribute(attribute); | ||
| if (model is not null) | ||
| { | ||
| builder.Add(model); | ||
| } | ||
| } |
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
Audits all four MTP
*-filenameCLI options so they consistently resolve placeholders through the existingArtifactNamingHelperservice and advertise it in their option description.--report-trx-filename{pname}/{pid}/{asm}/{tfm}/{time}, documented--hangdump-filename%p), undocumented%ptoken--report-html-filenameArtifactNamingHelper.ResolveTemplateand documents the placeholders--crashdump-filenameDbgMiniDumpName%p/%e/%t/%h); doing it client-side would be either a no-op or misleading ({pid}would be the controller PID, not the test host)Implementation notes
HtmlReportEngine.cs: newResolveUserFileNamemirrors the TRX pattern - resolve the template, then sanitize the file-name leaf viaPath.GetInvalidFileNameChars(), thenPath.Combinewith the output directory.Microsoft.Testing.Extensions.HtmlReport.csproj: linksArtifactNamingHelper.cs(TargetFrameworkParser.cswas already linked).ExtensionResources.resxupdated; localized XLF files regenerated viadotnet msbuild .../Microsoft.Testing.Extensions.HtmlReport.csproj /t:UpdateXlf(and the equivalent for HangDump).HelpInfoAllExtensionsTests.cs--helpand--infoexpectation blocks updated for both affected options.Tests
GenerateReportAsync_WithUserSuppliedFileName_ResolvesPlaceholdersinHtmlReportEngineTests.cs.dotnet test Microsoft.Testing.Extensions.UnitTests --filter ~HtmlReport(net9.0): 55/55 passed.dotnet test Microsoft.Testing.Platform.Acceptance.IntegrationTests --filter ~HelpInfoAllExtensionsTests(net11.0, after-pack): 9/9 passed.