Unit testing a background thread with an interface

Question

I have created a class, SenderClass, which will start and run a background worker from its constructor. The method, RunWorker(), runs is a while(true) loop which will pop elements from a queue, send them through the method SendMessage(), and sleep for a small amount of time to allow new elements to be added to the queue.

Here lies the problem: How do I test the method that sends the element from the queue, without exposing it to those who uses the class?

Implementation:

public class SenderClass : ISenderClass
{
    private Queue<int> _myQueue = new Queue<int>();
    private Thread _worker;

    public SenderClass()
    {
        //Create a background worker
        _worker = new Thread(RunWorker) {IsBackground = true};
        _worker.Start();
    }

    private void RunWorker() //This is the background worker's method
    {
        while (true) //Keep it running
        {
            lock (_myQueue) //No fiddling from other threads
            {
                while (_myQueue.Count != 0) //Pop elements if found
                    SendMessage(_myQueue.Dequeue()); //Send the element
            }
            Thread.Sleep(50); //Allow new elements to be inserted
        }
    }

    private void SendMessage(int element)
    {
        //This is what we want to test
    }

    public void AddToQueue(int element)
    {
        Task.Run(() => //Async method will return at ones, not slowing the caller
        {
            lock (_myQueue) //Lock queue to insert into it
            {
                _myQueue.Enqueue(element);
            }
        });
    }
}

Wanted interface:

public interface ISenderClass
{
    void AddToQueue(int element);
}

Needed interface for test purpose:

public interface ISenderClass
{
    void SendMessage(int element);
    void AddToQueue(int element);
}

There's a very simple solution, saying I have created my class incorrect due to the Single Responsability Principle, and my class' purpose is not to send messages, but actually run what sends them.

What I should have, is another class, TransmittingClass, which exposes the method SendMessage(int) through its own interface. This way I can test that class, and SenderClass should just call the method through that interface.

But what other options do I have with the current implementation?

I can make all private methods I wish to test (all of them) have a [assembly:InternalsVisibleTo("MyTests")], but does a third option exist?

Answer 1

Send message logic should be implemented in a separate class with a separate interface. This class should take the new class as a dependency. You can test the new class separately.

public interface IMessageQueue
{
    void AddToQueue(int element);
}

public interface IMessageSender
{
    void SendMessage(object message);
}

public class SenderClass : IMessageQueue
{
    private readonly IMessageSender _sender;
    public SenderClass(IMessageSender sender)
    {
        _sender = sender;
    }
    public void AddToQueue(int element)
    {
        /*...*/
    }

    private void SendMessage()
    {
        _sender.SendMessage(new object());
    }
}

public class DummyMessageSender : IMessageSender
{
    //you can use this in your test harness to check for the messages sent
    public Queue<object> Messages { get; private set; }

    public DummyMessageSender()
    {
        Messages = new Queue<object>();
    }
    public void SendMessage(object message)
    {
        Messages.Enqueue(message);
        //obviously you'll need to do some locking here too
    }
}

Edit

To address your comment, here is an implementation using Action<int>. This allows you to define your message sending action in your test class to mock the SendMessage method without worrying about creating another class. (Personally, I'd still prefer to define the classes/interfaces explicitly).

public class SenderClass : ISenderClass
    {
        private Queue<int> _myQueue = new Queue<int>();
        private Thread _worker;
        private readonly Action<int> _senderAction;

        public SenderClass()
        {
            _worker = new Thread(RunWorker) { IsBackground = true };
            _worker.Start();
            _senderAction = DefaultMessageSendingAction;
        }

        public SenderClass(Action<int> senderAction)
        {
            //Create a background worker
            _worker = new Thread(RunWorker) { IsBackground = true };
            _worker.Start();
            _senderAction = senderAction;
        }

        private void RunWorker() //This is the background worker's method
        {
            while (true) //Keep it running
            {
                lock (_myQueue) //No fiddling from other threads
                {
                    while (_myQueue.Count != 0) //Pop elements if found
                        SendMessage(_myQueue.Dequeue()); //Send the element
                }
                Thread.Sleep(50); //Allow new elements to be inserted
            }
        }

        private void SendMessage(int element)
        {
            _senderAction(element);
        }

        private void DefaultMessageSendingAction(int item)
        {
            /* whatever happens during sending */
        }

        public void AddToQueue(int element)
        {
            Task.Run(() => //Async method will return at ones, not slowing the caller
            {
                lock (_myQueue) //Lock queue to insert into it
                {
                    _myQueue.Enqueue(element);
                }
            });
        }
    }

    public class TestClass
{
    private SenderClass _sender;
    private Queue<int> _messages;

    [TestInitialize]
    public void SetUp()
    {
        _messages = new Queue<int>();
        _sender = new SenderClass(DummyMessageSendingAction);
    }

    private void DummyMessageSendingAction(int item)
    {
        _messages.Enqueue(item);
    }

    [TestMethod]
    public void TestMethod1()
    {
        //This isn't a great test, but I think you get the idea
        int message = 42;
        _sender.AddToQueue(message);
        Thread.Sleep(100);
        CollectionAssert.Contains(_messages, 42);            
    }
}

Answer 2

It looks like SenderClass should not perform any sending at all. It should simply maintain the queue. Inject an Action<int> through the constructor that does the sending. That way you can move SendMessage somewhere else and call it however you like.

As an added benefit your test of SendMessage is not cluttered with queue management.

Seeing your edit you don't seem to like this approach and you don't seem to like the InternalsVisibleTo approach either. You could expose SendMessage through a separate interface and implement that interface explicitly. That way SendMessage is still callable through that interface but by default it is not accessible without some casting contortions. It also does not show up in the intellisense autocomplete list.