Fix retry logic and add unit tests for ExecuteTaskAsync (#429)

* Add retry logic improvements and unit tests for ExecuteTaskAsync

* Refactor retry test setup to use SetupRetryTestFixture for consistency

Author: SpaceOgre

src/TickerQ/Properties/InternalsVisibleTo.cs

@@ -0,0 +1,3 @@
+using System.Runtime.CompilerServices;
+
+[assembly: InternalsVisibleTo("TickerQ.Tests")]

src/TickerQ/Src/TickerExecutionTaskHandler.cs

@@ -172,7 +172,7 @@ private async Task RunContextFunctionAsync(InternalFunctionContext context, bool
 
         for (var attempt = context.RetryCount; attempt <= context.Retries; attempt++)
         {
-            tickerFunctionContext.RetryCount = context.RetryCount;
+            tickerFunctionContext.RetryCount = attempt;
 
             // Update activity with current attempt information
             jobActivity?.SetTag("tickerq.job.current_attempt", attempt + 1);
@@ -304,18 +304,18 @@ private async Task<bool> WaitForRetry(InternalFunctionContext context, Cancellat
         if (attempt == 0)
             return false;
 
-        if (attempt >= context.Retries)
+        if (attempt > context.Retries)
             return true;
 
-        context.SetProperty(x => x.RetryCount, attempt + 1);
+        context.SetProperty(x => x.RetryCount, attempt);
 
         await _internalTickerManager.UpdateTickerAsync(context, cancellationToken);
 
         context.ResetUpdateProps();
 
         var retryInterval = (context.RetryIntervals?.Length > 0)
-            ? (attempt < context.RetryIntervals.Length
-                ? context.RetryIntervals[attempt]
+            ? (attempt - 1 < context.RetryIntervals.Length
+                ? context.RetryIntervals[attempt - 1]
                 : context.RetryIntervals[^1])
             : 30;
 

tests/TickerQ.Tests/RetryBehaviorTests.cs

@@ -0,0 +1,121 @@
+using FluentAssertions;
+using NSubstitute;
+using TickerQ.Utilities.Enums;
+using TickerQ.Utilities.Interfaces;
+using TickerQ.Utilities.Interfaces.Managers;
+using Microsoft.Extensions.DependencyInjection;
+using TickerQ.Utilities.Instrumentation;
+using TickerQ.Utilities.Models;
+
+namespace TickerQ.Tests;
+
+public class RetryBehaviorTests
+{
+    // End-to-end unit tests that call the public ExecuteTaskAsync with a CronTickerOccurrence
+    // so RunContextFunctionAsync + retry logic is exercised. Tests use short intervals (1..3s).
+
+    [Fact()]
+    public async Task ExecuteTaskAsync_CronTickerOccurrence_AppliesRetryIntervals_AndUpdatesRetryCount()
+    {
+        // Arrange: cron occurrence -> RunContextFunctionAsync path
+        // Use three distinct short intervals so we can verify mapping without overly long waits
+        var (handler, context, _, attempts) = SetupRetryTestFixture([1, 2, 3], retries: 3);
+
+        // Act
+        await handler.ExecuteTaskAsync(context, isDue: true);
+
+        // Assert - initial + 3 retries = 4 attempts
+        attempts.Should().HaveCount(4);
+        for (int i = 0; i < 4; i++)
+            attempts[i].RetryCount.Should().Be(i);
+
+        // Verify mapped retry intervals produced the expected spacing between attempts
+        var timeDiffs = new[]
+        {
+            (attempts[1].Timestamp - attempts[0].Timestamp).TotalSeconds,
+            (attempts[2].Timestamp - attempts[1].Timestamp).TotalSeconds,
+            (attempts[3].Timestamp - attempts[2].Timestamp).TotalSeconds,
+        };
+
+        // allow a small tolerance for timing, but ensure each spacing reflects the configured intervals
+        timeDiffs[0].Should().BeInRange(0.8, 1.2); // first retry uses ~1s
+        timeDiffs[1].Should().BeInRange(1.8, 2.2); // second retry uses ~2s
+        timeDiffs[2].Should().BeInRange(2.8, 3.2); // third retry uses ~3s
+    }
+
+    [Fact]
+    public async Task ExecuteTaskAsync_CronTickerOccurrence_UsesLastInterval_WhenRetriesExceedArrayLength()
+    {
+        // Use zero intervals for speed
+        var (handler, context, _, attempts) = SetupRetryTestFixture([0, 0], retries: 4);
+
+        await handler.ExecuteTaskAsync(context, isDue: true);
+
+        // initial + 4 retries = 5 attempts
+        attempts.Should().HaveCount(5);
+
+        // Ensure we captured attempts and they happened in order. Timing is intentionally tiny.
+        attempts.Select(a => a.Timestamp).Should().BeInAscendingOrder();
+    }
+
+    [Fact]
+    public async Task ExecuteTaskAsync_CronTickerOccurrence_StopsRetrying_WhenFunctionSucceeds()
+    {
+        // Arrange: succeed on RetryCount==2
+        // Use zero intervals for speed; succeed at retry=2
+        var (handler, context, _, attempts) = SetupRetryTestFixture([0, 0, 0, 0], retries: 4, succeedOnRetryCount: 2);
+
+        await handler.ExecuteTaskAsync(context, isDue: true);
+
+        // Should stop after success on attempt with RetryCount=2 => initial + retry1 + retry2 = 3 attempts
+        attempts.Should().HaveCount(3);
+        attempts.Last().RetryCount.Should().Be(2);
+    }
+
+    private record Attempt(DateTime Timestamp, int RetryCount);
+
+    // Helpers
+    private static (TickerExecutionTaskHandler handler, InternalFunctionContext context, IInternalTickerManager manager, List<Attempt> attempts) SetupRetryTestFixture(
+        int[] retryIntervals,
+        int retries,
+        int? succeedOnRetryCount = null)
+    {
+        var services = new ServiceCollection();
+        var clock = Substitute.For<ITickerClock>();
+        var internalManager = Substitute.For<IInternalTickerManager>();
+        var instrumentation = Substitute.For<ITickerQInstrumentation>();
+
+        clock.UtcNow.Returns(DateTime.UtcNow);
+
+        services.AddSingleton(internalManager);
+        services.AddSingleton(instrumentation);
+        var serviceProvider = services.BuildServiceProvider();
+
+        var handler = new TickerExecutionTaskHandler(serviceProvider, clock, instrumentation, internalManager);
+
+        var attempts = new List<Attempt>();
+
+        var context = new InternalFunctionContext
+        {
+            TickerId = Guid.NewGuid(),
+            FunctionName = "TestFunction",
+            Type = TickerType.CronTickerOccurrence,
+            ExecutionTime = DateTime.UtcNow,
+            RetryIntervals = retryIntervals,
+            Retries = retries,
+            RetryCount = 0,
+            Status = TickerStatus.Idle,
+            CachedDelegate = (ct, sp, tctx) =>
+            {
+                attempts.Add(new Attempt(DateTime.UtcNow, tctx.RetryCount));
+
+                if (succeedOnRetryCount.HasValue && tctx.RetryCount >= succeedOnRetryCount.Value)
+                    return Task.CompletedTask;
+
+                throw new InvalidOperationException("Fail for retry test");
+            }
+        };
+
+        return (handler, context, internalManager, attempts);
+    }
+}

tests/TickerQ.Tests/TickerQ.Tests.csproj

@@ -22,6 +22,7 @@
 
     <ItemGroup>
       <ProjectReference Include="..\..\src\TickerQ.Utilities\TickerQ.Utilities.csproj" />
+      <ProjectReference Include="..\..\src\TickerQ\TickerQ.csproj" />
     </ItemGroup>
 
 </Project>