views:

40

answers:

2

hey guys

general consensus

I've done quite a lot of reading up on the subject of testing complex classes and private methods.

The general consensus seems to be:

  • "if you need to test private methods then you're class is badly designed"
  • "if your class is complex, then you need to separate it out"

So, I need your help.

the problem Class

So I have a relatively simple class whose long running job it is to:

  • poll a datasource
  • do some very simple mapping of the data
  • send that data somewhere else

Aditionally:

  • it needs to be able to be quite fault tolerant by being able to retry various tasks in case of certain errors.

the testing problem

The point of the class is to abstract a lot of the fault tolerance and threading... basically by using a simple Timer Class and some internal lists to keep track of errors etc.

Because of the Timer, certain methods are called on different threads asynchronously... additionally a bunch of the methods rely on global private fields.

How should I test this class... particularly because so many methods are private?

cheers guys

UPDATE:

would love your input, here's the class, cheers!

    using System;
using System.Collections.Generic;
using System.Linq;
using System.Threading;
using System.Timers;
using AutoMapper;
using Castle.Core.Logging;
using NServiceBus;
using VioAd.Metadata.SyncService.Messages;
using Viomedia.Common.Extensions;
using Viomedia.Common.Extensions.FluentnHibernate;
using Viomedia.Core.Services;
using Viomedia.Messages.DTO;
using Timer = System.Timers.Timer;

namespace Viomedia.DataSync.Server
{
    public class Retry
    {
        public Exception error { get; set; }
        public int? PageNumber { get; set; }
    }

    public class VioadPollingUserPublisher
    {
        private readonly IBus _bus;
        private readonly ILogger _logger;
        private readonly IUserService _userService;
        private readonly string _targetQueue;
        private Timer _timer;

        public bool processing { get; private set; }
        public DateTime currentProcessDate { get; private set; }
        public int retryTimes { get; set;}
        public TimeSpan timeoutWait { get; set; }
        public int PageSize { get; set; }

        public VioadPollingUserPublisher(IBus bus, ILogger logger, IUserService userService, string targetQueue)
        {
            _bus = bus;
            _logger = logger;
            _userService = userService;
            _targetQueue = targetQueue;
            processing = false;
            PageSize = 500;
            retryTimes = 10;
            timeoutWait = TimeSpan.FromMinutes(1);

            logger.Info("{0} created:{1}target queue: {2}{1}page size: {3}{1}retry times: {4}{1}timeout error pause: {5} in seconds.",
                GetType().Name,
                Environment.NewLine,
                _targetQueue,
                PageSize,
                retryTimes,
                timeoutWait.TotalSeconds);
        }

        public void StartPolling(DateTime fromDate, TimeSpan Interval)
        {
            try
            {
                currentProcessDate = fromDate;
                _timer = new Timer { Interval = Interval.TotalMilliseconds };
                _timer.Elapsed += new ElapsedEventHandler(_timer_Elapsed);
                _timer.Start();

                _logger.Info("Started Polling at every {0} seconds. First Date: {1}", Interval.TotalSeconds, fromDate.ToString());
            }
            catch (Exception ex)
            {
                _logger.Fatal("Failed to start Poller.",ex);   
                StopPolling();
                throw;
            }            
        }

        private void _timer_Elapsed(object sender, ElapsedEventArgs e)
        {
            if (!processing)
            {
                processing = true;

                int totalUsers = (int)_userService.GetTotalCount();
                DateTime? newdate;

                List<Retry> retries = GetAndSendUsers(null, out newdate);

                for (int i = 1; i < retryTimes; i++)
                {
                    retries = GetAndSendUsers(retries, out newdate);
                    if (retries.Count == 0)
                    {
                        break;
                    }
                }

                foreach (Retry retry in retries)
                {
                    _logger.Fatal(string.Format("Retry failed for date {0} page {1}", currentProcessDate, retry.PageNumber), retry.error);
                }

                _bus.Send(_targetQueue, new DataMessage()
                {
                    TotalCount = totalUsers,
                    Users = new List<CmsUser>()
                });

                if (newdate.HasValue)
                {
                    currentProcessDate = newdate.Value;
                }

                processing = false;
            }
        }

        private List<Retry> GetAndSendUsers(List<Retry> toRetry, out DateTime? lastDate)
        {
            var newRetries = new List<Retry>();
            lastDate = currentProcessDate;
            int Retried = 0;

            try
            {       
                bool isRetry = (toRetry != null && toRetry.Count > 0);

                var pagedQueryModel = new PagedQueryModel
                {
                    SortOrder = "asc",
                    PageSize = PageSize,
                    PageNumber = 1,
                    SortColumn = "Updated"
                };               

                do
                {
                    try
                    {
                        if((isRetry &&  (toRetry.Where(r=>r.PageNumber == pagedQueryModel.PageNumber).Count() > 0)) || !isRetry)
                        {
                            if(isRetry)
                            {
                                Retried++;    
                            }

                            var users = _userService.ListPaged(currentProcessDate, pagedQueryModel);

                            if (users.Items.Count == 0)
                            {
                                if(!isRetry || (isRetry && Retried > toRetry.Count))
                                {
                                    break;    
                                }                                
                            }
                            else
                            {
                                _bus.Send(_targetQueue, new DataMessage()
                                {
                                    TotalCount = 0,
                                    Users = Mapper.Map<List<UserDTO>, List<CmsUser>>(users.Items.ToList())
                                });

                                lastDate = users.Items.Last().Updated;    
                            }
                        }
                    }
                    catch (Exception ex)
                    {
                        _logger.Error(string.Format("Error while getting/sending users for date {0} page {1}. Is Retry {2}", currentProcessDate, pagedQueryModel.PageNumber, isRetry), ex);

                        newRetries.Add(new Retry() {error = ex, PageNumber = pagedQueryModel.PageNumber });
                        LookForTimeoutsAndWait(ex);            
                    }

                    pagedQueryModel.PageNumber++;                 

                } while (true);

                return newRetries;
            }
            catch (Exception ex)
            {
                _logger.Fatal(string.Format("Failed poll/send for update date {0}", currentProcessDate), ex);
                throw;
            }
        }

        private void LookForTimeoutsAndWait(Exception possibleTimeout)
        {
            Exception timeout = possibleTimeout.ContainsAnywhere("timeout");

            _logger.Warn(string.Format("Timeout error found, pausing for {0} seconds",timeoutWait.TotalSeconds),timeout);

            if(timeout != null)
            {
                Thread.Sleep(timeoutWait);
            }
        }

        public void StopPolling()
        {
            _timer.Stop();
            processing = false;
        }
    }
}
+1  A: 

You could try using something like JMock. This will let you replace the real Timer with a mock Timer that you can control. You can then set up test cases that fire method calls in a defined order, and you can also set up error conditions by creating a mock data source.

EDIT: Whoops! Didn't see the C# tag. Maybe there's a C# equivalent to JMock.

Cameron Skinner
+1 thanks cameron, great point! I thought about that, but does that mean I should pass in the Timer as a dependency in the constructor. currently is instantiated internally. cheers man
andy
Tricky. Probably best not to change the interface just to suit your testing regime. I see that you pass the timer interval as an argument, so maybe you can get away with testing a range of error conditions (and non-error, of course) with a variety of timings.
Cameron Skinner
it's alright, there's rhino mocks!
andy
yeah.... it is a bit tricky isn't it... I was thinking of maybe mocking the ILogger, and then I could check when various Log messages are called...? a bit yuk but...?
andy
That's OK - nothing wrong with mocking the logger. Then you can check that it's emitting the expected log messages.
Cameron Skinner
+2  A: 

I would extract the code to poll the data into a separate class that can be mocked, and also extract the code that sends that data for the same reason. You might want to extract the data mapping code, depending on how trivial it is.

I would definitely use mock timers in the unit tests, otherwise your tests are hard to set up and slow to run. You can either pass in the timer in the constructor, or expose a property that you can set. I often create a regular timer in the constructor and then overwrite that from my unit test.

You might also be able to extract the retry logic so that it can be tested separately from the other code. Passing in a delegate of the code to try and retry might be a way to decouple the data code from the retry logic. You can also use IEnumerable and the yield statement to generate your data and feed it to the retry code. Essentially, I'm looking for ways to make it so that the retry code doesn't directly call the target code that it's supposed to try and retry. That makes it easier to test and generate all possible errors for, although you can get some of the same benefits by just mocking out that target code.

If you really have multithreaded scenarios that you need to test, there are some tools out there to coordinate threads from within a test. One of them is a port I created of Java MultithreadedTC called TickingTest.

Don Kirkby
+1 excellent answer, thanks.... I would like to decouple the code, but it seems overly dependent on global changing values. this is definitely a problem, but also it's what makes the class so simple to use...?
andy
I don't know if it will work in your case, @Andy, but using IEnumerable and yield often let you keep track of the state with local variables instead of member variables. That might make it easier to decouple the code and still keep it simple to use.
Don Kirkby