diff --git a/src/BenchmarkDotNet/ConsoleArguments/CommandLineOptions.cs b/src/BenchmarkDotNet/ConsoleArguments/CommandLineOptions.cs index d660db42ba..ff512d80ce 100644 --- a/src/BenchmarkDotNet/ConsoleArguments/CommandLineOptions.cs +++ b/src/BenchmarkDotNet/ConsoleArguments/CommandLineOptions.cs @@ -274,5 +274,127 @@ public static IEnumerable Examples } private static string Escape(string input) => UserInteractionHelper.EscapeCommandExample(input); + + /// + /// All options, keyed by short and long name (case-insensitive). + /// Value is the canonical long name. + /// Keep in sync with the [Option] attributes above (hardcoded by design, no reflection for NativeAOT/trimming safety). + /// + internal static readonly IReadOnlyDictionary CanonicalNames = + new Dictionary(StringComparer.OrdinalIgnoreCase) + { + // Aliases + ["j"] = "job", + ["r"] = "runtimes", + ["e"] = "exporters", + ["m"] = "memory", + ["t"] = "threading", + ["d"] = "disasm", + ["p"] = "profiler", + ["f"] = "filter", + ["h"] = "hide", + ["i"] = "inProcess", + ["a"] = "artifacts", + + // Names + ["job"] = "job", + ["runtimes"] = "runtimes", + ["exporters"] = "exporters", + ["memory"] = "memory", + ["threading"] = "threading", + ["exceptions"] = "exceptions", + ["disasm"] = "disasm", + ["profiler"] = "profiler", + ["filter"] = "filter", + ["hide"] = "hide", + ["inProcess"] = "inProcess", + ["artifacts"] = "artifacts", + ["outliers"] = "outliers", + ["affinity"] = "affinity", + ["allStats"] = "allStats", + ["allCategories"] = "allCategories", + ["anyCategories"] = "anyCategories", + ["anyJobCategories"] = "anyJobCategories", + ["attribute"] = "attribute", + ["join"] = "join", + ["title"] = "title", + ["keepFiles"] = "keepFiles", + ["noOverwrite"] = "noOverwrite", + ["counters"] = "counters", + ["cli"] = "cli", + ["packages"] = "packages", + ["freshPackages"] = "freshPackages", + ["coreRun"] = "coreRun", + ["monoPath"] = "monoPath", + ["ilCompilerVersion"] = "ilCompilerVersion", + ["ilcPackages"] = "ilcPackages", + ["launchCount"] = "launchCount", + ["warmupCount"] = "warmupCount", + ["minWarmupCount"] = "minWarmupCount", + ["maxWarmupCount"] = "maxWarmupCount", + ["iterationTime"] = "iterationTime", + ["iterationCount"] = "iterationCount", + ["minIterationCount"] = "minIterationCount", + ["maxIterationCount"] = "maxIterationCount", + ["invocationCount"] = "invocationCount", + ["unrollFactor"] = "unrollFactor", + ["strategy"] = "strategy", + ["platform"] = "platform", + ["runOncePerIteration"] = "runOncePerIteration", + ["info"] = "info", + ["apples"] = "apples", + ["list"] = "list", + ["disasmDepth"] = "disasmDepth", + ["disasmFilter"] = "disasmFilter", + ["disasmDiff"] = "disasmDiff", + ["logBuildOutput"] = "logBuildOutput", + ["generateBinLog"] = "generateBinLog", + ["buildTimeout"] = "buildTimeout", + ["wakeLock"] = "wakeLock", + ["stopOnFirstError"] = "stopOnFirstError", + ["statisticalTest"] = "statisticalTest", + ["disableLogFile"] = "disableLogFile", + ["maxWidth"] = "maxWidth", + ["envVars"] = "envVars", + ["memoryRandomization"] = "memoryRandomization", + ["jitTieringMode"] = "jitTieringMode", + ["wasmEngine"] = "wasmEngine", + ["wasmArgs"] = "wasmArgs", + ["wasmMainJsTemplate"] = "wasmMainJsTemplate", + ["customRuntimePack"] = "customRuntimePack", + ["AOTCompilerPath"] = "AOTCompilerPath", + ["wasmProcessTimeout"] = "wasmProcessTimeout", + ["noForcedGCs"] = "noForcedGCs", + ["evaluateOverhead"] = "evaluateOverhead", + ["consumeTasksSynchronously"] = "consumeTasksSynchronously", + ["resume"] = "resume" + }; + + /// + /// Canonical long names of multi values options (It can be specified multiple times) + /// + internal static readonly ISet MultiValueOptionNames = + new HashSet(StringComparer.OrdinalIgnoreCase) + { + "runtimes", + "exporters", + "filter", + "hide", + "allCategories", + "anyCategories", + "anyJobCategories", + "attribute", + "counters", + "coreRun", + "envVars", + "disasmFilter" + }; + + /// + /// Canonical long names of scalar value options. + /// These options must appear at most once. + /// + internal static readonly ISet ScalarValueOptionNames = + new HashSet(CanonicalNames.Values.Except(MultiValueOptionNames), StringComparer.OrdinalIgnoreCase); } } diff --git a/src/BenchmarkDotNet/ConsoleArguments/ConfigParser.cs b/src/BenchmarkDotNet/ConsoleArguments/ConfigParser.cs index 538c5e0a44..7f87804190 100644 --- a/src/BenchmarkDotNet/ConsoleArguments/ConfigParser.cs +++ b/src/BenchmarkDotNet/ConsoleArguments/ConfigParser.cs @@ -15,11 +15,12 @@ using BenchmarkDotNet.Reports; using BenchmarkDotNet.Toolchains; using BenchmarkDotNet.Toolchains.CoreRun; +using BenchmarkDotNet.Toolchains.Framework; using BenchmarkDotNet.Toolchains.Mono; -using BenchmarkDotNet.Toolchains.Wasm; using BenchmarkDotNet.Toolchains.NativeAot; -using BenchmarkDotNet.Toolchains.Framework; +using BenchmarkDotNet.Toolchains.NetCoreApp; using BenchmarkDotNet.Toolchains.R2R; +using BenchmarkDotNet.Toolchains.Wasm; using CommandLine; using Perfolizer.Horology; using Perfolizer.Mathematics.OutlierDetection; @@ -27,7 +28,6 @@ using System.Diagnostics.CodeAnalysis; using System.Globalization; using System.Text; -using BenchmarkDotNet.Toolchains.NetCoreApp; namespace BenchmarkDotNet.ConsoleArguments { @@ -127,6 +127,12 @@ public static (bool isSuccess, IConfig? config, CommandLineOptions? options) Par } args = expandedArgs; + if (TryFindDuplicateScalarOption(args, out string? duplicateOptionName)) + { + logger.WriteLineError($"Option '--{duplicateOptionName}' is defined multiple times."); + return (false, default, default); + } + using (var parser = CreateParser(logger)) { parser @@ -244,6 +250,12 @@ internal static bool TryUpdateArgs(string[] args, out string[]? updatedArgs, Act (bool isSuccess, CommandLineOptions? options) result = default; ILogger logger = NullLogger.Instance; + if (TryFindDuplicateScalarOption(args, out _)) + { + updatedArgs = null; + return false; + } + using (var parser = CreateParser(logger)) { parser @@ -267,6 +279,7 @@ internal static bool TryUpdateArgs(string[] args, out string[]? updatedArgs, Act private static Parser CreateParser(ILogger logger) => new Parser(settings => { + settings.AllowMultiInstance = true; settings.CaseInsensitiveEnumValues = true; settings.CaseSensitive = false; settings.EnableDashDash = true; @@ -275,6 +288,79 @@ private static Parser CreateParser(ILogger logger) settings.MaximumDisplayWidth = Math.Max(MinimumDisplayWidth, GetMaximumDisplayWidth()); }); + /// + /// Detects a repeated scalar option in the (response-file expanded) args. + /// + internal static bool TryFindDuplicateScalarOption(string[] args, out string? duplicateOptionName) + { + var hashSet = new HashSet(StringComparer.OrdinalIgnoreCase); + duplicateOptionName = null; + + foreach (string arg in args) + { + if (arg.Equals("--", StringComparison.Ordinal)) + break; + + if (arg.StartsWith("--", StringComparison.Ordinal)) + { + string text = arg.Substring(2); + if (text.Length == 0) + continue; + + int equalsIndex = text.IndexOf('='); + string name = equalsIndex >= 0 ? text.Substring(0, equalsIndex) : text; + if (name.Length == 0) + continue; + + if (!TryGetScalarValueOption(name, out var canonicalName)) + continue; + + if (!hashSet.Add(canonicalName)) + { + duplicateOptionName = canonicalName; + return true; + } + } + else if (arg.StartsWith("-", StringComparison.Ordinal) && arg.Length >= 2) + { + // Truncate an attached value (e.g. "-j=dry" scans only "j"), mirroring the long-option branch. + string shorts = arg.Substring(1); + int equalsIndex = shorts.IndexOf('='); + if (equalsIndex >= 0) + shorts = shorts.Substring(0, equalsIndex); + + // Handle bundling of single-character options (e.g. -tm is equivalent to -t -m). + // Scanning stops at the first unknown char: like the parser, the rest is treated as a value. + foreach (char shortChar in shorts) + { + string shortName = shortChar.ToString(); + if (!TryGetScalarValueOption(shortName, out var canonicalName)) + break; + + if (!hashSet.Add(canonicalName)) + { + duplicateOptionName = canonicalName; + return true; + } + } + } + } + + return false; + } + + private static bool TryGetScalarValueOption( + string name, + [NotNullWhen(true)] out string? canonicalName) + { + if (CommandLineOptions.CanonicalNames.TryGetValue(name, out canonicalName)) + return CommandLineOptions.ScalarValueOptionNames.Contains(canonicalName); + + // Specified name is not a known option or it's not a scalar value option. + canonicalName = null; + return false; + } + private static bool Validate(CommandLineOptions options, ILogger logger) { if (options.BaseJob.IsBlank() || !AvailableJobs.ContainsKey(options.BaseJob)) diff --git a/tests/BenchmarkDotNet.Tests/CommandLineOptionsTests.cs b/tests/BenchmarkDotNet.Tests/CommandLineOptionsTests.cs new file mode 100644 index 0000000000..fe3e0647c0 --- /dev/null +++ b/tests/BenchmarkDotNet.Tests/CommandLineOptionsTests.cs @@ -0,0 +1,76 @@ +using AwesomeAssertions; +using BenchmarkDotNet.ConsoleArguments; +using BenchmarkDotNet.Extensions; +using CommandLine; +using System.Reflection; + +namespace BenchmarkDotNet.Tests; + +public class CommandLineOptionsTests +{ + private sealed record OptionProperty(PropertyInfo Property, OptionAttribute Option); + + /// + /// Validates that the hardcoded dictionaries in CommandLineOptions (CanonicalNames, MultiInstanceOptionNames) + /// are consistent with the actual option definitions in the CommandLineOptions class. + /// + [Fact] + public void ValidateCommandLineOptionDefinitions() + { + // Arrange + // Collect all option definitions of CommandLineOptions via reflection. + var optionProperties = typeof(CommandLineOptions) + .GetProperties(BindingFlags.Public | BindingFlags.Instance) + .Select(property => (Property: property, Option: property.GetCustomAttribute())) + .Where(entry => entry.Option is not null) + .Select(x => new OptionProperty(x.Property, x.Option!)) + .ToArray(); + + // Gets all long names. + var longNames = optionProperties + .Select(entry => entry.Option.LongName) + .ToHashSet(StringComparer.OrdinalIgnoreCase); + + // Gets all names (long and short) + var allNames = longNames.ToHashSet(StringComparer.OrdinalIgnoreCase); + foreach (var option in optionProperties.Where(x => x.Option.ShortName.IsNotBlank())) + allNames.Add(option.Option.ShortName); + + // Act + var canonicalNames = CommandLineOptions.CanonicalNames; + var multiInstanceOptionNames = CommandLineOptions.MultiValueOptionNames; + + // Assert + // Validate CanonicalNames definitions. + canonicalNames.Keys.Should().BeEquivalentTo(allNames); + canonicalNames.Values.Distinct().Should().BeEquivalentTo(longNames); + + // Validate each short/long name resolves to its own canonical long name + // (Without this test, a swapped alias would pass the set checks above). + var expectedCanonicals = optionProperties + .SelectMany(entry => new[] + { + (Name: entry.Option!.LongName, Canonical: entry.Option.LongName), + (Name: entry.Option.ShortName, Canonical: entry.Option.LongName) + }) + .Where(entry => entry.Name.IsNotBlank()) + .ToArray(); + + foreach (var (name, expected) in expectedCanonicals) + { + bool isSuccess = canonicalNames.TryGetValue(name, out string? actual); + isSuccess.Should().BeTrue($"Name '{name}' should be resolve from CanonicalNames."); + actual.Should().Be(expected, $"Name '{name}' should resolve to '{expected}'."); + } + + // Validate MultiInstanceOptionNames definitions are consistent with the property types. + foreach (var (property, option) in optionProperties) + { + bool isMultiValue = property.PropertyType != typeof(string) && typeof(System.Collections.IEnumerable).IsAssignableFrom(property.PropertyType); + if (isMultiValue) + multiInstanceOptionNames.Should().Contain(option.LongName); + else + multiInstanceOptionNames.Should().NotContain(option.LongName); + } + } +} diff --git a/tests/BenchmarkDotNet.Tests/ConfigParserTests.cs b/tests/BenchmarkDotNet.Tests/ConfigParserTests.cs index 88f48e37e3..1abd91bbb1 100644 --- a/tests/BenchmarkDotNet.Tests/ConfigParserTests.cs +++ b/tests/BenchmarkDotNet.Tests/ConfigParserTests.cs @@ -6,30 +6,30 @@ using BenchmarkDotNet.Engines; using BenchmarkDotNet.Environments; using BenchmarkDotNet.Exporters; -using BenchmarkDotNet.Helpers; -using BenchmarkDotNet.Reports; using BenchmarkDotNet.Exporters.Csv; using BenchmarkDotNet.Exporters.Json; using BenchmarkDotNet.Exporters.OpenMetrics; using BenchmarkDotNet.Exporters.Xml; +using BenchmarkDotNet.Helpers; using BenchmarkDotNet.Jobs; using BenchmarkDotNet.Loggers; using BenchmarkDotNet.Portability; +using BenchmarkDotNet.Reports; using BenchmarkDotNet.Tests.Loggers; using BenchmarkDotNet.Tests.Mocks; using BenchmarkDotNet.Tests.XUnit; using BenchmarkDotNet.Toolchains; using BenchmarkDotNet.Toolchains.CoreRun; using BenchmarkDotNet.Toolchains.DotNetCli; +using BenchmarkDotNet.Toolchains.Framework; using BenchmarkDotNet.Toolchains.InProcess.Emit; using BenchmarkDotNet.Toolchains.Mono; -using BenchmarkDotNet.Toolchains.Wasm; using BenchmarkDotNet.Toolchains.NativeAot; -using BenchmarkDotNet.Toolchains.Framework; -using Perfolizer.Horology; -using System.Reflection; using BenchmarkDotNet.Toolchains.NetCoreApp; using BenchmarkDotNet.Toolchains.R2R; +using BenchmarkDotNet.Toolchains.Wasm; +using Perfolizer.Horology; +using System.Reflection; namespace BenchmarkDotNet.Tests { @@ -1025,6 +1025,7 @@ public void UserCanSpecifyWasmMainJsTemplate() [InlineData("--filter abc", "--filter *")] [InlineData("-f abc", "--filter *")] [InlineData("-f *", "--filter *")] + [InlineData("--filter abc -f abc", "--filter *")] [InlineData("--runtimes net7.0 --join", "--filter * --join --runtimes net7.0")] [InlineData("--join abc", "--filter * --join")] public void CheckUpdateValidArgs(string strArgs, string expected) @@ -1036,15 +1037,20 @@ public void CheckUpdateValidArgs(string strArgs, string expected) } [Theory] - [InlineData("--filter abc -f abc")] [InlineData("--runtimes net")] + [InlineData("--job dry --job short")] + [InlineData("--join --join")] public void CheckUpdateInvalidArgs(string strArgs) { + // Arrange var args = strArgs.Split(); + + // Act bool isSuccess = ConfigParser.TryUpdateArgs(args, out var updatedArgs, options => options.Filters = ["*"]); - Assert.Null(updatedArgs); - Assert.False(isSuccess); + // Assert + updatedArgs.Should().BeNull(); + isSuccess.Should().BeFalse(); } private string GetDummyWasmEngine() @@ -1052,5 +1058,252 @@ private string GetDummyWasmEngine() // We know, that this file exists, that's enough. return $"--wasmEngine={Assembly.GetExecutingAssembly().Location}"; } + + [Fact] + public void UserCanSpecifyRuntimes_WithMultipleOptions() + { + // Arrange + var logger = new OutputLogger(Output); + string[] args = ["--runtimes", "net8.0", "--runtimes", "net9.0"]; + + // Act + var (isSuccess, config, options) = ConfigParser.Parse(args, logger); + + // Assert + isSuccess.Should().BeTrue(); + config.Should().NotBeNull(); + options.Should().NotBeNull(); + options.Runtimes.Should().Equal("net8.0", "net9.0"); + config.GetJobs().Should().HaveCount(2); + config.GetJobs().First().Meta.Baseline.Should().BeTrue(); + logger.GetLog().Should().BeEmpty(); + } + + [Fact] + public void UserCanSpecifyExporters_WithMultipleOptions() + { + // Arrange + var logger = new OutputLogger(Output); + string[] args = ["--exporters", "json", "--exporters", "html"]; + + // Act + var (isSuccess, config, _) = ConfigParser.Parse(args, logger); + + // Assert + isSuccess.Should().BeTrue(); + config.Should().NotBeNull(); + config.GetExporters().Should().Contain(JsonExporter.Default); + config.GetExporters().Should().Contain(HtmlExporter.Default); + logger.GetLog().Should().BeEmpty(); + } + + [Theory] + [InlineData("--filter", "A", "--filter", "B")] + [InlineData("-f", "A", "--filter", "B")] + public void UserCanSpecifyFilter_WithMultipleOptions(params string[] args) + { + // Arrange + var logger = new OutputLogger(Output); + + // Act + var (isSuccess, _, options) = ConfigParser.Parse(args, logger); + + // Assert + isSuccess.Should().BeTrue(); + options.Should().NotBeNull(); + options.Filters.Should().Equal("A", "B"); + logger.GetLog().Should().BeEmpty(); + } + + [Fact] + public void UserCanSpecifyCoreRunPaths_WithMultipleOptions() + { + // Arrange + var logger = new OutputLogger(Output); + var path1 = typeof(object).Assembly.Location; + var path2 = typeof(ConfigParserTests).Assembly.Location; + + // Act + var (isSuccess, config, _) = ConfigParser.Parse(["--coreRun", path1, "--coreRun", path2], logger); + + // Assert + isSuccess.Should().BeTrue(); + config.Should().NotBeNull(); + config.GetJobs().Should().HaveCount(2); + logger.GetLog().Should().BeEmpty(); + } + + [Fact] + public void UserCanSpecifyEnvVars_WithMultipleOptions() + { + // Arrange + var logger = new OutputLogger(Output); + string[] args = ["--envVars", "K1:V1", "--envVars", "K2:V2"]; + + // Act + var (isSuccess, config, _) = ConfigParser.Parse(args, logger); + + // Assert + isSuccess.Should().BeTrue(); + config.Should().NotBeNull(); + config.GetJobs().Single().Environment.EnvironmentVariables.Should().HaveCount(2); + logger.GetLog().Should().BeEmpty(); + } + + [Fact] + public void UserCanSpecifyRuntimes_WithSingleOption() + { + // Arrange + var logger = new OutputLogger(Output); + string[] args = ["--runtimes", "net8.0", "net9.0"]; + + // Act + var (isSuccess, config, options) = ConfigParser.Parse(args, logger); + + // Assert + isSuccess.Should().BeTrue(); + config.Should().NotBeNull(); + options.Should().NotBeNull(); + config.GetJobs().Should().HaveCount(2); + logger.GetLog().Should().BeEmpty(); + } + + [Fact] + public void UserCanSpecifyCounters_WithMultipleOptions() + { + // Arrange + var logger = new OutputLogger(Output); + string[] args = + [ + "--counters", $"{nameof(HardwareCounter.CacheMisses)}+{nameof(HardwareCounter.InstructionRetired)}", + "--counters", nameof(HardwareCounter.BranchMispredictions) + ]; + + // Act + var (isSuccess, config, _) = ConfigParser.Parse(args, logger); + + // Assert + isSuccess.Should().BeTrue(); + config.Should().NotBeNull(); + config!.GetHardwareCounters().Should().HaveCount(3); + logger.GetLog().Should().BeEmpty(); + } + + [Fact] + public void OptionAfterDashDashIsTreatedAsValue() + { + // Arrange + var logger = new OutputLogger(Output); + string[] args = ["--job", "dry", "--", "--job", "short"]; + + // Act + var (isSuccess, _, _) = ConfigParser.Parse(args, logger); + + // Assert + isSuccess.Should().BeTrue(); + logger.GetLog().Should().BeEmpty(); + } + + [Fact] + public void UserCanNotSpecifyCounters_MoreThan3_RaiseError() + { + // Arrange + var logger = new OutputLogger(Output); + string[] args = + [ + "--counters", $"{nameof(HardwareCounter.CacheMisses)}+{nameof(HardwareCounter.InstructionRetired)}", + "--counters", $"{nameof(HardwareCounter.BranchMispredictions)}+{nameof(HardwareCounter.Timer)}" + ]; + + // Act + var (isSuccess, _, _) = ConfigParser.Parse(args, logger); + + // Assert + isSuccess.Should().BeFalse(); + + var log = logger.GetLog().Trim(); + log.Should().Be("You can't use more than 3 HardwareCounters at the same time."); + } + + [Fact] + public void UserCanNotSpecifyCounters_WrongName() + { + // Arrange + var logger = new OutputLogger(Output); + string[] args = ["--counters", nameof(HardwareCounter.CacheMisses), "--counters", "WRONG_NAME"]; + + // Act + var (isSuccess, _, _) = ConfigParser.Parse(args, logger); + + // Assert + isSuccess.Should().BeFalse(); + + var log = logger.GetLog().Trim(); + log.Should().Be("The provided hardware counter \"WRONG_NAME\" is invalid. Available options are: NotSet+Timer+TotalIssues+BranchInstructions+CacheMisses+BranchMispredictions+TotalCycles+UnhaltedCoreCycles+InstructionRetired+UnhaltedReferenceCycles+LlcReference+LlcMisses+BranchInstructionRetired+BranchMispredictsRetired."); + } + + [Theory] + [InlineData("--join", "--join")] + [InlineData("--inProcess", "--inProcess")] + [InlineData("--memory", "--memory")] + [InlineData("-tm", "-m")] + [InlineData("-mm")] + public void UserCanNotSpecify_MultipleSameBooleanOptions(params string[] args) + { + // Arrange + var logger = new OutputLogger(Output); + + // Act + var (isSuccess, _, _) = ConfigParser.Parse(args, logger); + + // Assert + isSuccess.Should().BeFalse(); + + var log = logger.GetLog().Trim(); + log.Should().Match($"Option '*' is defined multiple times."); + } + + [Theory] + [InlineData("-tm")] + [InlineData("-m", "-t")] + public void UserCanSpecify_BundledDifferentBooleanOptions(params string[] args) + { + // Arrange + var logger = new OutputLogger(Output); + + // Act + var (isSuccess, _, options) = ConfigParser.Parse(args, logger); + + // Assert + isSuccess.Should().BeTrue(); + logger.GetLog().Should().BeEmpty(); + + options.Should().NotBeNull(); + options.UseThreadingDiagnoser.Should().BeTrue(); + options.UseMemoryDiagnoser.Should().BeTrue(); + } + + [Theory] + [InlineData("--job", "dry", "--job", "short")] + [InlineData("--job=dry", "--job=short")] + [InlineData("-j", "dry", "-j", "short")] + [InlineData("-j", "dry", "--job", "short")] + [InlineData("--JOB", "dry", "--job", "short")] + [InlineData("--launchCount", "1", "--launchCount", "2")] + [InlineData("--title", "A", "--title", "B")] + public void UserCanNotSpecify_MultipleSameScalarOptions(params string[] args) + { + // Arrange + var logger = new OutputLogger(Output); + + // Act + var (isSuccess, _, _) = ConfigParser.Parse(args, logger); + + // Assert + isSuccess.Should().BeFalse(); + + var log = logger.GetLog().Trim(); + log.Should().Match("Option '*' is defined multiple times."); + } } }