Improve coverage (#2526)

- Add missing coverage identified by #2524 and #2525. - Avoid allocating empty delegates. - Fix-up some formatting. - Remove redundant code. - Increase coverage threshold for Polly.Specs.
App-vNext · Feb 21, 2025 · 3fb0897 · 3fb0897
1 parent 4083d90
commit 3fb0897
Show file tree

Hide file tree

Showing 43 changed files with 759 additions and 445 deletions.
diff --git a/src/Polly.Core/ResiliencePipelineBuilderExtensions.cs b/src/Polly.Core/ResiliencePipelineBuilderExtensions.cs
@@ -147,12 +147,7 @@ public static TBuilder AddStrategy<TBuilder>(
         this TBuilder builder,
         Func<StrategyBuilderContext, ResilienceStrategy> factory)
         where TBuilder : ResiliencePipelineBuilderBase
-    {
-        Guard.NotNull(builder);
-        Guard.NotNull(factory);
-
-        return builder.AddStrategy(factory, EmptyOptions.Instance);
-    }
+        => builder.AddStrategy(factory, EmptyOptions.Instance);
 
     /// <summary>
     /// Adds a proactive resilience strategy to the builder.
@@ -170,12 +165,7 @@ public static TBuilder AddStrategy<TBuilder>(
     public static ResiliencePipelineBuilder AddStrategy(
         this ResiliencePipelineBuilder builder,
         Func<StrategyBuilderContext, ResilienceStrategy<object>> factory)
-    {
-        Guard.NotNull(builder);
-        Guard.NotNull(factory);
-
-        return builder.AddStrategy(factory, EmptyOptions.Instance);
-    }
+        => builder.AddStrategy(factory, EmptyOptions.Instance);
 
     /// <summary>
     /// Adds a reactive resilience strategy to the builder.
@@ -194,12 +184,7 @@ public static ResiliencePipelineBuilder AddStrategy(
     public static ResiliencePipelineBuilder<TResult> AddStrategy<TResult>(
         this ResiliencePipelineBuilder<TResult> builder,
         Func<StrategyBuilderContext, ResilienceStrategy<TResult>> factory)
-    {
-        Guard.NotNull(builder);
-        Guard.NotNull(factory);
-
-        return builder.AddStrategy(factory, EmptyOptions.Instance);
-    }
+        => builder.AddStrategy(factory, EmptyOptions.Instance);
 
     internal sealed class EmptyOptions : ResilienceStrategyOptions
     {

diff --git a/src/Polly.Core/ResiliencePipelineT.Async.cs b/src/Polly.Core/ResiliencePipelineT.Async.cs
@@ -19,12 +19,7 @@ public ValueTask<TResult> ExecuteAsync<TResult, TState>(
         ResilienceContext context,
         TState state)
         where TResult : T
-    {
-        Guard.NotNull(callback);
-        Guard.NotNull(context);
-
-        return Pipeline.ExecuteAsync(callback, context, state);
-    }
+        => Pipeline.ExecuteAsync(callback, context, state);
 
     /// <summary>
     /// Executes the specified callback.
@@ -38,12 +33,7 @@ public ValueTask<TResult> ExecuteAsync<TResult>(
         Func<ResilienceContext, ValueTask<TResult>> callback,
         ResilienceContext context)
         where TResult : T
-    {
-        Guard.NotNull(callback);
-        Guard.NotNull(context);
-
-        return Pipeline.ExecuteAsync(callback, context);
-    }
+        => Pipeline.ExecuteAsync(callback, context);
 
     /// <summary>
     /// Executes the specified callback.
@@ -60,11 +50,7 @@ public ValueTask<TResult> ExecuteAsync<TResult, TState>(
         TState state,
         CancellationToken cancellationToken = default)
         where TResult : T
-    {
-        Guard.NotNull(callback);
-
-        return Pipeline.ExecuteAsync(callback, state, cancellationToken);
-    }
+        => Pipeline.ExecuteAsync(callback, state, cancellationToken);
 
     /// <summary>
     /// Executes the specified callback.
@@ -78,11 +64,7 @@ public ValueTask<TResult> ExecuteAsync<TResult>(
         Func<CancellationToken, ValueTask<TResult>> callback,
         CancellationToken cancellationToken = default)
         where TResult : T
-    {
-        Guard.NotNull(callback);
-
-        return Pipeline.ExecuteAsync(callback, cancellationToken);
-    }
+        => Pipeline.ExecuteAsync(callback, cancellationToken);
 
     /// <summary>
     /// Executes the specified outcome-based callback.
@@ -103,10 +85,5 @@ public ValueTask<Outcome<TResult>> ExecuteOutcomeAsync<TResult, TState>(
         ResilienceContext context,
         TState state)
         where TResult : T
-    {
-        Guard.NotNull(callback);
-        Guard.NotNull(context);
-
-        return Pipeline.ExecuteOutcomeAsync(callback, context, state);
-    }
+        => Pipeline.ExecuteOutcomeAsync(callback, context, state);
 }
diff --git a/src/Polly.Core/ResiliencePipelineT.Sync.cs b/src/Polly.Core/ResiliencePipelineT.Sync.cs
@@ -19,12 +19,7 @@ public TResult Execute<TResult, TState>(
         ResilienceContext context,
         TState state)
         where TResult : T
-    {
-        Guard.NotNull(callback);
-        Guard.NotNull(context);
-
-        return Pipeline.Execute(callback, context, state);
-    }
+        => Pipeline.Execute(callback, context, state);
 
     /// <summary>
     /// Executes the specified callback.
@@ -38,12 +33,7 @@ public TResult Execute<TResult>(
         Func<ResilienceContext, TResult> callback,
         ResilienceContext context)
         where TResult : T
-    {
-        Guard.NotNull(callback);
-        Guard.NotNull(context);
-
-        return Pipeline.Execute(callback, context);
-    }
+        => Pipeline.Execute(callback, context);
 
     /// <summary>
     /// Executes the specified callback.
@@ -57,11 +47,7 @@ public TResult Execute<TResult>(
         Func<CancellationToken, TResult> callback,
         CancellationToken cancellationToken = default)
         where TResult : T
-    {
-        Guard.NotNull(callback);
-
-        return Pipeline.Execute(callback, cancellationToken);
-    }
+        => Pipeline.Execute(callback, cancellationToken);
 
     /// <summary>
     /// Executes the specified callback.
@@ -72,11 +58,7 @@ public TResult Execute<TResult>(
     /// <exception cref="ArgumentNullException">Thrown when <paramref name="callback"/> is <see langword="null"/>.</exception>
     public TResult Execute<TResult>(Func<TResult> callback)
         where TResult : T
-    {
-        Guard.NotNull(callback);
-
-        return Pipeline.Execute(callback);
-    }
+        => Pipeline.Execute(callback);
 
     /// <summary>
     /// Executes the specified callback.
@@ -89,11 +71,7 @@ public TResult Execute<TResult>(Func<TResult> callback)
     /// <exception cref="ArgumentNullException">Thrown when <paramref name="callback"/> is <see langword="null"/>.</exception>
     public TResult Execute<TResult, TState>(Func<TState, TResult> callback, TState state)
         where TResult : T
-    {
-        Guard.NotNull(callback);
-
-        return Pipeline.Execute(callback, state);
-    }
+        => Pipeline.Execute(callback, state);
 
     /// <summary>
     /// Executes the specified callback.
@@ -110,9 +88,5 @@ public TResult Execute<TResult, TState>(
         TState state,
         CancellationToken cancellationToken = default)
         where TResult : T
-    {
-        Guard.NotNull(callback);
-
-        return Pipeline.Execute(callback, state, cancellationToken);
-    }
+        => Pipeline.Execute(callback, state, cancellationToken);
 }
diff --git a/src/Polly.Core/Telemetry/TelemetryUtil.cs b/src/Polly.Core/Telemetry/TelemetryUtil.cs
@@ -49,11 +49,9 @@ private static void ReportAttempt<TResult>(
         Outcome<TResult> outcome,
         ExecutionAttemptArguments args)
     {
-        if (!telemetry.Enabled)
+        if (telemetry.Enabled)
         {
-            return;
+            telemetry.Report(resilienceEvent, context, outcome, args);
         }
-
-        telemetry.Report<ExecutionAttemptArguments, TResult>(resilienceEvent, context, outcome, args);
     }
 }
diff --git a/src/Polly.Core/Timeout/TimeoutResiliencePipelineBuilderExtensions.cs b/src/Polly.Core/Timeout/TimeoutResiliencePipelineBuilderExtensions.cs
@@ -20,14 +20,10 @@ public static class TimeoutResiliencePipelineBuilderExtensions
     /// <exception cref="ValidationException">Thrown when the options produced from the arguments are invalid.</exception>
     public static TBuilder AddTimeout<TBuilder>(this TBuilder builder, TimeSpan timeout)
         where TBuilder : ResiliencePipelineBuilderBase
-    {
-        Guard.NotNull(builder);
-
-        return builder.AddTimeout(new TimeoutStrategyOptions
+        => builder.AddTimeout(new TimeoutStrategyOptions
         {
             Timeout = timeout
         });
-    }
 
     /// <summary>
     /// Adds a timeout to the builder.

diff --git a/src/Polly.Extensions/DependencyInjection/AddResiliencePipelineContext.cs b/src/Polly.Extensions/DependencyInjection/AddResiliencePipelineContext.cs
@@ -2,7 +2,6 @@
 using Microsoft.Extensions.DependencyInjection;
 using Microsoft.Extensions.Options;
 using Polly.Registry;
-using Polly.Utils;
 
 namespace Polly.DependencyInjection;
 
@@ -69,9 +68,5 @@ internal AddResiliencePipelineContext(ConfigureBuilderContext<TKey> registryCont
     /// </summary>
     /// <param name="callback">The callback delegate.</param>
     public void OnPipelineDisposed(Action callback)
-    {
-        Guard.NotNull(callback);
-
-        RegistryContext.OnPipelineDisposed(callback);
-    }
+        => RegistryContext.OnPipelineDisposed(callback);
 }
diff --git a/src/Polly.Extensions/Telemetry/TelemetryListenerImpl.cs b/src/Polly.Extensions/Telemetry/TelemetryListenerImpl.cs
@@ -117,31 +117,27 @@ private void MeterEvent<TResult, TArgs>(in TelemetryEventArguments<TResult, TArg
 
         if (GetArgs<TArgs, PipelineExecutedArguments>(args.Arguments, out var executionFinished))
         {
-            if (!ExecutionDuration.Enabled)
+            if (ExecutionDuration.Enabled)
             {
-                return;
+                var tags = TagsList.Get();
+                var context = new EnrichmentContext<TResult, TArgs>(in args, tags.Tags);
+                UpdateEnrichmentContext(in context, severity);
+                ExecutionDuration.Record(executionFinished.Duration.TotalMilliseconds, tags.TagsSpan);
+                TagsList.Return(tags);
             }
-
-            var tags = TagsList.Get();
-            var context = new EnrichmentContext<TResult, TArgs>(in args, tags.Tags);
-            UpdateEnrichmentContext(in context, severity);
-            ExecutionDuration.Record(executionFinished.Duration.TotalMilliseconds, tags.TagsSpan);
-            TagsList.Return(tags);
         }
         else if (GetArgs<TArgs, ExecutionAttemptArguments>(args.Arguments, out var executionAttempt))
         {
-            if (!AttemptDuration.Enabled)
+            if (AttemptDuration.Enabled)
             {
-                return;
+                var tags = TagsList.Get();
+                var context = new EnrichmentContext<TResult, TArgs>(in args, tags.Tags);
+                UpdateEnrichmentContext(in context, severity);
+                context.Tags.Add(new(ResilienceTelemetryTags.AttemptNumber, executionAttempt.AttemptNumber.AsBoxedInt()));
+                context.Tags.Add(new(ResilienceTelemetryTags.AttemptHandled, executionAttempt.Handled.AsBoxedBool()));
+                AttemptDuration.Record(executionAttempt.Duration.TotalMilliseconds, tags.TagsSpan);
+                TagsList.Return(tags);
             }
-
-            var tags = TagsList.Get();
-            var context = new EnrichmentContext<TResult, TArgs>(in args, tags.Tags);
-            UpdateEnrichmentContext(in context, severity);
-            context.Tags.Add(new(ResilienceTelemetryTags.AttemptNumber, executionAttempt.AttemptNumber.AsBoxedInt()));
-            context.Tags.Add(new(ResilienceTelemetryTags.AttemptHandled, executionAttempt.Handled.AsBoxedBool()));
-            AttemptDuration.Record(executionAttempt.Duration.TotalMilliseconds, tags.TagsSpan);
-            TagsList.Return(tags);
         }
         else if (Counter.Enabled)
         {

diff --git a/src/Polly.RateLimiting/RateLimiterResiliencePipelineBuilderExtensions.cs b/src/Polly.RateLimiting/RateLimiterResiliencePipelineBuilderExtensions.cs
@@ -27,8 +27,6 @@ public static TBuilder AddConcurrencyLimiter<TBuilder>(
         int queueLimit = 0)
         where TBuilder : ResiliencePipelineBuilderBase
     {
-        Guard.NotNull(builder);
-
         return builder.AddConcurrencyLimiter(new ConcurrencyLimiterOptions
         {
             PermitLimit = permitLimit,

diff --git a/src/Polly/Caching/CacheSyntax.cs b/src/Polly/Caching/CacheSyntax.cs
@@ -124,24 +124,7 @@ public static CachePolicy Cache(ISyncCacheProvider cacheProvider, TimeSpan ttl,
     /// <exception cref="ArgumentNullException">Thrown when <paramref name="ttlStrategy"/> is <see langword="null"/>.</exception>
     /// <exception cref="ArgumentNullException">Thrown when <paramref name="cacheKeyStrategy"/> is <see langword="null"/>.</exception>
     public static CachePolicy Cache(ISyncCacheProvider cacheProvider, ITtlStrategy ttlStrategy, Func<Context, string> cacheKeyStrategy, Action<Context, string, Exception>? onCacheError = null)
-    {
-        if (cacheProvider == null)
-        {
-            throw new ArgumentNullException(nameof(cacheProvider));
-        }
-
-        if (ttlStrategy == null)
-        {
-            throw new ArgumentNullException(nameof(ttlStrategy));
-        }
-
-        if (cacheKeyStrategy == null)
-        {
-            throw new ArgumentNullException(nameof(cacheKeyStrategy));
-        }
-
-        return Cache(cacheProvider, ttlStrategy, cacheKeyStrategy, EmptyCallback, EmptyCallback, EmptyCallback, onCacheError, onCacheError);
-    }
+        => Cache(cacheProvider, ttlStrategy, cacheKeyStrategy, EmptyCallback, EmptyCallback, EmptyCallback, onCacheError, onCacheError);
 
     /// <summary>
     /// <para>Builds a <see cref="Policy"/> that will function like a result cache for delegate executions returning a result.</para>

diff --git a/src/Polly/CircuitBreaker/AdvancedCircuitBreakerTResultSyntax.cs b/src/Polly/CircuitBreaker/AdvancedCircuitBreakerTResultSyntax.cs
@@ -29,17 +29,12 @@ public static class AdvancedCircuitBreakerTResultSyntax
     /// <exception cref="ArgumentOutOfRangeException">minimumThroughput;Value must be greater than one.</exception>
     /// <exception cref="ArgumentOutOfRangeException">durationOfBreak;Value must be greater than zero.</exception>
     /// <remarks>(see "Release It!" by Michael T. Nygard fi).</remarks>
-    public static CircuitBreakerPolicy<TResult> AdvancedCircuitBreaker<TResult>(this PolicyBuilder<TResult> policyBuilder, double failureThreshold, TimeSpan samplingDuration, int minimumThroughput, TimeSpan durationOfBreak)
-    {
-        Action<DelegateResult<TResult>, TimeSpan> doNothingOnBreak = (_, _) => { };
-        Action doNothingOnReset = () => { };
-
-        return policyBuilder.AdvancedCircuitBreaker(
+    public static CircuitBreakerPolicy<TResult> AdvancedCircuitBreaker<TResult>(this PolicyBuilder<TResult> policyBuilder, double failureThreshold, TimeSpan samplingDuration, int minimumThroughput, TimeSpan durationOfBreak) =>
+        policyBuilder.AdvancedCircuitBreaker(
             failureThreshold, samplingDuration, minimumThroughput,
             durationOfBreak,
-            doNothingOnBreak,
-            doNothingOnReset);
-    }
+            static (_, _) => { },
+            static () => { });
 
     /// <summary>
     /// <para>The circuit will break if, within any timeslice of duration <paramref name="samplingDuration"/>, the proportion of actions resulting in a handled exception or result exceeds <paramref name="failureThreshold"/>, provided also that the number of actions through the circuit in the timeslice is at least <paramref name="minimumThroughput" />. </para>
@@ -102,15 +97,14 @@ public static CircuitBreakerPolicy<TResult> AdvancedCircuitBreaker<TResult>(this
     /// <exception cref="ArgumentOutOfRangeException">durationOfBreak;Value must be greater than zero.</exception>
     /// <exception cref="ArgumentNullException">Thrown when <paramref name="onBreak"/> is <see langword="null"/>.</exception>
     /// <exception cref="ArgumentNullException">Thrown when <paramref name="onReset"/> is <see langword="null"/>.</exception>
-    public static CircuitBreakerPolicy<TResult> AdvancedCircuitBreaker<TResult>(this PolicyBuilder<TResult> policyBuilder, double failureThreshold, TimeSpan samplingDuration, int minimumThroughput, TimeSpan durationOfBreak, Action<DelegateResult<TResult>, TimeSpan, Context> onBreak, Action<Context> onReset)
-    {
-        Action doNothingOnHalfOpen = () => { };
-        return policyBuilder.AdvancedCircuitBreaker(failureThreshold, samplingDuration, minimumThroughput,
+    public static CircuitBreakerPolicy<TResult> AdvancedCircuitBreaker<TResult>(this PolicyBuilder<TResult> policyBuilder, double failureThreshold, TimeSpan samplingDuration, int minimumThroughput, TimeSpan durationOfBreak, Action<DelegateResult<TResult>, TimeSpan, Context> onBreak, Action<Context> onReset) =>
+        policyBuilder.AdvancedCircuitBreaker(
+            failureThreshold,
+            samplingDuration, minimumThroughput,
             durationOfBreak,
             onBreak,
             onReset,
-            doNothingOnHalfOpen);
-    }
+            static () => { });
 
     /// <summary>
     /// <para>The circuit will break if, within any timeslice of duration <paramref name="samplingDuration"/>, the proportion of actions resulting in a handled exception or result exceeds <paramref name="failureThreshold"/>, provided also that the number of actions through the circuit in the timeslice is at least <paramref name="minimumThroughput" />. </para>