Skip to content

Commit 0f47404

Browse files
committed
Merge main and fix code review issues in artifact naming service
- Remove <root> placeholder (fragile directory traversal) - Fix OperatingSystem.IsWindows/IsLinux/IsMacOS for netstandard2.0 compat (use RuntimeInformation) - Fix KeyValuePair deconstruction for netstandard2.0 compat - Fix Guard.NotNullOrEmpty (non-existent API) - Fix legacy %p pattern handling (move to HangDump caller, not the service) - Fix time format to use hyphens instead of colons (filesystem safe) - Remove unnecessary using directives - Simplify GetOperatingSystemName with ternary expression - Simplify TFM detection (remove hard-coded version map) - Fix tests: use Assert.ThrowsExactly, match constructor signature, update assertions
1 parent a15fe1f commit 0f47404

5 files changed

Lines changed: 76 additions & 255 deletions

File tree

docs/ArtifactNamingService.md

Lines changed: 7 additions & 57 deletions
Original file line numberDiff line numberDiff line change
@@ -2,9 +2,7 @@
22

33
The artifact naming service provides a standardized way to generate consistent names and paths for test artifacts across all extensions.
44

5-
## Features
6-
7-
### Template-Based Naming
5+
## Template-Based Naming
86

97
Use placeholders in angle brackets to create dynamic file names:
108

@@ -14,17 +12,7 @@ Use placeholders in angle brackets to create dynamic file names:
1412

1513
Resolves to: `MyTests_12345_a1b2c3d4_hang.dmp`
1614

17-
### Complex Path Templates
18-
19-
Create structured directory layouts:
20-
21-
```text
22-
<root>/artifacts/<os>/<asm>/dumps/<pname>_<pid>_<tfm>_<time>.dmp
23-
```
24-
25-
Resolves to: `c:/myproject/artifacts/linux/MyTests/dumps/my-child-process_10001_net9.0_2025-09-22T13:49:34.dmp`
26-
27-
### Available Placeholders
15+
## Available Placeholders
2816

2917
| Placeholder | Description | Example |
3018
|-------------|-------------|---------|
@@ -34,28 +22,17 @@ Resolves to: `c:/myproject/artifacts/linux/MyTests/dumps/my-child-process_10001_
3422
| `<os>` | Operating system | `windows`, `linux`, `macos` |
3523
| `<asm>` | Assembly name | `MyTests` |
3624
| `<tfm>` | Target framework moniker | `net9.0`, `net8.0` |
37-
| `<time>` | Timestamp (1-second precision) | `2025-09-22T13:49:34` |
38-
| `<root>` | Project root directory | Found via solution/git/working dir |
25+
| `<time>` | Timestamp (1-second precision) | `2025-09-22T13-49-34` |
3926

40-
### Backward Compatibility
27+
## Backward Compatibility
4128

42-
Legacy patterns are still supported:
29+
Legacy patterns like `%p` continue to work in the hang dump extension.
4330

44-
```csharp
45-
// Old pattern
46-
"myfile_%p.dmp"
47-
48-
// Works with legacy support
49-
service.ResolveTemplateWithLegacySupport("myfile_%p.dmp",
50-
legacyReplacements: new Dictionary<string, string> { ["%p"] = "12345" });
51-
```
52-
53-
### Custom Replacements
31+
## Custom Replacements
5432

5533
Override default values for specific scenarios:
5634

5735
```csharp
58-
// When dumping a different process than the test host
5936
var customReplacements = new Dictionary<string, string>
6037
{
6138
["pname"] = "Notepad",
@@ -66,41 +43,14 @@ string result = service.ResolveTemplate("<pname>_<pid>.dmp", customReplacements)
6643
// Result: "Notepad_1111.dmp"
6744
```
6845

69-
## Usage in Extensions
70-
71-
Extensions can use the service through dependency injection:
72-
73-
```csharp
74-
public class MyExtension
75-
{
76-
private readonly IArtifactNamingService _artifactNamingService;
77-
78-
public MyExtension(IServiceProvider serviceProvider)
79-
{
80-
_artifactNamingService = serviceProvider.GetArtifactNamingService();
81-
}
82-
83-
public void CreateArtifact(string template)
84-
{
85-
string fileName = _artifactNamingService.ResolveTemplate(template);
86-
// Use fileName for artifact creation
87-
}
88-
}
89-
```
90-
9146
## Hang Dump Integration
9247

93-
The hang dump extension now uses the artifact naming service and supports both legacy and modern patterns:
48+
The hang dump extension uses the artifact naming service and supports both legacy and modern patterns:
9449

9550
```text
9651
# Legacy pattern (still works)
9752
--hangdump-filename "mydump_%p.dmp"
9853
9954
# New template pattern
10055
--hangdump-filename "<pname>_<pid>_<id>_hang.dmp"
101-
102-
# Complex path template
103-
--hangdump-filename "<root>/dumps/<os>/<pname>_<pid>_<time>.dmp"
10456
```
105-
106-
This provides consistent artifact naming across all extensions while maintaining backward compatibility.

src/Platform/Microsoft.Testing.Extensions.HangDump/HangDumpProcessLifetimeHandler.cs

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -311,14 +311,18 @@ private async Task TakeDumpAsync(IProcess process, CancellationToken cancellatio
311311
ApplicationStateGuard.Ensure(_testHostProcessInformation is not null);
312312
ApplicationStateGuard.Ensure(_dumpType is not null);
313313

314+
string processId = process.Id.ToString(CultureInfo.InvariantCulture);
314315
var customReplacements = new Dictionary<string, string>(StringComparer.OrdinalIgnoreCase)
315316
{
316317
["pname"] = process.Name,
317-
["pid"] = process.Id.ToString(CultureInfo.InvariantCulture),
318-
["%p"] = process.Id.ToString(CultureInfo.InvariantCulture),
318+
["pid"] = processId,
319319
};
320320

321-
string finalDumpFileName = _artifactNamingService.ResolveTemplate(_dumpFileNamePattern ?? $"{process.Name}_%p_hang.dmp", customReplacements);
321+
string pattern = _dumpFileNamePattern ?? $"{process.Name}_%p_hang.dmp";
322+
323+
// First resolve <placeholder> templates, then handle legacy %p pattern for backward compatibility.
324+
string finalDumpFileName = _artifactNamingService.ResolveTemplate(pattern, customReplacements)
325+
.Replace("%p", processId);
322326
finalDumpFileName = Path.Combine(_configuration.GetTestResultDirectory(), finalDumpFileName);
323327

324328
ApplicationStateGuard.Ensure(_namedPipeClient is not null);

src/Platform/Microsoft.Testing.Platform/Services/ArtifactNamingService.cs

Lines changed: 24 additions & 94 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,10 @@ public ArtifactNamingService(
2828

2929
public string ResolveTemplate(string template, IDictionary<string, string>? customReplacements = null)
3030
{
31-
Guard.NotNullOrEmpty(template);
31+
if (RoslynString.IsNullOrEmpty(template))
32+
{
33+
throw new ArgumentException("Template cannot be null or empty.", nameof(template));
34+
}
3235

3336
Dictionary<string, string> defaultReplacements = GetDefaultReplacements();
3437
Dictionary<string, string> allReplacements = MergeReplacements(defaultReplacements, customReplacements);
@@ -67,14 +70,11 @@ private Dictionary<string, string> GetDefaultReplacements()
6770
}
6871

6972
// Time info (precision to 1 second)
70-
replacements["time"] = _clock.UtcNow.ToString("yyyy-MM-ddTHH:mm:ss", CultureInfo.InvariantCulture);
73+
replacements["time"] = _clock.UtcNow.ToString("yyyy-MM-ddTHH-mm-ss", CultureInfo.InvariantCulture);
7174

7275
// Random ID for uniqueness
7376
replacements["id"] = GenerateShortId();
7477

75-
// Root directory
76-
replacements["root"] = GetRootDirectory();
77-
7878
return replacements;
7979
}
8080

@@ -86,125 +86,55 @@ private static Dictionary<string, string> MergeReplacements(Dictionary<string, s
8686
}
8787

8888
var merged = new Dictionary<string, string>(defaultReplacements, StringComparer.OrdinalIgnoreCase);
89-
foreach ((string? key, string? value) in customReplacements)
89+
foreach (KeyValuePair<string, string> kvp in customReplacements)
9090
{
91-
merged[key] = value;
91+
merged[kvp.Key] = kvp.Value;
9292
}
9393

9494
return merged;
9595
}
9696

9797
private static string GetOperatingSystemName()
98-
{
99-
if (OperatingSystem.IsWindows())
100-
{
101-
return "windows";
102-
}
103-
104-
if (OperatingSystem.IsLinux())
105-
{
106-
return "linux";
107-
}
108-
109-
if (OperatingSystem.IsMacOS())
110-
{
111-
return "macos";
112-
}
113-
114-
// Fallback for unknown OS
115-
return "unknown";
116-
}
98+
=> RuntimeInformation.IsOSPlatform(OSPlatform.Windows) ? "windows"
99+
: RuntimeInformation.IsOSPlatform(OSPlatform.Linux) ? "linux"
100+
: RuntimeInformation.IsOSPlatform(OSPlatform.OSX) ? "macos"
101+
: "unknown";
117102

118103
private string GetTargetFrameworkMoniker()
119104
{
120-
// Extract TFM from current runtime
121105
string frameworkDescription = RuntimeInformation.FrameworkDescription;
122106

123-
if (frameworkDescription.Contains(".NET Core"))
107+
// .NET 5+ reports as ".NET X.Y.Z"
108+
if (frameworkDescription.StartsWith(".NET ", StringComparison.Ordinal) &&
109+
!frameworkDescription.StartsWith(".NET Framework", StringComparison.Ordinal) &&
110+
!frameworkDescription.StartsWith(".NET Core", StringComparison.Ordinal))
124111
{
125-
// Try to extract version from .NET Core description
126-
Match match = Regex.Match(frameworkDescription, @"\.NET Core (\d+\.\d+)");
112+
Match match = Regex.Match(frameworkDescription, @"\.NET (\d+)\.\d+");
127113
if (match.Success)
128114
{
129-
return $"netcoreapp{match.Groups[1].Value}";
115+
return $"net{match.Groups[1].Value}.0";
130116
}
131117
}
132-
else if (frameworkDescription.Contains(".NET "))
118+
else if (frameworkDescription.StartsWith(".NET Core", StringComparison.Ordinal))
133119
{
134-
// Try to extract version from .NET 5+ description
135-
Match match = Regex.Match(frameworkDescription, @"\.NET (\d+\.\d+)");
120+
Match match = Regex.Match(frameworkDescription, @"\.NET Core (\d+\.\d+)");
136121
if (match.Success)
137122
{
138-
string version = match.Groups[1].Value;
139-
return version switch
140-
{
141-
"5.0" => "net5.0",
142-
"6.0" => "net6.0",
143-
"7.0" => "net7.0",
144-
"8.0" => "net8.0",
145-
"9.0" => "net9.0",
146-
"10.0" => "net10.0",
147-
_ => $"net{version}",
148-
};
123+
return $"netcoreapp{match.Groups[1].Value}";
149124
}
150125
}
151-
else if (frameworkDescription.Contains(".NET Framework"))
126+
else if (frameworkDescription.StartsWith(".NET Framework", StringComparison.Ordinal))
152127
{
153-
// Try to extract version from .NET Framework description
154-
Match match = Regex.Match(frameworkDescription, @"\.NET Framework (\d+\.\d+)");
128+
Match match = Regex.Match(frameworkDescription, @"\.NET Framework (\d+)\.(\d+)");
155129
if (match.Success)
156130
{
157-
return $"net{match.Groups[1].Value.Replace(".", string.Empty)}";
131+
return $"net{match.Groups[1].Value}{match.Groups[2].Value}";
158132
}
159133
}
160134

161135
return _environment.Version.ToString();
162136
}
163137

164138
private static string GenerateShortId()
165-
=> Guid.NewGuid().ToString("N")[..8];
166-
167-
private string GetRootDirectory()
168-
{
169-
string currentDirectory = _testApplicationModuleInfo.GetCurrentTestApplicationDirectory();
170-
171-
// Try to find solution root, git root, or working directory
172-
string? rootDirectory = FindSolutionRoot(currentDirectory)
173-
?? FindGitRoot(currentDirectory)
174-
?? currentDirectory;
175-
176-
return rootDirectory;
177-
}
178-
179-
private static string? FindSolutionRoot(string startDirectory)
180-
{
181-
string? directory = startDirectory;
182-
while (directory is not null)
183-
{
184-
if (Directory.GetFiles(directory, "*.sln").Length > 0)
185-
{
186-
return directory;
187-
}
188-
189-
directory = Directory.GetParent(directory)?.FullName;
190-
}
191-
192-
return null;
193-
}
194-
195-
private static string? FindGitRoot(string startDirectory)
196-
{
197-
string? directory = startDirectory;
198-
while (directory is not null)
199-
{
200-
if (Directory.Exists(Path.Combine(directory, ".git")))
201-
{
202-
return directory;
203-
}
204-
205-
directory = Directory.GetParent(directory)?.FullName;
206-
}
207-
208-
return null;
209-
}
139+
=> Guid.NewGuid().ToString("N").Substring(0, 8);
210140
}

src/Platform/Microsoft.Testing.Platform/Services/IArtifactNamingService.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ namespace Microsoft.Testing.Platform.Services;
55

66
/// <summary>
77
/// Service for generating consistent artifact names and paths using template patterns.
8-
/// Supports placeholders like &lt;pname&gt;, &lt;pid&gt;, &lt;id&gt;, &lt;os&gt;, &lt;asm&gt;, &lt;tfm&gt;, &lt;time&gt;, &lt;root&gt;.
8+
/// Supports placeholders like &lt;pname&gt;, &lt;pid&gt;, &lt;id&gt;, &lt;os&gt;, &lt;asm&gt;, &lt;tfm&gt;, &lt;time&gt;.
99
/// </summary>
1010
internal interface IArtifactNamingService
1111
{

0 commit comments

Comments
 (0)