Centralized Artifact Naming Service#15681
Conversation
Introduce IArtifactNameProvider and supporting types for template-based
artifact file naming. This replaces fragmented, ad-hoc file naming
across extensions with a single service that resolves {Token} templates
to sanitized, collision-safe file paths.
New public types in ObjectModel.ArtifactNaming:
- IArtifactNameProvider: interface with Resolve() and ExpandTemplate()
- ArtifactNameRequest: request object with file/directory templates
- ArtifactNameTokens: well-known token name constants
- ArtifactNamingPresets: built-in ci/local/detailed/flat presets
- CollisionBehavior: AppendCounter, Overwrite, Fail, AppendTimestamp
Implementation (internal) in Common.ArtifactNaming:
- ArtifactNameProvider: template parser, path sanitization, collision
handling, auto-populated tokens (Timestamp, MachineName, etc.)
Template syntax: {Token}, {Token:format}, {Token?fallback}, {{/}}
Unresolved tokens kept literally in output for clear diagnostics.
Addresses: microsoft#2378 microsoft#15673 microsoft#3026 microsoft#1603 microsoft#2113
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Resolve() now returns ArtifactNameResult with FilePath, IsOverwrite,
and IsDirectoryOwner metadata instead of a bare string
- Add .vstest-run marker file for directory ownership detection
via FileMode.CreateNew (atomic)
- Drop {Token?fallback} syntax (? is invalid in Windows paths)
- Unresolved tokens are kept literally: {Tfm} stays as {Tfm}
- All 35 tests pass on net11.0 and net481
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Fix SanitizeFileName to strip trailing dots (Windows normalizes these)
- Add 23 new tests covering:
- Template parser: empty token, lone brace, whitespace token, unknown
with format, multiple colons, format on non-DateTime, triple braces,
case-insensitive lookup
- Sanitization: trailing dot, trailing space, all-invalid chars,
case-insensitive reserved names, reserved names in directory segments,
mixed dot-dot traversal, rooted unix paths
- Provider: null/empty extension, context overrides auto-timestamp,
case-insensitive context override, Fail when no collision,
AppendTimestamp fallback to counter, missing TestResultsDirectory
- Total: 58 tests passing on net11.0 and net481
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Add TargetArchitecture and TestRunTimestamp to DefaultLoggerParameterNames. These new parameters are extracted from RunSettings and passed to all loggers via TestLoggerManager.UpdateLoggerParameters(), making them available as tokens for the artifact naming service. - Add RunConfiguration.ArtifactRunTimestamp property for cross-process timestamp synchronization via RunSettings - Add TestLoggerManager.GetTargetArchitecture() to extract platform from RunConfiguration (only when explicitly set) - Add TestLoggerManager.GetOrGenerateTestRunTimestamp() that reads ArtifactRunTimestamp from RunSettings or generates ISO 8601 compact format (yyyyMMddTHHmmss.fff) - Add Func<DateTime> time provider to TestLoggerManager for testability - Update existing tests for 4 default logger parameters (was 2) - Add 10 new tests covering architecture extraction, timestamp generation, and end-to-end context propagation All 71 TestLoggerManager tests pass on net11.0 and net481. All 58 artifact naming tests pass on net11.0 and net481. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Adds a centralized artifact naming system to generate deterministic, collision-resistant test output paths across vstest extensions, and propagates additional run context (architecture + shared timestamp) to loggers.
Changes:
- Introduces new public ObjectModel API for artifact name resolution (requests/results/tokens/presets/collision behavior).
- Implements the artifact naming provider in Common (template expansion, sanitization, collision handling, directory ownership markers).
- Propagates
TargetArchitectureandTestRunTimestampthroughTestLoggerManagerand adds corresponding unit tests.
Reviewed changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| test/Microsoft.TestPlatform.CrossPlatEngine.UnitTests/TestLoggerManagerTests.cs | Updates parameter propagation assertions; adds tests for architecture + timestamp extraction and propagation. |
| test/Microsoft.TestPlatform.Common.UnitTests/ArtifactNaming/ArtifactNameTemplateTests.cs | Adds template expansion unit tests (escaping, formatting, edge cases). |
| test/Microsoft.TestPlatform.Common.UnitTests/ArtifactNaming/ArtifactNameProviderTests.cs | Adds provider tests for collision behaviors, sanitization, presets, and ownership markers. |
| src/Microsoft.TestPlatform.ObjectModel/RunSettings/RunConfiguration.cs | Adds ArtifactRunTimestamp runsettings property with XML round-trip. |
| src/Microsoft.TestPlatform.ObjectModel/PublicAPI/PublicAPI.Unshipped.txt | Records new public API surface. |
| src/Microsoft.TestPlatform.ObjectModel/Constants.cs | Adds new default logger parameter names for architecture and run timestamp. |
| src/Microsoft.TestPlatform.ObjectModel/ArtifactNaming/*.cs | Adds new public artifact naming API types (provider interface, request/result, tokens, presets, collision behavior). |
| src/Microsoft.TestPlatform.CrossPlatEngine/Client/TestLoggerManager.cs | Extracts architecture + shared timestamp from runsettings; injects a time provider; passes new parameters to loggers. |
| src/Microsoft.TestPlatform.Common/ArtifactNaming/ArtifactNameProvider.cs | Implements naming provider: token expansion, sanitization, collision handling, and directory marker ownership. |
| // Normalize to forward slashes for splitting, then restore platform separator. | ||
| string normalized = path.Replace('\\', '/'); | ||
| string[] segments = normalized.Split('/'); | ||
|
|
||
| var result = new StringBuilder(path.Length); | ||
| bool first = true; | ||
|
|
||
| foreach (string segment in segments) | ||
| { | ||
| if (!first) | ||
| { | ||
| result.Append(Path.DirectorySeparatorChar); | ||
| } | ||
|
|
||
| first = false; | ||
|
|
||
| // Preserve drive letters (e.g., "C:") and empty segments at the start (UNC paths). | ||
| if (segment.Length == 2 && segment[1] == ':' && char.IsLetter(segment[0])) | ||
| { | ||
| result.Append(segment); | ||
| continue; | ||
| } | ||
|
|
||
| if (segment.Length == 0) | ||
| { | ||
| continue; | ||
| } |
There was a problem hiding this comment.
SanitizePathSegments can accidentally produce rooted/absolute paths due to separator appends that occur even when a segment is skipped (e.g., leading empty segment from /tmp/foo will yield a leading directory separator on the next segment). This conflicts with the stated goal of keeping output under the results directory and also contradicts the comment about preserving leading empty segments (UNC), which the current logic does not preserve. Recommendation (mandatory): build sanitized segments first and join with the directory separator only between emitted segments, and explicitly strip/deny rooted paths unless the root comes from an intended drive-letter prefix (e.g., allow C: but reject/strip a leading empty segment that would imply an absolute path on Unix/Windows). Add/adjust unit tests to assert the sanitized output is not rooted when given /tmp/foo-like inputs.
| private ArtifactNameResult ResolveWithCounter(string directory, string fileName, string extension, bool isDirectoryOwner) | ||
| { | ||
| string path = Path.Combine(directory, fileName + extension); | ||
|
|
||
| if (!_fileExists(path)) | ||
| { | ||
| return new ArtifactNameResult(path, isOverwrite: false, isDirectoryOwner); | ||
| } | ||
|
|
||
| for (int i = 2; i < 10000; i++) | ||
| { | ||
| string candidate = Path.Combine(directory, fileName + "_" + i.ToString(CultureInfo.InvariantCulture) + extension); | ||
| if (!_fileExists(candidate)) | ||
| { | ||
| return new ArtifactNameResult(candidate, isOverwrite: false, isDirectoryOwner); | ||
| } | ||
| } |
There was a problem hiding this comment.
Collision avoidance is based on _fileExists checks, which are not atomic across processes; two parallel test hosts can both observe !_fileExists(candidate) and choose the same path, reintroducing the overwrite/race problems this PR is aiming to eliminate. Recommendation (mandatory): incorporate an atomic reservation mechanism when selecting a path (e.g., attempt FileMode.CreateNew on a placeholder file and only return a candidate if creation succeeds; if you can't create the real artifact yet, create a small reservation file next to it). Alternatively, update the API to return an opened exclusive stream/handle (or a reservation token) so callers can safely write without TOCTOU races.
| out string directoryTemplate, | ||
| out string fileTemplate, | ||
| out CollisionBehavior collision) | ||
| { |
There was a problem hiding this comment.
TryGetPreset will throw a NullReferenceException when presetName is null because it calls ToLowerInvariant() unconditionally. Since this is a public API and follows a 'Try*' pattern, it should return false for null/empty input (and set outputs to safe defaults) rather than throwing.
| { | |
| { | |
| if (string.IsNullOrEmpty(presetName)) | |
| { | |
| directoryTemplate = string.Empty; | |
| fileTemplate = string.Empty; | |
| collision = CollisionBehavior.AppendCounter; | |
| return false; | |
| } |
| if (ArtifactRunTimestamp is not null) | ||
| { | ||
| XmlElement artifactRunTimestamp = doc.CreateElement(nameof(ArtifactRunTimestamp)); | ||
| artifactRunTimestamp.InnerXml = ArtifactRunTimestamp; |
There was a problem hiding this comment.
Using InnerXml for a value that is conceptually plain text allows XML injection (e.g., a crafted timestamp containing markup could change the structure of the generated runsettings). Recommendation (mandatory): set InnerText (or create a text node) instead of InnerXml so the value is always escaped as text.
| artifactRunTimestamp.InnerXml = ArtifactRunTimestamp; | |
| artifactRunTimestamp.InnerText = ArtifactRunTimestamp; |
Centralized Artifact Naming Service
TL;DR
A centralized naming service that gives every vstest extension (TrxLogger, BlameCollector,
coverage, etc.) deterministic, collision-free output file paths. Users get predictable
TestResults/layouts without post-processing scripts, glob hacks, or silent data loss.Resolves #2378 #15673 #3026 #1603 #2113
The Problem Today
When you run
dotnet test, each extension names its output files independently.There is no shared naming convention — each extension has its own ad-hoc logic with
its own bugs:
TestResults/a3f7b2c1-4d5e-.../coverage.xml— unpredictable paths force glob patterns in CIjajares@DESKTOP-ABC 2026-04-15 10_04_49.trx— useless info, no per-run groupingRoot Cause
No shared naming service. Each extension re-invents file naming:
GetTrxFilePath()/SetDefaultTrxFilePath()Sequence_{Guid}.xmlEvent Log.xmlwith a counterEach has its own collision handling (or none), its own sanitization (or none), and its own
assumptions about directory structure.
Before & After
Multi-TFM CI (data loss → everything preserved)
Before:
After:
Each file is unique by construction:
{AssemblyName}_{Tfm}_{Architecture}.Coverage GUID directories → deterministic paths
Before:
CI pipelines need
**/coverage.cobertura.xmlglob patterns. Andrew Arnott (AArnott)has a PowerShell post-processing script in his
Library.Templateto recursively find files.After:
Local dev (machine-name spam → per-run folders)
Before:
After (
localpreset):Each run = one folder. Delete one run:
rm -rf TestResults/20260415T100449.123/.Latest = last alphabetically (ISO 8601 sorts chronologically).
How It Works
Template System
Extensions request a file path by providing a template with
{Token}placeholders:The service expands tokens from context (RunSettings, environment), sanitizes the
result for the file system, handles collisions, and returns a ready-to-use path.
Available Tokens
{TestResultsDirectory}TestResults{AssemblyName}MyProject.Tests{Tfm}net8.0{Architecture}x64{Timestamp}20260415T105100.123{MachineName}BUILD-01Environment.MachineName{UserName}devEnvironment.UserName{Pid}12345{RunId}a1b2c3d4{Date}2026-04-15Tokens can have format specifiers:
{Timestamp:yyyyMMdd}→20260415.Unknown tokens stay literal in the output (e.g.,
{Unknown}→{Unknown}), makingmisconfiguration immediately obvious.
Built-in Presets
ciTestResults/{AssemblyName}_{Tfm}_{Architecture}localTestResults/{Timestamp}/{AssemblyName}_{Tfm}_{Architecture}detailedTestResults/{Timestamp}/{AssemblyName}_{Tfm}_{Architecture}_{RunId}flatTestResults/{AssemblyName}Configuration
RunSettings (for the whole solution):
CLI (per-logger override):
Priority: CLI param > RunSettings template > RunSettings preset > extension default.
Safety Features
Path Sanitization
_CON,PRN,NUL, ...) →_CON,_PRN..path traversal allowed — output stays under results directoryOverwrite Detection
FileMode.CreateNewused to detect when a file already existsResolve()returnsArtifactNameResultwithIsOverwriteflagAtomic Directory Ownership
.vstest-runmarker fileFileMode.CreateNew+ PID contentCollision Behaviors
AppendCounterfile.trx→file_2.trx→file_3.trxOverwriteIsOverwritewarning)FailAppendTimestampCross-Process Timestamp Synchronization
When
dotnet test MySolution.slnruns, MSBuild spawns a separatedotnet testper project.For the
localpreset, all projects should share the same timestamped folder.Solution: The orchestrator injects
<ArtifactRunTimestamp>into RunSettings beforespawning child processes. Each child reads this shared value instead of generating its own.
This uses the same RunSettings propagation mechanism that already works for
ResultsDirectory,TargetPlatform, andTargetFramework.Changes in This PR
New files (ObjectModel — public API)
ArtifactNaming/IArtifactNameProvider.csResolve()→ArtifactNameResultArtifactNaming/ArtifactNameRequest.csArtifactNaming/ArtifactNameResult.csFilePath,IsOverwrite,IsDirectoryOwnerArtifactNaming/ArtifactNameTokens.csArtifactNaming/ArtifactNamingPresets.csArtifactNaming/CollisionBehavior.csNew files (Common — implementation)
ArtifactNaming/ArtifactNameProvider.csModified files
Constants.csDefaultLoggerParameterNames.TargetArchitecture,TestRunTimestampRunConfiguration.csArtifactRunTimestampproperty with XML round-tripTestLoggerManager.csPublicAPI.Unshipped.txtTest coverage
ArtifactNameTemplateTests.csArtifactNameProviderTests.csTestLoggerManagerTests.csAll 129 tests pass on both
net11.0andnet481.What We Don't Do
$BUILD_NUMBER) already covers thisBackward Compatibility
The service ships with templates that reproduce today's behavior exactly. Extensions
adopt the service with zero visible change. The improved defaults
(
{AssemblyName}_{Tfm}_{Architecture}) are a separate opt-in step announced inrelease notes. Users can pin the old format via
<ArtifactFileTemplate>in RunSettings.Next Steps (follow-up PRs)
IArtifactNameProviderinto TrxLogger<ArtifactNamingPreset>/<ArtifactFileTemplate>from RunConfiguration