From 23dfee38c1182792a900483436df366659897cf5 Mon Sep 17 00:00:00 2001 From: filzrev <103790468+filzrev@users.noreply.github.com> Date: Thu, 12 Jun 2025 19:21:49 +0900 Subject: [PATCH 1/4] feat: add validator for job that contains null runtime --- src/BenchmarkDotNet/Configs/DefaultConfig.cs | 1 + .../Validators/RuntimeValidator.cs | 35 ++++++ .../Validators/RuntimeValidatorTests.cs | 116 ++++++++++++++++++ 3 files changed, 152 insertions(+) create mode 100644 src/BenchmarkDotNet/Validators/RuntimeValidator.cs create mode 100644 tests/BenchmarkDotNet.Tests/Validators/RuntimeValidatorTests.cs diff --git a/src/BenchmarkDotNet/Configs/DefaultConfig.cs b/src/BenchmarkDotNet/Configs/DefaultConfig.cs index 8585a77797..d188b67415 100644 --- a/src/BenchmarkDotNet/Configs/DefaultConfig.cs +++ b/src/BenchmarkDotNet/Configs/DefaultConfig.cs @@ -72,6 +72,7 @@ public IEnumerable GetValidators() yield return DeferredExecutionValidator.FailOnError; yield return ParamsAllValuesValidator.FailOnError; yield return ParamsValidator.FailOnError; + yield return RuntimeValidator.DontFailOnError; } public IOrderer Orderer => null; diff --git a/src/BenchmarkDotNet/Validators/RuntimeValidator.cs b/src/BenchmarkDotNet/Validators/RuntimeValidator.cs new file mode 100644 index 0000000000..4ef0543f3e --- /dev/null +++ b/src/BenchmarkDotNet/Validators/RuntimeValidator.cs @@ -0,0 +1,35 @@ +using System.Collections.Generic; +using System.Collections.Immutable; +using System.Linq; +using BenchmarkDotNet.Jobs; + +namespace BenchmarkDotNet.Validators; + +/// +/// Validator for runtime characteristic. +/// +/// +public class RuntimeValidator : IValidator +{ + public static readonly IValidator DontFailOnError = new RuntimeValidator(); + + private RuntimeValidator() { } + + public bool TreatsWarningsAsErrors => false; + + public IEnumerable Validate(ValidationParameters input) + { + var allBenchmarks = input.Benchmarks.ToArray(); + + var runtimes = allBenchmarks.Select(x => x.Job.Environment.Runtime) + .Distinct() + .ToArray(); + + if (runtimes.Length > 1 && runtimes.Contains(null)) + { + // GetRuntime() method returns current environment's runtime if RuntimeCharacteristic is not set. + var message = "There are benchmarks that job don't have a Runtime characteristic. It's recommended explicitly specify runtime by `WithRuntime`."; + yield return new ValidationError(false, message); + } + } +} diff --git a/tests/BenchmarkDotNet.Tests/Validators/RuntimeValidatorTests.cs b/tests/BenchmarkDotNet.Tests/Validators/RuntimeValidatorTests.cs new file mode 100644 index 0000000000..45b47f0dad --- /dev/null +++ b/tests/BenchmarkDotNet.Tests/Validators/RuntimeValidatorTests.cs @@ -0,0 +1,116 @@ +using BenchmarkDotNet.Attributes; +using BenchmarkDotNet.Configs; +using BenchmarkDotNet.Environments; +using BenchmarkDotNet.Jobs; +using BenchmarkDotNet.Running; +using BenchmarkDotNet.Toolchains.CsProj; +using BenchmarkDotNet.Validators; +using System.Linq; +using Xunit; + +namespace BenchmarkDotNet.Tests.Validators; + +public class RuntimeValidatorTests +{ + [Fact] + public void SameRuntime_Should_Success() + { + // Arrange + var config = new TestConfig1().CreateImmutableConfig(); + var runInfo = BenchmarkConverter.TypeToBenchmarks(typeof(DummyBenchmark), config); + var parameters = new ValidationParameters(runInfo.BenchmarksCases, config); + + // Act + var errors = RuntimeValidator.DontFailOnError.Validate(parameters).Select(e => e.Message).ToArray(); + + // Assert + Assert.Empty(errors); + } + + [Fact] + public void NullRuntimeMixed_Should_Failed() + { + // Arrange + var config = new TestConfig2().CreateImmutableConfig(); + var runInfo = BenchmarkConverter.TypeToBenchmarks(typeof(DummyBenchmark), config); + var parameters = new ValidationParameters(runInfo.BenchmarksCases, config); + + // Act + var errors = RuntimeValidator.DontFailOnError.Validate(parameters).Select(e => e.Message).ToArray(); + + // Assert + var expectedMessage = "There are benchmarks that job don't have a Runtime characteristic. It's recommended explicitly specify runtime by `WithRuntime`."; + Assert.Contains(errors, s => s.Equals(expectedMessage)); + } + + [Fact] + public void NotNullRuntimeOnly_Should_Success() + { + // Arrange + var config = new TestConfig3().CreateImmutableConfig(); + var runInfo = BenchmarkConverter.TypeToBenchmarks(typeof(DummyBenchmark), config); + var parameters = new ValidationParameters(runInfo.BenchmarksCases, config); + + // Act + var errors = RuntimeValidator.DontFailOnError.Validate(parameters).Select(e => e.Message).ToArray(); + + // Assert + Assert.Empty(errors); + } + + public class DummyBenchmark + { + [Benchmark] + public void Benchmark() + { + } + } + + // TestConfig that expicitly specify runtime. + private class TestConfig1 : ManualConfig + { + public TestConfig1() + { + var baseJob = Job.Dry; + + WithOption(ConfigOptions.DisableOptimizationsValidator, true); + AddColumnProvider(DefaultConfig.Instance.GetColumnProviders().ToArray()); + + AddJob(baseJob.WithToolchain(CsProjCoreToolchain.NetCoreApp80)); + AddJob(baseJob.WithToolchain(CsProjCoreToolchain.NetCoreApp90)); + } + } + + // TestConfig that contains job that don't specify runtime. + private class TestConfig2 : ManualConfig + { + public TestConfig2() + { + var baseJob = Job.Dry; + + WithOption(ConfigOptions.DisableOptimizationsValidator, true); + AddColumnProvider(DefaultConfig.Instance.GetColumnProviders().ToArray()); + + AddJob(baseJob.WithToolchain(CsProjCoreToolchain.NetCoreApp80)); + AddJob(baseJob.WithToolchain(CsProjCoreToolchain.NetCoreApp90) + .WithRuntime(CoreRuntime.Core90)); + } + } + + // TestConfig that expicitly specify runtime. + private class TestConfig3 : ManualConfig + { + public TestConfig3() + { + var baseJob = Job.Dry; + + WithOption(ConfigOptions.DisableOptimizationsValidator, true); + AddColumnProvider(DefaultConfig.Instance.GetColumnProviders().ToArray()); + + AddJob(baseJob.WithToolchain(CsProjCoreToolchain.NetCoreApp80) + .WithRuntime(CoreRuntime.Core80)); ; + AddJob(baseJob.WithToolchain(CsProjCoreToolchain.NetCoreApp90) + .WithRuntime(CoreRuntime.Core90)); + } + } +} From c008f256cb6355e8961ef9db92eb4d4b934b43bf Mon Sep 17 00:00:00 2001 From: filzrev <103790468+filzrev@users.noreply.github.com> Date: Sun, 15 Jun 2025 11:59:39 +0900 Subject: [PATCH 2/4] chore: update validator logics --- .../Validators/RuntimeValidator.cs | 26 ++++++++++++------- .../Validators/RuntimeValidatorTests.cs | 13 ++++++++-- 2 files changed, 28 insertions(+), 11 deletions(-) diff --git a/src/BenchmarkDotNet/Validators/RuntimeValidator.cs b/src/BenchmarkDotNet/Validators/RuntimeValidator.cs index 4ef0543f3e..041c72967f 100644 --- a/src/BenchmarkDotNet/Validators/RuntimeValidator.cs +++ b/src/BenchmarkDotNet/Validators/RuntimeValidator.cs @@ -1,7 +1,6 @@ using System.Collections.Generic; -using System.Collections.Immutable; using System.Linq; -using BenchmarkDotNet.Jobs; +using BenchmarkDotNet.Characteristics; namespace BenchmarkDotNet.Validators; @@ -20,16 +19,25 @@ private RuntimeValidator() { } public IEnumerable Validate(ValidationParameters input) { var allBenchmarks = input.Benchmarks.ToArray(); + var nullRuntimeBenchmarks = allBenchmarks.Where(x => x.Job.Environment.Runtime == null).ToArray(); - var runtimes = allBenchmarks.Select(x => x.Job.Environment.Runtime) - .Distinct() - .ToArray(); + // There is no validation error if all the runtimes are set or if all the runtimes are null. + if (allBenchmarks.Length == nullRuntimeBenchmarks.Length) + { + return []; + } - if (runtimes.Length > 1 && runtimes.Contains(null)) + var errors = new List(); + foreach (var benchmark in nullRuntimeBenchmarks) { - // GetRuntime() method returns current environment's runtime if RuntimeCharacteristic is not set. - var message = "There are benchmarks that job don't have a Runtime characteristic. It's recommended explicitly specify runtime by `WithRuntime`."; - yield return new ValidationError(false, message); + var job = benchmark.Job; + var jobText = job.HasValue(CharacteristicObject.IdCharacteristic) + ? job.Id + : CharacteristicSetPresenter.Display.ToPresentation(job); // Use job text representation instead for auto generated JobId. + + var message = $"Job({jobText}) don't have a Runtime characteristic. It's recommended to specify runtime by using WithRuntime explicitly."; + errors.Add(new ValidationError(false, message)); } + return errors; } } diff --git a/tests/BenchmarkDotNet.Tests/Validators/RuntimeValidatorTests.cs b/tests/BenchmarkDotNet.Tests/Validators/RuntimeValidatorTests.cs index 45b47f0dad..799e4a8c6a 100644 --- a/tests/BenchmarkDotNet.Tests/Validators/RuntimeValidatorTests.cs +++ b/tests/BenchmarkDotNet.Tests/Validators/RuntimeValidatorTests.cs @@ -39,8 +39,14 @@ public void NullRuntimeMixed_Should_Failed() var errors = RuntimeValidator.DontFailOnError.Validate(parameters).Select(e => e.Message).ToArray(); // Assert - var expectedMessage = "There are benchmarks that job don't have a Runtime characteristic. It's recommended explicitly specify runtime by `WithRuntime`."; - Assert.Contains(errors, s => s.Equals(expectedMessage)); + { + var expectedMessage = "Job(Dry) don't have a Runtime characteristic. It's recommended to specify runtime by using WithRuntime explicitly."; + Assert.Contains(expectedMessage, errors); + } + { + var expectedMessage = "Job(Toolchain=.NET 10.0) don't have a Runtime characteristic. It's recommended to specify runtime by using WithRuntime explicitly."; + Assert.Contains(expectedMessage, errors); + } } [Fact] @@ -94,6 +100,9 @@ public TestConfig2() AddJob(baseJob.WithToolchain(CsProjCoreToolchain.NetCoreApp80)); AddJob(baseJob.WithToolchain(CsProjCoreToolchain.NetCoreApp90) .WithRuntime(CoreRuntime.Core90)); + + // Validate error message for auto generated jobid. + AddJob(Job.Default.WithToolchain(CsProjCoreToolchain.NetCoreApp10_0)); } } From 91b0d8e8fa3f7b7408103627084455b812c12d05 Mon Sep 17 00:00:00 2001 From: filzrev <103790468+filzrev@users.noreply.github.com> Date: Sun, 15 Jun 2025 13:02:57 +0900 Subject: [PATCH 3/4] Update src/BenchmarkDotNet/Validators/RuntimeValidator.cs Co-authored-by: Tim Cassell --- src/BenchmarkDotNet/Validators/RuntimeValidator.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/BenchmarkDotNet/Validators/RuntimeValidator.cs b/src/BenchmarkDotNet/Validators/RuntimeValidator.cs index 041c72967f..7be547890c 100644 --- a/src/BenchmarkDotNet/Validators/RuntimeValidator.cs +++ b/src/BenchmarkDotNet/Validators/RuntimeValidator.cs @@ -35,7 +35,7 @@ public IEnumerable Validate(ValidationParameters input) ? job.Id : CharacteristicSetPresenter.Display.ToPresentation(job); // Use job text representation instead for auto generated JobId. - var message = $"Job({jobText}) don't have a Runtime characteristic. It's recommended to specify runtime by using WithRuntime explicitly."; + var message = $"Job({jobText}) doesn't have a Runtime characteristic. It's recommended to specify runtime by using WithRuntime explicitly."; errors.Add(new ValidationError(false, message)); } return errors; From ea68e8395c523fa4d5639796f614ed82fde5b31b Mon Sep 17 00:00:00 2001 From: filzrev <103790468+filzrev@users.noreply.github.com> Date: Sun, 15 Jun 2025 13:04:56 +0900 Subject: [PATCH 4/4] chore: fix test validation messages --- .../BenchmarkDotNet.Tests/Validators/RuntimeValidatorTests.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/BenchmarkDotNet.Tests/Validators/RuntimeValidatorTests.cs b/tests/BenchmarkDotNet.Tests/Validators/RuntimeValidatorTests.cs index 799e4a8c6a..9eac2dd6f2 100644 --- a/tests/BenchmarkDotNet.Tests/Validators/RuntimeValidatorTests.cs +++ b/tests/BenchmarkDotNet.Tests/Validators/RuntimeValidatorTests.cs @@ -40,11 +40,11 @@ public void NullRuntimeMixed_Should_Failed() // Assert { - var expectedMessage = "Job(Dry) don't have a Runtime characteristic. It's recommended to specify runtime by using WithRuntime explicitly."; + var expectedMessage = "Job(Dry) doesn't have a Runtime characteristic. It's recommended to specify runtime by using WithRuntime explicitly."; Assert.Contains(expectedMessage, errors); } { - var expectedMessage = "Job(Toolchain=.NET 10.0) don't have a Runtime characteristic. It's recommended to specify runtime by using WithRuntime explicitly."; + var expectedMessage = "Job(Toolchain=.NET 10.0) doesn't have a Runtime characteristic. It's recommended to specify runtime by using WithRuntime explicitly."; Assert.Contains(expectedMessage, errors); } }