[CrashDump] Support {placeholder} tokens in --crashdump-filename via ArtifactNamingHelper

[Evangelink]

Summary

Follow-up to PR #8951 (Copilot review comments) — wires ArtifactNamingHelper into the CrashDump extension so --crashdump-filename supports the same {placeholder} template syntax as HangDump and the report extensions.

What changed

CrashDumpEnvironmentVariableProvider

• Resolves {pname} / {pid} / {asm} / {tfm} / {time} tokens in the user-supplied filename via ArtifactNamingHelper.ResolveTemplate.
• Because CrashDump only hands a filename pattern to the .NET runtime's createdump before the testhost launches, the testhost PID / executable name are not yet known. We:
  • resolve {asm} / {tfm} / {time} to literal values up-front, and
  • map {pid} → %p and {pname} → %e so the runtime expands them at crash-write time.
• The sequence file name is now derived from the resolved user pattern (with {X} substituted) rather than the raw user input, keeping the sequence file aligned with the actual dump file name.

CrashDumpProcessLifetimeHandler — crash-report lookup fix

• The previous lookup line string expectedCrashReportFile = $"{expectedDumpFile}{CrashReportFileExtension}" only replaces %p with the PID. When the dump pattern contains %e (now possible via {pname}), the literal expected path would not exist on disk and the crash report would be silently dropped.
• Switched to regex-based enumeration that reuses the existing testhostDumpRegex (which already turns %X placeholders into .* wildcards) so any .crashreport.json whose prefix matches the dump pattern is correctly discovered and published.

Help text / resx

• Expanded CrashDumpFileNameOptionDescription to document all supported placeholders, including the {pname}→%e / {pid}→%p runtime-deferred mapping and that {time} is captured when the crashdump environment is configured.
• Integration test HelpInfoAllExtensionsTests updated to match.

Glossary

• ArtifactNamingHelper entry expanded to list direct consumers (HangDump, CrashDump) and indirect consumers (HtmlReport / JUnitReport / TrxReport via ReportFileNameHelper), plus a note about the CrashDump runtime-placeholder mapping.
• ⚠ overlaps with PR [docs] Address glossary review feedback from PR #8951 #8953 on a single line — will need a manual rebase of that one line if [docs] Address glossary review feedback from PR #8951 #8953 merges first.

Tests

• New unit test OnTestHostProcessExitedAsync_CrashReport_PatternWithRuntimePlaceholders_PublishesMatchedReport covers the regex-based crash-report matching when the dump pattern contains %e.
• Full Microsoft.Testing.Extensions.UnitTests suite passes on net8 / net9 / net462 / net472.
• Default behavior ({testAppName}_%p_crash.dmp) is unchanged when --crashdump-filename is not supplied.

[Copilot]

Pull request overview

This PR updates the CrashDump MTP extension to support the same {placeholder} filename template syntax used by other extensions, by wiring in ArtifactNamingHelper and improving crash-report discovery when runtime placeholders (e.g. %e) are involved.

Changes:

• Added {pname}/{pid}/{asm}/{tfm}/{time} template resolution for --crashdump-filename, mapping {pid}→%p and {pname}→%e for runtime-time expansion.
• Fixed crash-report discovery to handle dump name patterns containing runtime placeholders beyond %p.
• Updated help text, resources, glossary, and added a unit test covering crash-report matching with %e.

Show a summary per file

File	Description
test/UnitTests/Microsoft.Testing.Extensions.UnitTests/CrashDumpTests.cs	Adds a unit test validating crash-report publication when the dump pattern includes runtime placeholders like %e.
test/IntegrationTests/Microsoft.Testing.Platform.Acceptance.IntegrationTests/HelpInfoAllExtensionsTests.cs	Updates expected --help / --info output for the expanded --crashdump-filename placeholder documentation.
src/Platform/Microsoft.Testing.Extensions.CrashDump/Resources/xlf/CrashDumpResources.zh-Hant.xlf	Updates localized resource entry for the expanded crashdump filename description.
src/Platform/Microsoft.Testing.Extensions.CrashDump/Resources/xlf/CrashDumpResources.zh-Hans.xlf	Updates localized resource entry for the expanded crashdump filename description.
src/Platform/Microsoft.Testing.Extensions.CrashDump/Resources/xlf/CrashDumpResources.tr.xlf	Updates localized resource entry for the expanded crashdump filename description.
src/Platform/Microsoft.Testing.Extensions.CrashDump/Resources/xlf/CrashDumpResources.ru.xlf	Updates localized resource entry for the expanded crashdump filename description.
src/Platform/Microsoft.Testing.Extensions.CrashDump/Resources/xlf/CrashDumpResources.pt-BR.xlf	Updates localized resource entry for the expanded crashdump filename description.
src/Platform/Microsoft.Testing.Extensions.CrashDump/Resources/xlf/CrashDumpResources.pl.xlf	Updates localized resource entry for the expanded crashdump filename description.
src/Platform/Microsoft.Testing.Extensions.CrashDump/Resources/xlf/CrashDumpResources.ko.xlf	Updates localized resource entry for the expanded crashdump filename description.
src/Platform/Microsoft.Testing.Extensions.CrashDump/Resources/xlf/CrashDumpResources.ja.xlf	Updates localized resource entry for the expanded crashdump filename description.
src/Platform/Microsoft.Testing.Extensions.CrashDump/Resources/xlf/CrashDumpResources.it.xlf	Updates localized resource entry for the expanded crashdump filename description.
src/Platform/Microsoft.Testing.Extensions.CrashDump/Resources/xlf/CrashDumpResources.fr.xlf	Updates localized resource entry for the expanded crashdump filename description.
src/Platform/Microsoft.Testing.Extensions.CrashDump/Resources/xlf/CrashDumpResources.es.xlf	Updates localized resource entry for the expanded crashdump filename description.
src/Platform/Microsoft.Testing.Extensions.CrashDump/Resources/xlf/CrashDumpResources.de.xlf	Updates localized resource entry for the expanded crashdump filename description.
src/Platform/Microsoft.Testing.Extensions.CrashDump/Resources/xlf/CrashDumpResources.cs.xlf	Updates localized resource entry for the expanded crashdump filename description.
src/Platform/Microsoft.Testing.Extensions.CrashDump/Resources/CrashDumpResources.resx	Expands CrashDumpFileNameOptionDescription to document placeholder tokens and runtime-deferred mappings.
src/Platform/Microsoft.Testing.Extensions.CrashDump/Microsoft.Testing.Extensions.CrashDump.csproj	Links in ArtifactNamingHelper (and its dependency TargetFrameworkParser) into the CrashDump extension project.
src/Platform/Microsoft.Testing.Extensions.CrashDump/CrashDumpProcessLifetimeHandler.cs	Updates crash-report lookup to use regex-based matching so %e / other runtime placeholders don’t break discovery.
src/Platform/Microsoft.Testing.Extensions.CrashDump/CrashDumpExtensions.cs	Passes IClock to the environment-variable provider to support {time} resolution.
src/Platform/Microsoft.Testing.Extensions.CrashDump/CrashDumpEnvironmentVariableProvider.cs	Resolves user --crashdump-filename templates via ArtifactNamingHelper and derives sequence filename from the resolved template.
docs/glossary.md	Expands the ArtifactNamingHelper glossary entry with consumer details and CrashDump runtime-placeholder mapping notes.

Copilot's findings

• Files reviewed: 21/21 changed files
• Comments generated: 2

[Evangelink]

Test Completeness & Coverage — 1 ISSUE

The crash-report regex matching (primary new behavior) is well covered by the new PatternWithRuntimePlaceholders test and the existing multi-placeholder/child-crash tests. ArtifactNamingHelper itself has thorough standalone tests.

The one gap is that ResolveUserDumpFileNameTemplate is new logic that is never exercised through UpdateAsync in any test. The specific contract — {pname} → %e (not %p) and {pid} → %p (not %e) — is not pinned. A swapped-argument regression silently produces a wrong dump-file pattern whose crash-report regex would then not match the file actually written by the runtime. See the inline comment for a suggested [DataRow] test.

Generated by Expert Code Review (on open) for issue #8957 · sonnet46 13.2M

[Youssef1313]

This is introducing a regression.

CrashDump should never replace placeholders that are known to runtime. The placeholders should remain as-is so that they flow properly to child processes.

[Youssef1313]

Other placeholders that are not known by runtime might as well be confusing if the crash happens in a child process.

[Evangelink]

Sorry @Youssef1313, I think I miscommunicated the intent in the PR description — let me clarify and we can decide where to go from here.

What I actually wanted from this PR: just make --crashdump-filename accept the same {placeholder} vocabulary ({asm}, {tfm}, {time}, {pname}, {pid}) that --hangdump-filename, --report-trx-filename, --report-html-filename, and --report-junit-filename already accept. The motivation is purely API consistency — a user who learned the {...} syntax from one extension shouldn''t hit a different vocabulary on another. I do not want to change default behavior (no --crashdump-filename still resolves to {testAppName}_%p_crash.dmp), and I do not want to break the runtime per-process expansion contract.

On "should never replace placeholders known to runtime": agreed, and the PR doesn''t touch %p / %e / %h / %t — ArtifactNamingHelper.ResolveTemplate only matches {...} tokens and preserves everything else verbatim, so a user pattern like Dump_%p.dmp flows to the runtime unchanged. The {pid} → %p and {pname} → %e mapping was specifically intended to preserve runtime per-child-process expansion (we substitute {pid} with the literal string %p so the runtime still fills it in per crashing process). But I take your point that this overlaps confusingly with the runtime placeholders that already exist — happy to drop {pid} / {pname} entirely and let users keep using %p / %e directly if you''d prefer.

On "other placeholders... confusing if the crash happens in a child process": also a fair concern for {asm} / {tfm} — they''d resolve to the parent (entry assembly / TFM) values regardless of which child crashes. My read is they still describe the test run identity (which is the same across the child tree) rather than the crashing process identity, so they''re informative rather than misleading, but I see the argument the other way.

Concretely, three options I see — let me know which you''d like:

1. Close this PR — accept that CrashDump should not adopt the {placeholder} vocabulary at all because of the runtime-driven, child-process-aware nature of dump generation.
2. Scope down to deterministic-only tokens — keep only {asm} / {tfm} / {time} (no {pid} / {pname} since the runtime equivalents already exist). This drops the need for the CrashDumpProcessLifetimeHandler regex change because no new %e is introduced.
3. Keep the current shape — full {asm} / {tfm} / {time} / {pid} / {pname} set, with {pid} / {pname} translated to %p / %e so the runtime continues to expand them per child.

My slight preference is (2) — it gives users the consistency win without overlapping with runtime placeholders or requiring the crash-report-lookup regex change — but I''m fine with (1) if you''d rather not add the surface area at all. Will you let me know which direction to take?

[Copilot]

Copilot's findings

• Files reviewed: 20/20 changed files
• Comments generated: 1

[Evangelink]

🧪 Test quality grade — PR #8957

Δ	Test	Grade	Band	Notes
new	CrashDumpTests. UpdateAsync_ UserFilenameTemplate_ SetsCorrectDumpFileNamePattern	D	60–69	Conditional if (expectedSuffix is not null) drives two distinct assertion branches in one body; the null-suffix case tests different properties than the string cases — prefer two focused test methods.
new	CrashDumpTests. OnTestHostProcessExitedAsync_ CrashReport_ PatternWithRuntimePlaceholders_ PublishesMatchedReport	B	80–89	Clear regression scenario with positive sequence-equality + negative no-warning assertions; arrange phase is slightly long but well-commented.
mod	HelpInfoAllExtensionsTests. Help_ WithAllExtensionsRegistered_ OutputFullHelpContent	A	90–100	No issues found.
mod	HelpInfoAllExtensionsTests. Info_ WithAllExtensionsRegistered_ OutputFullInfoContent	A	90–100	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. Posted via a maintainer's GitHub token, so it appears under their account — the account owner did not write or approve this content personally. Generated by the Grade Tests on PR (on open / sync) workflow. · 465.9 AIC · ⌖ 13.2 AIC · ⊞ 45.6K · [◷]( · ◷)

[Evangelink]

🔴 Build Failure Analysis

Root cause: 8 × IDE0008 ("Use explicit type instead of var") compiler errors in the new test methods added by this PR, all in one file.

---

Affected file

test/UnitTests/Microsoft.Testing.Platform.UnitTests/OutputDevice/Terminal/TerminalTestReporterTests.cs

Errors (all the same root cause)

Line	Code	Message
1468	IDE0008	Use explicit type instead of var
1492	IDE0008	Use explicit type instead of var
1513	IDE0008	Use explicit type instead of var
1537	IDE0008	Use explicit type instead of var

(each line is reported twice because the project is compiled for two target frameworks)

Root cause analysis

The four new test methods each declare var terminalReporter = CreateOrchestratorReporter(...). The repo's .editorconfig enforces IDE0008 as an error, which requires an explicit type whenever the type is not apparent from the right-hand side of the assignment. Because CreateOrchestratorReporter is a method call (not a new expression), the type is not considered apparent and var is disallowed.

CreateOrchestratorReporter returns TerminalTestReporter, so the fix is straightforward.

Fix

Replace var with TerminalTestReporter at all four sites:

// Lines 1468, 1492, 1513, 1537 — change:
var terminalReporter = CreateOrchestratorReporter(...);

// To:
TerminalTestReporter terminalReporter = CreateOrchestratorReporter(...);

Inline suggestion comments are posted below for each affected line.

🤖 Automated content by GitHub Copilot. Posted via a maintainer's GitHub token, so it appears under their account — the account owner did not write or approve this content personally. Generated by the Build Failure Analysis workflow. · 211.7 AIC · ⌖ 12.3 AIC · ⊞ 46.9K · [◷]( · ◷)

[Evangelink]

🤖 Automated content by GitHub Copilot. Posted via a maintainer's GitHub token, so it appears under their account — the account owner did not write or approve this content personally. Generated by the Build Failure Analysis workflow. · 211.7 AIC · ⌖ 12.3 AIC · ⊞ 46.9K · ◷