Add missing coverage

src/Polly.Extensions/DependencyInjection/PollyServiceCollectionExtensions.cs

@@ -234,7 +234,6 @@ public static IServiceCollection AddResiliencePipelineRegistry<TKey>(this IServi
             return services;
         }
 
-        services.AddOptions();
         services.Add(RegistryMarker<TKey>.ServiceDescriptor);
         services.AddResiliencePipelineBuilder();
 

src/Polly/Bulkhead/AsyncBulkheadTResultSyntax.cs

@@ -11,10 +11,7 @@ public partial class Policy
     /// <param name="maxParallelization">The maximum number of concurrent actions that may be executing through the policy.</param>
     /// <returns>The policy instance.</returns>
     public static AsyncBulkheadPolicy<TResult> BulkheadAsync<TResult>(int maxParallelization)
-    {
-        Func<Context, Task> doNothingAsync = _ => TaskHelper.EmptyTask;
-        return BulkheadAsync<TResult>(maxParallelization, 0, doNothingAsync);
-    }
+        => BulkheadAsync<TResult>(maxParallelization, 0, EmptyHandler);
 
     /// <summary>
     /// <para>Builds a bulkhead isolation <see cref="AsyncPolicy{TResult}"/>, which limits the maximum concurrency of actions executed through the policy.  Imposing a maximum concurrency limits the potential of governed actions, when faulting, to bring down the system.</para>
@@ -40,10 +37,7 @@ public static AsyncBulkheadPolicy<TResult> BulkheadAsync<TResult>(int maxParalle
     /// <exception cref="ArgumentOutOfRangeException">maxParallelization;Value must be greater than zero.</exception>
     /// <exception cref="ArgumentOutOfRangeException">maxQueuingActions;Value must be greater than or equal to zero.</exception>
     public static AsyncBulkheadPolicy<TResult> BulkheadAsync<TResult>(int maxParallelization, int maxQueuingActions)
-    {
-        Func<Context, Task> doNothingAsync = _ => TaskHelper.EmptyTask;
-        return BulkheadAsync<TResult>(maxParallelization, maxQueuingActions, doNothingAsync);
-    }
+        => BulkheadAsync<TResult>(maxParallelization, maxQueuingActions, EmptyHandler);
 
     /// <summary>
     /// Builds a bulkhead isolation <see cref="AsyncPolicy{TResult}" />, which limits the maximum concurrency of actions executed through the policy.  Imposing a maximum concurrency limits the potential of governed actions, when faulting, to bring down the system.
@@ -79,4 +73,6 @@ public static AsyncBulkheadPolicy<TResult> BulkheadAsync<TResult>(int maxParalle
             maxQueuingActions,
             onBulkheadRejectedAsync);
     }
+
+    private static Task EmptyHandler(Context context) => TaskHelper.EmptyTask;
 }

src/Polly/Caching/CacheSyntax.cs

@@ -90,9 +90,7 @@ public static CachePolicy Cache(ISyncCacheProvider cacheProvider, ITtlStrategy t
             throw new ArgumentNullException(nameof(cacheKeyStrategy));
         }
 
-        Action<Context, string> emptyDelegate = (_, _) => { };
-
-        return Cache(cacheProvider, ttlStrategy, cacheKeyStrategy.GetCacheKey, emptyDelegate, emptyDelegate, emptyDelegate, onCacheError, onCacheError);
+        return Cache(cacheProvider, ttlStrategy, cacheKeyStrategy.GetCacheKey, EmptyCallback, EmptyCallback, EmptyCallback, onCacheError, onCacheError);
     }
 
     /// <summary>
@@ -142,9 +140,7 @@ public static CachePolicy Cache(ISyncCacheProvider cacheProvider, ITtlStrategy t
             throw new ArgumentNullException(nameof(cacheKeyStrategy));
         }
 
-        Action<Context, string> emptyDelegate = (_, _) => { };
-
-        return Cache(cacheProvider, ttlStrategy, cacheKeyStrategy, emptyDelegate, emptyDelegate, emptyDelegate, onCacheError, onCacheError);
+        return Cache(cacheProvider, ttlStrategy, cacheKeyStrategy, EmptyCallback, EmptyCallback, EmptyCallback, onCacheError, onCacheError);
     }
 
     /// <summary>

src/Polly/Policy.ContextAndKeys.cs

@@ -25,16 +25,7 @@ public Policy WithPolicyKey(string policyKey)
     /// </summary>
     /// <param name="policyKey">The unique, used-definable key to assign to this <see cref="Policy"/> instance.</param>
     /// <returns>An instance of <see cref="ISyncPolicy"/>.</returns>
-    ISyncPolicy ISyncPolicy.WithPolicyKey(string policyKey)
-    {
-        if (policyKeyInternal != null)
-        {
-            throw PolicyKeyMustBeImmutableException(nameof(policyKey));
-        }
-
-        policyKeyInternal = policyKey;
-        return this;
-    }
+    ISyncPolicy ISyncPolicy.WithPolicyKey(string policyKey) => WithPolicyKey(policyKey);
 }
 
 #pragma warning disable CA1724
@@ -64,14 +55,5 @@ public Policy<TResult> WithPolicyKey(string policyKey)
     /// </summary>
     /// <param name="policyKey">The unique, used-definable key to assign to this <see cref="Policy{TResult}"/> instance.</param>
     /// <returns>An instance of <see cref="ISyncPolicy{TResult}"/>.</returns>
-    ISyncPolicy<TResult> ISyncPolicy<TResult>.WithPolicyKey(string policyKey)
-    {
-        if (policyKeyInternal != null)
-        {
-            throw PolicyKeyMustBeImmutableException(nameof(policyKey));
-        }
-
-        policyKeyInternal = policyKey;
-        return this;
-    }
+    ISyncPolicy<TResult> ISyncPolicy<TResult>.WithPolicyKey(string policyKey) => WithPolicyKey(policyKey);
 }

src/Polly/Registry/PolicyRegistry.cs

@@ -7,7 +7,7 @@
 /// <remarks>Uses ConcurrentDictionary to store the collection.</remarks>
 public class PolicyRegistry : IConcurrentPolicyRegistry<string>
 {
-    private readonly IDictionary<string, IsPolicy> _registry = new ConcurrentDictionary<string, IsPolicy>();
+    private readonly ConcurrentDictionary<string, IsPolicy> _registry = [];
 
     /// <summary>
     /// Initializes a new instance of the <see cref="PolicyRegistry"/> class, with <see cref="string"/> keys.
@@ -26,22 +26,9 @@ public PolicyRegistry()
     /// </summary>
     /// <param name="registry">a dictionary containing keys and policies used for testing.</param>
     internal PolicyRegistry(IDictionary<string, IsPolicy> registry)
-    {
-#pragma warning disable S3236 // Remove this argument from the method call; it hides the caller information.
-        Debug.Assert(registry != null, "This constructor is for testing only, and should not be called with a null registry.");
-#pragma warning restore S3236 // Remove this argument from the method call; it hides the caller information.
-        _registry = registry;
-    }
-
-    private ConcurrentDictionary<string, IsPolicy> ThrowIfNotConcurrentImplementation()
-    {
-        if (_registry is ConcurrentDictionary<string, IsPolicy> concurrentRegistry)
-        {
-            return concurrentRegistry;
-        }
-
-        throw new InvalidOperationException($"This {nameof(PolicyRegistry)} is not configured for concurrent operations. This exception should never be thrown in production code as the only public constructors create {nameof(PolicyRegistry)} instances of the correct form.");
-    }
+#pragma warning disable IDE0306 // Simplify collection initialization
+        => _registry = new(registry);
+#pragma warning restore IDE0306 // Simplify collection initialization
 
     /// <summary>
     /// Gets the total number of policies in the registry.
@@ -57,7 +44,11 @@ private ConcurrentDictionary<string, IsPolicy> ThrowIfNotConcurrentImplementatio
     /// <exception cref="ArgumentNullException">Thrown when <paramref name="key"/> is <see langword="null"/>.</exception>
     /// <exception cref="ArgumentException">A Policy with same <paramref name="key"/> already exists.</exception>
     public void Add<TPolicy>(string key, TPolicy policy)
-        where TPolicy : IsPolicy => _registry.Add(key, policy);
+        where TPolicy : IsPolicy
+    {
+        IDictionary<string, IsPolicy> dictionary = _registry;
+        dictionary.Add(key, policy);
+    }
 
     /// <summary>
     /// Adds a policy with the provided key and policy to the registry.
@@ -68,11 +59,7 @@ public void Add<TPolicy>(string key, TPolicy policy)
     /// <returns>True if Policy was added. False otherwise.</returns>
     public bool TryAdd<TPolicy>(string key, TPolicy policy)
         where TPolicy : IsPolicy
-    {
-        var registry = ThrowIfNotConcurrentImplementation();
-
-        return registry.TryAdd(key, policy);
-    }
+        => _registry.TryAdd(key, policy);
 
     /// <summary>
     /// Gets of sets the <see cref="IsPolicy"/> with the specified key.
@@ -145,8 +132,11 @@ public bool ContainsKey(string key) =>
     /// <param name="key">The <paramref name="key"/> of the policy to remove.</param>
     /// <returns>True if the policy is successfully removed. Otherwise false.</returns>
     /// <exception cref="ArgumentNullException"><paramref name="key"/> is <see langword="null"/>.</exception>
-    public bool Remove(string key) =>
-        _registry.Remove(key);
+    public bool Remove(string key)
+    {
+        IDictionary<string, IsPolicy> dictionary = _registry;
+        return dictionary.Remove(key);
+    }
 
     /// <summary>
     /// Removes the policy stored under the specified <paramref name="key"/> from the registry.
@@ -162,10 +152,8 @@ public bool Remove(string key) =>
     public bool TryRemove<TPolicy>(string key, out TPolicy policy)
         where TPolicy : IsPolicy
     {
-        var registry = ThrowIfNotConcurrentImplementation();
-
         policy = default;
-        bool got = registry.TryRemove(key, out IsPolicy value);
+        bool got = _registry.TryRemove(key, out IsPolicy value);
 
         if (got)
         {
@@ -188,11 +176,7 @@ public bool TryRemove<TPolicy>(string key, out TPolicy policy)
     /// </returns>
     public bool TryUpdate<TPolicy>(string key, TPolicy newPolicy, TPolicy comparisonPolicy)
         where TPolicy : IsPolicy
-    {
-        var registry = ThrowIfNotConcurrentImplementation();
-
-        return registry.TryUpdate(key, newPolicy, comparisonPolicy);
-    }
+        => _registry.TryUpdate(key, newPolicy, comparisonPolicy);
 
     /// <summary>
     /// Adds a policy with the provided key and policy to the registry
@@ -206,11 +190,7 @@ public bool TryUpdate<TPolicy>(string key, TPolicy newPolicy, TPolicy comparison
     /// if the key was not in the registry.</returns>
     public TPolicy GetOrAdd<TPolicy>(string key, Func<string, TPolicy> policyFactory)
         where TPolicy : IsPolicy
-    {
-        var registry = ThrowIfNotConcurrentImplementation();
-
-        return (TPolicy)registry.GetOrAdd(key, k => policyFactory(k));
-    }
+        => (TPolicy)_registry.GetOrAdd(key, k => policyFactory(k));
 
     /// <summary>
     /// Adds a key/policy pair to the registry
@@ -223,11 +203,7 @@ public TPolicy GetOrAdd<TPolicy>(string key, Func<string, TPolicy> policyFactory
     /// key is already in the registry, or the new policy if the key was not in the registry.</returns>
     public TPolicy GetOrAdd<TPolicy>(string key, TPolicy policy)
         where TPolicy : IsPolicy
-    {
-        var registry = ThrowIfNotConcurrentImplementation();
-
-        return (TPolicy)registry.GetOrAdd(key, policy);
-    }
+        => (TPolicy)_registry.GetOrAdd(key, policy);
 
     /// <summary>
     /// Adds a key/policy pair to the registry if the key does not already
@@ -243,11 +219,7 @@ public TPolicy GetOrAdd<TPolicy>(string key, TPolicy policy)
     /// absent) or the result of updatePolicyFactory (if the key was present).</returns>
     public TPolicy AddOrUpdate<TPolicy>(string key, Func<string, TPolicy> addPolicyFactory, Func<string, TPolicy, TPolicy> updatePolicyFactory)
         where TPolicy : IsPolicy
-    {
-        var registry = ThrowIfNotConcurrentImplementation();
-
-        return (TPolicy)registry.AddOrUpdate(key, k => addPolicyFactory(k), (k, e) => updatePolicyFactory(k, (TPolicy)e));
-    }
+        => (TPolicy)_registry.AddOrUpdate(key, k => addPolicyFactory(k), (k, e) => updatePolicyFactory(k, (TPolicy)e));
 
     /// <summary>
     /// Adds a key/policy pair to the registry if the key does not already
@@ -263,11 +235,7 @@ public TPolicy AddOrUpdate<TPolicy>(string key, Func<string, TPolicy> addPolicyF
     /// absent) or the result of updatePolicyFactory (if the key was present).</returns>
     public TPolicy AddOrUpdate<TPolicy>(string key, TPolicy addPolicy, Func<string, TPolicy, TPolicy> updatePolicyFactory)
         where TPolicy : IsPolicy
-    {
-        var registry = ThrowIfNotConcurrentImplementation();
-
-        return (TPolicy)registry.AddOrUpdate(key, addPolicy, (k, e) => updatePolicyFactory(k, (TPolicy)e));
-    }
+        => (TPolicy)_registry.AddOrUpdate(key, addPolicy, (k, e) => updatePolicyFactory(k, (TPolicy)e));
 
     /// <summary>Returns an enumerator that iterates through the policy objects in the <see
     /// cref="PolicyRegistry"/>.</summary>

test/Polly.Extensions.Tests/DependencyInjection/PollyServiceCollectionExtensionTests.cs

@@ -22,24 +22,34 @@ public class PollyServiceCollectionExtensionTests
     public void AddResiliencePipeline_ArgValidation()
     {
         _services = null!;
-        Assert.Throws<ArgumentNullException>(() => AddResiliencePipeline(Key));
-        Assert.Throws<ArgumentNullException>(() => AddResiliencePipeline<string>(Key));
+        Assert.Throws<ArgumentNullException>("services", () => AddResiliencePipeline(Key));
+        Assert.Throws<ArgumentNullException>("services", () => AddResiliencePipeline<string>(Key));
+        Assert.Throws<ArgumentNullException>("services", () => _services.AddResiliencePipeline(Key, (_) => { }));
+        Assert.Throws<ArgumentNullException>("services", () => _services.AddResiliencePipeline(Key, (_, _) => { }));
+        Assert.Throws<ArgumentNullException>("services", () => _services.AddResiliencePipeline<string, string>(Key, (_) => { }));
+        Assert.Throws<ArgumentNullException>("services", () => _services.AddResiliencePipeline<string, string>(Key, (_, _) => { }));
 
         _services = [];
-        Assert.Throws<ArgumentNullException>(() => _services.AddResiliencePipeline(
+        Assert.Throws<ArgumentNullException>("configure", () => _services.AddResiliencePipeline(
             Key,
             (Action<ResiliencePipelineBuilder, AddResiliencePipelineContext<string>>)null!));
-        Assert.Throws<ArgumentNullException>(() => _services.AddResiliencePipeline(
+        Assert.Throws<ArgumentNullException>("configure", () => _services.AddResiliencePipeline(
             Key,
             (Action<ResiliencePipelineBuilder<string>, AddResiliencePipelineContext<string>>)null!));
 
-        Assert.Throws<ArgumentNullException>(() => _services.AddResiliencePipeline(
+        Assert.Throws<ArgumentNullException>("configure", () => _services.AddResiliencePipeline(
             Key,
             (Action<ResiliencePipelineBuilder>)null!));
 
-        Assert.Throws<ArgumentNullException>(() => _services.AddResiliencePipeline(
+        Assert.Throws<ArgumentNullException>("configure", () => _services.AddResiliencePipeline(
             Key,
             (Action<ResiliencePipelineBuilder<string>>)null!));
+
+        _services.AddResiliencePipelines<string>(ctx =>
+        {
+            Assert.Throws<ArgumentNullException>("configure", () => ctx.AddResiliencePipeline(Key, null!));
+            Assert.Throws<ArgumentNullException>("configure", () => ctx.AddResiliencePipeline<string>(Key, null!));
+        });
     }
 
     [InlineData(true)]
@@ -62,6 +72,7 @@ public void AddResiliencePipeline_EnsureRegisteredServices(bool generic)
         serviceProvider.GetServices<ResiliencePipelineRegistry<string>>().ShouldNotBeNull();
         serviceProvider.GetServices<ResiliencePipelineProvider<string>>().ShouldNotBeNull();
         serviceProvider.GetServices<ResiliencePipelineBuilder>().ShouldNotBeSameAs(serviceProvider.GetServices<ResiliencePipelineBuilder>());
+        serviceProvider.GetRequiredService<IOptions<ResiliencePipelineRegistryOptions<string>>>().ShouldNotBeNull();
     }
 
     [Fact]

test/Polly.Extensions.Tests/DisposablePipelineTests.cs

@@ -68,6 +68,23 @@ public async Task DisposePipeline_EnsureExternalPipelineNotDisposed()
         Should.Throw<ObjectDisposedException>(() => pipeline2.Execute(() => string.Empty));
     }
 
+    [Fact]
+    public void DisposePipeline_OnPipelineDisposed_Throws_NullArg()
+    {
+        var asserted = false;
+
+        var provider = new ServiceCollection()
+            .AddResiliencePipeline("my-pipeline", (_, context) =>
+            {
+                Assert.Throws<ArgumentNullException>("callback", () => context.OnPipelineDisposed(null!));
+                asserted = true;
+            })
+            .BuildServiceProvider();
+
+        provider.GetRequiredService<ResiliencePipelineProvider<string>>().GetPipeline("my-pipeline");
+        asserted.ShouldBeTrue();
+    }
+
     private static bool IsDisposed(RateLimiter limiter)
     {
         try

test/Polly.Extensions.Tests/Registry/ConfigureBuilderContextExtensionsTEsts.cs → test/Polly.Extensions.Tests/Registry/ConfigureBuilderContextExtensionsTests.cs

@@ -7,6 +7,22 @@ namespace Polly.Extensions.Tests.Registry;
 
 public class ConfigureBuilderContextExtensionsTests
 {
+    [Fact]
+    public async Task ConfigureReloads_NullArguments_Throws()
+    {
+        await using var registry = new ResiliencePipelineRegistry<string>();
+        registry.GetOrAddPipeline("pipeline", (_, builderContext) =>
+        {
+            ConfigureBuilderContext<string> nullContext = null!;
+            IOptionsMonitor<Options> nullMonitor = null!;
+
+            var monitor = Substitute.For<IOptionsMonitor<Options>>();
+
+            Assert.Throws<ArgumentNullException>("context", () => nullContext.EnableReloads(monitor));
+            Assert.Throws<ArgumentNullException>("optionsMonitor", () => builderContext.EnableReloads(nullMonitor));
+        });
+    }
+
     [Fact]
     public async Task ConfigureReloads_MonitorRegistrationReturnsNull_DoesNotThrow()
     {
@@ -27,5 +43,7 @@ public async Task ConfigureReloads_MonitorRegistrationReturnsNull_DoesNotThrow()
         listener.Events.ShouldBeEmpty();
     }
 
+#pragma warning disable S2094 // Classes should not be empty
     public class Options;
+#pragma warning restore S2094 // Classes should not be empty
 }

test/Polly.Extensions.Tests/Telemetry/TelemetryOptionsTests.cs

@@ -24,6 +24,13 @@ public void Ctor_EnsureDefaults()
         options.ResultFormatter(resilienceContext, response).ShouldBe(200);
     }
 
+    [Fact]
+    public void CopyCtor_OtherNull_Throws()
+    {
+        var options = new TelemetryOptions();
+        Assert.Throws<ArgumentNullException>("other", () => new TelemetryOptions(null!));
+    }
+
     [Fact]
     public void CopyCtor_Ok()
     {

test/Polly.Extensions.Tests/Telemetry/TelemetryResiliencePipelineBuilderExtensionsTests.cs

@@ -1,4 +1,5 @@
 using System.ComponentModel.DataAnnotations;
+using Microsoft.Extensions.Logging;
 using Microsoft.Extensions.Logging.Abstractions;
 using Polly.Telemetry;
 
@@ -28,6 +29,24 @@ public void ConfigureTelemetry_EnsureLogging()
         fakeLogger.GetRecords().Count().ShouldBe(2);
     }
 
+    [Fact]
+    public void ConfigureTelemetry_NullArguments_Throws()
+    {
+        ResiliencePipelineBuilder builder = null!;
+
+        using var loggerFactory = TestUtilities.CreateLoggerFactory(out var fakeLogger);
+        var options = new TelemetryOptions();
+
+        Assert.Throws<ArgumentNullException>("builder", () => builder.ConfigureTelemetry(loggerFactory));
+        Assert.Throws<ArgumentNullException>("builder", () => builder.ConfigureTelemetry(options));
+
+        ILoggerFactory nulLoggerFactory = null!;
+        TelemetryOptions nulOptions = null!;
+
+        Assert.Throws<ArgumentNullException>("loggerFactory", () => _builder.ConfigureTelemetry(nulLoggerFactory));
+        Assert.Throws<ArgumentNullException>("options", () => _builder.ConfigureTelemetry(nulOptions));
+    }
+
     [Fact]
     public void ConfigureTelemetry_InvalidOptions_Throws()
     {