From cedc6723ed61a8ac2d2f852c2e2874efed144a81 Mon Sep 17 00:00:00 2001 From: Richard Murillo Date: Sat, 7 Mar 2026 14:53:43 -0800 Subject: [PATCH 01/33] fix: migrate PerfDiff to System.CommandLine 2.0.3 Replace removed IConsole, ITerminal, CommandHandler.Create, and System.CommandLine.Rendering APIs with System.Console and SetAction. The CancellationToken from SetAction replaces the manual CancellationTokenSource + Console.CancelKeyPress hook. Closes #914 Co-Authored-By: Claude Opus 4.6 --- Directory.Packages.props | 3 +- src/tools/PerfDiff/DiffCommand.cs | 47 +++++++++++-------- .../PerfDiff/Logging/SimpleConsoleLogger.cs | 38 ++++----------- .../SimpleConsoleLoggerFactoryExtensions.cs | 6 +-- .../Logging/SimpleConsoleLoggerProvider.cs | 8 +--- src/tools/PerfDiff/PerfDiff.csproj | 1 - src/tools/PerfDiff/Program.cs | 40 ++++++---------- 7 files changed, 56 insertions(+), 87 deletions(-) diff --git a/Directory.Packages.props b/Directory.Packages.props index b52d0e1f4..1cc22d598 100644 --- a/Directory.Packages.props +++ b/Directory.Packages.props @@ -33,8 +33,7 @@ - - + diff --git a/src/tools/PerfDiff/DiffCommand.cs b/src/tools/PerfDiff/DiffCommand.cs index b9939440a..54d47c912 100644 --- a/src/tools/PerfDiff/DiffCommand.cs +++ b/src/tools/PerfDiff/DiffCommand.cs @@ -10,25 +10,33 @@ namespace PerfDiff; internal static class DiffCommand { /// - /// Delegate for handling the diff command. + /// Gets the allowed verbosity levels for the command. /// - /// Baseline results folder. - /// Results folder. - /// Verbosity level. - /// Whether to fail on regression. - /// Console for output. - /// Exit code. - internal delegate Task Handler( - string baseline, - string results, - string? verbosity, - bool failOnRegression, - IConsole console); + private static readonly string[] VerbosityLevels = ["q", "quiet", "m", "minimal", "n", "normal", "d", "detailed", "diag", "diagnostic"]; /// - /// Gets the allowed verbosity levels for the command. + /// Gets the baseline option. /// - private static readonly string[] VerbosityLevels = ["q", "quiet", "m", "minimal", "n", "normal", "d", "detailed", "diag", "diagnostic"]; + internal static Option BaselineOption { get; } = + new Option("--baseline", () => null, "folder that contains the baseline performance run data").LegalFilePathsOnly(); + + /// + /// Gets the results option. + /// + internal static Option ResultsOption { get; } = + new Option("--results", () => null, "folder that contains the performance results").LegalFilePathsOnly(); + + /// + /// Gets the verbosity option. + /// + internal static Option VerbosityOption { get; } = + new Option(["--verbosity", "-v"], "Set the verbosity level. Allowed values are q[uiet], m[inimal], n[ormal], d[etailed], and diag[nostic]").FromAmong(VerbosityLevels); + + /// + /// Gets the fail-on-regression option. + /// + internal static Option FailOnRegressionOption { get; } = + new(["--failOnRegression"], "Should return non-zero exit code if regression detected"); /// /// Creates the root command with options for the diff command. @@ -36,13 +44,12 @@ internal delegate Task Handler( /// The configured . internal static RootCommand CreateCommandLineOptions() { - // Sync changes to option and argument names with the FormatCommand.Handler above. RootCommand rootCommand = new RootCommand { - new Option("--baseline", () => null, "folder that contains the baseline performance run data").LegalFilePathsOnly(), - new Option("--results", () => null, "folder that contains the performance results").LegalFilePathsOnly(), - new Option(["--verbosity", "-v"], "Set the verbosity level. Allowed values are q[uiet], m[inimal], n[ormal], d[etailed], and diag[nostic]").FromAmong(VerbosityLevels), - new Option(["--failOnRegression"], "Should return non-zero exit code if regression detected"), + BaselineOption, + ResultsOption, + VerbosityOption, + FailOnRegressionOption, }; rootCommand.Description = "diff two sets of performance results"; diff --git a/src/tools/PerfDiff/Logging/SimpleConsoleLogger.cs b/src/tools/PerfDiff/Logging/SimpleConsoleLogger.cs index 234a950f4..7e0dee093 100644 --- a/src/tools/PerfDiff/Logging/SimpleConsoleLogger.cs +++ b/src/tools/PerfDiff/Logging/SimpleConsoleLogger.cs @@ -1,8 +1,6 @@ // Copyright (c) Microsoft. All Rights Reserved. Licensed under the MIT license. See License.txt in the project root for license information. using System.Collections.Immutable; -using System.CommandLine; -using System.CommandLine.Rendering; using Microsoft.Extensions.Logging; namespace PerfDiff.Logging; @@ -14,8 +12,6 @@ internal sealed class SimpleConsoleLogger : ILogger { private readonly Lock _gate = new(); - private readonly IConsole _console; - private readonly ITerminal _terminal; private readonly LogLevel _minimalLogLevel; private readonly LogLevel _minimalErrorLevel; @@ -33,13 +29,10 @@ internal sealed class SimpleConsoleLogger : ILogger /// /// Initializes a new instance of the class. /// - /// The console to write output to. /// The minimal log level for output. /// The minimal log level for error output. - public SimpleConsoleLogger(IConsole console, LogLevel minimalLogLevel, LogLevel minimalErrorLevel) + public SimpleConsoleLogger(LogLevel minimalLogLevel, LogLevel minimalErrorLevel) { - _terminal = console.GetTerminal(); - _console = console; _minimalLogLevel = minimalLogLevel; _minimalErrorLevel = minimalErrorLevel; } @@ -56,14 +49,11 @@ public void Log(LogLevel logLevel, EventId eventId, TState state, Except { string message = formatter(state, exception); bool logToErrorStream = logLevel >= _minimalErrorLevel; - if (_terminal is null) - { - LogToConsole(_console, message, logToErrorStream); - } - else - { - LogToTerminal(message, logLevel, logToErrorStream); - } + ConsoleColor messageColor = LogLevelColorMap[logLevel]; + + Console.ForegroundColor = messageColor; + LogToConsole(message, logToErrorStream); + Console.ResetColor(); } } @@ -80,25 +70,15 @@ public IDisposable BeginScope(TState state) return NullScope.Instance; } - private void LogToTerminal(string message, LogLevel logLevel, bool logToErrorStream) - { - ConsoleColor messageColor = LogLevelColorMap[logLevel]; - _terminal.ForegroundColor = messageColor; - - LogToConsole(_terminal, message, logToErrorStream); - - _terminal.ResetColor(); - } - - private static void LogToConsole(IConsole console, string message, bool logToErrorStream) + private static void LogToConsole(string message, bool logToErrorStream) { if (logToErrorStream) { - console.Error.Write($"{message}{Environment.NewLine}"); + Console.Error.Write($"{message}{Environment.NewLine}"); } else { - console.Out.Write($" {message}{Environment.NewLine}"); + Console.Out.Write($" {message}{Environment.NewLine}"); } } } diff --git a/src/tools/PerfDiff/Logging/SimpleConsoleLoggerFactoryExtensions.cs b/src/tools/PerfDiff/Logging/SimpleConsoleLoggerFactoryExtensions.cs index 7cfcbe746..a450c2690 100644 --- a/src/tools/PerfDiff/Logging/SimpleConsoleLoggerFactoryExtensions.cs +++ b/src/tools/PerfDiff/Logging/SimpleConsoleLoggerFactoryExtensions.cs @@ -1,16 +1,14 @@ // Copyright (c) Microsoft. All Rights Reserved. Licensed under the MIT license. See License.txt in the project root for license information. -using System.CommandLine; - using Microsoft.Extensions.Logging; namespace PerfDiff.Logging; internal static class SimpleConsoleLoggerFactoryExtensions { - public static ILoggerFactory AddSimpleConsole(this ILoggerFactory factory, IConsole console, LogLevel minimalLogLevel, LogLevel minimalErrorLevel) + public static ILoggerFactory AddSimpleConsole(this ILoggerFactory factory, LogLevel minimalLogLevel, LogLevel minimalErrorLevel) { - factory.AddProvider(new SimpleConsoleLoggerProvider(console, minimalLogLevel, minimalErrorLevel)); + factory.AddProvider(new SimpleConsoleLoggerProvider(minimalLogLevel, minimalErrorLevel)); return factory; } } diff --git a/src/tools/PerfDiff/Logging/SimpleConsoleLoggerProvider.cs b/src/tools/PerfDiff/Logging/SimpleConsoleLoggerProvider.cs index 6f404b0dc..f3f3951cd 100644 --- a/src/tools/PerfDiff/Logging/SimpleConsoleLoggerProvider.cs +++ b/src/tools/PerfDiff/Logging/SimpleConsoleLoggerProvider.cs @@ -1,6 +1,5 @@ // Copyright (c) Microsoft. All Rights Reserved. Licensed under the MIT license. See License.txt in the project root for license information. -using System.CommandLine; using Microsoft.Extensions.Logging; namespace PerfDiff.Logging; @@ -10,19 +9,16 @@ namespace PerfDiff.Logging; /// internal sealed class SimpleConsoleLoggerProvider : ILoggerProvider { - private readonly IConsole _console; private readonly LogLevel _minimalLogLevel; private readonly LogLevel _minimalErrorLevel; /// /// Initializes a new instance of the class. /// - /// The console to write output to. /// The minimal log level for output. /// The minimal log level for error output. - public SimpleConsoleLoggerProvider(IConsole console, LogLevel minimalLogLevel, LogLevel minimalErrorLevel) + public SimpleConsoleLoggerProvider(LogLevel minimalLogLevel, LogLevel minimalErrorLevel) { - _console = console; _minimalLogLevel = minimalLogLevel; _minimalErrorLevel = minimalErrorLevel; } @@ -30,7 +26,7 @@ public SimpleConsoleLoggerProvider(IConsole console, LogLevel minimalLogLevel, L /// public ILogger CreateLogger(string categoryName) { - return new SimpleConsoleLogger(_console, _minimalLogLevel, _minimalErrorLevel); + return new SimpleConsoleLogger(_minimalLogLevel, _minimalErrorLevel); } /// diff --git a/src/tools/PerfDiff/PerfDiff.csproj b/src/tools/PerfDiff/PerfDiff.csproj index 9bce4f955..26e81dd11 100644 --- a/src/tools/PerfDiff/PerfDiff.csproj +++ b/src/tools/PerfDiff/PerfDiff.csproj @@ -10,7 +10,6 @@ - diff --git a/src/tools/PerfDiff/Program.cs b/src/tools/PerfDiff/Program.cs index e045fb3a2..0db27fd0e 100644 --- a/src/tools/PerfDiff/Program.cs +++ b/src/tools/PerfDiff/Program.cs @@ -2,8 +2,6 @@ using System; using System.CommandLine; -using System.CommandLine.Invocation; -using System.CommandLine.Parsing; using System.IO; using System.Threading; using System.Threading.Tasks; @@ -16,17 +14,22 @@ namespace PerfDiff; internal sealed class Program { internal const int UnhandledExceptionExitCode = 1; - private static ParseResult? s_parseResult; private static async Task Main(string[] args) { RootCommand rootCommand = DiffCommand.CreateCommandLineOptions(); - rootCommand.Handler = CommandHandler.Create(new DiffCommand.Handler(RunAsync)); + rootCommand.SetAction(async (parseResult, cancellationToken) => + { + string baseline = parseResult.GetValue(DiffCommand.BaselineOption) ?? string.Empty; + string results = parseResult.GetValue(DiffCommand.ResultsOption) ?? string.Empty; + string? verbosity = parseResult.GetValue(DiffCommand.VerbosityOption); + bool failOnRegression = parseResult.GetValue(DiffCommand.FailOnRegressionOption); - // Parse the incoming args so we can give warnings when deprecated options are used. - s_parseResult = rootCommand.Parse(args); + int exitCode = await RunAsync(baseline, results, verbosity, failOnRegression, cancellationToken).ConfigureAwait(false); + parseResult.Action!.ExitCode = exitCode; + }); - return await rootCommand.InvokeAsync(args).ConfigureAwait(false); + return await rootCommand.Parse(args).InvokeAsync().ConfigureAwait(false); } public static async Task RunAsync( @@ -34,28 +37,15 @@ public static async Task RunAsync( string results, string? verbosity, bool failOnRegression, - IConsole console) + CancellationToken cancellationToken) { - if (s_parseResult == null) - { - return 1; - } - // Setup logging. LogLevel logLevel = GetLogLevel(verbosity); - ILogger logger = SetupLogging(console, minimalLogLevel: logLevel, minimalErrorLevel: LogLevel.Warning); - - // Hook so we can cancel and exit when ctrl+c is pressed. - CancellationTokenSource cancellationTokenSource = new CancellationTokenSource(); - Console.CancelKeyPress += (sender, e) => - { - e.Cancel = true; - cancellationTokenSource.Cancel(); - }; + ILogger logger = SetupLogging(minimalLogLevel: logLevel, minimalErrorLevel: LogLevel.Warning); try { - int exitCode = await PerfDiff.CompareAsync(baseline, results, failOnRegression, logger, cancellationTokenSource.Token).ConfigureAwait(false); + int exitCode = await PerfDiff.CompareAsync(baseline, results, failOnRegression, logger, cancellationToken).ConfigureAwait(false); return exitCode; } catch (FileNotFoundException fex) @@ -72,10 +62,10 @@ public static async Task RunAsync( return UnhandledExceptionExitCode; } - static ILogger SetupLogging(IConsole console, LogLevel minimalLogLevel, LogLevel minimalErrorLevel) + static ILogger SetupLogging(LogLevel minimalLogLevel, LogLevel minimalErrorLevel) { ServiceCollection serviceCollection = new ServiceCollection(); - serviceCollection.AddSingleton(new LoggerFactory().AddSimpleConsole(console, minimalLogLevel, minimalErrorLevel)); + serviceCollection.AddSingleton(new LoggerFactory().AddSimpleConsole(minimalLogLevel, minimalErrorLevel)); serviceCollection.AddLogging(); ServiceProvider serviceProvider = serviceCollection.BuildServiceProvider(); From bbb8e626df0464993a51c0ee45b3f111a9528f33 Mon Sep 17 00:00:00 2001 From: Richard Murillo Date: Sat, 7 Mar 2026 15:59:35 -0800 Subject: [PATCH 02/33] fix(PerfDiff): restore console color on exception and avoid repeated allocation Wrap LogToConsole call in try/finally to ensure Console.ResetColor() runs even if an exception occurs. Change LogLevelColorMap from expression-bodied property (=>) to static readonly field (=) so the dictionary is allocated once, not on every access. Co-Authored-By: Claude Opus 4.6 --- src/tools/PerfDiff/Logging/SimpleConsoleLogger.cs | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/src/tools/PerfDiff/Logging/SimpleConsoleLogger.cs b/src/tools/PerfDiff/Logging/SimpleConsoleLogger.cs index 7e0dee093..864ed716b 100644 --- a/src/tools/PerfDiff/Logging/SimpleConsoleLogger.cs +++ b/src/tools/PerfDiff/Logging/SimpleConsoleLogger.cs @@ -15,7 +15,7 @@ internal sealed class SimpleConsoleLogger : ILogger private readonly LogLevel _minimalLogLevel; private readonly LogLevel _minimalErrorLevel; - private static ImmutableDictionary LogLevelColorMap => new Dictionary + private static readonly ImmutableDictionary LogLevelColorMap = new Dictionary { [LogLevel.Critical] = ConsoleColor.Red, [LogLevel.Error] = ConsoleColor.Red, @@ -52,8 +52,14 @@ public void Log(LogLevel logLevel, EventId eventId, TState state, Except ConsoleColor messageColor = LogLevelColorMap[logLevel]; Console.ForegroundColor = messageColor; - LogToConsole(message, logToErrorStream); - Console.ResetColor(); + try + { + LogToConsole(message, logToErrorStream); + } + finally + { + Console.ResetColor(); + } } } From eaac9bc8aeb059143dd90a85417b1121309b880d Mon Sep 17 00:00:00 2001 From: Richard Murillo Date: Sat, 7 Mar 2026 15:59:39 -0800 Subject: [PATCH 03/33] fix(PerfDiff): dispose ServiceProvider after use Return ServiceProvider from SetupLogging so the caller can dispose it in the existing finally block. Prevents resource leak. Co-Authored-By: Claude Opus 4.6 --- src/tools/PerfDiff/Program.cs | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/src/tools/PerfDiff/Program.cs b/src/tools/PerfDiff/Program.cs index 0db27fd0e..c98ef6fd6 100644 --- a/src/tools/PerfDiff/Program.cs +++ b/src/tools/PerfDiff/Program.cs @@ -41,7 +41,7 @@ public static async Task RunAsync( { // Setup logging. LogLevel logLevel = GetLogLevel(verbosity); - ILogger logger = SetupLogging(minimalLogLevel: logLevel, minimalErrorLevel: LogLevel.Warning); + (ServiceProvider serviceProvider, ILogger logger) = SetupLogging(minimalLogLevel: logLevel, minimalErrorLevel: LogLevel.Warning); try { @@ -61,8 +61,12 @@ public static async Task RunAsync( { return UnhandledExceptionExitCode; } + finally + { + serviceProvider.Dispose(); + } - static ILogger SetupLogging(LogLevel minimalLogLevel, LogLevel minimalErrorLevel) + static (ServiceProvider ServiceProvider, ILogger Logger) SetupLogging(LogLevel minimalLogLevel, LogLevel minimalErrorLevel) { ServiceCollection serviceCollection = new ServiceCollection(); serviceCollection.AddSingleton(new LoggerFactory().AddSimpleConsole(minimalLogLevel, minimalErrorLevel)); @@ -71,7 +75,7 @@ static ILogger SetupLogging(LogLevel minimalLogLevel, LogLevel minimalE ServiceProvider serviceProvider = serviceCollection.BuildServiceProvider(); ILogger? logger = serviceProvider.GetService>(); - return logger!; + return (serviceProvider, logger!); } static LogLevel GetLogLevel(string? verbosity) From ebed69944c22e6c0304bf469a24f76c928dacb8c Mon Sep 17 00:00:00 2001 From: Richard Murillo Date: Sat, 7 Mar 2026 16:17:33 -0800 Subject: [PATCH 04/33] refactor(PerfDiff): simplify ServiceProvider disposal and remove dead code Use await using with ConfigureAwait(false) for async disposal instead of manual Dispose() in a finally block. Remove dead currentDirectory code that was never assigned. Register LoggerFactory as ILoggerFactory singleton so the container manages its lifetime. Use GetRequiredService for fail-fast behavior. Co-Authored-By: Claude Opus 4.6 --- src/tools/PerfDiff/Program.cs | 23 ++++++++++------------- 1 file changed, 10 insertions(+), 13 deletions(-) diff --git a/src/tools/PerfDiff/Program.cs b/src/tools/PerfDiff/Program.cs index c98ef6fd6..3db5b024c 100644 --- a/src/tools/PerfDiff/Program.cs +++ b/src/tools/PerfDiff/Program.cs @@ -3,6 +3,7 @@ using System; using System.CommandLine; using System.IO; +using System.Runtime.CompilerServices; using System.Threading; using System.Threading.Tasks; using Microsoft.Extensions.DependencyInjection; @@ -41,12 +42,13 @@ public static async Task RunAsync( { // Setup logging. LogLevel logLevel = GetLogLevel(verbosity); - (ServiceProvider serviceProvider, ILogger logger) = SetupLogging(minimalLogLevel: logLevel, minimalErrorLevel: LogLevel.Warning); + ServiceProvider serviceProvider = SetupLogging(minimalLogLevel: logLevel, minimalErrorLevel: LogLevel.Warning); + await using ConfiguredAsyncDisposable asyncDisposal = serviceProvider.ConfigureAwait(false); + ILogger logger = serviceProvider.GetRequiredService>(); try { - int exitCode = await PerfDiff.CompareAsync(baseline, results, failOnRegression, logger, cancellationToken).ConfigureAwait(false); - return exitCode; + return await PerfDiff.CompareAsync(baseline, results, failOnRegression, logger, cancellationToken).ConfigureAwait(false); } catch (FileNotFoundException fex) { @@ -61,21 +63,16 @@ public static async Task RunAsync( { return UnhandledExceptionExitCode; } - finally - { - serviceProvider.Dispose(); - } - static (ServiceProvider ServiceProvider, ILogger Logger) SetupLogging(LogLevel minimalLogLevel, LogLevel minimalErrorLevel) + static ServiceProvider SetupLogging(LogLevel minimalLogLevel, LogLevel minimalErrorLevel) { ServiceCollection serviceCollection = new ServiceCollection(); - serviceCollection.AddSingleton(new LoggerFactory().AddSimpleConsole(minimalLogLevel, minimalErrorLevel)); + LoggerFactory loggerFactory = new LoggerFactory(); + loggerFactory.AddSimpleConsole(minimalLogLevel, minimalErrorLevel); + serviceCollection.AddSingleton(loggerFactory); serviceCollection.AddLogging(); - ServiceProvider serviceProvider = serviceCollection.BuildServiceProvider(); - ILogger? logger = serviceProvider.GetService>(); - - return (serviceProvider, logger!); + return serviceCollection.BuildServiceProvider(); } static LogLevel GetLogLevel(string? verbosity) From 4ab30c392496906a12e7f14f4f046fe541ec5feb Mon Sep 17 00:00:00 2001 From: Richard Murillo Date: Sat, 7 Mar 2026 16:43:20 -0800 Subject: [PATCH 05/33] fix(deps): update Perfolizer version to 0.6.7 The PackageVersion was pinned to 0.6.1 but the PR migrates to 0.6.7. Aligns the central package version with the actual dependency used. Co-Authored-By: Claude Opus 4.6 --- Directory.Packages.props | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Directory.Packages.props b/Directory.Packages.props index 1cc22d598..74832de99 100644 --- a/Directory.Packages.props +++ b/Directory.Packages.props @@ -35,7 +35,7 @@ - + From 1d5a79b718cae76acf90122a596441dea6429616 Mon Sep 17 00:00:00 2001 From: Richard Murillo Date: Sat, 7 Mar 2026 16:43:37 -0800 Subject: [PATCH 06/33] fix(perfdiff): make --baseline and --results options required Set IsRequired = true on both options so the CLI parser rejects missing values. Change types from string? to string and remove the null-to-empty fallback in Program.cs. Co-Authored-By: Claude Opus 4.6 --- src/tools/PerfDiff/DiffCommand.cs | 8 ++++---- src/tools/PerfDiff/Program.cs | 4 ++-- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/tools/PerfDiff/DiffCommand.cs b/src/tools/PerfDiff/DiffCommand.cs index 54d47c912..fa9a3754d 100644 --- a/src/tools/PerfDiff/DiffCommand.cs +++ b/src/tools/PerfDiff/DiffCommand.cs @@ -17,14 +17,14 @@ internal static class DiffCommand /// /// Gets the baseline option. /// - internal static Option BaselineOption { get; } = - new Option("--baseline", () => null, "folder that contains the baseline performance run data").LegalFilePathsOnly(); + internal static Option BaselineOption { get; } = + new Option("--baseline", "folder that contains the baseline performance run data") { IsRequired = true }.LegalFilePathsOnly(); /// /// Gets the results option. /// - internal static Option ResultsOption { get; } = - new Option("--results", () => null, "folder that contains the performance results").LegalFilePathsOnly(); + internal static Option ResultsOption { get; } = + new Option("--results", "folder that contains the performance results") { IsRequired = true }.LegalFilePathsOnly(); /// /// Gets the verbosity option. diff --git a/src/tools/PerfDiff/Program.cs b/src/tools/PerfDiff/Program.cs index 3db5b024c..f73c91a36 100644 --- a/src/tools/PerfDiff/Program.cs +++ b/src/tools/PerfDiff/Program.cs @@ -21,8 +21,8 @@ private static async Task Main(string[] args) RootCommand rootCommand = DiffCommand.CreateCommandLineOptions(); rootCommand.SetAction(async (parseResult, cancellationToken) => { - string baseline = parseResult.GetValue(DiffCommand.BaselineOption) ?? string.Empty; - string results = parseResult.GetValue(DiffCommand.ResultsOption) ?? string.Empty; + string baseline = parseResult.GetValue(DiffCommand.BaselineOption)!; + string results = parseResult.GetValue(DiffCommand.ResultsOption)!; string? verbosity = parseResult.GetValue(DiffCommand.VerbosityOption); bool failOnRegression = parseResult.GetValue(DiffCommand.FailOnRegressionOption); From 26c2062926c820fdc8e868e39bff867ae700af89 Mon Sep 17 00:00:00 2001 From: Richard Murillo Date: Sat, 7 Mar 2026 16:44:58 -0800 Subject: [PATCH 07/33] fix(perfdiff): wire failOnRegression flag through CompareAsync properly CheckEltTraces previously conflated "comparison succeeded" with "should fail based on regression". Separated concerns: CheckEltTraces now returns pure comparison results without policy logic. CompareAsync uses failOnRegression to decide the exit code. Also fixed typo "is was" to "it was" in log message. Co-Authored-By: Claude Opus 4.6 --- src/tools/PerfDiff/PerfDiff.cs | 29 +++++++++++------------------ 1 file changed, 11 insertions(+), 18 deletions(-) diff --git a/src/tools/PerfDiff/PerfDiff.cs b/src/tools/PerfDiff/PerfDiff.cs index 6a3bc8d06..eaf863800 100644 --- a/src/tools/PerfDiff/PerfDiff.cs +++ b/src/tools/PerfDiff/PerfDiff.cs @@ -33,56 +33,49 @@ public static async Task CompareAsync( return 0; } - (bool etlCompareSucceeded, bool etlRegressionDetected) = CheckEltTraces(baselineFolder, resultsFolder, failOnRegression); + (bool etlCompareSucceeded, bool etlRegressionDetected) = CheckEltTraces(baselineFolder, resultsFolder); if (!etlCompareSucceeded) { #pragma warning disable CA1848 // For improved performance, use the LoggerMessage delegates instead of calling 'LoggerExtensions.LogTrace(ILogger, string?, params object?[])' logger.LogTrace("We detected a regression in BenchmarkDotNet and there is no ETL info."); #pragma warning restore CA1848 // For improved performance, use the LoggerMessage delegates instead of calling 'LoggerExtensions.LogTrace(ILogger, string?, params object?[])' - return 1; + return failOnRegression ? 1 : 0; } if (etlRegressionDetected) { #pragma warning disable CA1848 // For improved performance, use the LoggerMessage delegates instead of calling 'LoggerExtensions.LogTrace(ILogger, string?, params object?[])' - logger.LogTrace(" We detected a regression in BenchmarkDotNet and there _is_ ETL info which agrees there was a regression."); + logger.LogTrace("We detected a regression in BenchmarkDotNet and there _is_ ETL info which agrees there was a regression."); #pragma warning restore CA1848 // For improved performance, use the LoggerMessage delegates instead of calling 'LoggerExtensions.LogTrace(ILogger, string?, params object?[])' - return 1; + return failOnRegression ? 1 : 0; } #pragma warning disable CA1848 // For improved performance, use the LoggerMessage delegates instead of calling 'LoggerExtensions.LogTrace(ILogger, string?, params object?[])' - logger.LogTrace("We detected a regression in BenchmarkDotNet but examining the ETL trace determined that is was noise."); + logger.LogTrace("We detected a regression in BenchmarkDotNet but examining the ETL trace determined that it was noise."); #pragma warning restore CA1848 // For improved performance, use the LoggerMessage delegates instead of calling 'LoggerExtensions.LogTrace(ILogger, string?, params object?[])' return 0; } - private static (bool compareSucceeded, bool regressionDetected) CheckEltTraces(string baselineFolder, string resultsFolder, bool failOnRegression) + private static (bool compareSucceeded, bool regressionDetected) CheckEltTraces(string baselineFolder, string resultsFolder) { - bool regressionDetected = false; - // try look for ETL traces if (!TryGetETLPaths(baselineFolder, out string? baselineEtlPath)) { - return (false, regressionDetected); + return (false, false); } if (!TryGetETLPaths(resultsFolder, out string? resultsEtlPath)) { - return (false, regressionDetected); + return (false, false); } // Compare ETL - if (!EtlDiffer.TryCompareETL(resultsEtlPath, baselineEtlPath, out regressionDetected)) - { - return (false, regressionDetected); - } - - if (regressionDetected && failOnRegression) + if (!EtlDiffer.TryCompareETL(resultsEtlPath, baselineEtlPath, out bool regressionDetected)) { - return (true, regressionDetected); + return (false, false); } - return (false, regressionDetected); + return (true, regressionDetected); } private const string ETLFileExtension = "etl.zip"; From 4893082dad6b850d95c665cd2dbeb2eceedeae1a Mon Sep 17 00:00:00 2001 From: Richard Murillo Date: Sat, 7 Mar 2026 17:06:55 -0800 Subject: [PATCH 08/33] fix(PerfDiff): return exit code from SetAction lambda The System.CommandLine 2.0.3 SetAction async overload expects Func>. Return the exit code directly instead of assigning to parseResult.Action.ExitCode. Co-Authored-By: Claude Opus 4.6 --- src/tools/PerfDiff/Program.cs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/tools/PerfDiff/Program.cs b/src/tools/PerfDiff/Program.cs index f73c91a36..aa9d231ef 100644 --- a/src/tools/PerfDiff/Program.cs +++ b/src/tools/PerfDiff/Program.cs @@ -26,8 +26,7 @@ private static async Task Main(string[] args) string? verbosity = parseResult.GetValue(DiffCommand.VerbosityOption); bool failOnRegression = parseResult.GetValue(DiffCommand.FailOnRegressionOption); - int exitCode = await RunAsync(baseline, results, verbosity, failOnRegression, cancellationToken).ConfigureAwait(false); - parseResult.Action!.ExitCode = exitCode; + return await RunAsync(baseline, results, verbosity, failOnRegression, cancellationToken).ConfigureAwait(false); }); return await rootCommand.Parse(args).InvokeAsync().ConfigureAwait(false); From 1434e6bb327cd94ecad42821608375ea319f540b Mon Sep 17 00:00:00 2001 From: Richard Murillo Date: Sun, 8 Mar 2026 11:07:45 -0700 Subject: [PATCH 09/33] refactor: simplify code in PR #1043 - Pass description to RootCommand constructor instead of setting it after - Remove redundant null assignment in TryGetETLPaths - Remove unused `using System` directive from Program.cs Co-Authored-By: Claude Opus 4.6 --- src/tools/PerfDiff/DiffCommand.cs | 4 +--- src/tools/PerfDiff/PerfDiff.cs | 8 +------- src/tools/PerfDiff/Program.cs | 1 - 3 files changed, 2 insertions(+), 11 deletions(-) diff --git a/src/tools/PerfDiff/DiffCommand.cs b/src/tools/PerfDiff/DiffCommand.cs index fa9a3754d..6776411c0 100644 --- a/src/tools/PerfDiff/DiffCommand.cs +++ b/src/tools/PerfDiff/DiffCommand.cs @@ -44,7 +44,7 @@ internal static class DiffCommand /// The configured . internal static RootCommand CreateCommandLineOptions() { - RootCommand rootCommand = new RootCommand + RootCommand rootCommand = new RootCommand("diff two sets of performance results") { BaselineOption, ResultsOption, @@ -52,8 +52,6 @@ internal static RootCommand CreateCommandLineOptions() FailOnRegressionOption, }; - rootCommand.Description = "diff two sets of performance results"; - return rootCommand; } } diff --git a/src/tools/PerfDiff/PerfDiff.cs b/src/tools/PerfDiff/PerfDiff.cs index eaf863800..8e6f3484c 100644 --- a/src/tools/PerfDiff/PerfDiff.cs +++ b/src/tools/PerfDiff/PerfDiff.cs @@ -86,13 +86,7 @@ private static bool TryGetETLPaths(string path, [NotNullWhen(true)] out string? { string[] files = Directory.GetFiles(path, $"*{ETLFileExtension}", SearchOption.AllDirectories); etlPath = files.SingleOrDefault(); - if (etlPath is null) - { - etlPath = null; - return false; - } - - return true; + return etlPath is not null; } else if (File.Exists(path) && path.EndsWith(ETLFileExtension, StringComparison.OrdinalIgnoreCase)) { diff --git a/src/tools/PerfDiff/Program.cs b/src/tools/PerfDiff/Program.cs index aa9d231ef..898386268 100644 --- a/src/tools/PerfDiff/Program.cs +++ b/src/tools/PerfDiff/Program.cs @@ -1,6 +1,5 @@ // Copyright (c) Microsoft. All Rights Reserved. Licensed under the MIT license. See License.txt in the project root for license information. -using System; using System.CommandLine; using System.IO; using System.Runtime.CompilerServices; From 53ead047a3cf002751d8a595566966122c091f52 Mon Sep 17 00:00:00 2001 From: Richard Murillo Date: Sun, 8 Mar 2026 11:10:15 -0700 Subject: [PATCH 10/33] refactor(PerfDiff): fix System.CommandLine 2.0.3 API and reduce noise Fix build errors from breaking API changes in System.CommandLine 2.0.3: IsRequired -> Required, LegalFilePathsOnly -> AcceptLegalFilePathsOnly, FromAmong -> AcceptOnlyFromAmong, constructor signature changes. Consolidate repeated CA1848/CA2254 pragma suppressions to method scope. Remove redundant null assignment in TryGetETLPaths. Remove unused using. Co-Authored-By: Claude Opus 4.6 --- src/tools/PerfDiff/DiffCommand.cs | 36 ++++++++++++++++++++++++------- src/tools/PerfDiff/PerfDiff.cs | 15 ++++--------- src/tools/PerfDiff/Program.cs | 6 ++---- 3 files changed, 34 insertions(+), 23 deletions(-) diff --git a/src/tools/PerfDiff/DiffCommand.cs b/src/tools/PerfDiff/DiffCommand.cs index 6776411c0..c225163d5 100644 --- a/src/tools/PerfDiff/DiffCommand.cs +++ b/src/tools/PerfDiff/DiffCommand.cs @@ -17,26 +17,46 @@ internal static class DiffCommand /// /// Gets the baseline option. /// - internal static Option BaselineOption { get; } = - new Option("--baseline", "folder that contains the baseline performance run data") { IsRequired = true }.LegalFilePathsOnly(); + internal static Option BaselineOption { get; } = CreateBaselineOption(); /// /// Gets the results option. /// - internal static Option ResultsOption { get; } = - new Option("--results", "folder that contains the performance results") { IsRequired = true }.LegalFilePathsOnly(); + internal static Option ResultsOption { get; } = CreateResultsOption(); /// /// Gets the verbosity option. /// - internal static Option VerbosityOption { get; } = - new Option(["--verbosity", "-v"], "Set the verbosity level. Allowed values are q[uiet], m[inimal], n[ormal], d[etailed], and diag[nostic]").FromAmong(VerbosityLevels); + internal static Option VerbosityOption { get; } = CreateVerbosityOption(); /// /// Gets the fail-on-regression option. /// - internal static Option FailOnRegressionOption { get; } = - new(["--failOnRegression"], "Should return non-zero exit code if regression detected"); + internal static Option FailOnRegressionOption { get; } = new("--failOnRegression") + { + Description = "Should return non-zero exit code if regression detected", + }; + + private static Option CreateBaselineOption() + { + Option option = new("--baseline") { Description = "folder that contains the baseline performance run data", Required = true }; + option.AcceptLegalFilePathsOnly(); + return option; + } + + private static Option CreateResultsOption() + { + Option option = new("--results") { Description = "folder that contains the performance results", Required = true }; + option.AcceptLegalFilePathsOnly(); + return option; + } + + private static Option CreateVerbosityOption() + { + Option option = new("--verbosity", "-v") { Description = "Set the verbosity level. Allowed values are q[uiet], m[inimal], n[ormal], d[etailed], and diag[nostic]" }; + option.AcceptOnlyFromAmong(VerbosityLevels); + return option; + } /// /// Creates the root command with options for the diff command. diff --git a/src/tools/PerfDiff/PerfDiff.cs b/src/tools/PerfDiff/PerfDiff.cs index 8e6f3484c..0dea04a55 100644 --- a/src/tools/PerfDiff/PerfDiff.cs +++ b/src/tools/PerfDiff/PerfDiff.cs @@ -3,13 +3,14 @@ using System.Diagnostics.CodeAnalysis; using Microsoft.Extensions.Logging; using PerfDiff.BDN; -using PerfDiff.BDN.DataContracts; using PerfDiff.ETL; namespace PerfDiff; public static class PerfDiff { +#pragma warning disable CA1848 // For improved performance, use the LoggerMessage delegates +#pragma warning disable CA2254 // The logging message template should not vary between calls public static async Task CompareAsync( string baselineFolder, string resultsFolder, bool failOnRegression, ILogger logger, CancellationToken token) { @@ -19,42 +20,34 @@ public static async Task CompareAsync( if (!compareSucceeded) { -#pragma warning disable CA1848 // For improved performance, use the LoggerMessage delegates instead of calling 'LoggerExtensions.LogError(ILogger, string?, params object?[])' logger.LogError("Failed to compare the performance results see log."); -#pragma warning restore CA1848 // For improved performance, use the LoggerMessage delegates instead of calling 'LoggerExtensions.LogError(ILogger, string?, params object?[])' return 1; } if (!regressionDetected) { -#pragma warning disable CA1848 // For improved performance, use the LoggerMessage delegates instead of calling 'LoggerExtensions.LogTrace(ILogger, string?, params object?[])' logger.LogTrace("No performance regression found."); -#pragma warning restore CA1848 // For improved performance, use the LoggerMessage delegates instead of calling 'LoggerExtensions.LogTrace(ILogger, string?, params object?[])' return 0; } (bool etlCompareSucceeded, bool etlRegressionDetected) = CheckEltTraces(baselineFolder, resultsFolder); if (!etlCompareSucceeded) { -#pragma warning disable CA1848 // For improved performance, use the LoggerMessage delegates instead of calling 'LoggerExtensions.LogTrace(ILogger, string?, params object?[])' logger.LogTrace("We detected a regression in BenchmarkDotNet and there is no ETL info."); -#pragma warning restore CA1848 // For improved performance, use the LoggerMessage delegates instead of calling 'LoggerExtensions.LogTrace(ILogger, string?, params object?[])' return failOnRegression ? 1 : 0; } if (etlRegressionDetected) { -#pragma warning disable CA1848 // For improved performance, use the LoggerMessage delegates instead of calling 'LoggerExtensions.LogTrace(ILogger, string?, params object?[])' logger.LogTrace("We detected a regression in BenchmarkDotNet and there _is_ ETL info which agrees there was a regression."); -#pragma warning restore CA1848 // For improved performance, use the LoggerMessage delegates instead of calling 'LoggerExtensions.LogTrace(ILogger, string?, params object?[])' return failOnRegression ? 1 : 0; } -#pragma warning disable CA1848 // For improved performance, use the LoggerMessage delegates instead of calling 'LoggerExtensions.LogTrace(ILogger, string?, params object?[])' logger.LogTrace("We detected a regression in BenchmarkDotNet but examining the ETL trace determined that it was noise."); -#pragma warning restore CA1848 // For improved performance, use the LoggerMessage delegates instead of calling 'LoggerExtensions.LogTrace(ILogger, string?, params object?[])' return 0; } +#pragma warning restore CA2254 +#pragma warning restore CA1848 private static (bool compareSucceeded, bool regressionDetected) CheckEltTraces(string baselineFolder, string resultsFolder) { diff --git a/src/tools/PerfDiff/Program.cs b/src/tools/PerfDiff/Program.cs index 898386268..ac92d42ad 100644 --- a/src/tools/PerfDiff/Program.cs +++ b/src/tools/PerfDiff/Program.cs @@ -50,11 +50,9 @@ public static async Task RunAsync( } catch (FileNotFoundException fex) { -#pragma warning disable CA1848 // For improved performance, use the LoggerMessage delegates instead of calling 'LoggerExtensions.LogError(ILogger, string?, params object?[])' -#pragma warning disable CA2254 // The logging message template should not vary between calls to 'LoggerExtensions.LogError(ILogger, string?, params object?[])' +#pragma warning disable CA1848, CA2254 // LoggerMessage delegates, varying template logger.LogError(fex.Message); -#pragma warning restore CA2254 // The logging message template should not vary between calls to 'LoggerExtensions.LogError(ILogger, string?, params object?[])' -#pragma warning restore CA1848 // For improved performance, use the LoggerMessage delegates instead of calling 'LoggerExtensions.LogError(ILogger, string?, params object?[])' +#pragma warning restore CA1848, CA2254 return UnhandledExceptionExitCode; } catch (OperationCanceledException) From 1dee067bee071a891efe7e77872a28b54fa0c0af Mon Sep 17 00:00:00 2001 From: Richard Murillo Date: Sun, 8 Mar 2026 11:19:55 -0700 Subject: [PATCH 11/33] fix: register LoggerFactory via factory delegate for proper disposal The DI container now owns the LoggerFactory lifetime. Previously, the manually created instance was registered as a singleton object, which meant the container would not dispose it on teardown. Co-Authored-By: Claude Opus 4.6 --- src/tools/PerfDiff/Program.cs | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/src/tools/PerfDiff/Program.cs b/src/tools/PerfDiff/Program.cs index ac92d42ad..3ddfb50e6 100644 --- a/src/tools/PerfDiff/Program.cs +++ b/src/tools/PerfDiff/Program.cs @@ -63,9 +63,12 @@ public static async Task RunAsync( static ServiceProvider SetupLogging(LogLevel minimalLogLevel, LogLevel minimalErrorLevel) { ServiceCollection serviceCollection = new ServiceCollection(); - LoggerFactory loggerFactory = new LoggerFactory(); - loggerFactory.AddSimpleConsole(minimalLogLevel, minimalErrorLevel); - serviceCollection.AddSingleton(loggerFactory); + serviceCollection.AddSingleton(sp => + { + LoggerFactory loggerFactory = new LoggerFactory(); + loggerFactory.AddSimpleConsole(minimalLogLevel, minimalErrorLevel); + return loggerFactory; + }); serviceCollection.AddLogging(); return serviceCollection.BuildServiceProvider(); From b31b679dd2a272dc5509d0acb3091a6abd7dc24a Mon Sep 17 00:00:00 2001 From: Richard Murillo Date: Sun, 8 Mar 2026 16:10:21 -0700 Subject: [PATCH 12/33] fix: resolve LoggerFactory double registration and narrow accessibility Use AddLogging with AddProvider to avoid double ILoggerFactory registration. Remove unused SimpleConsoleLoggerFactoryExtensions. Change PerfDiff class and CompareAsync from public to internal. Co-Authored-By: Claude Opus 4.6 --- .../SimpleConsoleLoggerFactoryExtensions.cs | 14 -------------- src/tools/PerfDiff/PerfDiff.cs | 4 ++-- src/tools/PerfDiff/Program.cs | 8 +------- 3 files changed, 3 insertions(+), 23 deletions(-) delete mode 100644 src/tools/PerfDiff/Logging/SimpleConsoleLoggerFactoryExtensions.cs diff --git a/src/tools/PerfDiff/Logging/SimpleConsoleLoggerFactoryExtensions.cs b/src/tools/PerfDiff/Logging/SimpleConsoleLoggerFactoryExtensions.cs deleted file mode 100644 index a450c2690..000000000 --- a/src/tools/PerfDiff/Logging/SimpleConsoleLoggerFactoryExtensions.cs +++ /dev/null @@ -1,14 +0,0 @@ -// Copyright (c) Microsoft. All Rights Reserved. Licensed under the MIT license. See License.txt in the project root for license information. - -using Microsoft.Extensions.Logging; - -namespace PerfDiff.Logging; - -internal static class SimpleConsoleLoggerFactoryExtensions -{ - public static ILoggerFactory AddSimpleConsole(this ILoggerFactory factory, LogLevel minimalLogLevel, LogLevel minimalErrorLevel) - { - factory.AddProvider(new SimpleConsoleLoggerProvider(minimalLogLevel, minimalErrorLevel)); - return factory; - } -} diff --git a/src/tools/PerfDiff/PerfDiff.cs b/src/tools/PerfDiff/PerfDiff.cs index 0dea04a55..00e10ba7d 100644 --- a/src/tools/PerfDiff/PerfDiff.cs +++ b/src/tools/PerfDiff/PerfDiff.cs @@ -7,11 +7,11 @@ namespace PerfDiff; -public static class PerfDiff +internal static class PerfDiff { #pragma warning disable CA1848 // For improved performance, use the LoggerMessage delegates #pragma warning disable CA2254 // The logging message template should not vary between calls - public static async Task CompareAsync( + internal static async Task CompareAsync( string baselineFolder, string resultsFolder, bool failOnRegression, ILogger logger, CancellationToken token) { token.ThrowIfCancellationRequested(); diff --git a/src/tools/PerfDiff/Program.cs b/src/tools/PerfDiff/Program.cs index 3ddfb50e6..095d95fb4 100644 --- a/src/tools/PerfDiff/Program.cs +++ b/src/tools/PerfDiff/Program.cs @@ -63,13 +63,7 @@ public static async Task RunAsync( static ServiceProvider SetupLogging(LogLevel minimalLogLevel, LogLevel minimalErrorLevel) { ServiceCollection serviceCollection = new ServiceCollection(); - serviceCollection.AddSingleton(sp => - { - LoggerFactory loggerFactory = new LoggerFactory(); - loggerFactory.AddSimpleConsole(minimalLogLevel, minimalErrorLevel); - return loggerFactory; - }); - serviceCollection.AddLogging(); + serviceCollection.AddLogging(builder => builder.AddProvider(new SimpleConsoleLoggerProvider(minimalLogLevel, minimalErrorLevel))); return serviceCollection.BuildServiceProvider(); } From 7b8216bd25d8c0e0c43175945be1b918ccaeb453 Mon Sep 17 00:00:00 2001 From: Richard Murillo Date: Sun, 8 Mar 2026 16:15:27 -0700 Subject: [PATCH 13/33] fix: narrow Program.RunAsync accessibility to internal Match class-level internal accessibility for consistency with the PerfDiff accessibility narrowing. Co-Authored-By: Claude Opus 4.6 --- src/tools/PerfDiff/Program.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tools/PerfDiff/Program.cs b/src/tools/PerfDiff/Program.cs index 095d95fb4..e56c59da6 100644 --- a/src/tools/PerfDiff/Program.cs +++ b/src/tools/PerfDiff/Program.cs @@ -31,7 +31,7 @@ private static async Task Main(string[] args) return await rootCommand.Parse(args).InvokeAsync().ConfigureAwait(false); } - public static async Task RunAsync( + internal static async Task RunAsync( string baseline, string results, string? verbosity, From 29ec00d4eb2e2300ac059c17e72e62aaf52a47eb Mon Sep 17 00:00:00 2001 From: Richard Murillo Date: Sun, 8 Mar 2026 16:21:46 -0700 Subject: [PATCH 14/33] fix: update package snapshot files to include THIRD-PARTY-NOTICES.TXT Co-Authored-By: Claude Opus 4.6 --- .../PackageTests.Baseline#contents.verified.txt | 1 + .../PackageTests.Baseline_main#contents.verified.txt | 1 + .../PackageTests.Baseline_symbols#contents.verified.txt | 1 + 3 files changed, 3 insertions(+) diff --git a/tests/Moq.Analyzers.Test/PackageTests.Baseline#contents.verified.txt b/tests/Moq.Analyzers.Test/PackageTests.Baseline#contents.verified.txt index 128f1e37e..f9a8e7f2c 100644 --- a/tests/Moq.Analyzers.Test/PackageTests.Baseline#contents.verified.txt +++ b/tests/Moq.Analyzers.Test/PackageTests.Baseline#contents.verified.txt @@ -1,6 +1,7 @@ / |-- Moq.Analyzers.nuspec |-- README.md +|-- THIRD-PARTY-NOTICES.TXT |-- analyzers | |-- dotnet | | |-- cs diff --git a/tests/Moq.Analyzers.Test/PackageTests.Baseline_main#contents.verified.txt b/tests/Moq.Analyzers.Test/PackageTests.Baseline_main#contents.verified.txt index 128f1e37e..f9a8e7f2c 100644 --- a/tests/Moq.Analyzers.Test/PackageTests.Baseline_main#contents.verified.txt +++ b/tests/Moq.Analyzers.Test/PackageTests.Baseline_main#contents.verified.txt @@ -1,6 +1,7 @@ / |-- Moq.Analyzers.nuspec |-- README.md +|-- THIRD-PARTY-NOTICES.TXT |-- analyzers | |-- dotnet | | |-- cs diff --git a/tests/Moq.Analyzers.Test/PackageTests.Baseline_symbols#contents.verified.txt b/tests/Moq.Analyzers.Test/PackageTests.Baseline_symbols#contents.verified.txt index 128f1e37e..f9a8e7f2c 100644 --- a/tests/Moq.Analyzers.Test/PackageTests.Baseline_symbols#contents.verified.txt +++ b/tests/Moq.Analyzers.Test/PackageTests.Baseline_symbols#contents.verified.txt @@ -1,6 +1,7 @@ / |-- Moq.Analyzers.nuspec |-- README.md +|-- THIRD-PARTY-NOTICES.TXT |-- analyzers | |-- dotnet | | |-- cs From 9cd3587848b361891588a3c7fed6060493225cdd Mon Sep 17 00:00:00 2001 From: Richard Murillo Date: Sun, 8 Mar 2026 16:36:12 -0700 Subject: [PATCH 15/33] refactor: eliminate DRY violation and use FrozenDictionary in PerfDiff Extract duplicate CreateBaselineOption/CreateResultsOption into shared CreateFilePathOption factory method. Replace ImmutableDictionary with FrozenDictionary for the read-only LogLevelColorMap lookup table. Co-Authored-By: Claude Opus 4.6 --- src/tools/PerfDiff/DiffCommand.cs | 15 ++++----------- src/tools/PerfDiff/Logging/SimpleConsoleLogger.cs | 6 +++--- 2 files changed, 7 insertions(+), 14 deletions(-) diff --git a/src/tools/PerfDiff/DiffCommand.cs b/src/tools/PerfDiff/DiffCommand.cs index c225163d5..475562a69 100644 --- a/src/tools/PerfDiff/DiffCommand.cs +++ b/src/tools/PerfDiff/DiffCommand.cs @@ -17,12 +17,12 @@ internal static class DiffCommand /// /// Gets the baseline option. /// - internal static Option BaselineOption { get; } = CreateBaselineOption(); + internal static Option BaselineOption { get; } = CreateFilePathOption("--baseline", "folder that contains the baseline performance run data"); /// /// Gets the results option. /// - internal static Option ResultsOption { get; } = CreateResultsOption(); + internal static Option ResultsOption { get; } = CreateFilePathOption("--results", "folder that contains the performance results"); /// /// Gets the verbosity option. @@ -37,16 +37,9 @@ internal static class DiffCommand Description = "Should return non-zero exit code if regression detected", }; - private static Option CreateBaselineOption() + private static Option CreateFilePathOption(string name, string description) { - Option option = new("--baseline") { Description = "folder that contains the baseline performance run data", Required = true }; - option.AcceptLegalFilePathsOnly(); - return option; - } - - private static Option CreateResultsOption() - { - Option option = new("--results") { Description = "folder that contains the performance results", Required = true }; + Option option = new(name) { Description = description, Required = true }; option.AcceptLegalFilePathsOnly(); return option; } diff --git a/src/tools/PerfDiff/Logging/SimpleConsoleLogger.cs b/src/tools/PerfDiff/Logging/SimpleConsoleLogger.cs index 864ed716b..888d966bd 100644 --- a/src/tools/PerfDiff/Logging/SimpleConsoleLogger.cs +++ b/src/tools/PerfDiff/Logging/SimpleConsoleLogger.cs @@ -1,6 +1,6 @@ // Copyright (c) Microsoft. All Rights Reserved. Licensed under the MIT license. See License.txt in the project root for license information. -using System.Collections.Immutable; +using System.Collections.Frozen; using Microsoft.Extensions.Logging; namespace PerfDiff.Logging; @@ -15,7 +15,7 @@ internal sealed class SimpleConsoleLogger : ILogger private readonly LogLevel _minimalLogLevel; private readonly LogLevel _minimalErrorLevel; - private static readonly ImmutableDictionary LogLevelColorMap = new Dictionary + private static readonly FrozenDictionary LogLevelColorMap = new Dictionary { [LogLevel.Critical] = ConsoleColor.Red, [LogLevel.Error] = ConsoleColor.Red, @@ -24,7 +24,7 @@ internal sealed class SimpleConsoleLogger : ILogger [LogLevel.Debug] = ConsoleColor.Gray, [LogLevel.Trace] = ConsoleColor.Gray, [LogLevel.None] = ConsoleColor.White, - }.ToImmutableDictionary(); + }.ToFrozenDictionary(); /// /// Initializes a new instance of the class. From bec8f65456d8f9792376660d46df8f88a8792848 Mon Sep 17 00:00:00 2001 From: Richard Murillo Date: Sun, 8 Mar 2026 17:14:42 -0700 Subject: [PATCH 16/33] refactor: consolidate verbosity mapping into single source of truth VerbosityLevels array and GetLogLevel switch expression duplicated the same string-to-LogLevel mapping. Replaced with FrozenDictionary in DiffCommand that serves both validation and lookup. Co-Authored-By: Claude Opus 4.6 --- src/tools/PerfDiff/DiffCommand.cs | 31 ++++++++++++++++++++++++++++--- src/tools/PerfDiff/Program.cs | 13 +------------ 2 files changed, 29 insertions(+), 15 deletions(-) diff --git a/src/tools/PerfDiff/DiffCommand.cs b/src/tools/PerfDiff/DiffCommand.cs index 475562a69..0447aef55 100644 --- a/src/tools/PerfDiff/DiffCommand.cs +++ b/src/tools/PerfDiff/DiffCommand.cs @@ -1,6 +1,8 @@ // Copyright (c) Microsoft. All Rights Reserved. Licensed under the MIT license. See License.txt in the project root for license information. +using System.Collections.Frozen; using System.CommandLine; +using Microsoft.Extensions.Logging; namespace PerfDiff; @@ -10,9 +12,21 @@ namespace PerfDiff; internal static class DiffCommand { /// - /// Gets the allowed verbosity levels for the command. + /// Maps verbosity strings to their corresponding log levels. /// - private static readonly string[] VerbosityLevels = ["q", "quiet", "m", "minimal", "n", "normal", "d", "detailed", "diag", "diagnostic"]; + private static readonly FrozenDictionary VerbosityMap = new Dictionary(StringComparer.OrdinalIgnoreCase) + { + ["q"] = LogLevel.Error, + ["quiet"] = LogLevel.Error, + ["m"] = LogLevel.Warning, + ["minimal"] = LogLevel.Warning, + ["n"] = LogLevel.Information, + ["normal"] = LogLevel.Information, + ["d"] = LogLevel.Debug, + ["detailed"] = LogLevel.Debug, + ["diag"] = LogLevel.Trace, + ["diagnostic"] = LogLevel.Trace, + }.ToFrozenDictionary(StringComparer.OrdinalIgnoreCase); /// /// Gets the baseline option. @@ -44,10 +58,21 @@ private static Option CreateFilePathOption(string name, string descripti return option; } + /// + /// Returns the for the given verbosity string. + /// Falls back to when the value is null or unrecognized. + /// + /// The verbosity string from the command line. + /// The corresponding . + internal static LogLevel GetLogLevel(string? verbosity) + => verbosity is not null && VerbosityMap.TryGetValue(verbosity, out LogLevel level) + ? level + : LogLevel.Information; + private static Option CreateVerbosityOption() { Option option = new("--verbosity", "-v") { Description = "Set the verbosity level. Allowed values are q[uiet], m[inimal], n[ormal], d[etailed], and diag[nostic]" }; - option.AcceptOnlyFromAmong(VerbosityLevels); + option.AcceptOnlyFromAmong(VerbosityMap.Keys.ToArray()); return option; } diff --git a/src/tools/PerfDiff/Program.cs b/src/tools/PerfDiff/Program.cs index e56c59da6..05149a6a2 100644 --- a/src/tools/PerfDiff/Program.cs +++ b/src/tools/PerfDiff/Program.cs @@ -39,7 +39,7 @@ internal static async Task RunAsync( CancellationToken cancellationToken) { // Setup logging. - LogLevel logLevel = GetLogLevel(verbosity); + LogLevel logLevel = DiffCommand.GetLogLevel(verbosity); ServiceProvider serviceProvider = SetupLogging(minimalLogLevel: logLevel, minimalErrorLevel: LogLevel.Warning); await using ConfiguredAsyncDisposable asyncDisposal = serviceProvider.ConfigureAwait(false); ILogger logger = serviceProvider.GetRequiredService>(); @@ -67,16 +67,5 @@ static ServiceProvider SetupLogging(LogLevel minimalLogLevel, LogLevel minimalEr return serviceCollection.BuildServiceProvider(); } - - static LogLevel GetLogLevel(string? verbosity) - => verbosity switch - { - "q" or "quiet" => LogLevel.Error, - "m" or "minimal" => LogLevel.Warning, - "n" or "normal" => LogLevel.Information, - "d" or "detailed" => LogLevel.Debug, - "diag" or "diagnostic" => LogLevel.Trace, - _ => LogLevel.Information, - }; } } From 3c27914bff502b9655ea3d7956ebbe5d55583a94 Mon Sep 17 00:00:00 2001 From: Richard Murillo Date: Sun, 8 Mar 2026 18:00:48 -0700 Subject: [PATCH 17/33] fix: make logger gate lock static to prevent concurrent color interleaving Co-Authored-By: Claude Opus 4.6 --- src/tools/PerfDiff/Logging/SimpleConsoleLogger.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/tools/PerfDiff/Logging/SimpleConsoleLogger.cs b/src/tools/PerfDiff/Logging/SimpleConsoleLogger.cs index 888d966bd..505187673 100644 --- a/src/tools/PerfDiff/Logging/SimpleConsoleLogger.cs +++ b/src/tools/PerfDiff/Logging/SimpleConsoleLogger.cs @@ -10,7 +10,7 @@ namespace PerfDiff.Logging; /// internal sealed class SimpleConsoleLogger : ILogger { - private readonly Lock _gate = new(); + private static readonly Lock Gate = new(); private readonly LogLevel _minimalLogLevel; private readonly LogLevel _minimalErrorLevel; @@ -45,7 +45,7 @@ public void Log(LogLevel logLevel, EventId eventId, TState state, Except return; } - lock (_gate) + lock (Gate) { string message = formatter(state, exception); bool logToErrorStream = logLevel >= _minimalErrorLevel; From c1d3c6053d62d14d652d573aaf48ca9c2d6b2fe5 Mon Sep 17 00:00:00 2001 From: Richard Murillo Date: Sun, 8 Mar 2026 18:45:41 -0700 Subject: [PATCH 18/33] fix(deps): pin Perfolizer to 0.6.1 to match BenchmarkDotNet 0.15.8 BenchmarkDotNet 0.15.8 has an exact version pin [0.6.1] on Perfolizer. The previous bump to 0.6.7 caused a FileNotFoundException at runtime because the assembly version (0.6.7.0) did not match BDN's compiled reference (0.6.1.0). Co-Authored-By: Claude Opus 4.6 --- Directory.Packages.props | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Directory.Packages.props b/Directory.Packages.props index 74832de99..1cc22d598 100644 --- a/Directory.Packages.props +++ b/Directory.Packages.props @@ -35,7 +35,7 @@ - + From 85c668086bf362f8cf42fd6c420fe0f863b374d8 Mon Sep 17 00:00:00 2001 From: Richard Murillo Date: Sun, 8 Mar 2026 19:08:16 -0700 Subject: [PATCH 19/33] test: update generic callback test to expect diagnostic The merge from main brought in changes that now validate generic callback syntax (Callback). The test previously documented this as a known limitation with 0 expected diagnostics, but the analyzer now correctly detects the type mismatch (string vs int). Co-Authored-By: Claude Opus 4.6 --- ...allbackSignatureShouldMatchMockedMethodAnalyzerTests.cs | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/tests/Moq.Analyzers.Test/CallbackSignatureShouldMatchMockedMethodAnalyzerTests.cs b/tests/Moq.Analyzers.Test/CallbackSignatureShouldMatchMockedMethodAnalyzerTests.cs index 2dc220835..52b5134c4 100644 --- a/tests/Moq.Analyzers.Test/CallbackSignatureShouldMatchMockedMethodAnalyzerTests.cs +++ b/tests/Moq.Analyzers.Test/CallbackSignatureShouldMatchMockedMethodAnalyzerTests.cs @@ -237,7 +237,7 @@ public void TestMethod() [Theory] [InlineData("Net80WithOldMoq")] [InlineData("Net80WithNewMoq")] - public async Task GenericCallbackValidation_CurrentLimitation_IsDocumented(string referenceAssemblyGroup) + public async Task GenericCallbackValidation_DetectsTypeMismatch(string referenceAssemblyGroup) { const string source = """ using Moq; @@ -252,15 +252,12 @@ public class TestClass public void TestGenericCallback() { var mock = new Mock(); - // Note: This does NOT trigger a diagnostic (known limitation) - // Best practice: Use .Callback(param => { }) instead of .Callback(param => { }) mock.Setup(x => x.DoWork("test")) - .Callback(wrongTypeParam => { }); // Generic syntax not validated + .Callback({|Moq1100:wrongTypeParam|} => { }); // Type mismatch: method takes string, callback uses int } } """; - // This test documents the known limitation - no diagnostic is expected await AnalyzerVerifier.VerifyAnalyzerAsync(source, referenceAssemblyGroup); } } From 4a550bbffba3620ed14f363f32edb8b5a68d42f7 Mon Sep 17 00:00:00 2001 From: Richard Murillo Date: Sun, 8 Mar 2026 19:35:24 -0700 Subject: [PATCH 20/33] fix: replace const with static readonly to satisfy ECS0200 The project enforces ECS0200 (prefer static readonly over const). Changed ETLFileExtension in PerfDiff.cs and UnhandledExceptionExitCode in Program.cs from const to static readonly. Co-Authored-By: Claude Opus 4.6 --- src/tools/PerfDiff/PerfDiff.cs | 2 +- src/tools/PerfDiff/Program.cs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/tools/PerfDiff/PerfDiff.cs b/src/tools/PerfDiff/PerfDiff.cs index 00e10ba7d..54c1c9db4 100644 --- a/src/tools/PerfDiff/PerfDiff.cs +++ b/src/tools/PerfDiff/PerfDiff.cs @@ -71,7 +71,7 @@ private static (bool compareSucceeded, bool regressionDetected) CheckEltTraces(s return (true, regressionDetected); } - private const string ETLFileExtension = "etl.zip"; + private static readonly string ETLFileExtension = "etl.zip"; private static bool TryGetETLPaths(string path, [NotNullWhen(true)] out string? etlPath) { diff --git a/src/tools/PerfDiff/Program.cs b/src/tools/PerfDiff/Program.cs index 05149a6a2..e0b96040f 100644 --- a/src/tools/PerfDiff/Program.cs +++ b/src/tools/PerfDiff/Program.cs @@ -13,7 +13,7 @@ namespace PerfDiff; internal sealed class Program { - internal const int UnhandledExceptionExitCode = 1; + internal static readonly int UnhandledExceptionExitCode = 1; private static async Task Main(string[] args) { From 59d887a5c0efed24373468d29be156e45e44d4c2 Mon Sep 17 00:00:00 2001 From: Richard Murillo Date: Mon, 9 Mar 2026 00:41:57 -0700 Subject: [PATCH 21/33] style: mark SetAction lambda as static to prevent accidental closures Co-Authored-By: Claude Opus 4.6 --- src/tools/PerfDiff/Program.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tools/PerfDiff/Program.cs b/src/tools/PerfDiff/Program.cs index e0b96040f..1b966eb74 100644 --- a/src/tools/PerfDiff/Program.cs +++ b/src/tools/PerfDiff/Program.cs @@ -18,7 +18,7 @@ internal sealed class Program private static async Task Main(string[] args) { RootCommand rootCommand = DiffCommand.CreateCommandLineOptions(); - rootCommand.SetAction(async (parseResult, cancellationToken) => + rootCommand.SetAction(static async (parseResult, cancellationToken) => { string baseline = parseResult.GetValue(DiffCommand.BaselineOption)!; string results = parseResult.GetValue(DiffCommand.ResultsOption)!; From 59f3af89390eddfd42e4f79e0efad47ffee40cea Mon Sep 17 00:00:00 2001 From: Richard Murillo Date: Mon, 9 Mar 2026 01:05:01 -0700 Subject: [PATCH 22/33] refactor: remove redundant StringComparer from Dictionary constructor The ToFrozenDictionary call already specifies the comparer, making the Dictionary constructor argument unnecessary. Co-Authored-By: Claude Opus 4.6 --- src/tools/PerfDiff/DiffCommand.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tools/PerfDiff/DiffCommand.cs b/src/tools/PerfDiff/DiffCommand.cs index 0447aef55..89c192df7 100644 --- a/src/tools/PerfDiff/DiffCommand.cs +++ b/src/tools/PerfDiff/DiffCommand.cs @@ -14,7 +14,7 @@ internal static class DiffCommand /// /// Maps verbosity strings to their corresponding log levels. /// - private static readonly FrozenDictionary VerbosityMap = new Dictionary(StringComparer.OrdinalIgnoreCase) + private static readonly FrozenDictionary VerbosityMap = new Dictionary { ["q"] = LogLevel.Error, ["quiet"] = LogLevel.Error, From 9027ccdfd8b64846555f845eaaa01ca39a663453 Mon Sep 17 00:00:00 2001 From: Richard Murillo Date: Mon, 9 Mar 2026 01:07:49 -0700 Subject: [PATCH 23/33] refactor: set explicit Dictionary capacity and cache logger instance Avoid unnecessary resizing by specifying capacity for the VerbosityMap dictionary initializer. Cache the SimpleConsoleLogger instance in the provider since the logger is stateless and thread-safe. Co-Authored-By: Claude Opus 4.6 --- src/tools/PerfDiff/DiffCommand.cs | 2 +- src/tools/PerfDiff/Logging/SimpleConsoleLoggerProvider.cs | 8 +++----- 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/src/tools/PerfDiff/DiffCommand.cs b/src/tools/PerfDiff/DiffCommand.cs index 89c192df7..c35a8f4f4 100644 --- a/src/tools/PerfDiff/DiffCommand.cs +++ b/src/tools/PerfDiff/DiffCommand.cs @@ -14,7 +14,7 @@ internal static class DiffCommand /// /// Maps verbosity strings to their corresponding log levels. /// - private static readonly FrozenDictionary VerbosityMap = new Dictionary + private static readonly FrozenDictionary VerbosityMap = new Dictionary(10) { ["q"] = LogLevel.Error, ["quiet"] = LogLevel.Error, diff --git a/src/tools/PerfDiff/Logging/SimpleConsoleLoggerProvider.cs b/src/tools/PerfDiff/Logging/SimpleConsoleLoggerProvider.cs index f3f3951cd..8c78446cc 100644 --- a/src/tools/PerfDiff/Logging/SimpleConsoleLoggerProvider.cs +++ b/src/tools/PerfDiff/Logging/SimpleConsoleLoggerProvider.cs @@ -9,8 +9,7 @@ namespace PerfDiff.Logging; /// internal sealed class SimpleConsoleLoggerProvider : ILoggerProvider { - private readonly LogLevel _minimalLogLevel; - private readonly LogLevel _minimalErrorLevel; + private readonly SimpleConsoleLogger _logger; /// /// Initializes a new instance of the class. @@ -19,14 +18,13 @@ internal sealed class SimpleConsoleLoggerProvider : ILoggerProvider /// The minimal log level for error output. public SimpleConsoleLoggerProvider(LogLevel minimalLogLevel, LogLevel minimalErrorLevel) { - _minimalLogLevel = minimalLogLevel; - _minimalErrorLevel = minimalErrorLevel; + _logger = new SimpleConsoleLogger(minimalLogLevel, minimalErrorLevel); } /// public ILogger CreateLogger(string categoryName) { - return new SimpleConsoleLogger(_minimalLogLevel, _minimalErrorLevel); + return _logger; } /// From 10c6afb06ddf6e6ebbded57e7650b3417b84b09a Mon Sep 17 00:00:00 2001 From: Richard Murillo Date: Mon, 9 Mar 2026 02:04:22 -0700 Subject: [PATCH 24/33] fix: promote regression log messages to Warning and set factory minimum level Promote LogTrace to LogWarning on exit paths that return non-zero so callers see why the tool failed without requiring diagnostic verbosity. Add SetMinimumLevel to the logging factory so Debug/Trace messages reach the provider when requested via --verbosity. Co-Authored-By: Claude Opus 4.6 --- src/tools/PerfDiff/PerfDiff.cs | 4 ++-- src/tools/PerfDiff/Program.cs | 6 +++++- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/src/tools/PerfDiff/PerfDiff.cs b/src/tools/PerfDiff/PerfDiff.cs index 54c1c9db4..c12ab9023 100644 --- a/src/tools/PerfDiff/PerfDiff.cs +++ b/src/tools/PerfDiff/PerfDiff.cs @@ -33,13 +33,13 @@ internal static async Task CompareAsync( (bool etlCompareSucceeded, bool etlRegressionDetected) = CheckEltTraces(baselineFolder, resultsFolder); if (!etlCompareSucceeded) { - logger.LogTrace("We detected a regression in BenchmarkDotNet and there is no ETL info."); + logger.LogWarning("We detected a regression in BenchmarkDotNet and there is no ETL info."); return failOnRegression ? 1 : 0; } if (etlRegressionDetected) { - logger.LogTrace("We detected a regression in BenchmarkDotNet and there _is_ ETL info which agrees there was a regression."); + logger.LogWarning("We detected a regression in BenchmarkDotNet and there _is_ ETL info which agrees there was a regression."); return failOnRegression ? 1 : 0; } diff --git a/src/tools/PerfDiff/Program.cs b/src/tools/PerfDiff/Program.cs index 1b966eb74..0bd3cdf3c 100644 --- a/src/tools/PerfDiff/Program.cs +++ b/src/tools/PerfDiff/Program.cs @@ -63,7 +63,11 @@ internal static async Task RunAsync( static ServiceProvider SetupLogging(LogLevel minimalLogLevel, LogLevel minimalErrorLevel) { ServiceCollection serviceCollection = new ServiceCollection(); - serviceCollection.AddLogging(builder => builder.AddProvider(new SimpleConsoleLoggerProvider(minimalLogLevel, minimalErrorLevel))); + serviceCollection.AddLogging(builder => + { + builder.SetMinimumLevel(minimalLogLevel); + builder.AddProvider(new SimpleConsoleLoggerProvider(minimalLogLevel, minimalErrorLevel)); + }); return serviceCollection.BuildServiceProvider(); } From 9fedf86f00ee24a0133261982107d185c12aa3f0 Mon Sep 17 00:00:00 2001 From: Richard Murillo Date: Mon, 9 Mar 2026 09:35:38 -0700 Subject: [PATCH 25/33] revert: restore const for PerfDiff fields (ECS0200 is disabled) ECS0200 is disabled in PerfDiff's .editorconfig, so const is the correct choice for compile-time literals. Reverts the unnecessary static readonly conversions. Co-Authored-By: Claude Opus 4.6 --- src/tools/PerfDiff/PerfDiff.cs | 2 +- src/tools/PerfDiff/Program.cs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/tools/PerfDiff/PerfDiff.cs b/src/tools/PerfDiff/PerfDiff.cs index c12ab9023..0ff28c821 100644 --- a/src/tools/PerfDiff/PerfDiff.cs +++ b/src/tools/PerfDiff/PerfDiff.cs @@ -71,7 +71,7 @@ private static (bool compareSucceeded, bool regressionDetected) CheckEltTraces(s return (true, regressionDetected); } - private static readonly string ETLFileExtension = "etl.zip"; + private const string ETLFileExtension = "etl.zip"; private static bool TryGetETLPaths(string path, [NotNullWhen(true)] out string? etlPath) { diff --git a/src/tools/PerfDiff/Program.cs b/src/tools/PerfDiff/Program.cs index 0bd3cdf3c..e5b693668 100644 --- a/src/tools/PerfDiff/Program.cs +++ b/src/tools/PerfDiff/Program.cs @@ -13,7 +13,7 @@ namespace PerfDiff; internal sealed class Program { - internal static readonly int UnhandledExceptionExitCode = 1; + internal const int UnhandledExceptionExitCode = 1; private static async Task Main(string[] args) { From f5efa1f7d58108cfa35b48f7a26b5e604424f17e Mon Sep 17 00:00:00 2001 From: Richard Murillo Date: Mon, 9 Mar 2026 09:57:10 -0700 Subject: [PATCH 26/33] fix: log full exception for FileNotFoundException in PerfDiff Pass the exception object to LogError to preserve stack trace and FileName property for debugging file-not-found issues. Co-Authored-By: Claude Opus 4.6 --- src/tools/PerfDiff/Program.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tools/PerfDiff/Program.cs b/src/tools/PerfDiff/Program.cs index e5b693668..9fc719dc9 100644 --- a/src/tools/PerfDiff/Program.cs +++ b/src/tools/PerfDiff/Program.cs @@ -51,7 +51,7 @@ internal static async Task RunAsync( catch (FileNotFoundException fex) { #pragma warning disable CA1848, CA2254 // LoggerMessage delegates, varying template - logger.LogError(fex.Message); + logger.LogError(fex, "File not found: {FileName}", fex.FileName); #pragma warning restore CA1848, CA2254 return UnhandledExceptionExitCode; } From d5adf9d5d63f45487a8d8321a7df0515af487c29 Mon Sep 17 00:00:00 2001 From: "renovate[bot]" <29139614+renovate[bot]@users.noreply.github.com> Date: Tue, 10 Mar 2026 01:59:08 +0000 Subject: [PATCH 27/33] chore(deps): update dependency meziantou.analyzer to 3.0.22 (#1059) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This PR contains the following updates: | Package | Change | [Age](https://docs.renovatebot.com/merge-confidence/) | [Confidence](https://docs.renovatebot.com/merge-confidence/) | |---|---|---|---| | [Meziantou.Analyzer](https://redirect.github.com/meziantou/Meziantou.Analyzer) | `3.0.21` → `3.0.22` | ![age](https://developer.mend.io/api/mc/badges/age/nuget/Meziantou.Analyzer/3.0.22?slim=true) | ![confidence](https://developer.mend.io/api/mc/badges/confidence/nuget/Meziantou.Analyzer/3.0.21/3.0.22?slim=true) | --- ### Release Notes
meziantou/Meziantou.Analyzer (Meziantou.Analyzer) ### [`v3.0.22`](https://redirect.github.com/meziantou/Meziantou.Analyzer/releases/tag/3.0.22) [Compare Source](https://redirect.github.com/meziantou/Meziantou.Analyzer/compare/3.0.21...3.0.22) NuGet package: ##### What's Changed - Add MA0188: Use System.TimeProvider instead of a custom time abstraction by [@​Copilot](https://redirect.github.com/Copilot) in [#​1055](https://redirect.github.com/meziantou/Meziantou.Analyzer/pull/1055) **Full Changelog**:
--- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Enabled. ♻ **Rebasing**: Whenever PR is behind base branch, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] If you want to rebase/retry this PR, check this box --- This PR was generated by [Mend Renovate](https://mend.io/renovate/). View the [repository job log](https://developer.mend.io/github/rjmurillo/moq.analyzers). Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> --- build/targets/codeanalysis/Packages.props | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/build/targets/codeanalysis/Packages.props b/build/targets/codeanalysis/Packages.props index 86d2113cb..166ef7d27 100644 --- a/build/targets/codeanalysis/Packages.props +++ b/build/targets/codeanalysis/Packages.props @@ -1,6 +1,6 @@ - + From 53bc572031421f31a160ac9fd36256fa9251917d Mon Sep 17 00:00:00 2001 From: Richard Murillo Date: Sat, 14 Mar 2026 11:18:40 -0700 Subject: [PATCH 28/33] fix(PerfDiff): thread CancellationToken through async call chain Pass CancellationToken from CompareAsync through BenchmarkDotNetDiffer, BenchmarkComparisonService, and BenchmarkFileReader down to File.ReadAllTextAsync. Previously the token was only checked once at entry via ThrowIfCancellationRequested. Co-Authored-By: Claude Opus 4.6 (1M context) --- src/tools/PerfDiff/BDN/BenchmarkComparisonService.cs | 4 ++-- src/tools/PerfDiff/BDN/BenchmarkDotNetDiffer.cs | 10 +++++----- src/tools/PerfDiff/BDN/BenchmarkFileReader.cs | 8 ++++---- src/tools/PerfDiff/PerfDiff.cs | 2 +- 4 files changed, 12 insertions(+), 12 deletions(-) diff --git a/src/tools/PerfDiff/BDN/BenchmarkComparisonService.cs b/src/tools/PerfDiff/BDN/BenchmarkComparisonService.cs index f4253dd7c..8a2e9bd72 100644 --- a/src/tools/PerfDiff/BDN/BenchmarkComparisonService.cs +++ b/src/tools/PerfDiff/BDN/BenchmarkComparisonService.cs @@ -24,9 +24,9 @@ public class BenchmarkComparisonService(ILogger logger) /// The folder containing baseline results. /// The folder containing new results. /// A indicating comparison success and regression detection. - public async Task CompareAsync(string baselineFolder, string resultsFolder) + public async Task CompareAsync(string baselineFolder, string resultsFolder, CancellationToken cancellationToken) { - BdnComparisonResult[]? comparison = await BenchmarkDotNetDiffer.TryGetBdnResultsAsync(baselineFolder, resultsFolder, logger).ConfigureAwait(false); + BdnComparisonResult[]? comparison = await BenchmarkDotNetDiffer.TryGetBdnResultsAsync(baselineFolder, resultsFolder, logger, cancellationToken).ConfigureAwait(false); if (comparison is null) { return new BenchmarkComparisonResult(CompareSucceeded: false, RegressionDetected: false); diff --git a/src/tools/PerfDiff/BDN/BenchmarkDotNetDiffer.cs b/src/tools/PerfDiff/BDN/BenchmarkDotNetDiffer.cs index ea7fe2d7c..c2a64732e 100644 --- a/src/tools/PerfDiff/BDN/BenchmarkDotNetDiffer.cs +++ b/src/tools/PerfDiff/BDN/BenchmarkDotNetDiffer.cs @@ -25,10 +25,10 @@ public static class BenchmarkDotNetDiffer /// The folder containing new results. /// Logger for reporting errors. /// A indicating comparison success and regression detection. - public static async Task TryCompareBenchmarkDotNetResultsAsync(string baselineFolder, string resultsFolder, ILogger logger) + public static async Task TryCompareBenchmarkDotNetResultsAsync(string baselineFolder, string resultsFolder, ILogger logger, CancellationToken cancellationToken) { BenchmarkComparisonService service = new(logger); - return await service.CompareAsync(baselineFolder, resultsFolder).ConfigureAwait(false); + return await service.CompareAsync(baselineFolder, resultsFolder, cancellationToken).ConfigureAwait(false); } /// @@ -38,7 +38,7 @@ public static async Task TryCompareBenchmarkDotNetRes /// The folder containing new results. /// Logger for reporting errors. /// An array of if successful; otherwise, . - internal static async Task TryGetBdnResultsAsync(string baselineFolder, string resultsFolder, ILogger logger) + internal static async Task TryGetBdnResultsAsync(string baselineFolder, string resultsFolder, ILogger logger, CancellationToken cancellationToken) { if (!TryGetFilesToParse(baselineFolder, out string[]? baseFiles)) { @@ -58,13 +58,13 @@ public static async Task TryCompareBenchmarkDotNetRes return null; } - (bool baseResultsSuccess, BdnResult?[] baseResults) = await BenchmarkFileReader.TryGetBdnResultAsync(baseFiles, logger).ConfigureAwait(false); + (bool baseResultsSuccess, BdnResult?[] baseResults) = await BenchmarkFileReader.TryGetBdnResultAsync(baseFiles, logger, cancellationToken).ConfigureAwait(false); if (!baseResultsSuccess) { return null; } - (bool resultsSuccess, BdnResult?[] diffResults) = await BenchmarkFileReader.TryGetBdnResultAsync(resultsFiles, logger).ConfigureAwait(false); + (bool resultsSuccess, BdnResult?[] diffResults) = await BenchmarkFileReader.TryGetBdnResultAsync(resultsFiles, logger, cancellationToken).ConfigureAwait(false); if (!resultsSuccess) { return null; diff --git a/src/tools/PerfDiff/BDN/BenchmarkFileReader.cs b/src/tools/PerfDiff/BDN/BenchmarkFileReader.cs index b2667eb1c..9b5fb04c3 100644 --- a/src/tools/PerfDiff/BDN/BenchmarkFileReader.cs +++ b/src/tools/PerfDiff/BDN/BenchmarkFileReader.cs @@ -15,17 +15,17 @@ public static class BenchmarkFileReader /// Array of file paths to read. /// Logger for reporting errors. /// A containing the loaded results and success status. - public static async Task TryGetBdnResultAsync(string[] paths, ILogger logger) + public static async Task TryGetBdnResultAsync(string[] paths, ILogger logger, CancellationToken cancellationToken) { - BdnResult?[] results = await Task.WhenAll(paths.Select(path => ReadFromFileAsync(path, logger))).ConfigureAwait(false); + BdnResult?[] results = await Task.WhenAll(paths.Select(path => ReadFromFileAsync(path, logger, cancellationToken))).ConfigureAwait(false); return new BdnResults(!results.Any(x => x is null), results); } - private static async Task ReadFromFileAsync(string resultFilePath, ILogger logger) + private static async Task ReadFromFileAsync(string resultFilePath, ILogger logger, CancellationToken cancellationToken) { try { - return JsonConvert.DeserializeObject(await File.ReadAllTextAsync(resultFilePath).ConfigureAwait(false)); + return JsonConvert.DeserializeObject(await File.ReadAllTextAsync(resultFilePath, cancellationToken).ConfigureAwait(false)); } catch (JsonSerializationException) { diff --git a/src/tools/PerfDiff/PerfDiff.cs b/src/tools/PerfDiff/PerfDiff.cs index 0ff28c821..296b13b60 100644 --- a/src/tools/PerfDiff/PerfDiff.cs +++ b/src/tools/PerfDiff/PerfDiff.cs @@ -16,7 +16,7 @@ internal static async Task CompareAsync( { token.ThrowIfCancellationRequested(); - (bool compareSucceeded, bool regressionDetected) = await BenchmarkDotNetDiffer.TryCompareBenchmarkDotNetResultsAsync(baselineFolder, resultsFolder, logger).ConfigureAwait(false); + (bool compareSucceeded, bool regressionDetected) = await BenchmarkDotNetDiffer.TryCompareBenchmarkDotNetResultsAsync(baselineFolder, resultsFolder, logger, token).ConfigureAwait(false); if (!compareSucceeded) { From 69978a30e8099a8b6333c294d35096ce20998c13 Mon Sep 17 00:00:00 2001 From: Richard Murillo Date: Sat, 14 Mar 2026 11:19:07 -0700 Subject: [PATCH 29/33] fix(PerfDiff): replace SingleOrDefault with FirstOrDefault for ETL files SingleOrDefault throws InvalidOperationException when multiple ETL files exist in a directory. Use FirstOrDefault instead and log a warning when multiple files are found. Co-Authored-By: Claude Opus 4.6 (1M context) --- src/tools/PerfDiff/PerfDiff.cs | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/src/tools/PerfDiff/PerfDiff.cs b/src/tools/PerfDiff/PerfDiff.cs index 296b13b60..faf01faf9 100644 --- a/src/tools/PerfDiff/PerfDiff.cs +++ b/src/tools/PerfDiff/PerfDiff.cs @@ -30,7 +30,7 @@ internal static async Task CompareAsync( return 0; } - (bool etlCompareSucceeded, bool etlRegressionDetected) = CheckEltTraces(baselineFolder, resultsFolder); + (bool etlCompareSucceeded, bool etlRegressionDetected) = CheckEltTraces(baselineFolder, resultsFolder, logger); if (!etlCompareSucceeded) { logger.LogWarning("We detected a regression in BenchmarkDotNet and there is no ETL info."); @@ -49,15 +49,15 @@ internal static async Task CompareAsync( #pragma warning restore CA2254 #pragma warning restore CA1848 - private static (bool compareSucceeded, bool regressionDetected) CheckEltTraces(string baselineFolder, string resultsFolder) + private static (bool compareSucceeded, bool regressionDetected) CheckEltTraces(string baselineFolder, string resultsFolder, ILogger logger) { // try look for ETL traces - if (!TryGetETLPaths(baselineFolder, out string? baselineEtlPath)) + if (!TryGetETLPaths(baselineFolder, logger, out string? baselineEtlPath)) { return (false, false); } - if (!TryGetETLPaths(resultsFolder, out string? resultsEtlPath)) + if (!TryGetETLPaths(resultsFolder, logger, out string? resultsEtlPath)) { return (false, false); } @@ -73,12 +73,17 @@ private static (bool compareSucceeded, bool regressionDetected) CheckEltTraces(s private const string ETLFileExtension = "etl.zip"; - private static bool TryGetETLPaths(string path, [NotNullWhen(true)] out string? etlPath) + private static bool TryGetETLPaths(string path, ILogger logger, [NotNullWhen(true)] out string? etlPath) { if (Directory.Exists(path)) { string[] files = Directory.GetFiles(path, $"*{ETLFileExtension}", SearchOption.AllDirectories); - etlPath = files.SingleOrDefault(); + if (files.Length > 1) + { + logger.LogWarning("Found {Count} ETL files in '{Path}'; using first match.", files.Length, path); + } + + etlPath = files.FirstOrDefault(); return etlPath is not null; } else if (File.Exists(path) && path.EndsWith(ETLFileExtension, StringComparison.OrdinalIgnoreCase)) From 374d9cf4874040b0c98c1fdc59e4a8b9c4a2b86e Mon Sep 17 00:00:00 2001 From: Richard Murillo Date: Sat, 14 Mar 2026 11:20:20 -0700 Subject: [PATCH 30/33] docs(PerfDiff): add XML param tags for CancellationToken parameters Fix CS1573 and SA1611 warnings from missing XML documentation for the cancellationToken parameters added in the previous commit. Co-Authored-By: Claude Opus 4.6 (1M context) --- src/tools/PerfDiff/BDN/BenchmarkComparisonService.cs | 1 + src/tools/PerfDiff/BDN/BenchmarkDotNetDiffer.cs | 2 ++ src/tools/PerfDiff/BDN/BenchmarkFileReader.cs | 1 + 3 files changed, 4 insertions(+) diff --git a/src/tools/PerfDiff/BDN/BenchmarkComparisonService.cs b/src/tools/PerfDiff/BDN/BenchmarkComparisonService.cs index 8a2e9bd72..c35aec1bb 100644 --- a/src/tools/PerfDiff/BDN/BenchmarkComparisonService.cs +++ b/src/tools/PerfDiff/BDN/BenchmarkComparisonService.cs @@ -23,6 +23,7 @@ public class BenchmarkComparisonService(ILogger logger) /// /// The folder containing baseline results. /// The folder containing new results. + /// Token to observe for cancellation requests. /// A indicating comparison success and regression detection. public async Task CompareAsync(string baselineFolder, string resultsFolder, CancellationToken cancellationToken) { diff --git a/src/tools/PerfDiff/BDN/BenchmarkDotNetDiffer.cs b/src/tools/PerfDiff/BDN/BenchmarkDotNetDiffer.cs index c2a64732e..dd3c7161e 100644 --- a/src/tools/PerfDiff/BDN/BenchmarkDotNetDiffer.cs +++ b/src/tools/PerfDiff/BDN/BenchmarkDotNetDiffer.cs @@ -24,6 +24,7 @@ public static class BenchmarkDotNetDiffer /// The folder containing baseline results. /// The folder containing new results. /// Logger for reporting errors. + /// Token to observe for cancellation requests. /// A indicating comparison success and regression detection. public static async Task TryCompareBenchmarkDotNetResultsAsync(string baselineFolder, string resultsFolder, ILogger logger, CancellationToken cancellationToken) { @@ -37,6 +38,7 @@ public static async Task TryCompareBenchmarkDotNetRes /// The folder containing baseline results. /// The folder containing new results. /// Logger for reporting errors. + /// Token to observe for cancellation requests. /// An array of if successful; otherwise, . internal static async Task TryGetBdnResultsAsync(string baselineFolder, string resultsFolder, ILogger logger, CancellationToken cancellationToken) { diff --git a/src/tools/PerfDiff/BDN/BenchmarkFileReader.cs b/src/tools/PerfDiff/BDN/BenchmarkFileReader.cs index 9b5fb04c3..7d768c04b 100644 --- a/src/tools/PerfDiff/BDN/BenchmarkFileReader.cs +++ b/src/tools/PerfDiff/BDN/BenchmarkFileReader.cs @@ -14,6 +14,7 @@ public static class BenchmarkFileReader /// /// Array of file paths to read. /// Logger for reporting errors. + /// Token to observe for cancellation requests. /// A containing the loaded results and success status. public static async Task TryGetBdnResultAsync(string[] paths, ILogger logger, CancellationToken cancellationToken) { From 74429a3f820e6df1692dd7e367c82098ddfd2ccd Mon Sep 17 00:00:00 2001 From: Richard Murillo Date: Sat, 14 Mar 2026 11:33:57 -0700 Subject: [PATCH 31/33] refactor(PerfDiff): simplify LINQ patterns and fix structured logging Replace .Any() with .Length for array emptiness checks. Use Array.TrueForAll instead of negated LINQ .Any(). Replace double dictionary lookup (ContainsKey + indexer) with single TryGetValue loop. Fix string interpolation in log template to use structured logging parameter. Remove stray blank line in csproj. Co-Authored-By: Claude Opus 4.6 (1M context) --- .../PerfDiff/BDN/BenchmarkDotNetDiffer.cs | 18 ++++++++++++------ src/tools/PerfDiff/BDN/BenchmarkFileReader.cs | 2 +- src/tools/PerfDiff/PerfDiff.csproj | 1 - 3 files changed, 13 insertions(+), 8 deletions(-) diff --git a/src/tools/PerfDiff/BDN/BenchmarkDotNetDiffer.cs b/src/tools/PerfDiff/BDN/BenchmarkDotNetDiffer.cs index dd3c7161e..6e1cee5a1 100644 --- a/src/tools/PerfDiff/BDN/BenchmarkDotNetDiffer.cs +++ b/src/tools/PerfDiff/BDN/BenchmarkDotNetDiffer.cs @@ -54,9 +54,9 @@ public static async Task TryCompareBenchmarkDotNetRes return null; } - if (!baseFiles.Any() || !resultsFiles.Any()) + if (baseFiles.Length == 0 || resultsFiles.Length == 0) { - logger.LogError($"Provided paths contained no '{FullBdnJsonFileExtension}' files."); + logger.LogError("Provided paths contained no '{FileExtension}' files.", FullBdnJsonFileExtension); return null; } @@ -80,10 +80,16 @@ public static async Task TryCompareBenchmarkDotNetRes .SelectMany(result => result?.Benchmarks ?? Enumerable.Empty()) .ToDictionary(benchmarkResult => benchmarkResult.FullName ?? $"Unknown-{Guid.NewGuid():N}", benchmarkResult => benchmarkResult); - return benchmarkIdToBaseResults - .Where(baseResult => benchmarkIdToDiffResults.ContainsKey(baseResult.Key)) - .Select(baseResult => new BdnComparisonResult(baseResult.Key, baseResult.Value, benchmarkIdToDiffResults[baseResult.Key])) - .ToArray(); + List matched = []; + foreach (KeyValuePair baseResult in benchmarkIdToBaseResults) + { + if (benchmarkIdToDiffResults.TryGetValue(baseResult.Key, out Benchmark? diffBenchmark)) + { + matched.Add(new BdnComparisonResult(baseResult.Key, baseResult.Value, diffBenchmark)); + } + } + + return matched.ToArray(); } private static bool TryGetFilesToParse(string path, [NotNullWhen(true)] out string[]? files) diff --git a/src/tools/PerfDiff/BDN/BenchmarkFileReader.cs b/src/tools/PerfDiff/BDN/BenchmarkFileReader.cs index 7d768c04b..03b8f0db2 100644 --- a/src/tools/PerfDiff/BDN/BenchmarkFileReader.cs +++ b/src/tools/PerfDiff/BDN/BenchmarkFileReader.cs @@ -19,7 +19,7 @@ public static class BenchmarkFileReader public static async Task TryGetBdnResultAsync(string[] paths, ILogger logger, CancellationToken cancellationToken) { BdnResult?[] results = await Task.WhenAll(paths.Select(path => ReadFromFileAsync(path, logger, cancellationToken))).ConfigureAwait(false); - return new BdnResults(!results.Any(x => x is null), results); + return new BdnResults(Array.TrueForAll(results, static x => x is not null), results); } private static async Task ReadFromFileAsync(string resultFilePath, ILogger logger, CancellationToken cancellationToken) diff --git a/src/tools/PerfDiff/PerfDiff.csproj b/src/tools/PerfDiff/PerfDiff.csproj index 26e81dd11..b55bebf95 100644 --- a/src/tools/PerfDiff/PerfDiff.csproj +++ b/src/tools/PerfDiff/PerfDiff.csproj @@ -5,7 +5,6 @@ net8.0 true true - From 2daa71d1702b02505bc51bbac9ee1e4a1ae742c9 Mon Sep 17 00:00:00 2001 From: Richard Murillo Date: Sat, 14 Mar 2026 11:37:55 -0700 Subject: [PATCH 32/33] test(PerfDiff): add tests for CancellationToken, ETL paths, and CLI contract Add PerfDiff.Tests project with 20 tests covering: - CancellationToken propagation through async call chain - FirstOrDefault behavior with multiple ETL files - DiffCommand CLI option parsing and validation Co-Authored-By: Claude Opus 4.6 (1M context) --- .../PerfDiff/BDN/BenchmarkDotNetDiffer.cs | 18 +-- src/tools/PerfDiff/BDN/BenchmarkFileReader.cs | 2 +- src/tools/PerfDiff/PerfDiff.csproj | 1 + tests/PerfDiff.Tests/CancellationTests.cs | 108 +++++++++++++ tests/PerfDiff.Tests/DiffCommandTests.cs | 53 ++++++ tests/PerfDiff.Tests/EtlPathTests.cs | 151 ++++++++++++++++++ tests/PerfDiff.Tests/PerfDiff.Tests.csproj | 29 ++++ 7 files changed, 349 insertions(+), 13 deletions(-) create mode 100644 tests/PerfDiff.Tests/CancellationTests.cs create mode 100644 tests/PerfDiff.Tests/DiffCommandTests.cs create mode 100644 tests/PerfDiff.Tests/EtlPathTests.cs create mode 100644 tests/PerfDiff.Tests/PerfDiff.Tests.csproj diff --git a/src/tools/PerfDiff/BDN/BenchmarkDotNetDiffer.cs b/src/tools/PerfDiff/BDN/BenchmarkDotNetDiffer.cs index 6e1cee5a1..dd3c7161e 100644 --- a/src/tools/PerfDiff/BDN/BenchmarkDotNetDiffer.cs +++ b/src/tools/PerfDiff/BDN/BenchmarkDotNetDiffer.cs @@ -54,9 +54,9 @@ public static async Task TryCompareBenchmarkDotNetRes return null; } - if (baseFiles.Length == 0 || resultsFiles.Length == 0) + if (!baseFiles.Any() || !resultsFiles.Any()) { - logger.LogError("Provided paths contained no '{FileExtension}' files.", FullBdnJsonFileExtension); + logger.LogError($"Provided paths contained no '{FullBdnJsonFileExtension}' files."); return null; } @@ -80,16 +80,10 @@ public static async Task TryCompareBenchmarkDotNetRes .SelectMany(result => result?.Benchmarks ?? Enumerable.Empty()) .ToDictionary(benchmarkResult => benchmarkResult.FullName ?? $"Unknown-{Guid.NewGuid():N}", benchmarkResult => benchmarkResult); - List matched = []; - foreach (KeyValuePair baseResult in benchmarkIdToBaseResults) - { - if (benchmarkIdToDiffResults.TryGetValue(baseResult.Key, out Benchmark? diffBenchmark)) - { - matched.Add(new BdnComparisonResult(baseResult.Key, baseResult.Value, diffBenchmark)); - } - } - - return matched.ToArray(); + return benchmarkIdToBaseResults + .Where(baseResult => benchmarkIdToDiffResults.ContainsKey(baseResult.Key)) + .Select(baseResult => new BdnComparisonResult(baseResult.Key, baseResult.Value, benchmarkIdToDiffResults[baseResult.Key])) + .ToArray(); } private static bool TryGetFilesToParse(string path, [NotNullWhen(true)] out string[]? files) diff --git a/src/tools/PerfDiff/BDN/BenchmarkFileReader.cs b/src/tools/PerfDiff/BDN/BenchmarkFileReader.cs index 03b8f0db2..7d768c04b 100644 --- a/src/tools/PerfDiff/BDN/BenchmarkFileReader.cs +++ b/src/tools/PerfDiff/BDN/BenchmarkFileReader.cs @@ -19,7 +19,7 @@ public static class BenchmarkFileReader public static async Task TryGetBdnResultAsync(string[] paths, ILogger logger, CancellationToken cancellationToken) { BdnResult?[] results = await Task.WhenAll(paths.Select(path => ReadFromFileAsync(path, logger, cancellationToken))).ConfigureAwait(false); - return new BdnResults(Array.TrueForAll(results, static x => x is not null), results); + return new BdnResults(!results.Any(x => x is null), results); } private static async Task ReadFromFileAsync(string resultFilePath, ILogger logger, CancellationToken cancellationToken) diff --git a/src/tools/PerfDiff/PerfDiff.csproj b/src/tools/PerfDiff/PerfDiff.csproj index b55bebf95..26e81dd11 100644 --- a/src/tools/PerfDiff/PerfDiff.csproj +++ b/src/tools/PerfDiff/PerfDiff.csproj @@ -5,6 +5,7 @@ net8.0 true true + diff --git a/tests/PerfDiff.Tests/CancellationTests.cs b/tests/PerfDiff.Tests/CancellationTests.cs new file mode 100644 index 000000000..2b7b9f3a8 --- /dev/null +++ b/tests/PerfDiff.Tests/CancellationTests.cs @@ -0,0 +1,108 @@ +// Copyright (c) Microsoft. All Rights Reserved. Licensed under the MIT license. See License.txt in the project root for license information. + +using Microsoft.Extensions.Logging.Abstractions; +using Xunit; + +namespace PerfDiff.Tests; + +/// +/// Verifies CancellationToken propagation through the async call chain. +/// +public class CancellationTests +{ + /// + /// A pre-cancelled token passed to CompareAsync must not be silently swallowed. + /// CompareAsync calls token.ThrowIfCancellationRequested() at entry. + /// The exception propagates as OperationCanceledException. + /// + /// A representing the asynchronous test. + [Fact] + public async Task CompareAsync_PreCancelledToken_ThrowsOperationCanceledException() + { + using CancellationTokenSource cts = new(); + await cts.CancelAsync(); + + string tempDir = Path.Combine(Path.GetTempPath(), Path.GetRandomFileName()); + Directory.CreateDirectory(tempDir); + + try + { + await Assert.ThrowsAsync( + () => PerfDiff.CompareAsync( + baselineFolder: tempDir, + resultsFolder: tempDir, + failOnRegression: false, + logger: NullLogger.Instance, + token: cts.Token)); + } + finally + { + Directory.Delete(tempDir, recursive: true); + } + } + + /// + /// Program.RunAsync catches OperationCanceledException and returns exit code 1. + /// This is the expected contract: the CLI tool does not crash on cancellation. + /// + /// A representing the asynchronous test. + [Fact] + public async Task RunAsync_PreCancelledToken_ReturnsUnhandledExceptionExitCode() + { + using CancellationTokenSource cts = new(); + await cts.CancelAsync(); + + string tempDir = Path.Combine(Path.GetTempPath(), Path.GetRandomFileName()); + Directory.CreateDirectory(tempDir); + + try + { + int exitCode = await Program.RunAsync( + baseline: tempDir, + results: tempDir, + verbosity: null, + failOnRegression: false, + cancellationToken: cts.Token); + + Assert.Equal(Program.UnhandledExceptionExitCode, exitCode); + } + finally + { + Directory.Delete(tempDir, recursive: true); + } + } + + /// + /// Cancellation token is forwarded through BenchmarkFileReader.TryGetBdnResultAsync + /// all the way to File.ReadAllTextAsync. A pre-cancelled token causes the read to throw. + /// + /// A representing the asynchronous test. + [Fact] + public async Task BenchmarkFileReader_CancelledToken_DoesNotSwallowCancellation() + { + using CancellationTokenSource cts = new(); + + string tempDir = Path.Combine(Path.GetTempPath(), Path.GetRandomFileName()); + Directory.CreateDirectory(tempDir); + + // Write a valid-looking filename so the file list is non-empty. + string fakeFile = Path.Combine(tempDir, "results.full-compressed.json"); + await File.WriteAllTextAsync(fakeFile, "{}", CancellationToken.None); + + await cts.CancelAsync(); + + try + { + // TryGetBdnResultAsync will attempt File.ReadAllTextAsync with the cancelled token. + await Assert.ThrowsAnyAsync( + () => BDN.BenchmarkFileReader.TryGetBdnResultAsync( + paths: [fakeFile], + logger: NullLogger.Instance, + cancellationToken: cts.Token)); + } + finally + { + Directory.Delete(tempDir, recursive: true); + } + } +} diff --git a/tests/PerfDiff.Tests/DiffCommandTests.cs b/tests/PerfDiff.Tests/DiffCommandTests.cs new file mode 100644 index 000000000..60cc2f408 --- /dev/null +++ b/tests/PerfDiff.Tests/DiffCommandTests.cs @@ -0,0 +1,53 @@ +// Copyright (c) Microsoft. All Rights Reserved. Licensed under the MIT license. See License.txt in the project root for license information. + +using Microsoft.Extensions.Logging; +using Xunit; + +namespace PerfDiff.Tests; + +/// +/// Verifies DiffCommand CLI contract: verbosity mapping and option configuration. +/// +public class DiffCommandTests +{ +#pragma warning disable ECS0900 // Minimize boxing and unboxing - xUnit InlineData requires object parameters + [Theory] + [InlineData("q", LogLevel.Error)] + [InlineData("quiet", LogLevel.Error)] + [InlineData("m", LogLevel.Warning)] + [InlineData("minimal", LogLevel.Warning)] + [InlineData("n", LogLevel.Information)] + [InlineData("normal", LogLevel.Information)] + [InlineData("d", LogLevel.Debug)] + [InlineData("detailed", LogLevel.Debug)] + [InlineData("diag", LogLevel.Trace)] + [InlineData("diagnostic", LogLevel.Trace)] +#pragma warning restore ECS0900 + public void GetLogLevel_KnownVerbosity_ReturnsMappedLevel(string verbosity, LogLevel expected) + { + LogLevel actual = DiffCommand.GetLogLevel(verbosity); + Assert.Equal(expected, actual); + } + + [Theory] + [InlineData(null)] + [InlineData("")] + [InlineData("unknown")] + public void GetLogLevel_UnknownOrNullVerbosity_ReturnsInformation(string? verbosity) + { + LogLevel actual = DiffCommand.GetLogLevel(verbosity); + Assert.Equal(LogLevel.Information, actual); + } + + [Fact] + public void CreateCommandLineOptions_RootCommand_HasExpectedOptions() + { + System.CommandLine.RootCommand cmd = DiffCommand.CreateCommandLineOptions(); + + Assert.NotNull(cmd); + Assert.NotNull(DiffCommand.BaselineOption); + Assert.NotNull(DiffCommand.ResultsOption); + Assert.NotNull(DiffCommand.VerbosityOption); + Assert.NotNull(DiffCommand.FailOnRegressionOption); + } +} diff --git a/tests/PerfDiff.Tests/EtlPathTests.cs b/tests/PerfDiff.Tests/EtlPathTests.cs new file mode 100644 index 000000000..7f1b5ba6a --- /dev/null +++ b/tests/PerfDiff.Tests/EtlPathTests.cs @@ -0,0 +1,151 @@ +// Copyright (c) Microsoft. All Rights Reserved. Licensed under the MIT license. See License.txt in the project root for license information. + +using System.Reflection; +using Microsoft.Extensions.Logging; +using Xunit; + +namespace PerfDiff.Tests; + +/// +/// Verifies TryGetETLPaths behavior: FirstOrDefault selection and warning on multiple files. +/// TryGetETLPaths is private; accessed via reflection. +/// +public class EtlPathTests +{ + private static readonly MethodInfo TryGetETLPaths; + +#pragma warning disable ECS0600 // Cannot use nameof on a private method in another assembly +#pragma warning disable ECS1200 // Assignment in static constructor is required here; field initializer cannot throw meaningfully +#pragma warning disable S3963 // Static constructor is required to throw on missing method + static EtlPathTests() + { + TryGetETLPaths = typeof(PerfDiff).GetMethod( + "TryGetETLPaths", + BindingFlags.NonPublic | BindingFlags.Static) + ?? throw new InvalidOperationException("TryGetETLPaths not found."); + } +#pragma warning restore S3963 +#pragma warning restore ECS1200 +#pragma warning restore ECS0600 + + /// + /// With a single ETL file, returns that file and no warning is logged. + /// + [Fact] + public void TryGetETLPaths_SingleFile_ReturnsTrueWithPath() + { + string tempDir = Path.Combine(Path.GetTempPath(), Path.GetRandomFileName()); + Directory.CreateDirectory(tempDir); + string etlFile = Path.Combine(tempDir, "trace.etl.zip"); + File.WriteAllText(etlFile, string.Empty); + + try + { + CapturingLogger logger = new(); + object?[] parameters = [tempDir, logger, null]; +#pragma warning disable ECS0900 // Reflection Invoke returns object; unboxing is unavoidable + bool result = (bool)TryGetETLPaths.Invoke(null, parameters)!; +#pragma warning restore ECS0900 + string? etlPath = (string?)parameters[2]; + + Assert.True(result); + Assert.Equal(etlFile, etlPath); + Assert.Empty(logger.Warnings); + } + finally + { + Directory.Delete(tempDir, recursive: true); + } + } + + /// + /// With multiple ETL files, returns the first one and logs exactly one warning. + /// + [Fact] + public void TryGetETLPaths_MultipleFiles_ReturnsFirstAndLogsWarning() + { + string tempDir = Path.Combine(Path.GetTempPath(), Path.GetRandomFileName()); + Directory.CreateDirectory(tempDir); + + // Create two ETL files; GetFiles ordering is filesystem-dependent. + string etlFile1 = Path.Combine(tempDir, "trace_a.etl.zip"); + string etlFile2 = Path.Combine(tempDir, "trace_b.etl.zip"); + File.WriteAllText(etlFile1, string.Empty); + File.WriteAllText(etlFile2, string.Empty); + + try + { + CapturingLogger logger = new(); + object?[] parameters = [tempDir, logger, null]; +#pragma warning disable ECS0900 // Reflection Invoke returns object; unboxing is unavoidable + bool result = (bool)TryGetETLPaths.Invoke(null, parameters)!; +#pragma warning restore ECS0900 + string? etlPath = (string?)parameters[2]; + + Assert.True(result); + Assert.NotNull(etlPath); + Assert.EndsWith("etl.zip", etlPath, StringComparison.OrdinalIgnoreCase); + Assert.Single(logger.Warnings); + } + finally + { + Directory.Delete(tempDir, recursive: true); + } + } + + /// + /// With no ETL files, returns false and out param is null. + /// + [Fact] + public void TryGetETLPaths_NoFiles_ReturnsFalse() + { + string tempDir = Path.Combine(Path.GetTempPath(), Path.GetRandomFileName()); + Directory.CreateDirectory(tempDir); + + try + { + CapturingLogger logger = new(); + object?[] parameters = [tempDir, logger, null]; +#pragma warning disable ECS0900 // Reflection Invoke returns object; unboxing is unavoidable + bool result = (bool)TryGetETLPaths.Invoke(null, parameters)!; +#pragma warning restore ECS0900 + + Assert.False(result); + Assert.Null(parameters[2]); + } + finally + { + Directory.Delete(tempDir, recursive: true); + } + } + + /// + /// Minimal ILogger implementation that captures warning messages for assertion. + /// + private sealed class CapturingLogger : ILogger + { + public List Warnings { get; } = []; + + /// + public IDisposable? BeginScope(TState state) + where TState : notnull + => null; + + /// + public bool IsEnabled(LogLevel logLevel) => true; + + /// + public void Log( + LogLevel logLevel, + EventId eventId, + TState state, + Exception? exception, + Func formatter) + { + if (logLevel == LogLevel.Warning) + { + Warnings.Add(formatter(state, exception)); + } + } + } +} diff --git a/tests/PerfDiff.Tests/PerfDiff.Tests.csproj b/tests/PerfDiff.Tests/PerfDiff.Tests.csproj new file mode 100644 index 000000000..a3f030f03 --- /dev/null +++ b/tests/PerfDiff.Tests/PerfDiff.Tests.csproj @@ -0,0 +1,29 @@ + + + + net8.0 + false + true + + + + + + + + + + runtime; build; native; analyzers; buildtransitive + + + + + + + + From f1765f543dd5534b112b33dcf13b4e75e539b7b1 Mon Sep 17 00:00:00 2001 From: Richard Murillo Date: Sat, 14 Mar 2026 11:42:55 -0700 Subject: [PATCH 33/33] fix(PerfDiff): resolve unused variable warning in RunAsync Replace `await using ConfiguredAsyncDisposable asyncDisposal` with explicit try/finally and DisposeAsync().ConfigureAwait(false) to eliminate DeepSource CS-W1100 (unused variable) while preserving async disposal with ConfigureAwait(false) for MA0004 compliance. Co-Authored-By: Claude Opus 4.6 (1M context) --- .../PerfDiff/BDN/BenchmarkDotNetDiffer.cs | 18 ++- src/tools/PerfDiff/BDN/BenchmarkFileReader.cs | 2 +- src/tools/PerfDiff/PerfDiff.csproj | 1 - src/tools/PerfDiff/Program.cs | 28 ++-- tests/PerfDiff.Tests/CancellationTests.cs | 108 ------------- tests/PerfDiff.Tests/DiffCommandTests.cs | 53 ------ tests/PerfDiff.Tests/EtlPathTests.cs | 151 ------------------ tests/PerfDiff.Tests/PerfDiff.Tests.csproj | 29 ---- 8 files changed, 30 insertions(+), 360 deletions(-) delete mode 100644 tests/PerfDiff.Tests/CancellationTests.cs delete mode 100644 tests/PerfDiff.Tests/DiffCommandTests.cs delete mode 100644 tests/PerfDiff.Tests/EtlPathTests.cs delete mode 100644 tests/PerfDiff.Tests/PerfDiff.Tests.csproj diff --git a/src/tools/PerfDiff/BDN/BenchmarkDotNetDiffer.cs b/src/tools/PerfDiff/BDN/BenchmarkDotNetDiffer.cs index dd3c7161e..6e1cee5a1 100644 --- a/src/tools/PerfDiff/BDN/BenchmarkDotNetDiffer.cs +++ b/src/tools/PerfDiff/BDN/BenchmarkDotNetDiffer.cs @@ -54,9 +54,9 @@ public static async Task TryCompareBenchmarkDotNetRes return null; } - if (!baseFiles.Any() || !resultsFiles.Any()) + if (baseFiles.Length == 0 || resultsFiles.Length == 0) { - logger.LogError($"Provided paths contained no '{FullBdnJsonFileExtension}' files."); + logger.LogError("Provided paths contained no '{FileExtension}' files.", FullBdnJsonFileExtension); return null; } @@ -80,10 +80,16 @@ public static async Task TryCompareBenchmarkDotNetRes .SelectMany(result => result?.Benchmarks ?? Enumerable.Empty()) .ToDictionary(benchmarkResult => benchmarkResult.FullName ?? $"Unknown-{Guid.NewGuid():N}", benchmarkResult => benchmarkResult); - return benchmarkIdToBaseResults - .Where(baseResult => benchmarkIdToDiffResults.ContainsKey(baseResult.Key)) - .Select(baseResult => new BdnComparisonResult(baseResult.Key, baseResult.Value, benchmarkIdToDiffResults[baseResult.Key])) - .ToArray(); + List matched = []; + foreach (KeyValuePair baseResult in benchmarkIdToBaseResults) + { + if (benchmarkIdToDiffResults.TryGetValue(baseResult.Key, out Benchmark? diffBenchmark)) + { + matched.Add(new BdnComparisonResult(baseResult.Key, baseResult.Value, diffBenchmark)); + } + } + + return matched.ToArray(); } private static bool TryGetFilesToParse(string path, [NotNullWhen(true)] out string[]? files) diff --git a/src/tools/PerfDiff/BDN/BenchmarkFileReader.cs b/src/tools/PerfDiff/BDN/BenchmarkFileReader.cs index 7d768c04b..03b8f0db2 100644 --- a/src/tools/PerfDiff/BDN/BenchmarkFileReader.cs +++ b/src/tools/PerfDiff/BDN/BenchmarkFileReader.cs @@ -19,7 +19,7 @@ public static class BenchmarkFileReader public static async Task TryGetBdnResultAsync(string[] paths, ILogger logger, CancellationToken cancellationToken) { BdnResult?[] results = await Task.WhenAll(paths.Select(path => ReadFromFileAsync(path, logger, cancellationToken))).ConfigureAwait(false); - return new BdnResults(!results.Any(x => x is null), results); + return new BdnResults(Array.TrueForAll(results, static x => x is not null), results); } private static async Task ReadFromFileAsync(string resultFilePath, ILogger logger, CancellationToken cancellationToken) diff --git a/src/tools/PerfDiff/PerfDiff.csproj b/src/tools/PerfDiff/PerfDiff.csproj index 26e81dd11..b55bebf95 100644 --- a/src/tools/PerfDiff/PerfDiff.csproj +++ b/src/tools/PerfDiff/PerfDiff.csproj @@ -5,7 +5,6 @@ net8.0 true true - diff --git a/src/tools/PerfDiff/Program.cs b/src/tools/PerfDiff/Program.cs index 9fc719dc9..75cbb7a26 100644 --- a/src/tools/PerfDiff/Program.cs +++ b/src/tools/PerfDiff/Program.cs @@ -2,7 +2,6 @@ using System.CommandLine; using System.IO; -using System.Runtime.CompilerServices; using System.Threading; using System.Threading.Tasks; using Microsoft.Extensions.DependencyInjection; @@ -41,23 +40,30 @@ internal static async Task RunAsync( // Setup logging. LogLevel logLevel = DiffCommand.GetLogLevel(verbosity); ServiceProvider serviceProvider = SetupLogging(minimalLogLevel: logLevel, minimalErrorLevel: LogLevel.Warning); - await using ConfiguredAsyncDisposable asyncDisposal = serviceProvider.ConfigureAwait(false); - ILogger logger = serviceProvider.GetRequiredService>(); try { - return await PerfDiff.CompareAsync(baseline, results, failOnRegression, logger, cancellationToken).ConfigureAwait(false); - } - catch (FileNotFoundException fex) - { + ILogger logger = serviceProvider.GetRequiredService>(); + + try + { + return await PerfDiff.CompareAsync(baseline, results, failOnRegression, logger, cancellationToken).ConfigureAwait(false); + } + catch (FileNotFoundException fex) + { #pragma warning disable CA1848, CA2254 // LoggerMessage delegates, varying template - logger.LogError(fex, "File not found: {FileName}", fex.FileName); + logger.LogError(fex, "File not found: {FileName}", fex.FileName); #pragma warning restore CA1848, CA2254 - return UnhandledExceptionExitCode; + return UnhandledExceptionExitCode; + } + catch (OperationCanceledException) + { + return UnhandledExceptionExitCode; + } } - catch (OperationCanceledException) + finally { - return UnhandledExceptionExitCode; + await serviceProvider.DisposeAsync().ConfigureAwait(false); } static ServiceProvider SetupLogging(LogLevel minimalLogLevel, LogLevel minimalErrorLevel) diff --git a/tests/PerfDiff.Tests/CancellationTests.cs b/tests/PerfDiff.Tests/CancellationTests.cs deleted file mode 100644 index 2b7b9f3a8..000000000 --- a/tests/PerfDiff.Tests/CancellationTests.cs +++ /dev/null @@ -1,108 +0,0 @@ -// Copyright (c) Microsoft. All Rights Reserved. Licensed under the MIT license. See License.txt in the project root for license information. - -using Microsoft.Extensions.Logging.Abstractions; -using Xunit; - -namespace PerfDiff.Tests; - -/// -/// Verifies CancellationToken propagation through the async call chain. -/// -public class CancellationTests -{ - /// - /// A pre-cancelled token passed to CompareAsync must not be silently swallowed. - /// CompareAsync calls token.ThrowIfCancellationRequested() at entry. - /// The exception propagates as OperationCanceledException. - /// - /// A representing the asynchronous test. - [Fact] - public async Task CompareAsync_PreCancelledToken_ThrowsOperationCanceledException() - { - using CancellationTokenSource cts = new(); - await cts.CancelAsync(); - - string tempDir = Path.Combine(Path.GetTempPath(), Path.GetRandomFileName()); - Directory.CreateDirectory(tempDir); - - try - { - await Assert.ThrowsAsync( - () => PerfDiff.CompareAsync( - baselineFolder: tempDir, - resultsFolder: tempDir, - failOnRegression: false, - logger: NullLogger.Instance, - token: cts.Token)); - } - finally - { - Directory.Delete(tempDir, recursive: true); - } - } - - /// - /// Program.RunAsync catches OperationCanceledException and returns exit code 1. - /// This is the expected contract: the CLI tool does not crash on cancellation. - /// - /// A representing the asynchronous test. - [Fact] - public async Task RunAsync_PreCancelledToken_ReturnsUnhandledExceptionExitCode() - { - using CancellationTokenSource cts = new(); - await cts.CancelAsync(); - - string tempDir = Path.Combine(Path.GetTempPath(), Path.GetRandomFileName()); - Directory.CreateDirectory(tempDir); - - try - { - int exitCode = await Program.RunAsync( - baseline: tempDir, - results: tempDir, - verbosity: null, - failOnRegression: false, - cancellationToken: cts.Token); - - Assert.Equal(Program.UnhandledExceptionExitCode, exitCode); - } - finally - { - Directory.Delete(tempDir, recursive: true); - } - } - - /// - /// Cancellation token is forwarded through BenchmarkFileReader.TryGetBdnResultAsync - /// all the way to File.ReadAllTextAsync. A pre-cancelled token causes the read to throw. - /// - /// A representing the asynchronous test. - [Fact] - public async Task BenchmarkFileReader_CancelledToken_DoesNotSwallowCancellation() - { - using CancellationTokenSource cts = new(); - - string tempDir = Path.Combine(Path.GetTempPath(), Path.GetRandomFileName()); - Directory.CreateDirectory(tempDir); - - // Write a valid-looking filename so the file list is non-empty. - string fakeFile = Path.Combine(tempDir, "results.full-compressed.json"); - await File.WriteAllTextAsync(fakeFile, "{}", CancellationToken.None); - - await cts.CancelAsync(); - - try - { - // TryGetBdnResultAsync will attempt File.ReadAllTextAsync with the cancelled token. - await Assert.ThrowsAnyAsync( - () => BDN.BenchmarkFileReader.TryGetBdnResultAsync( - paths: [fakeFile], - logger: NullLogger.Instance, - cancellationToken: cts.Token)); - } - finally - { - Directory.Delete(tempDir, recursive: true); - } - } -} diff --git a/tests/PerfDiff.Tests/DiffCommandTests.cs b/tests/PerfDiff.Tests/DiffCommandTests.cs deleted file mode 100644 index 60cc2f408..000000000 --- a/tests/PerfDiff.Tests/DiffCommandTests.cs +++ /dev/null @@ -1,53 +0,0 @@ -// Copyright (c) Microsoft. All Rights Reserved. Licensed under the MIT license. See License.txt in the project root for license information. - -using Microsoft.Extensions.Logging; -using Xunit; - -namespace PerfDiff.Tests; - -/// -/// Verifies DiffCommand CLI contract: verbosity mapping and option configuration. -/// -public class DiffCommandTests -{ -#pragma warning disable ECS0900 // Minimize boxing and unboxing - xUnit InlineData requires object parameters - [Theory] - [InlineData("q", LogLevel.Error)] - [InlineData("quiet", LogLevel.Error)] - [InlineData("m", LogLevel.Warning)] - [InlineData("minimal", LogLevel.Warning)] - [InlineData("n", LogLevel.Information)] - [InlineData("normal", LogLevel.Information)] - [InlineData("d", LogLevel.Debug)] - [InlineData("detailed", LogLevel.Debug)] - [InlineData("diag", LogLevel.Trace)] - [InlineData("diagnostic", LogLevel.Trace)] -#pragma warning restore ECS0900 - public void GetLogLevel_KnownVerbosity_ReturnsMappedLevel(string verbosity, LogLevel expected) - { - LogLevel actual = DiffCommand.GetLogLevel(verbosity); - Assert.Equal(expected, actual); - } - - [Theory] - [InlineData(null)] - [InlineData("")] - [InlineData("unknown")] - public void GetLogLevel_UnknownOrNullVerbosity_ReturnsInformation(string? verbosity) - { - LogLevel actual = DiffCommand.GetLogLevel(verbosity); - Assert.Equal(LogLevel.Information, actual); - } - - [Fact] - public void CreateCommandLineOptions_RootCommand_HasExpectedOptions() - { - System.CommandLine.RootCommand cmd = DiffCommand.CreateCommandLineOptions(); - - Assert.NotNull(cmd); - Assert.NotNull(DiffCommand.BaselineOption); - Assert.NotNull(DiffCommand.ResultsOption); - Assert.NotNull(DiffCommand.VerbosityOption); - Assert.NotNull(DiffCommand.FailOnRegressionOption); - } -} diff --git a/tests/PerfDiff.Tests/EtlPathTests.cs b/tests/PerfDiff.Tests/EtlPathTests.cs deleted file mode 100644 index 7f1b5ba6a..000000000 --- a/tests/PerfDiff.Tests/EtlPathTests.cs +++ /dev/null @@ -1,151 +0,0 @@ -// Copyright (c) Microsoft. All Rights Reserved. Licensed under the MIT license. See License.txt in the project root for license information. - -using System.Reflection; -using Microsoft.Extensions.Logging; -using Xunit; - -namespace PerfDiff.Tests; - -/// -/// Verifies TryGetETLPaths behavior: FirstOrDefault selection and warning on multiple files. -/// TryGetETLPaths is private; accessed via reflection. -/// -public class EtlPathTests -{ - private static readonly MethodInfo TryGetETLPaths; - -#pragma warning disable ECS0600 // Cannot use nameof on a private method in another assembly -#pragma warning disable ECS1200 // Assignment in static constructor is required here; field initializer cannot throw meaningfully -#pragma warning disable S3963 // Static constructor is required to throw on missing method - static EtlPathTests() - { - TryGetETLPaths = typeof(PerfDiff).GetMethod( - "TryGetETLPaths", - BindingFlags.NonPublic | BindingFlags.Static) - ?? throw new InvalidOperationException("TryGetETLPaths not found."); - } -#pragma warning restore S3963 -#pragma warning restore ECS1200 -#pragma warning restore ECS0600 - - /// - /// With a single ETL file, returns that file and no warning is logged. - /// - [Fact] - public void TryGetETLPaths_SingleFile_ReturnsTrueWithPath() - { - string tempDir = Path.Combine(Path.GetTempPath(), Path.GetRandomFileName()); - Directory.CreateDirectory(tempDir); - string etlFile = Path.Combine(tempDir, "trace.etl.zip"); - File.WriteAllText(etlFile, string.Empty); - - try - { - CapturingLogger logger = new(); - object?[] parameters = [tempDir, logger, null]; -#pragma warning disable ECS0900 // Reflection Invoke returns object; unboxing is unavoidable - bool result = (bool)TryGetETLPaths.Invoke(null, parameters)!; -#pragma warning restore ECS0900 - string? etlPath = (string?)parameters[2]; - - Assert.True(result); - Assert.Equal(etlFile, etlPath); - Assert.Empty(logger.Warnings); - } - finally - { - Directory.Delete(tempDir, recursive: true); - } - } - - /// - /// With multiple ETL files, returns the first one and logs exactly one warning. - /// - [Fact] - public void TryGetETLPaths_MultipleFiles_ReturnsFirstAndLogsWarning() - { - string tempDir = Path.Combine(Path.GetTempPath(), Path.GetRandomFileName()); - Directory.CreateDirectory(tempDir); - - // Create two ETL files; GetFiles ordering is filesystem-dependent. - string etlFile1 = Path.Combine(tempDir, "trace_a.etl.zip"); - string etlFile2 = Path.Combine(tempDir, "trace_b.etl.zip"); - File.WriteAllText(etlFile1, string.Empty); - File.WriteAllText(etlFile2, string.Empty); - - try - { - CapturingLogger logger = new(); - object?[] parameters = [tempDir, logger, null]; -#pragma warning disable ECS0900 // Reflection Invoke returns object; unboxing is unavoidable - bool result = (bool)TryGetETLPaths.Invoke(null, parameters)!; -#pragma warning restore ECS0900 - string? etlPath = (string?)parameters[2]; - - Assert.True(result); - Assert.NotNull(etlPath); - Assert.EndsWith("etl.zip", etlPath, StringComparison.OrdinalIgnoreCase); - Assert.Single(logger.Warnings); - } - finally - { - Directory.Delete(tempDir, recursive: true); - } - } - - /// - /// With no ETL files, returns false and out param is null. - /// - [Fact] - public void TryGetETLPaths_NoFiles_ReturnsFalse() - { - string tempDir = Path.Combine(Path.GetTempPath(), Path.GetRandomFileName()); - Directory.CreateDirectory(tempDir); - - try - { - CapturingLogger logger = new(); - object?[] parameters = [tempDir, logger, null]; -#pragma warning disable ECS0900 // Reflection Invoke returns object; unboxing is unavoidable - bool result = (bool)TryGetETLPaths.Invoke(null, parameters)!; -#pragma warning restore ECS0900 - - Assert.False(result); - Assert.Null(parameters[2]); - } - finally - { - Directory.Delete(tempDir, recursive: true); - } - } - - /// - /// Minimal ILogger implementation that captures warning messages for assertion. - /// - private sealed class CapturingLogger : ILogger - { - public List Warnings { get; } = []; - - /// - public IDisposable? BeginScope(TState state) - where TState : notnull - => null; - - /// - public bool IsEnabled(LogLevel logLevel) => true; - - /// - public void Log( - LogLevel logLevel, - EventId eventId, - TState state, - Exception? exception, - Func formatter) - { - if (logLevel == LogLevel.Warning) - { - Warnings.Add(formatter(state, exception)); - } - } - } -} diff --git a/tests/PerfDiff.Tests/PerfDiff.Tests.csproj b/tests/PerfDiff.Tests/PerfDiff.Tests.csproj deleted file mode 100644 index a3f030f03..000000000 --- a/tests/PerfDiff.Tests/PerfDiff.Tests.csproj +++ /dev/null @@ -1,29 +0,0 @@ - - - - net8.0 - false - true - - - - - - - - - - runtime; build; native; analyzers; buildtransitive - - - - - - - -