Unit testing whether or not method used in System.Threading.Timer constructor (TimerCallback) has been called using Moq

Question

I am looking to make sure a callback passed into a System.Threading.Timer with TimerCallback in the signature:

public Timer (System.Threading.TimerCallback callback, object? state, TimeSpan dueTime, TimeSpan period);

I have a class holding a timer and need a way to make sure the callback (MethodC) has test coverage. Also, I am not going to wait 15 minutes to see what happens with MethodC.

using System.Threading;
public class MyClass
{
    private Timer? myTimer;

    public MyClass() { }

    public void MethodA()
    {
        MethodB();
    }

    private void MethodB()
    {
        myTimer = new Timer(
            MethodC, // TimerCallback
            new Object(),
            TimeSpan.FromMinutes(15),
            TimeSpan.FromMinutes(15));
        // do some things
    }

    private void MethodC(object? state)
    {
        // do something
    }
}

Based on my search, it seems the answer is to move the timer to an interface and then mock interface. I am using Moq. However, I am still missing some understanding of achieving my goal. I don't know how I can test MethodC if the callback is given in the constructor of myTimer.

I ultimately want to check if the code is reached with Moq's Verify and maybe other behavior.

I have tried to move timer code to an interface:

    interface ITimer
    {
        void SetTimer(TimerCallback callback, object state, TimeSpan dueTime, TimeSpan period);
        void Dispose();
    }

    class MyTimer : ITimer, IDisposable
    {
        private Timer timer;
        private TimerCallback callback;

        public MyTimer(TimerCallback callback, Object state, TimeSpan dueTime, TimeSpan period)
        {
            this.callback = callback;
            this.SetTimer(this.callback, state, dueTime, period);
        }

        public void SetTimer(TimerCallback callback, object state, TimeSpan dueTime, TimeSpan period)
        {
            this.timer = new Timer(callback, state, dueTime, period);
        }

        public void Dispose()
        {
            timer.Dispose();
        }
    }


.......



using System.Threading;
public class MyClass : IDisposable {


  private ITimer myTimer;
  
  public MyClass(){}

  public void MethodA(){
    MethodB();
  }

  private void MethodB(){
    this.myTimer = new MyTimer(this.MethodC, // TimerCallback
                             new Object(), 
                             TimeSpan.FromMinutes(15), 
                             TimeSpan.FromMinutes(15))
    // do some things
  }

  private void MethodC(){
    // do something
  }
  
}

The timer callback is given to the Timer object through the constructor. There is no method to call, and therefore Moq's Setup(Expression<Action<T>> expression) doesn't seem to work here. After a certain amount of time is called, the callback MethodC is just called on another thread, also it is private.

I try to make my MyTimer class create the Timer in another method, however I want to test MethodC, not SetTimer.

The design seems convoluted and untestable. But I am also not that familiar with Moq. Is there some suggestion on how I can improve this, so that I can test MethodC?

Answer 1

It's trivially possible to write code that can't easily be tested. The OP is one such example. You should expect to have to modify the System Under Test (SUT) in some way in order to make it testable. Sometimes, this can lead to a more modular design in general, so this isn't necessarily a bad thing.

The simplest thing that immediately springs to mind is to first define an interface:

public interface ITimer
{
    void Start(
        TimerCallback callback,
        object? state,
        TimeSpan dueTime,
        TimeSpan period);
}

According to the Dependency Inversion Principle, one doesn't have to define more members with more details than the SUT needs, and in the case a method that looks like the constructor is enough.

Then modify the SUT to use Constructor Injection:

public class MyClass
{
    private readonly ITimer timer;

    public MyClass(ITimer timer)
    {
        this.timer = timer;
    }

    public void MethodA()
    {
        MethodB();
    }

    private void MethodB()
    {
        timer.Start(
            MethodC, // TimerCallback
            new Object(),
            TimeSpan.FromMinutes(15),
            TimeSpan.FromMinutes(15));
        // do some things
    }

    private void MethodC(object? state)
    {
        // do something
    }
}

You can now verify that the Start method gets called:

[Fact]
public void MethodAInvokesMethodC()
{
    var mockTimer = new Mock<ITimer>();
    var sut = new MyClass(mockTimer.Object);

    sut.MethodA();
    
    mockTimer.Verify(t => t.Start(
        It.IsAny<TimerCallback>(),
        It.IsAny<object>(),
        TimeSpan.FromMinutes(15),
        TimeSpan.FromMinutes(15)));
}

If you also want to verify which method is designated as the callback, you may consider changing the Verify call to this:

mockTimer.Verify(t => t.Start(
    It.Is<TimerCallback>(cb => cb.Method.Name == "MethodC"),
    It.IsAny<object>(),
    TimeSpan.FromMinutes(15),
    TimeSpan.FromMinutes(15)));

This, however, makes the test more brittle, because if you change the name of MethodC, this test is going to fail. Instead, consider making MethodC public:

public void MethodC(object? state)
{
    // do something
}

This then enables you to write the assertion like this:

mockTimer.Verify(t => t.Start(
    sut.MethodC,
    It.IsAny<object>(),
    TimeSpan.FromMinutes(15),
    TimeSpan.FromMinutes(15)));

Now that MethodC is public it also means that you can write tests against it, so making it public isn't necessarily a bad thing.

For production use, you can implement the interface using the real Timer class:

public sealed class TimerTimer : ITimer
{
    private Timer? timer;

    public void Start(
        TimerCallback callback,
        object? state,
        TimeSpan dueTime,
        TimeSpan period)
    {
        timer = new Timer(callback, state, dueTime, period);
    }
}