Refactor distributed lock resilience tests

- Consolidated test logic: streamlined test providers, injected services, and reusable test patterns.
- Simplified `TestDistributedLockProvider` implementation with enhanced initialization and failure simulation.
- Reorganized tests for transient acquisition/release failures to use parameterized `Theory` for improved maintainability.

Author: sfmskywalker

test/component/Elsa.Workflows.ComponentTests/Scenarios/DistributedLockResilience/DistributedLockResilienceTests.cs

@@ -13,131 +13,82 @@ namespace Elsa.Workflows.ComponentTests.Scenarios.DistributedLockResilience;
 
 public class DistributedLockResilienceTests(App app) : AppComponentTest(app)
 {
-    [Fact]
-    public async Task AcquireLockWithRetry_TransientFailureOnAcquisition_ShouldRetryAndSucceed()
+    private TestDistributedLockProvider MockProvider => Scope.ServiceProvider.GetRequiredService<TestDistributedLockProvider>();
+    private IDistributedLockProvider LockProvider => Scope.ServiceProvider.GetRequiredService<IDistributedLockProvider>();
+    private ITransientExceptionDetectionService TransientDetectionService => Scope.ServiceProvider.GetRequiredService<ITransientExceptionDetectionService>();
+    private ILogger<DistributedLockResilienceTests> Logger => Scope.ServiceProvider.GetRequiredService<ILogger<DistributedLockResilienceTests>>();
+    private DistributedLockingOptions LockOptions => Scope.ServiceProvider.GetRequiredService<IOptions<DistributedLockingOptions>>().Value;
+    private ResiliencePipeline RetryPipeline => CreateRetryPipeline(TransientDetectionService, Logger);
+
+    [Theory]
+    [InlineData(1, 2, false)] // Single failure, succeeds on retry
+    [InlineData(2, 3, false)] // Two failures, succeeds on third attempt
+    [InlineData(4, 4, true)]  // Four failures, exhausts retries (MaxRetryAttempts = 3)
+    public async Task AcquireLockWithRetry_AcquisitionFailures_BehavesAsExpected(int failureCount, int expectedAttemptCount, bool shouldThrow)
     {
         // Arrange
-        var mockProvider = Scope.ServiceProvider.GetRequiredService<TestDistributedLockProvider>();
-        mockProvider.Reset();
-        mockProvider.FailAcquisitionOnce();
-
-        var lockProvider = Scope.ServiceProvider.GetRequiredService<IDistributedLockProvider>();
-        var transientDetectionService = Scope.ServiceProvider.GetRequiredService<ITransientExceptionDetectionService>();
-        var logger = Scope.ServiceProvider.GetRequiredService<ILogger<DistributedLockResilienceTests>>();
-        var lockOptions = Scope.ServiceProvider.GetRequiredService<IOptions<DistributedLockingOptions>>();
-
-        var retryPipeline = CreateRetryPipeline(transientDetectionService, logger);
-
-        // Act - should succeed after retry
-        var handle = await retryPipeline.ExecuteAsync(async ct =>
-            await lockProvider.AcquireLockAsync("test-lock-1", lockOptions.Value.LockAcquisitionTimeout, ct),
-            CancellationToken.None);
+        MockProvider.Reset();
+        MockProvider.FailAcquisitionTimes(failureCount);
 
-        // Assert
-        Assert.NotNull(handle);
-        Assert.Equal(2, mockProvider.AcquisitionAttemptCount); // First attempt fails, second succeeds
-
-        await handle.DisposeAsync();
-    }
-
-    [Fact]
-    public async Task AcquireLockWithRetry_TransientFailureOnRelease_ShouldLogButNotThrow()
-    {
-        // Arrange
-        var mockProvider = Scope.ServiceProvider.GetRequiredService<TestDistributedLockProvider>();
-        mockProvider.Reset();
-        mockProvider.FailReleaseOnce();
-
-        var lockProvider = Scope.ServiceProvider.GetRequiredService<IDistributedLockProvider>();
-        var lockOptions = Scope.ServiceProvider.GetRequiredService<IOptions<DistributedLockingOptions>>();
-        var logger = Scope.ServiceProvider.GetRequiredService<ILogger<DistributedLockResilienceTests>>();
-
-        var handle = await lockProvider.AcquireLockAsync("test-lock-2", lockOptions.Value.LockAcquisitionTimeout);
-
-        // Act & Assert - should not throw despite release failure (mimics production try-catch behavior)
-        try
+        // Act & Assert
+        if (shouldThrow)
         {
-            await handle.DisposeAsync();
+            await Assert.ThrowsAsync<TimeoutException>(async () => await AcquireLockWithRetryAsync($"test-lock-{failureCount}"));
         }
-        catch (Exception ex)
+        else
         {
-            logger.LogWarning(ex, "Failed to release distributed lock (expected test behavior)");
+            await using var handle = await AcquireLockWithRetryAsync($"test-lock-{failureCount}");
+            Assert.NotNull(handle);
         }
 
-        Assert.Equal(1, mockProvider.ReleaseAttemptCount);
+        Assert.Equal(expectedAttemptCount, MockProvider.AcquisitionAttemptCount);
     }
 
     [Fact]
-    public async Task AcquireLockWithRetry_MultipleTransientFailures_ShouldRetryAndSucceed()
+    public async Task AcquireLockWithRetry_TransientFailureOnRelease_ShouldLogButNotThrow()
     {
         // Arrange
-        var mockProvider = Scope.ServiceProvider.GetRequiredService<TestDistributedLockProvider>();
-        mockProvider.Reset();
-        mockProvider.FailAcquisitionTimes(2); // Fail twice, succeed on third attempt
+        MockProvider.Reset();
+        MockProvider.FailReleaseOnce();
 
-        var lockProvider = Scope.ServiceProvider.GetRequiredService<IDistributedLockProvider>();
-        var transientDetectionService = Scope.ServiceProvider.GetRequiredService<ITransientExceptionDetectionService>();
-        var logger = Scope.ServiceProvider.GetRequiredService<ILogger<DistributedLockResilienceTests>>();
-        var lockOptions = Scope.ServiceProvider.GetRequiredService<IOptions<DistributedLockingOptions>>();
+        // Act & Assert - mimics production try-catch behavior where release exceptions are logged but not thrown
+        await using var handle = await LockProvider.AcquireLockAsync("test-lock-release", LockOptions.LockAcquisitionTimeout);
+        await SafeDisposeAsync(handle);
 
-        var retryPipeline = CreateRetryPipeline(transientDetectionService, logger);
+        Assert.Equal(1, MockProvider.ReleaseAttemptCount);
+    }
 
-        // Act - should succeed after multiple retries
-        var handle = await retryPipeline.ExecuteAsync(async ct =>
-            await lockProvider.AcquireLockAsync("test-lock-3", lockOptions.Value.LockAcquisitionTimeout, ct),
+    private async Task<IDistributedSynchronizationHandle?> AcquireLockWithRetryAsync(string lockName) =>
+        await RetryPipeline.ExecuteAsync(async ct =>
+            await LockProvider.AcquireLockAsync(lockName, LockOptions.LockAcquisitionTimeout, ct),
             CancellationToken.None);
 
-        // Assert
-        Assert.NotNull(handle);
-        Assert.Equal(3, mockProvider.AcquisitionAttemptCount); // Two attempts fail, third succeeds
-
-        await handle.DisposeAsync();
-    }
-
-    [Fact]
-    public async Task AcquireLockWithRetry_ExhaustedRetries_ShouldThrowException()
+    private async Task SafeDisposeAsync(IDistributedSynchronizationHandle handle)
     {
-        // Arrange
-        var mockProvider = Scope.ServiceProvider.GetRequiredService<TestDistributedLockProvider>();
-        mockProvider.Reset();
-        mockProvider.FailAcquisitionTimes(4); // Fail 4 times (exceeds max retry attempts of 3)
-
-        var lockProvider = Scope.ServiceProvider.GetRequiredService<IDistributedLockProvider>();
-        var transientDetectionService = Scope.ServiceProvider.GetRequiredService<ITransientExceptionDetectionService>();
-        var logger = Scope.ServiceProvider.GetRequiredService<ILogger<DistributedLockResilienceTests>>();
-        var lockOptions = Scope.ServiceProvider.GetRequiredService<IOptions<DistributedLockingOptions>>();
-
-        var retryPipeline = CreateRetryPipeline(transientDetectionService, logger);
-
-        // Act & Assert - should throw after exhausting retries
-        await Assert.ThrowsAsync<TimeoutException>(async () =>
+        try
         {
-            await retryPipeline.ExecuteAsync(async ct =>
-                await lockProvider.AcquireLockAsync("test-lock-4", lockOptions.Value.LockAcquisitionTimeout, ct),
-                CancellationToken.None);
-        });
-
-        Assert.Equal(4, mockProvider.AcquisitionAttemptCount);
+            await handle.DisposeAsync();
+        }
+        catch (Exception ex)
+        {
+            Logger.LogWarning(ex, "Failed to release distributed lock (expected test behavior)");
+        }
     }
 
-    private static ResiliencePipeline CreateRetryPipeline(
-        ITransientExceptionDetectionService transientExceptionDetectionService,
-        ILogger logger)
-    {
-        return new ResiliencePipelineBuilder()
+    private static ResiliencePipeline CreateRetryPipeline(ITransientExceptionDetectionService transientExceptionDetectionService, ILogger logger) =>
+        new ResiliencePipelineBuilder()
             .AddRetry(new()
             {
                 MaxRetryAttempts = 3,
-                Delay = TimeSpan.FromMilliseconds(10), // Shorter delay for tests
+                Delay = TimeSpan.FromMilliseconds(10),
                 BackoffType = DelayBackoffType.Constant,
                 UseJitter = false,
-                ShouldHandle = new PredicateBuilder().Handle<Exception>(ex => transientExceptionDetectionService.IsTransient(ex)),
+                ShouldHandle = new PredicateBuilder().Handle<Exception>(transientExceptionDetectionService.IsTransient),
                 OnRetry = args =>
                 {
                     logger.LogWarning(args.Outcome.Exception, "Transient error acquiring lock. Attempt {AttemptNumber} of {MaxAttempts}.", args.AttemptNumber + 1, 3);
                     return ValueTask.CompletedTask;
                 }
             })
             .Build();
-    }
 }

test/component/Elsa.Workflows.ComponentTests/Scenarios/DistributedLockResilience/Mocks/TestDistributedLockProvider.cs

@@ -5,26 +5,19 @@ namespace Elsa.Workflows.ComponentTests.Scenarios.DistributedLockResilience.Mock
 /// <summary>
 /// Test implementation of IDistributedLockProvider that allows simulating transient failures.
 /// </summary>
-public class TestDistributedLockProvider : IDistributedLockProvider
+public class TestDistributedLockProvider(IDistributedLockProvider innerProvider) : IDistributedLockProvider
 {
-    private readonly IDistributedLockProvider _innerProvider;
     private int _acquisitionFailuresRemaining;
     private int _releaseFailuresRemaining;
     private int _acquisitionAttemptCount;
     private int _releaseAttemptCount;
 
-    public TestDistributedLockProvider(IDistributedLockProvider innerProvider)
-    {
-        _innerProvider = innerProvider;
-    }
-
     public int AcquisitionAttemptCount => _acquisitionAttemptCount;
     public int ReleaseAttemptCount => _releaseAttemptCount;
 
     public void FailAcquisitionOnce() => _acquisitionFailuresRemaining = 1;
     public void FailAcquisitionTimes(int count) => _acquisitionFailuresRemaining = count;
     public void FailReleaseOnce() => _releaseFailuresRemaining = 1;
-    public void FailReleaseTimes(int count) => _releaseFailuresRemaining = count;
 
     public void Reset()
     {
@@ -36,30 +29,26 @@ public void Reset()
 
     public IDistributedLock CreateLock(string name)
     {
-        return new TestDistributedLock(
-            _innerProvider.CreateLock(name),
-            this);
+        return new TestDistributedLock(innerProvider.CreateLock(name), this);
     }
 
     internal bool ShouldFailAcquisition()
     {
         _acquisitionAttemptCount++;
-        if (_acquisitionFailuresRemaining > 0)
-        {
-            _acquisitionFailuresRemaining--;
-            return true;
-        }
-        return false;
+        if (_acquisitionFailuresRemaining <= 0)
+            return false;
+
+        _acquisitionFailuresRemaining--;
+        return true;
     }
 
     internal bool ShouldFailRelease()
     {
         _releaseAttemptCount++;
-        if (_releaseFailuresRemaining > 0)
-        {
-            _releaseFailuresRemaining--;
-            return true;
-        }
-        return false;
+        if (_releaseFailuresRemaining <= 0)
+            return false;
+
+        _releaseFailuresRemaining--;
+        return true;
     }
 }