Using FakeTimeProvider in PeriodicTimer

[olstakh]

I have the following abstraction of a PeriodicTimer (simplified)

internal class PeriodicTimerAdapter : IPeriodicTimer
{
    /// <inheritdoc />
    public async Task RunPeriodicallyAsync(TimeSpan period, Func<CancellationToken, Task> action, CancellationToken cancellationToken, TimeProvider? timeProvider = null)
    {
        timeProvider ??= TimeProvider.System;
        using var timer = new PeriodicTimer(period, timeProvider);

        // try/catch here
        while (!cancellationToken.IsCancellationRequested && await timer.WaitForNextTickAsync(cancellationToken))
        {
            await action(cancellationToken);
        }
    }
}

Since it accepts TimeProvider as a constructor parameter - i'm attempting to test it using FakeTimeProvider in the following way

[Fact]
public async Task CallsActionUntilCancelled()
{
    var period = TimeSpan.FromSeconds(2);
    using var cts = new CancellationTokenSource();
    var periodicTimer = new PeriodicTimerAdapter();
        
    var timeProvider = new FakeTimeProvider() { AutoAdvanceAmount = period };

    var actionCalledCount = 0;
    var actionToCall = async (CancellationToken _) =>
    {
        actionCalledCount++; // Called the first time, never get the second call

        if (actionCalledCount == 3)
        {
            await cts.CancelAsync();
        }
        timeProvider.Advance(period * 2); // The "right" time? Second tick never happens
    };

    var runTask = periodicTimer.RunPeriodicallyAsync(period, actionToCall, cts.Token, timeProvider);
    timeProvider.Advance(period); // Advancing time to trigger the first tick
    await runTask;

    Assert.Equal(3, actionCalledCount);
}

What i'm observing is that advancing fake time provider in between the ticks - doesn't seem to achieve anything (i.e. the second tick never happens).

Am i using the time provider wrong? There seem to be no other hook points (apart from in between the ticks) to advance the time. And if this is by design - what's the purpose of passing time provider to periodic timer in the first place?

I'm curious where i am mistaken here

[svick]

I suspect the problem is here:

https://github.com/dotnet/extensions/blob/1664044d/src/Libraries/Microsoft.Extensions.TimeProvider.Testing/FakeTimeProvider.cs#L294-L298

                if (oldTicks != newTicks)
                {
                    // time changed while in the callback, readjust the wake time accordingly
                    candidate.WakeupTime = newTicks + candidate.Period;
                }

You are advancing time while the callback is executing, which results in the callback getting rescheduled to a later point, which then never happens.

[olstakh]

@svick perhaps, but in that case - what is the proper way to use TimeProvider? There's no other that i see, apart from a callback, where i can actually utilize the time provider and forward the time, instead of actually waiting. I'm assuming PeriodicTimer exposes it for a reason, but if callback is not the right place to use it - what is?

[lilinus]

In the remarks section in documentation of TimeProvider.CreateTimer it says:

The delegate specified by the callback parameter is invoked once after dueTime elapses, and thereafter each time the period time interval elapses.

IMO either FakeTimeProvider should be explicit in documentation or perhaps it should be fixed so that it is triggered when advancing time.