Testing callback with observable pattern

Question

I want to write some Unittests with NUnit for our wpf application. The application downloads some data with System.Net.WebClient in the background using the observer pattern.

Here is an example:

Download.cs

public class Download : IObservable<string>
{
    private string url { get; }
    private List<IObserver<string>> observers = new List<IObserver<string>>();
    private bool closed = false;
    private string data = null;

    public Download(string url)
    {
        this.url = url;

        startDownload();
    }

    public IDisposable Subscribe(IObserver<string> observer)
    {
        if (!observers.Contains(observer))
        {
            if (!closed)
            {
                observers.Add(observer);
            }
            else
            {
                sendAndComplete(observer);
            }

        }

        return new Unsubscriber(observer, observers);
    }

    private void startDownload()
    {
        WebClient client = new WebClient();
        client.DownloadStringCompleted += new DownloadStringCompletedEventHandler((object sender, DownloadStringCompletedEventArgs e) => {
            if (e.Error != null)
            {
                data = e.Result;
            }

            closed = true;
            sendAndComplete();
        });

        client.DownloadStringAsync(new Uri(url));
    }

    private void sendAndComplete()
    {
        foreach (var observer in observers)
        {
            sendAndComplete(observer);
        }

        observers.Clear();
    }

    private void sendAndComplete(IObserver<string> observer)
    {
        if (data != null)
        {
            observer.OnNext(data);
        }
        else
        {
            observer.OnError(new Exception("Download failed!"));
        }

        observer.OnCompleted();
    }

    private class Unsubscriber : IDisposable
    {
        private IObserver<string> _observer { get; }
        private List<IObserver<string>> _observers { get; }

        public Unsubscriber(IObserver<string> _observer, List<IObserver<string>> _observers)
        {
            this._observer = _observer;
            this._observers = _observers;
        }

        public void Dispose()
        {
            if (_observer != null && _observers.Contains(_observer))
            {
                _observers.Remove(_observer);
            }
        }
    }
}

DownloadInspector.cs

public class DownloadInspector : IObserver<string>
{
    private Action<string> onSuccessAction { get; }
    private Action<Exception> onErrorAction { get; }
    private Action onCompleteAction { get; }

    public DownloadInspector(Action<string> onSuccessAction, Action<Exception> onErrorAction, Action onCompleteAction)
    {
        this.onSuccessAction = onSuccessAction;
        this.onErrorAction = onErrorAction;
        this.onCompleteAction = onCompleteAction;
    }

    public void OnCompleted()
    {
        onCompleteAction.Invoke();
    }

    public void OnError(Exception error)
    {
        onErrorAction.Invoke(error);
    }

    public void OnNext(string value)
    {
        onSuccessAction.Invoke(value);
    }
}

example (usage)

Download download = new Download("http://stackoverflow.com");
DownloadInspector inspector = new DownloadInspector(
    (string data) =>
    {
        Debug.WriteLine("HANDLE DATA");
    },
    (Exception error) =>
    {
        Debug.WriteLine("HANDLE ERROR");
    },
    () =>
    {
        Debug.WriteLine("HANDLE COMPLETE");
    }
    );

I'm still new in c# and not very familiar with asynchronous programming in that language. I know the await and async keywords and know that they work with NUnit, but the current construct don't use this keywords.

Can you help me creating a unit test for this case? Its okay to change/remove the observer pattern.

Answer 1

The constructor for the Download class starts the download, which means that I can't subscribe an observer until after the download has started. That's a race condition. It's possible (although unlikely) that observers will be notified before they can be subscribed.

public Download(string url)
{
    this.url = url;
    startDownload();
}

But I can go ahead and test because I'm subscribing an observer before that can happen. If you can I'd recommend not doing that. Allow the caller to construct the class in one step and then start the download with a method call.

I also had to change this method. I figured that testing for an error would be the easiest first step, but it needs to do data = e.Result if there is no error, not if there is an error.

private void StartDownload()
{
    WebClient client = new WebClient();
    client.DownloadStringCompleted += new DownloadStringCompletedEventHandler((object sender, DownloadStringCompletedEventArgs e) =>
    {
        if (e.Error == null) // <== because of this
        {
            data = e.Result;
        }

        closed = true;
        sendAndComplete();
    });    

    client.DownloadStringAsync(new Uri(url));
}

What I didn't see coming is that WebClient.DownloadStringAsync isn't actually async. It doesn't return a Task. It just takes a callback. What that means is that there's no sure way to know whether it's done except to wait for it to notify the observer that the download is complete.

My NUnit test runner wasn't running, so I used MsTest. It's the same thing.

The basic approach is that I'm creating some flags, and the inspector responds to notifications by setting the flags. That way I can see which notifications were raised.

The last problem is that because DownloadStringComplete is a callback, the test exits before the Assert. That means it will always pass. So in order to fix it I had to do something I've never seen before, which I found here:

[TestMethod]
public void download_raises_error_notification()
{
    var success = false;
    bool error = false;
    bool complete = false;
    var pause = new ManualResetEvent(false);

    var download = new Download("http://NoSuchUrlAnywhere.com");
    var inspector = new DownloadInspector(
        onSuccessAction: s => success = true,
        onCompleteAction: () =>
        {
            complete = true;
            pause.Set();
        },
        onErrorAction: s => error = true
        );

    download.Subscribe(inspector);

    // allow 500ms for the download to fail. This is a race condition.
    pause.WaitOne(500);

    Assert.IsTrue(error,"onErrorAction was not called.");
}

This is technically an integration test since it must actually attempt a download in order to run. That could be remedied by mocking WebClient.