diff --git a/src/BenchmarkDotNet/Extensions/ReflectionExtensions.cs b/src/BenchmarkDotNet/Extensions/ReflectionExtensions.cs index d3669d4029..655bacd906 100644 --- a/src/BenchmarkDotNet/Extensions/ReflectionExtensions.cs +++ b/src/BenchmarkDotNet/Extensions/ReflectionExtensions.cs @@ -156,14 +156,14 @@ private static MethodInfo[] GetBenchmarks(this TypeInfo typeInfo) .Where(method => method.GetCustomAttributes(true).OfType().Any()) .ToArray(); - internal static (string Name, TAttribute Attribute, bool IsPrivate, bool IsStatic, Type ParameterType)[] GetTypeMembersWithGivenAttribute(this Type type, BindingFlags reflectionFlags) + internal static (string Name, TAttribute Attribute, bool IsPublic, bool IsStatic, Type ParameterType)[] GetTypeMembersWithGivenAttribute(this Type type, BindingFlags reflectionFlags) where TAttribute : Attribute { var allFields = type.GetFields(reflectionFlags) .Select(f => ( Name: f.Name, Attribute: f.ResolveAttribute(), - IsPrivate: f.IsPrivate, + IsPublic: f.IsPublic, IsStatic: f.IsStatic, ParameterType: f.FieldType)); @@ -171,16 +171,12 @@ internal static (string Name, TAttribute Attribute, bool IsPrivate, bool IsStati .Select(p => ( Name: p.Name, Attribute: p.ResolveAttribute(), - IsPrivate: p.GetSetMethod() == null, + IsPublic: p.GetSetMethod() != null && p.GetSetMethod().IsPublic, IsStatic: p.GetSetMethod() != null && p.GetSetMethod().IsStatic, PropertyType: p.PropertyType)); - var joined = allFields.Concat(allProperties).Where(member => member.Attribute != null).ToArray(); - - foreach (var member in joined.Where(m => m.IsPrivate)) - throw new InvalidOperationException($"Member \"{member.Name}\" must be public if it has the [{typeof(TAttribute).Name}] attribute applied to it"); - - return joined; + var joined = allFields.Concat(allProperties).Where(member => member.Attribute != null); + return joined.ToArray(); } internal static bool IsStackOnlyWithImplicitCast(this Type argumentType, object argumentInstance) diff --git a/src/BenchmarkDotNet/Running/BenchmarkConverter.cs b/src/BenchmarkDotNet/Running/BenchmarkConverter.cs index 754cf9f9d1..0a217c25c6 100644 --- a/src/BenchmarkDotNet/Running/BenchmarkConverter.cs +++ b/src/BenchmarkDotNet/Running/BenchmarkConverter.cs @@ -179,6 +179,10 @@ IEnumerable GetDefinitions(Func(reflectionFlags); + + foreach (var member in allMembers.Where(m => !m.IsPublic)) + throw new InvalidOperationException($"Member \"{member.Name}\" must be public if it has the [{typeof(TAttribute).Name}] attribute applied to it"); + return allMembers.Select(member => new ParameterDefinition( member.Name, diff --git a/src/BenchmarkDotNet/Toolchains/InProcess.NoEmit/InProcessNoEmitRunner.cs b/src/BenchmarkDotNet/Toolchains/InProcess.NoEmit/InProcessNoEmitRunner.cs index e890000683..80719e2ddf 100644 --- a/src/BenchmarkDotNet/Toolchains/InProcess.NoEmit/InProcessNoEmitRunner.cs +++ b/src/BenchmarkDotNet/Toolchains/InProcess.NoEmit/InProcessNoEmitRunner.cs @@ -4,6 +4,7 @@ using BenchmarkDotNet.Environments; using BenchmarkDotNet.Exporters; using BenchmarkDotNet.Jobs; +using BenchmarkDotNet.Parameters; using BenchmarkDotNet.Running; using JetBrains.Annotations; @@ -61,39 +62,43 @@ public static int Run(IHost host, BenchmarkCase benchmarkCase) } } - /// Fills the properties of the instance of the object used to run the benchmark. - /// The instance. - /// The benchmark. + /// Fills the properties of the instance of the object used to run the benchmark. Arguments are not supported internal static void FillMembers(object instance, BenchmarkCase benchmarkCase) { foreach (var parameter in benchmarkCase.Parameters.Items) { - var flags = BindingFlags.Public; - flags |= parameter.IsStatic ? BindingFlags.Static : BindingFlags.Instance; + FillMember(instance, benchmarkCase, parameter); + } + } - var targetType = benchmarkCase.Descriptor.Type; - var paramProperty = targetType.GetProperty(parameter.Name, flags); + /// Fills the property of the instance of the object used to run the benchmark. Arguments are not supported + internal static void FillMember(object instance, BenchmarkCase benchmarkCase, ParameterInstance parameter) + { + var flags = BindingFlags.Public; + flags |= parameter.IsStatic ? BindingFlags.Static : BindingFlags.Instance; - if (paramProperty == null) - { - var paramField = targetType.GetField(parameter.Name, flags); - if (paramField == null) - throw new InvalidOperationException( - $"Type {targetType.FullName}: no property or field {parameter.Name} found."); + var targetType = benchmarkCase.Descriptor.Type; + var paramProperty = targetType.GetProperty(parameter.Name, flags); - var callInstance = paramField.IsStatic ? null : instance; - paramField.SetValue(callInstance, parameter.Value); - } - else - { - var setter = paramProperty.GetSetMethod(); - if (setter == null) - throw new InvalidOperationException( - $"Type {targetType.FullName}: no settable property {parameter.Name} found."); + if (paramProperty == null) + { + var paramField = targetType.GetField(parameter.Name, flags); + if (paramField == null) + throw new InvalidOperationException( + $"Type {targetType.FullName}: no property or field {parameter.Name} found."); - var callInstance = setter.IsStatic ? null : instance; - setter.Invoke(callInstance, new[] { parameter.Value }); - } + var callInstance = paramField.IsStatic ? null : instance; + paramField.SetValue(callInstance, parameter.Value); + } + else + { + var setter = paramProperty.GetSetMethod(); + if (setter == null) + throw new InvalidOperationException( + $"Type {targetType.FullName}: no settable property {parameter.Name} found."); + + var callInstance = setter.IsStatic ? null : instance; + setter.Invoke(callInstance, new[] { parameter.Value }); } } diff --git a/src/BenchmarkDotNet/Validators/ExecutionValidator.cs b/src/BenchmarkDotNet/Validators/ExecutionValidator.cs index 79f3037126..211f07aaf5 100644 --- a/src/BenchmarkDotNet/Validators/ExecutionValidator.cs +++ b/src/BenchmarkDotNet/Validators/ExecutionValidator.cs @@ -1,6 +1,5 @@ using System; using System.Collections.Generic; -using BenchmarkDotNet.Running; namespace BenchmarkDotNet.Validators { @@ -12,20 +11,20 @@ public class ExecutionValidator : ExecutionValidatorBase private ExecutionValidator(bool failOnError) : base(failOnError) { } - protected override void ExecuteBenchmarks(object benchmarkTypeInstance, IEnumerable benchmarks, List errors) + protected override void ExecuteBenchmarks(IEnumerable executors, List errors) { - foreach (var benchmark in benchmarks) + foreach (var executor in executors) { try { - benchmark.Descriptor.WorkloadMethod.Invoke(benchmarkTypeInstance, null); + executor.Invoke(); } catch (Exception ex) { errors.Add(new ValidationError( TreatsWarningsAsErrors, - $"Failed to execute benchmark '{benchmark.DisplayInfo}', exception was: '{GetDisplayExceptionMessage(ex)}'", - benchmark)); + $"Failed to execute benchmark '{executor.BenchmarkCase.DisplayInfo}', exception was: '{GetDisplayExceptionMessage(ex)}'", + executor.BenchmarkCase)); } } } diff --git a/src/BenchmarkDotNet/Validators/ExecutionValidatorBase.cs b/src/BenchmarkDotNet/Validators/ExecutionValidatorBase.cs index 92d7422ae2..2572dd70b7 100644 --- a/src/BenchmarkDotNet/Validators/ExecutionValidatorBase.cs +++ b/src/BenchmarkDotNet/Validators/ExecutionValidatorBase.cs @@ -6,6 +6,7 @@ using BenchmarkDotNet.Attributes; using BenchmarkDotNet.Extensions; using BenchmarkDotNet.Running; +using BenchmarkDotNet.Toolchains.InProcess.NoEmit; namespace BenchmarkDotNet.Validators { @@ -24,29 +25,34 @@ public IEnumerable Validate(ValidationParameters validationPara foreach (var typeGroup in validationParameters.Benchmarks.GroupBy(benchmark => benchmark.Descriptor.Type)) { - if (!TryCreateBenchmarkTypeInstance(typeGroup.Key, errors, out var benchmarkTypeInstance)) - { - continue; - } + var executors = new List(); - if (!TryToSetParamsFields(benchmarkTypeInstance, errors)) + foreach (var benchmark in typeGroup) { - continue; - } + if (!TryCreateBenchmarkTypeInstance(typeGroup.Key, errors, out var benchmarkTypeInstance)) + continue; - if (!TryToSetParamsProperties(benchmarkTypeInstance, errors)) - { - continue; - } + if (!TryToFillParameters(benchmark, benchmarkTypeInstance, errors)) + continue; - if (!TryToCallGlobalSetup(benchmarkTypeInstance, errors)) - { - continue; + if (!TryToCallGlobalSetup(benchmarkTypeInstance, errors)) + continue; + + if (!TryToCallIterationSetup(benchmarkTypeInstance, errors)) + continue; + + executors.Add(new BenchmarkExecutor(benchmarkTypeInstance, benchmark)); } - ExecuteBenchmarks(benchmarkTypeInstance, typeGroup, errors); + ExecuteBenchmarks(executors, errors); + + foreach (var executor in executors) + { + if (!TryToCallIterationCleanup(executor.Instance, errors)) + continue; - TryToCallGlobalCleanup(benchmarkTypeInstance, errors); + TryToCallGlobalCleanup(executor.Instance, errors); + } } return errors; @@ -73,15 +79,25 @@ private bool TryCreateBenchmarkTypeInstance(Type type, List err private bool TryToCallGlobalSetup(object benchmarkTypeInstance, List errors) { - return TryToCallGlobalMethod(benchmarkTypeInstance, errors); + return TryToCallGlobalMethod(benchmarkTypeInstance, errors, true); } private void TryToCallGlobalCleanup(object benchmarkTypeInstance, List errors) { - TryToCallGlobalMethod(benchmarkTypeInstance, errors); + TryToCallGlobalMethod(benchmarkTypeInstance, errors, true); } - private bool TryToCallGlobalMethod(object benchmarkTypeInstance, List errors) + private bool TryToCallIterationSetup(object benchmarkTypeInstance, List errors) + { + return TryToCallGlobalMethod(benchmarkTypeInstance, errors, false); + } + + private bool TryToCallIterationCleanup(object benchmarkTypeInstance, List errors) + { + return TryToCallGlobalMethod(benchmarkTypeInstance, errors, false); + } + + private bool TryToCallGlobalMethod(object benchmarkTypeInstance, List errors, bool canBeAsync) where T : Attribute { var methods = benchmarkTypeInstance .GetType() @@ -107,7 +123,16 @@ private bool TryToCallGlobalMethod(object benchmarkTypeInstance, List(object benchmarkTypeInstance, List type.Name.Replace("Attribute", string.Empty); - - private void TryToGetTaskResult(object result) + private static bool TryAwaitTask(object task, out object result) { - if (result == null) + result = null; + + if (task is null) { - return; + return false; } - var returnType = result.GetType(); + var returnType = task.GetType(); if (returnType.IsGenericType && returnType.GetGenericTypeDefinition() == typeof(ValueTask<>)) { - var asTaskMethod = result.GetType().GetMethod("AsTask"); - result = asTaskMethod.Invoke(result, null); + var asTaskMethod = task.GetType().GetMethod("AsTask"); + task = asTaskMethod.Invoke(task, null); } - if (result is Task task) - { - task.GetAwaiter().GetResult(); - } - else if (result is ValueTask valueTask) + if (task is ValueTask valueTask) + task = valueTask.AsTask(); + + if (task is Task t) { - valueTask.GetAwaiter().GetResult(); + if (TryGetTaskResult(t, out var taskResult)) + result = taskResult; + + return true; } + + return false; } - private bool TryToSetParamsFields(object benchmarkTypeInstance, List errors) + // https://stackoverflow.com/a/52500763 + private static bool TryGetTaskResult(Task task, out object result) { - var paramFields = benchmarkTypeInstance - .GetType() - .GetAllFields() - .Where(fieldInfo => fieldInfo.GetCustomAttributes(false).OfType().Any()) - .ToArray(); + task.GetAwaiter().GetResult(); - if (!paramFields.Any()) + result = null; + + var voidTaskType = typeof(Task<>).MakeGenericType(Type.GetType("System.Threading.Tasks.VoidTaskResult")); + if (voidTaskType.IsInstanceOfType(task)) { - return true; + return false; } - foreach (var paramField in paramFields) + var property = task.GetType().GetProperty("Result", BindingFlags.Public | BindingFlags.Instance); + if (property is null) { - if (!paramField.IsPublic) - { - errors.Add(new ValidationError( - TreatsWarningsAsErrors, - $"Fields marked with [Params] must be public, {paramField.Name} of {benchmarkTypeInstance.GetType().Name} is not")); - - return false; - } - - var values = paramField.GetCustomAttributes(false).OfType().Single().Values; - if (!values.Any()) - { - errors.Add(new ValidationError( - TreatsWarningsAsErrors, - $"Fields marked with [Params] must have some values defined, {paramField.Name} of {benchmarkTypeInstance.GetType().Name} has none")); - - return false; - } - - try - { - paramField.SetValue(benchmarkTypeInstance, values.First()); - } - catch (Exception ex) - { - errors.Add(new ValidationError( - TreatsWarningsAsErrors, - $"Failed to set {paramField.Name} of {benchmarkTypeInstance.GetType().Name} to {values.First()}, exception was: {GetDisplayExceptionMessage(ex)}")); - - return false; - } + return false; } + result = property.GetValue(task); return true; } - private bool TryToSetParamsProperties(object benchmarkTypeInstance, List errors) + private bool TryToFillParameters(BenchmarkCase benchmark, object benchmarkTypeInstance, List errors) { - var paramProperties = benchmarkTypeInstance - .GetType() - .GetAllProperties() - .Where(propertyInfo => propertyInfo.GetCustomAttributes(false).OfType().Any()) - .ToArray(); + if (ValidateMembers(benchmarkTypeInstance, errors)) + return false; - if (!paramProperties.Any()) - { - return true; - } + if (ValidateMembers(benchmarkTypeInstance, errors)) + return false; + + bool hasError = false; - foreach (var paramProperty in paramProperties) + foreach (var parameter in benchmark.Parameters.Items) { - var setter = paramProperty.SetMethod; - if (setter == null || !setter.IsPublic) + // InProcessNoEmitToolchain does not support arguments + if (!parameter.IsArgument) { - errors.Add(new ValidationError( - TreatsWarningsAsErrors, - $"Properties marked with [Params] must have public setter, {paramProperty.Name} of {benchmarkTypeInstance.GetType().Name} has not")); - - return false; + try + { + InProcessNoEmitRunner.FillMember(benchmarkTypeInstance, benchmark, parameter); + } + catch (Exception ex) + { + hasError = true; + errors.Add(new ValidationError( + TreatsWarningsAsErrors, + ex.Message, + benchmark)); + } } + } - var values = paramProperty.GetCustomAttributes(false).OfType().Single().Values; - if (!values.Any()) - { - errors.Add(new ValidationError( - TreatsWarningsAsErrors, - $"Properties marked with [Params] must have some values defined, {paramProperty.Name} of {benchmarkTypeInstance.GetType().Name} has not")); + return !hasError; + } - return false; - } + private bool ValidateMembers(object benchmarkTypeInstance, List errors) where T : Attribute + { + const BindingFlags reflectionFlags = BindingFlags.Static | BindingFlags.Instance | BindingFlags.Public | BindingFlags.NonPublic; - try - { - setter.Invoke(benchmarkTypeInstance, new[] { values.First() }); - } - catch (Exception ex) + bool hasError = false; + + foreach (var member in benchmarkTypeInstance.GetType().GetTypeMembersWithGivenAttribute(reflectionFlags)) + { + if (!member.IsPublic) { errors.Add(new ValidationError( TreatsWarningsAsErrors, - $"Failed to set {paramProperty.Name} of {benchmarkTypeInstance.GetType().Name} to {values.First()}, exception was: {GetDisplayExceptionMessage(ex)}")); + $"Member \"{member.Name}\" must be public if it has the [{GetAttributeName(typeof(T))}] attribute applied to it, {member.Name} of {benchmarkTypeInstance.GetType().Name} has not")); - return false; + hasError = true; } } - return true; + return hasError; } + private static string GetAttributeName(Type type) => type.Name.Replace("Attribute", string.Empty); + protected static string GetDisplayExceptionMessage(Exception ex) { if (ex is TargetInvocationException targetInvocationException) @@ -258,6 +263,33 @@ protected static string GetDisplayExceptionMessage(Exception ex) return ex?.Message ?? "Unknown error"; } - protected abstract void ExecuteBenchmarks(object benchmarkTypeInstance, IEnumerable benchmarks, List errors); + protected abstract void ExecuteBenchmarks(IEnumerable executors, List errors); + + protected class BenchmarkExecutor + { + public object Instance { get; } + public BenchmarkCase BenchmarkCase { get; } + + public BenchmarkExecutor(object instance, BenchmarkCase benchmarkCase) + { + Instance = instance; + BenchmarkCase = benchmarkCase; + } + + public object Invoke() + { + var arguments = BenchmarkCase.Parameters.Items + .Where(parameter => parameter.IsArgument) + .Select(argument => argument.Value) + .ToArray(); + + var result = BenchmarkCase.Descriptor.WorkloadMethod.Invoke(Instance, arguments); + + if (TryAwaitTask(result, out var taskResult)) + result = taskResult; + + return result; + } + } } } \ No newline at end of file diff --git a/src/BenchmarkDotNet/Validators/ReturnValueValidator.cs b/src/BenchmarkDotNet/Validators/ReturnValueValidator.cs index f4b5b7b63d..17d0e959d8 100644 --- a/src/BenchmarkDotNet/Validators/ReturnValueValidator.cs +++ b/src/BenchmarkDotNet/Validators/ReturnValueValidator.cs @@ -4,11 +4,9 @@ using System.Diagnostics.CodeAnalysis; using System.Linq; using System.Reflection; - using BenchmarkDotNet.Extensions; using BenchmarkDotNet.Parameters; using BenchmarkDotNet.Running; -using BenchmarkDotNet.Toolchains.InProcess.NoEmit; namespace BenchmarkDotNet.Validators { @@ -20,22 +18,21 @@ public class ReturnValueValidator : ExecutionValidatorBase private ReturnValueValidator(bool failOnError) : base(failOnError) { } - protected override void ExecuteBenchmarks(object benchmarkTypeInstance, IEnumerable benchmarks, List errors) + protected override void ExecuteBenchmarks(IEnumerable executors, List errors) { - foreach (var parameterGroup in benchmarks.GroupBy(i => i.Parameters, ParameterInstancesEqualityComparer.Instance)) + foreach (var parameterGroup in executors.GroupBy(i => i.BenchmarkCase.Parameters, ParameterInstancesEqualityComparer.Instance)) { var results = new List<(BenchmarkCase benchmark, object returnValue)>(); bool hasErrorsInGroup = false; - foreach (var benchmark in parameterGroup.DistinctBy(i => i.Descriptor.WorkloadMethod)) + foreach (var executor in parameterGroup.DistinctBy(e => e.BenchmarkCase.Descriptor.WorkloadMethod)) { try { - InProcessNoEmitRunner.FillMembers(benchmarkTypeInstance, benchmark); - var result = benchmark.Descriptor.WorkloadMethod.Invoke(benchmarkTypeInstance, null); + var result = executor.Invoke(); - if (benchmark.Descriptor.WorkloadMethod.ReturnType != typeof(void)) - results.Add((benchmark, result)); + if (executor.BenchmarkCase.Descriptor.WorkloadMethod.ReturnType != typeof(void)) + results.Add((executor.BenchmarkCase, result)); } catch (Exception ex) { @@ -43,8 +40,8 @@ protected override void ExecuteBenchmarks(object benchmarkTypeInstance, IEnumera errors.Add(new ValidationError( TreatsWarningsAsErrors, - $"Failed to execute benchmark '{benchmark.DisplayInfo}', exception was: '{GetDisplayExceptionMessage(ex)}'", - benchmark)); + $"Failed to execute benchmark '{executor.BenchmarkCase.DisplayInfo}', exception was: '{GetDisplayExceptionMessage(ex)}'", + executor.BenchmarkCase)); } } @@ -75,7 +72,11 @@ public bool Equals(ParameterInstances x, ParameterInstances y) if (x.Count != y.Count) return false; - return x.Items.OrderBy(i => i.Name).Zip(y.Items.OrderBy(i => i.Name), (a, b) => a.Name == b.Name && Equals(a.Value, b.Value)).All(i => i); + return x.Items + .Where(i => !i.IsArgument) + .OrderBy(i => i.Name) + .Zip(y.Items.OrderBy(i => i.Name), (a, b) => a.Name == b.Name && Equals(a.Value, b.Value)) + .All(i => i); } public int GetHashCode(ParameterInstances obj) @@ -84,7 +85,7 @@ public int GetHashCode(ParameterInstances obj) return 0; var hashCode = new HashCode(); - foreach (var instance in obj.Items.OrderBy(i => i.Name)) + foreach (var instance in obj.Items.Where(i => !i.IsArgument).OrderBy(i => i.Name)) { hashCode.Add(instance.Name); hashCode.Add(instance.Value); @@ -137,7 +138,7 @@ private bool CompareStructural(object x, object y) return false; - Array ToStructuralEquatable(object obj) + static Array ToStructuralEquatable(object obj) { switch (obj) { diff --git a/tests/BenchmarkDotNet.Tests/Validators/ExecutionValidatorTests.cs b/tests/BenchmarkDotNet.Tests/Validators/ExecutionValidatorTests.cs index 6ca7394f4d..6e5a9ddc46 100644 --- a/tests/BenchmarkDotNet.Tests/Validators/ExecutionValidatorTests.cs +++ b/tests/BenchmarkDotNet.Tests/Validators/ExecutionValidatorTests.cs @@ -1,4 +1,5 @@ using System; +using System.Collections.Generic; using System.Linq; using System.Threading.Tasks; using BenchmarkDotNet.Attributes; @@ -9,6 +10,8 @@ namespace BenchmarkDotNet.Tests.Validators { + // xUnit bug: https://github.com/xunit/xunit/issues/2587" + [Collection("Disable parallelism")] public class ExecutionValidatorTests { [Fact] @@ -164,17 +167,16 @@ public class OverridesGlobalCleanup : BaseClassWithThrowingGlobalCleanup } [Fact] - public void NonFailingGlobalSetupsAreOmitted() + public void ParamsAreSetBeforeGlobalSetup() { - var validationErrors = ExecutionValidator.FailOnError.Validate(BenchmarkConverter.TypeToBenchmarks(typeof(GlobalSetupThatRequiresParamsToBeSetFirst))); + var validationErrors = ExecutionValidator.FailOnError.Validate(BenchmarkConverter.TypeToBenchmarks(typeof(ParamsAreSetBeforeGlobalSetupClass))); Assert.Empty(validationErrors); } - public class GlobalSetupThatRequiresParamsToBeSetFirst + public class ParamsAreSetBeforeGlobalSetupClass { [Params(100)] - [UsedImplicitly] public int Field; [GlobalSetup] @@ -189,17 +191,45 @@ public void NonThrowing() { } } [Fact] - public void NonFailingGlobalCleanupsAreOmitted() + public void ParamsSourceAreSetBeforeGlobalSetup() { - var validationErrors = ExecutionValidator.FailOnError.Validate(BenchmarkConverter.TypeToBenchmarks(typeof(GlobalCleanupThatRequiresParamsToBeSetFirst))); + var validationErrors = ExecutionValidator.FailOnError.Validate(BenchmarkConverter.TypeToBenchmarks(typeof(ParamsSourceAreSetBeforeGlobalSetupClass))); Assert.Empty(validationErrors); } - public class GlobalCleanupThatRequiresParamsToBeSetFirst + public class ParamsSourceAreSetBeforeGlobalSetupClass + { + [ParamsSource(nameof(GetParams))] + public int Field; + + [GlobalSetup] + public void Failing() + { + if (Field == default) + throw new Exception("This should have never happened"); + } + + [Benchmark] + public void NonThrowing() { } + + public IEnumerable GetParams() + { + yield return 100; + } + } + + [Fact] + public void ParamsAreSetBeforeGlobalCleanup() + { + var validationErrors = ExecutionValidator.FailOnError.Validate(BenchmarkConverter.TypeToBenchmarks(typeof(ParamsAreSetBeforeGlobalCleanupClass))); + + Assert.Empty(validationErrors); + } + + public class ParamsAreSetBeforeGlobalCleanupClass { [Params(100)] - [UsedImplicitly] public int Field; [GlobalCleanup] @@ -270,12 +300,8 @@ public void NonThrowing() { } [Fact] public void NonPublicFieldsWithParamsAreDiscovered() { - var validationErrors = ExecutionValidator.FailOnError - .Validate(BenchmarkConverter.TypeToBenchmarks(typeof(NonPublicFieldWithParams))) - .ToList(); - - Assert.NotEmpty(validationErrors); - Assert.StartsWith("Fields marked with [Params] must be public", validationErrors.Single().Message); + Assert.Throws( + () => ExecutionValidator.FailOnError.Validate(BenchmarkConverter.TypeToBenchmarks(typeof(NonPublicFieldWithParams)))); } public class NonPublicFieldWithParams @@ -290,6 +316,29 @@ public class NonPublicFieldWithParams public void NonThrowing() { } } + [Fact] + public void NonPublicFieldsWithParamsSourceAreDiscovered() + { + Assert.Throws( + () => ExecutionValidator.FailOnError.Validate(BenchmarkConverter.TypeToBenchmarks(typeof(NonPublicFieldWithParamsSource)))); + } + + public class NonPublicFieldWithParamsSource + { +#pragma warning disable CS0649 + [ParamsSource(nameof(Get))] + internal int Field; +#pragma warning restore CS0649 + + [Benchmark] + public void NonThrowing() { } + + public IEnumerable Get() + { + yield return 0; + } + } + [Fact] public void NonPublicPropertiesWithParamsAreDiscovered() { @@ -597,5 +646,241 @@ public async ValueTask GlobalCleanup() [Benchmark] public void NonThrowing() { } } + + [Fact] + public void IterationSetupIsSupported() + { + var validationErrors = ExecutionValidator.FailOnError.Validate(BenchmarkConverter.TypeToBenchmarks(typeof(IterationSetupIsSupportedClass))).ToList(); + + Assert.Empty(validationErrors); + } + + public class IterationSetupIsSupportedClass + { + [IterationSetup] + public void Setup() { } + + [Benchmark] + public void Foo() { } + } + + [Fact] + public void IterationCleanupIsSupported() + { + var validationErrors = ExecutionValidator.FailOnError.Validate(BenchmarkConverter.TypeToBenchmarks(typeof(IterationCleanupIsSupportedClass))).ToList(); + Assert.Empty(validationErrors); + } + + public class IterationCleanupIsSupportedClass + { + [IterationCleanup] + public void Cleanup() { } + + [Benchmark] + public void Foo() { } + } + + [Fact] + public void AsyncIterationSetupIsNotAllowed() + { + var validationErrors = ExecutionValidator.FailOnError.Validate(BenchmarkConverter.TypeToBenchmarks(typeof(AsyncIterationSetupIsNotAllowedClass))).ToList(); + + Assert.NotEmpty(validationErrors); + Assert.StartsWith("[IterationSetup] cannot be async. Error in type ", validationErrors.Single().Message); + } + + public class AsyncIterationSetupIsNotAllowedClass + { + [IterationSetup] + public Task Setup() => Task.CompletedTask; + + [Benchmark] + public void Foo() { } + } + + [Fact] + public void AsyncIterationCleanupIsNotAllowed() + { + var validationErrors = ExecutionValidator.FailOnError.Validate(BenchmarkConverter.TypeToBenchmarks(typeof(AsyncIterationCleanupIsNotAllowedClass))).ToList(); + + Assert.NotEmpty(validationErrors); + Assert.StartsWith("[IterationCleanup] cannot be async. Error in type ", validationErrors.Single().Message); + } + + public class AsyncIterationCleanupIsNotAllowedClass + { + [IterationCleanup] + public Task Cleanup() => Task.CompletedTask; + + [Benchmark] + public void Foo() { } + } + + [Fact] + public void SetupsWithCleanupsAreCalledInCorrectOrder() + { + var validationErrors = ExecutionValidator.FailOnError.Validate(BenchmarkConverter.TypeToBenchmarks(typeof(SetupsAndCleanups))).ToList(); + + Assert.True(SetupsAndCleanups.GlobalSetupIsCalled); + Assert.True(SetupsAndCleanups.IterationSetupIsCalled); + Assert.True(SetupsAndCleanups.BenchmarkIsCalled); + Assert.True(SetupsAndCleanups.IterationCleanupIsCalled); + Assert.True(SetupsAndCleanups.GlobalCleanupIsCalled); + + Assert.Empty(validationErrors); + } + + public class SetupsAndCleanups + { + public static bool GlobalSetupIsCalled; + public static bool IterationSetupIsCalled; + public static bool BenchmarkIsCalled; + public static bool IterationCleanupIsCalled; + public static bool GlobalCleanupIsCalled; + + [GlobalSetup] + public void GlobalSetup() => + GlobalSetupIsCalled = true; + + [IterationSetup] + public void IterationSetup() + { + if (!GlobalSetupIsCalled) + throw new Exception("[GlobalSetup] is not called"); + + IterationSetupIsCalled = true; + } + + [Benchmark] + public void Benchmark() + { + if (!IterationSetupIsCalled) + throw new Exception("[IterationSetup] is not called"); + + BenchmarkIsCalled = true; + } + + [IterationCleanup] + public void IterationCleanup() + { + if (!BenchmarkIsCalled) + throw new Exception("[Benchmark] is not called"); + + IterationCleanupIsCalled = true; + } + + [GlobalCleanup] + public void GlobalCleanup() + { + if (!IterationCleanupIsCalled) + throw new Exception("[IterationCleanup] is not called"); + + GlobalCleanupIsCalled = true; + } + } + + [Fact] + public void AsyncSetupsWithCleanupsAreCalledInCorrectOrder() + { + var validationErrors = ExecutionValidator.FailOnError.Validate(BenchmarkConverter.TypeToBenchmarks(typeof(AsyncSetupsAndCleanups))).ToList(); + + Assert.True(AsyncSetupsAndCleanups.AsyncGlobalSetupIsCalled); + Assert.True(AsyncSetupsAndCleanups.IterationSetupIsCalled); + Assert.True(AsyncSetupsAndCleanups.AsyncBenchmarkIsCalled); + Assert.True(AsyncSetupsAndCleanups.IterationCleanupIsCalled); + Assert.True(AsyncSetupsAndCleanups.AsyncGlobalCleanupIsCalled); + + Assert.Empty(validationErrors); + } + + public class AsyncSetupsAndCleanups + { + public static bool AsyncGlobalSetupIsCalled; + public static bool IterationSetupIsCalled; + public static bool AsyncBenchmarkIsCalled; + public static bool IterationCleanupIsCalled; + public static bool AsyncGlobalCleanupIsCalled; + + [GlobalSetup] + public async Task GlobalSetup() + { + await Task.Delay(1); + AsyncGlobalSetupIsCalled = true; + } + + [IterationSetup] + public void IterationSetup() + { + if (!AsyncGlobalSetupIsCalled) + throw new Exception("[GlobalSetup] is not called"); + + IterationSetupIsCalled = true; + } + + [Benchmark] + public async Task Benchmark() + { + if (!IterationSetupIsCalled) + throw new Exception("[IterationSetup] is not called"); + + await Task.Delay(1); + AsyncBenchmarkIsCalled = true; + } + + [IterationCleanup] + public void IterationCleanup() + { + if (!AsyncBenchmarkIsCalled) + throw new Exception("[Benchmark] is not called"); + + IterationCleanupIsCalled = true; + } + + [GlobalCleanup] + public async Task GlobalCleanup() + { + if (!IterationCleanupIsCalled) + throw new Exception("[IterationCleanup] is not called"); + + await Task.Delay(1); + AsyncGlobalCleanupIsCalled = true; + } + } + + [Fact] + public void BenchmarksMustBeIndependent() + { + var validationErrors = ExecutionValidator.FailOnError.Validate(BenchmarkConverter.TypeToBenchmarks(typeof(BenchmarksMustBeIndependentClass))).ToList(); + + Assert.Empty(validationErrors); + } + + public class BenchmarksMustBeIndependentClass + { + [Params(1, 2)] + public int N; + + private bool isBenchmarkExecuted; + + [Benchmark] + [Arguments(1)] + [Arguments(2)] + public void Foo() + { + if (isBenchmarkExecuted) + throw new Exception("Each benchmark must be called on a new instance"); + + isBenchmarkExecuted = true; + } + + [Benchmark] + public void Bar() + { + if (isBenchmarkExecuted) + throw new Exception("Each benchmark must be called on a new instance"); + + isBenchmarkExecuted = true; + } + } } } \ No newline at end of file diff --git a/tests/BenchmarkDotNet.Tests/Validators/ReturnValueValidatorTests.cs b/tests/BenchmarkDotNet.Tests/Validators/ReturnValueValidatorTests.cs index 40acc04fdc..cbc345dd04 100644 --- a/tests/BenchmarkDotNet.Tests/Validators/ReturnValueValidatorTests.cs +++ b/tests/BenchmarkDotNet.Tests/Validators/ReturnValueValidatorTests.cs @@ -2,6 +2,8 @@ using System.Collections.Generic; using System.Linq; using System.Text.RegularExpressions; +using System.Threading.Tasks; +using BenchmarkDotNet.Extensions; using BenchmarkDotNet.Attributes; using BenchmarkDotNet.Running; using BenchmarkDotNet.Validators; @@ -9,6 +11,8 @@ namespace BenchmarkDotNet.Tests.Validators { + // xUnit bug: https://github.com/xunit/xunit/issues/2587" + [Collection("Disable parallelism")] public class ReturnValueValidatorTests { private const string ErrorMessagePrefix = "Inconsistent benchmark return values"; @@ -253,6 +257,83 @@ public class ConsistentAlterParam public int Bar() => ++Value; } + [Fact] + public void ArgumentsAreSupported() + => AssertConsistent(); + + public class ArgumentsAreSupportedClass + { + [Arguments(1)] + [Arguments(2)] + [Benchmark] + public int Foo(int arg) => arg; + + [Arguments(2)] + [Arguments(4)] + [Benchmark] + public int Bar(int arg) => arg / 2; + } + + [Fact] + public void ArgumentsSourceAreSupported() + => AssertConsistent(); + + public class ArgumentsSourceAreSupportedClass + { + [ArgumentsSource(nameof(GetArguments))] + [Benchmark] + public int Foo(int arg) => arg; + + [ArgumentsSource(nameof(GetArguments))] + [Benchmark] + public int Bar(int arg) => arg; + + public IEnumerable GetArguments() + { + yield return 1; + yield return 2; + } + } + + [Fact] + public void ArgumentsWithArgumentsSourceAreSupported() + => AssertConsistent(); + + public class ArgumentsWithArgumentsSourceAreSupportedClass + { + [ArgumentsSource(nameof(GetArguments))] + [Benchmark] + public int Foo(int arg) => arg; + + [Benchmark] + [Arguments(1)] + [Arguments(2)] + public int Bar(int arg) => arg; + + public IEnumerable GetArguments() + { + yield return 1; + yield return 2; + } + } + + [Fact] + public void AsyncSetupIsSupported() + => AssertConsistent(); + + public class AsyncSetupIsSupportedClass + { + [Benchmark] + public async Task Foo() + { + await Task.Delay(1); + return 1; + } + + [Benchmark] + public int Bar() => 1; + } + private static void AssertConsistent() { var validationErrors = ReturnValueValidator.FailOnError.Validate(BenchmarkConverter.TypeToBenchmarks(typeof(TBenchmark))).ToList();