1

I wrote this class:

using System;
using System.Threading;
using System.Threading.Tasks;

    public interface IJobScheduler {
        void RunDaily(Task task, int hour, int minutes );
    }

    public class JobScheduler : IJobScheduler, IDisposable
    {
        Timer timer;

        /// <summary>
        /// Run the provided task every day at the defined time.
        /// </summary>
        /// <param name="task">Action to execute</param>
        /// <param name="hour">At what time (hour) the task have to be executed. LOCAL time.</param>
        /// <param name="minutes">At what time (minutes) the task have to be executed. LOCAL time.</param>
        public void RunDaily(Task task, int hour, int minutes)
        {
            var todayRun = DateTime.Today.Add(new TimeSpan(hour, minutes, 0));

            var timeToGo = todayRun > DateTime.Now ?
                (todayRun - DateTime.Now) :          // run today
                todayRun.AddDays(1) - DateTime.Now;  // run tomorrow

            timer = new Timer(x => { if (isEnabled) task.RunSynchronously(); },
                state: null,
                dueTime: (int)timeToGo.TotalMilliseconds,
                period: 24 * 60 * 60 * 1000 /* 24h */);
        }

        public void Dispose()
        {
            try { timer?.Dispose(); } catch { }
        }
    }

Now, this is the initial test I wrote:

        [Test, Category("long_test")]
        public void TaskScheduler_RunDaily__should__execute_at_the_specified_time() {

            // scheduler has a precision of 1 minute so...
            var runAt = DateTime.Now.AddMinutes(1);

            var runCounter = 0;
            Task task = new Task(
                () => runCounter++
                );

            int hours = runAt.Hour;
            int minutes = runAt.Minute;
            var scheduler = new JobScheduler();
            scheduler.RunDaily(task, hours, minutes);

            // scheduler has a precision of 1 minute so...
            Thread.Sleep(TimeSpan.FromSeconds(60+2));

            runCounter.Should().Be(1);
        }

It can be improved to exit as soon as the Task is executed or, introducing seconds as parameter (with default zero) maybe I'll be able to reduce the test time to 1 or 2 seconds.

But my real quetion is this:
How can I check that the Scheduler has a Timer set to 24 hours (= it will run the task after 24 hours) ?

I started thinking to verify the internal Timer...

        [Test]
        public void TaskScheduler_RunDaily__should__have_an_internal_timer_et_to_24_hours()
        {
            // scheduler has precision of 1 minute so...
            var runAt = DateTime.Now.AddMinutes(1);

            var runCounter = 0;
            Task task = new Task(
                () => runCounter++
                );

            int hours = runAt.Hour;
            int minutes = runAt.Minute;
            var scheduler = new JobScheduler();
            scheduler.RunDaily(task, hours, minutes);

            // use reflection to get the check internal Timer
            var timerField = typeof(JobScheduler).GetField("timer",
                System.Reflection.BindingFlags.NonPublic | System.Reflection.BindingFlags.Instance);

            if (timerField == null) Assert.Fail("Cannot read the Timer field");

            var timer = timerField.As<System.Threading.Timer>();

            // what can I test here ?  
            timer.<internal "enabled"> Should().Be(true);           
            timer.<internal "interval"> Should().Be(24_HOURS);
        }

and I think it is ok to use reflection and check some internal implementation of MY code, but it is not acceptable to verify the internal implementation of Timer itself.

The real behavior to test here is the fact that the task run after 24 hours.
I can think of "expose" that 24 hours so to mock it but I really don't like the idea: DailyRun means 24 hours, why should I expose this value as parameter.
The other kind of rule I like to follow is this: "Don't make the code ugly just because it has to be testable, prefer simplicity."
To be clear, the RunDaily method is 10 lines of code and it get only the needed input, I don't want to change it because there is no practical way to unit test it as it is.

[Update] It is worth to explain why I'm so reluctant to change a simple implementation with ... something else.
This is the current implementation I found in a project:
enter image description here

and this is CurrentTimeProvider:

    public class CurrentDateTimeProvider : ICurrentDateTimeProvider
    {
        public DateTime GetNow()
        {
            return DateTime.Now;
        }
    }

and SchedulerTimeProvider:

    public class SchedulerTimeProvider : ISchedulerTimeProvider
    {
        #region Fields

        private readonly ICurrentDateTimeProvider _currentDateTimeProvider;

        #endregion

        #region Constructor

        public SchedulerTimeProvider(ICurrentDateTimeProvider currentDateTimeProvider)
        {
            _currentDateTimeProvider = currentDateTimeProvider;
        }

        #endregion

        #region Public Methods

        public ISchedulerTimeInfo GetSchedulerTimeInfo(string scheduledTime)
        {
            var defaultSchedulerTime = scheduledTime.Split(":");
            var defaultSchedulerHour = int.Parse(defaultSchedulerTime[0]);
            var defaultSchedulerMinute = int.Parse(defaultSchedulerTime[1]);

            TimeSpan due;
            TimeSpan period;


            var now = _currentDateTimeProvider.GetNow();

            DateTime dueDateTime = new DateTime(now.Year, now.Month, now.Day, defaultSchedulerHour,
                defaultSchedulerMinute, 0);


            if (now > dueDateTime)
            {
                var tomorrowsDate = now.AddDays(1);
                var tomorrowsScheduleDateTime = new DateTime(tomorrowsDate.Year, tomorrowsDate.Month, tomorrowsDate.Day, defaultSchedulerHour, defaultSchedulerMinute, 0);
                due = tomorrowsScheduleDateTime.Subtract(now);
                dueDateTime = tomorrowsScheduleDateTime;
            }
            else
            {
                dueDateTime = new DateTime(now.Year, now.Month, now.Day, defaultSchedulerHour, defaultSchedulerMinute,
                    0);
                due = dueDateTime
                    .Subtract(now);
            }


            period = due.Add(new TimeSpan(1, 0, 0, 0));
            //period = due.Add(new TimeSpan(0, 0, 2, 0));


            return new SchedulerTimeInfo(due, period, dueDateTime);
        }

        #endregion

    }

(just 2 of the 6 interfaces/classes as example)

Now, that scheduler is used/started in this way:

private void StartProcess()
        {
            if (_scheduleTimer == null)
                _scheduleTimer = new Timer(ExecuteExporterProcess);

            var schedulerTimeInfo = _schedulerTimeProvider.GetSchedulerTimeInfo(_config.RegistrationsSchedulerTime);

            var due = schedulerTimeInfo.Due;
            var period = schedulerTimeInfo.Period;

            _scheduleTimer.Change(due, period);
        }

The ExecuteExporterProcess method does something and then call again StartProcess.

I spent some time to understand it and I wrote my simple implementation.
The current scheduler code is not tested in this solution/project because it is copied from another project and I strongly suspect there are no tests also there (I knew the developer that wrote it).
Also the class that coontains StartProcess is not testd, so the timer itself is not tested at all. I put my simple implementation in a common package but before making it available to everyone I want to fully test it; because I put the Timer "logic" inside the new implementation I want to test it too.

As you can see the current implementation is a little bit "verbose" and I think I'm able to introduce some interface to make it testable but less complicated.
But, I hope, you can see WHY I'm reluctant/scared to go on the direction to write so "verbose" code.

7
  • 2
    Learn to use mocks, spies, stubs, and fakes? – rwong Apr 29 '20 at 10:35
  • Sorry, are you suggesting to mock the Timer and verify that it is used with the expected values (24 hours) ? Or, there is a way to test that code without passing more parameters than the ones it has and I should be able to find it when I'll be more experienced/trained on mocks, spies, stubs and fakes? – Alex 75 Apr 29 '20 at 10:41
  • 1
    not willing to change RunDaily may be main reason why you are stuck. I once managed to overcome such an unwillingness and rewrote all such methods to accept additional time related parameters (so that there were nothing like DateTime.Now left inside). And I was rewarded with a perfectly testable code allowing me to implement any logic I wanted fully covered by unit tests. Even though I disliked additional verbosity it turned out very well worth it – gnat Apr 29 '20 at 10:49
  • I would suggest querying the Scheduler, rather than querying the timer directly; extend the Scheduler api with the query methods that you need, and use the test to verify those values are what you expect. – VoiceOfUnreason Apr 29 '20 at 11:48
  • 1
    One option is not to test for it, if the risk is acceptable. If the goal is to check if you've configured the timer with the correct parameters, you can pass the timer as an abstract dependency to the constructor, and replace it with a mock in the test, where you can check the parameter values. – Filip Milovanović Apr 29 '20 at 14:03
2

I have dealt with the same problem in the past, and I came up with a solution which on one hand is uncompromising in its perfection, but on the other hand it is, unfortunately, quite a bit involved.

The solution starts with abstracting everything related to time, timers, tasks, and asynchronous events, and rewriting all of our code to use this abstraction instead of the untestable functionality provided by dotnet out of the box.

So, we introduce an interface which describes an event-driven system, with its time-query function, and its tick function, and its timer factory function, and what not, and we provide two implementations for it: one which is going to be our fake system that we are going to be using for testing, and one which delegates to the underlying dotnet system, so it reads the true system time, (which should never be used by testing code,) and it creates those System.Threading.Timer objects (which are completely untestable.)

The idea behind the fake event-driven system is that of course it has a fake notion of time, but more importantly than that, it has a highly non-linear notion of the passage of time. Instead of advancing time at the rate of "real" (wall clock) time, it advances time at the fastest possible rate so as to service the events at the time that they are supposed to happen.

So, in order to determine at any given moment by how much it should advance the current time, the fake event-driven system looks at all registered timers, sorts them by the order by which the next firing event will be, and picks the one which is going to fire at the earliest time. The time at which this timer will fire becomes the new "current time", and the system then proceeds with firing that timer.

What this means is that you may register a timer to fire in 24 hours from now, and under test conditions it will actually fire immediately, but the time reported by the system at that moment will be as if 24 hours have actually passed, so everything will look normal!

3
  • Now I need your Time-Abstraction framework, :-) Do you have a base class that I can use as starting point for the Timer-Abstraction, I'll try to do the rest myself. Should I pass an interface or a factory ? (sorry if this is a mere implementation detail, I still have to digest your suggestion and figure out how to implement it). – Alex 75 May 4 '20 at 15:09
  • Do you think Microsoft.Fakes will be another way to solve my problem? Shims is what MS offer to "test" Timer and DateTime.Now. Have you ever used or considered it ? (I can't at work because we use VS Professional that does not have Fakes) – Alex 75 May 4 '20 at 15:09
  • Unfortunately, my event-driven-system abstraction framework is part of a bigger thing and so i cannot isolate it, otherwise I would have open-sourced it already. As for Microsoft fakes and shims, I know nothing of them. – Mike Nakis May 4 '20 at 17:56

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.