Skip to content

Commit a3bf778

Browse files
BertkBertk
committed
Improve RestoreOriginalModule and ignore running test assembly
- Refactor InstrumentationHelper backup/restore logic: - Skip and log if a module or symbol file is already in the backup list instead of throwing. - Check for backup existence before restore; log warnings or verbose messages as needed. - Add IsCurrentlyRunningAssembly to avoid restoring the running assembly. - Log the number of modules to restore and skip missing or running assemblies. - Update CoverletMTPCommandLineTests to match the current set of supported options. - Update HelpCommandTests to reflect new, clearer option descriptions.
1 parent 71d8ecf commit a3bf778

File tree

6 files changed

+136
-51
lines changed

6 files changed

+136
-51
lines changed

‎src/coverlet.MTP/CommandLine/CoverletCommandLineOptionDefinitions.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ public static IReadOnlyCollection<CommandLineOption> GetAllOptions()
3030
new CommandLineOption(CoverletOptionNames.SkipAutoProps, "Skip auto-implemented properties.", ArgumentArity.Zero, isHidden: false),
3131
new CommandLineOption(CoverletOptionNames.DoesNotReturnAttribute, "Attributes that mark methods as not returning.", ArgumentArity.ZeroOrMore, isHidden: false),
3232
new CommandLineOption(CoverletOptionNames.ExcludeAssembliesWithoutSources, "Exclude assemblies without source code.", ArgumentArity.ZeroOrOne, isHidden: false),
33-
new CommandLineOption(CoverletOptionNames.SourceMappingFile, "Output path for SourceRootsMappings file.", ArgumentArity.ZeroOrOne, isHidden: false)
33+
// new CommandLineOption(CoverletOptionNames.SourceMappingFile, "Output path for SourceRootsMappings file.", ArgumentArity.ZeroOrOne, isHidden: false)
3434
];
3535
}
3636
}

‎src/coverlet.MTP/CommandLine/CoverletOptionNames.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,5 +18,5 @@ internal static class CoverletOptionNames
1818
public const string SkipAutoProps = "coverage-skip-auto-props";
1919
public const string DoesNotReturnAttribute = "coverage-does-not-return-attribute";
2020
public const string ExcludeAssembliesWithoutSources = "coverage-exclude-assemblies-without-sources";
21-
public const string SourceMappingFile = "coverage-source-mapping-file";
21+
//public const string SourceMappingFile = "coverage-source-mapping-file";
2222
}

‎src/coverlet.MTP/Configuration/CoverageConfiguration.cs‎

Lines changed: 16 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -55,21 +55,21 @@ public string[] GetOutputFormats()
5555
return defaultFormats;
5656
}
5757

58-
public string GetOutputSourceMappingDirectory()
59-
{
60-
if (_commandLineOptions.TryGetOptionArgumentList(
61-
CoverletOptionNames.SourceMappingFile,
62-
out string[]? outputPath))
63-
{
64-
LogOptionValue(CoverletOptionNames.SourceMappingFile, outputPath, isExplicit: true);
65-
return outputPath[0];
66-
}
67-
// Default: TestResults folder next to test assembly
68-
string testDir = Path.GetDirectoryName(GetTestAssemblyPath()) ?? AppContext.BaseDirectory;
69-
string defaultPath = Path.Combine(testDir, "TestResults");
70-
LogOptionValue(CoverletOptionNames.Output, new[] { defaultPath }, isExplicit: false);
71-
return defaultPath;
72-
}
58+
//public string GetOutputSourceMappingDirectory()
59+
//{
60+
// if (_commandLineOptions.TryGetOptionArgumentList(
61+
// CoverletOptionNames.SourceMappingFile,
62+
// out string[]? outputPath))
63+
// {
64+
// LogOptionValue(CoverletOptionNames.SourceMappingFile, outputPath, isExplicit: true);
65+
// return outputPath[0];
66+
// }
67+
// // Default: TestResults folder next to test assembly
68+
// string testDir = Path.GetDirectoryName(GetTestAssemblyPath()) ?? AppContext.BaseDirectory;
69+
// string defaultPath = Path.Combine(testDir, "TestResults");
70+
// LogOptionValue(CoverletOptionNames.SourceMappingFile, new[] { defaultPath }, isExplicit: false);
71+
// return defaultPath;
72+
//}
7373

7474
public string GetOutputDirectory()
7575
{
@@ -242,7 +242,7 @@ public void LogConfigurationSummary()
242242
_logger.LogInformation($"Include Test Assembly: {IncludeTestAssembly}");
243243
_logger.LogInformation($"Skip Auto Properties: {SkipAutoProps}");
244244
_logger.LogInformation($"Exclude Assemblies Without Sources: {ExcludeAssembliesWithoutSources}");
245-
_logger.LogInformation($"Output directory for source mapping file: {GetOutputSourceMappingDirectory()}");
245+
//_logger.LogInformation($"Output directory for source mapping file: {GetOutputSourceMappingDirectory()}");
246246
_logger.LogInformation("========================================");
247247
}
248248

‎src/coverlet.core/Helpers/InstrumentationHelper.cs‎

Lines changed: 92 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -247,7 +247,8 @@ public void BackupOriginalModule(string module, string identifier, bool disableM
247247
_fileSystem.Copy(module, backupPath, true);
248248
if (!_backupList.TryAdd(module, backupPath))
249249
{
250-
throw new ArgumentException($"Key already added '{module}'");
250+
_logger.LogVerbose($"Module already in backup list, skipping: '{module}'");
251+
return;
251252
}
252253

253254
string symbolFile = Path.ChangeExtension(module, ".pdb");
@@ -256,7 +257,7 @@ public void BackupOriginalModule(string module, string identifier, bool disableM
256257
_fileSystem.Copy(symbolFile, backupSymbolPath, true);
257258
if (!_backupList.TryAdd(symbolFile, backupSymbolPath))
258259
{
259-
throw new ArgumentException($"Key already added '{module}'");
260+
_logger.LogVerbose($"Symbol file already in backup list, skipping: '{symbolFile}'");
260261
}
261262
}
262263
}
@@ -272,6 +273,20 @@ public virtual void RestoreOriginalModule(string module, string identifier)
272273
string backupPath = GetBackupPath(module, identifier);
273274
string backupSymbolPath = Path.ChangeExtension(backupPath, ".pdb");
274275

276+
// Guard: Check if module is in backup list and backup file exists
277+
if (!_backupList.ContainsKey(module))
278+
{
279+
_logger.LogVerbose($"Module not in backup list (already restored or never backed up): '{module}'");
280+
return;
281+
}
282+
283+
if (!_fileSystem.Exists(backupPath))
284+
{
285+
_logger.LogWarning($"Backup file not found, cannot restore module: '{backupPath}'");
286+
_backupList.TryRemove(module, out _);
287+
return;
288+
}
289+
275290
// Restore the original module - retry up to 10 times, since the destination file could be locked
276291
// See: https://github.com/tonerdo/coverlet/issues/25
277292
Func<TimeSpan> retryStrategy = CreateRetryStrategy();
@@ -280,35 +295,65 @@ public virtual void RestoreOriginalModule(string module, string identifier)
280295
{
281296
_fileSystem.Copy(backupPath, module, true);
282297
_fileSystem.Delete(backupPath);
283-
_backupList.TryRemove(module, out string _);
298+
_backupList.TryRemove(module, out _);
299+
_logger.LogVerbose($"Restored module from backup: '{module}'");
284300
}, retryStrategy, RetryAttempts);
285301

286302
_retryHelper.Retry(() =>
287303
{
304+
string symbolFile = Path.ChangeExtension(module, ".pdb");
288305
if (_fileSystem.Exists(backupSymbolPath))
289306
{
290-
string symbolFile = Path.ChangeExtension(module, ".pdb");
291307
_fileSystem.Copy(backupSymbolPath, symbolFile, true);
292308
_fileSystem.Delete(backupSymbolPath);
293-
_backupList.TryRemove(symbolFile, out string _);
309+
_backupList.TryRemove(symbolFile, out _);
310+
_logger.LogVerbose($"Restored symbol file from backup: '{symbolFile}'");
294311
}
295312
}, retryStrategy, RetryAttempts);
296313
}
297314

298315
public virtual void RestoreOriginalModules()
299316
{
317+
// Early exit if nothing to restore
318+
if (_backupList.IsEmpty)
319+
{
320+
return;
321+
}
322+
323+
_logger.LogVerbose($"RestoreOriginalModules: {_backupList.Count} modules to restore");
324+
300325
// Restore the original module - retry up to 10 times, since the destination file could be locked
301326
// See: https://github.com/tonerdo/coverlet/issues/25
302327
Func<TimeSpan> retryStrategy = CreateRetryStrategy();
303328

304329
foreach (string key in _backupList.Keys.ToList())
305330
{
306-
string backupPath = _backupList[key];
331+
// Skip the currently running assembly - it cannot be restored while running
332+
if (IsCurrentlyRunningAssembly(key))
333+
{
334+
_logger.LogVerbose($"Skipping restore of currently running assembly: '{key}'");
335+
_backupList.TryRemove(key, out _);
336+
continue;
337+
}
338+
339+
if (!_backupList.TryGetValue(key, out string backupPath))
340+
{
341+
continue;
342+
}
343+
344+
if (!_fileSystem.Exists(backupPath))
345+
{
346+
_logger.LogWarning($"Backup file not found during bulk restore: '{backupPath}'");
347+
_backupList.TryRemove(key, out _);
348+
continue;
349+
}
350+
307351
_retryHelper.Retry(() =>
308352
{
309353
_fileSystem.Copy(backupPath, key, true);
310354
_fileSystem.Delete(backupPath);
311-
_backupList.TryRemove(key, out string _);
355+
_backupList.TryRemove(key, out _);
356+
_logger.LogVerbose($"Restored from backup (ProcessExit): '{key}'");
312357
}, retryStrategy, RetryAttempts);
313358
}
314359
}
@@ -522,5 +567,45 @@ private static bool IsAssembly(string filePath)
522567
return false;
523568
}
524569
}
570+
571+
/// <summary>
572+
/// Determines whether the specified module path corresponds to the currently running assembly.
573+
/// </summary>
574+
/// <param name="modulePath">The file path of the module to check.</param>
575+
/// <returns>
576+
/// <c>true</c> if the specified module path matches the currently running assembly's location;
577+
/// otherwise, <c>false</c>.
578+
/// </returns>
579+
/// <remarks>
580+
/// This method first attempts to get the entry assembly's location. If that is not available
581+
/// (e.g., in certain hosting scenarios), it falls back to using the current process's main module filename.
582+
/// The comparison is case-insensitive and uses full paths to ensure accurate matching.
583+
/// Any exceptions during the check are caught and result in returning <c>false</c>.
584+
/// </remarks>
585+
private static bool IsCurrentlyRunningAssembly(string modulePath)
586+
{
587+
try
588+
{
589+
string currentAssembly = System.Reflection.Assembly.GetEntryAssembly()?.Location;
590+
if (string.IsNullOrEmpty(currentAssembly))
591+
{
592+
currentAssembly = System.Diagnostics.Process.GetCurrentProcess().MainModule?.FileName;
593+
}
594+
595+
if (string.IsNullOrEmpty(currentAssembly))
596+
{
597+
return false;
598+
}
599+
600+
return string.Equals(
601+
Path.GetFullPath(modulePath),
602+
Path.GetFullPath(currentAssembly),
603+
StringComparison.OrdinalIgnoreCase);
604+
}
605+
catch
606+
{
607+
return false;
608+
}
609+
}
525610
}
526611
}

‎test/coverlet.MTP.unit.tests/CoverletMTPCommandLineTests.cs‎

Lines changed: 14 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -92,20 +92,20 @@ public void GetCommandLineOptions_Returns_AllExpectedOptions()
9292

9393
var expectedOptions = new[]
9494
{
95-
CoverletOptionNames.Coverage,
96-
CoverletOptionNames.Formats,
97-
CoverletOptionNames.Exclude,
98-
CoverletOptionNames.Include,
99-
CoverletOptionNames.ExcludeByFile,
100-
CoverletOptionNames.IncludeDirectory,
101-
CoverletOptionNames.ExcludeByAttribute,
102-
CoverletOptionNames.IncludeTestAssembly,
103-
CoverletOptionNames.SingleHit,
104-
CoverletOptionNames.SkipAutoProps,
105-
CoverletOptionNames.DoesNotReturnAttribute,
106-
CoverletOptionNames.ExcludeAssembliesWithoutSources
107-
//CoverletOptionNames.SourceMappingFile
108-
};
95+
CoverletOptionNames.Coverage,
96+
CoverletOptionNames.Formats,
97+
CoverletOptionNames.Output,
98+
CoverletOptionNames.Include,
99+
CoverletOptionNames.IncludeDirectory,
100+
CoverletOptionNames.Exclude,
101+
CoverletOptionNames.ExcludeByFile,
102+
CoverletOptionNames.ExcludeByAttribute,
103+
CoverletOptionNames.IncludeTestAssembly,
104+
CoverletOptionNames.SingleHit,
105+
CoverletOptionNames.SkipAutoProps,
106+
CoverletOptionNames.DoesNotReturnAttribute,
107+
CoverletOptionNames.ExcludeAssembliesWithoutSources,
108+
};
109109

110110
Assert.Equal(expectedOptions.Length, options.Count);
111111
Assert.All(expectedOptions, name => Assert.Contains(options, o => o.Name == name));

‎test/coverlet.MTP.validation.tests/HelpCommandTests.cs‎

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -119,7 +119,7 @@ public async Task Help_ShowsCoverageOption()
119119

120120
// Assert - Check for --coverlet-coverage option that enables coverage collection
121121
Assert.Contains("--coverage", result.StandardOutput);
122-
Assert.Contains("Enable code coverage collection", result.StandardOutput);
122+
Assert.Contains("Enable code coverage data collection.", result.StandardOutput);
123123
}
124124

125125
[Fact]
@@ -155,7 +155,7 @@ public async Task Help_ShowsFormatsOption()
155155

156156
// Assert - Check for formats option from CoverletExtensionCommandLineProvider
157157
Assert.Contains("--coverage-output-format", result.StandardOutput);
158-
Assert.Contains("Specifies the output formats for the coverage report", result.StandardOutput);
158+
Assert.Contains("Output format(s) for coverage report (json, lcov, opencover, cobertura)", result.StandardOutput);
159159
}
160160

161161
[Fact]
@@ -173,7 +173,7 @@ public async Task Help_ShowsExcludeOption()
173173

174174
// Assert
175175
Assert.Contains("--coverage-exclude", result.StandardOutput);
176-
Assert.Contains("Filter expressions to exclude specific modules and types", result.StandardOutput);
176+
Assert.Contains("Exclude assemblies matching filters (e.g., [Assembly]Type)", result.StandardOutput);
177177
}
178178

179179
[Fact]
@@ -191,7 +191,7 @@ public async Task Help_ShowsIncludeOption()
191191

192192
// Assert
193193
Assert.Contains("--coverage-include", result.StandardOutput);
194-
Assert.Contains("Filter expressions to include only specific modules and type", result.StandardOutput);
194+
Assert.Contains("Include assemblies matching filters (e.g., [Assembly]Type)", result.StandardOutput);
195195
}
196196

197197
[Fact]
@@ -209,7 +209,7 @@ public async Task Help_ShowsExcludeByFileOption()
209209

210210
// Assert
211211
Assert.Contains("--coverage-exclude-by-file", result.StandardOutput);
212-
Assert.Contains("Glob patterns specifying source files to exclude", result.StandardOutput);
212+
Assert.Contains("Exclude source files matching glob patterns", result.StandardOutput);
213213
}
214214

215215
[Fact]
@@ -227,7 +227,7 @@ public async Task Help_ShowsIncludeDirectoryOption()
227227

228228
// Assert
229229
Assert.Contains("--coverage-include-directory", result.StandardOutput);
230-
Assert.Contains("Include directories containing additional assemblies", result.StandardOutput);
230+
Assert.Contains("Include additional directories for instrumentation", result.StandardOutput);
231231
}
232232

233233
[Fact]
@@ -245,7 +245,7 @@ public async Task Help_ShowsExcludeByAttributeOption()
245245

246246
// Assert
247247
Assert.Contains("--coverage-exclude-by-attribute", result.StandardOutput);
248-
Assert.Contains("Attributes to exclude from code coverage", result.StandardOutput);
248+
Assert.Contains("Exclude methods/classes decorated with attributes.", result.StandardOutput);
249249
}
250250

251251
[Fact]
@@ -263,7 +263,7 @@ public async Task Help_ShowsIncludeTestAssemblyOption()
263263

264264
// Assert
265265
Assert.Contains("--coverage-include-test-assembly", result.StandardOutput);
266-
Assert.Contains("Specifies whether to report code coverage of the test assembly", result.StandardOutput);
266+
Assert.Contains("Include test assembly in coverage", result.StandardOutput);
267267
}
268268

269269
[Fact]
@@ -281,7 +281,7 @@ public async Task Help_ShowsSingleHitOption()
281281

282282
// Assert
283283
Assert.Contains("--coverage-single-hit", result.StandardOutput);
284-
Assert.Contains("limit code coverage hit reporting to a single hit", result.StandardOutput);
284+
Assert.Contains("Limit the number of hits to one for each location", result.StandardOutput);
285285
}
286286

287287
[Fact]
@@ -299,7 +299,7 @@ public async Task Help_ShowsSkipAutoPropsOption()
299299

300300
// Assert
301301
Assert.Contains("--coverage-skip-auto-props", result.StandardOutput);
302-
Assert.Contains("Neither track nor record auto-implemented properties", result.StandardOutput);
302+
Assert.Contains("Skip auto-implemented properties", result.StandardOutput);
303303
}
304304

305305
[Fact]
@@ -317,7 +317,7 @@ public async Task Help_ShowsDoesNotReturnAttributeOption()
317317

318318
// Assert
319319
Assert.Contains("--coverage-does-not-return-attribute", result.StandardOutput);
320-
Assert.Contains("Attributes that mark methods that do not return", result.StandardOutput);
320+
Assert.Contains("Attributes that mark methods as not returning", result.StandardOutput);
321321
}
322322

323323
[Fact]
@@ -335,7 +335,7 @@ public async Task Help_ShowsExcludeAssembliesWithoutSourcesOption()
335335

336336
// Assert
337337
Assert.Contains("--coverage-exclude-assemblies-without-sources", result.StandardOutput);
338-
Assert.Contains("Specifies behavior of heuristic to ignore assemblies with missing source documents", result.StandardOutput);
338+
Assert.Contains("Exclude assemblies without source code", result.StandardOutput);
339339
}
340340

341341
//[Fact]

0 commit comments

Comments
 (0)