views:

107

answers:

4

I'm having a problem unit testing a class which fires events when a thread starts and finishes. A cut down version of the offending source is as follows:

public class ThreadRunner
{
    private bool keepRunning;

    public event EventHandler Started;
    public event EventHandler Finished;

    public void StartThreadTest()
    {
        this.keepRunning = true;
        var thread = new Thread(new ThreadStart(this.LongRunningMethod));
        thread.Start();
    }

    public void FinishThreadTest()
    {
        this.keepRunning = false;
    }

    protected void OnStarted()
    {
        if (this.Started != null)
            this.Started(this, new EventArgs());
    }

    protected void OnFinished()
    {
        if (this.Finished != null)
            this.Finished(this, new EventArgs());
    }

    private void LongRunningMethod()
    {   
        this.OnStarted();

        while (this.keepRunning)
            Thread.Sleep(100);

        this.OnFinished();
    }
}

I then have a test to check that the Finished event fires after the LongRunningMethod has finished as follows:

[TestClass]
public class ThreadRunnerTests
{
    [TestMethod]
    public void CheckFinishedEventFiresTest()
    {
        var threadTest = new ThreadRunner();

        bool finished = false;

        object locker = new object();

        threadTest.Finished += delegate(object sender, EventArgs e)
        {
            lock (locker)
            {
                finished = true;
                Monitor.Pulse(locker);
            }
        };

        threadTest.StartThreadTest();
        threadTest.FinishThreadTest();

        lock (locker)
        {
            Monitor.Wait(locker, 1000);
            Assert.IsTrue(finished);
        }
    }
}

So the idea here being that the test will block for a maximum of one second - or until the Finish event is fired - before checking whether the finished flag is set.

Clearly I've done something wrong as sometimes the test will pass, sometimes it won't. Debugging seems very difficult as well as the breakpoints I'd expect to be hit (the OnFinished method, for example) don't always seem to be.

I'm assuming this is just my misunderstanding of the way threading works, so hopefully someone can enlighten me.

+2  A: 

You test seems to be wrong. Assume that after threadTest.FinishThreadTest(); lock is obtained by the code in CheckFinishedEventFiresTest(). Then the test is going to fail. You've got a clear race condition here.

Note that return from FinishThreadTest() doesn't guarantee that the thread is finished. It just sets the flag for the thread, which may be taken into consideration at any moment (nothing basically guarantees that the thread gets run by the scheduler immediately).

In your case, the thread will be most probably busy Sleep()ing. After calling threadTest.FinishThreadTest(); the lock will most probably acquired by the thread where CheckFinishedEventFiresTest() is executed. The monitor will wait 1 second and then give up. After that lock will be released, so the delegate will be able to lock only at that moment.

Vlad
Ah, certainly makes sense now. Thanks Vlad.
Dougc
+2  A: 

Vlad is absolutely right, but I'll take another shot at clarifying the problem:

// This runs on the other thread
threadTest.Finished += delegate(object sender, EventArgs e) {
    // I can't get this lock if the test thread gets here first!
    lock (locker) {
        finished = true;
        Monitor.Pulse(locker);
    }
};

You can do it with a wait handle of some kind. I'd use a ManualResetEvent:

ManualResetEvent waitHandle = new ManualResetEvent(false);
threadTest.Finished += delegate(object sender, EventArgs e) {
    finished = true;
    waitHandle.Set();
};

threadTest.StartThreadTest();
threadTest.FinishThreadTest();

// Specify a timeout so your test isn't hostage forever
if (waitHandle.WaitOne(timeout, true)) {
    Assert.IsTrue(finished);
}
Jeff Sternal
Thanks Jeff. There still seems to be a weird issue where the thread sometimes doesn't exit after the keepRunning flag has been switched by the FinishThreadTest method? Even if I up the timeout to a very high value.
Dougc
`LongRunningMethod` doesn't exit? Maybe there's some funny business with the second WaitOne parameter (exit context). I'd try nobugz' sample, which aims to accomplish the same thing, but is better anyway (dispensing with the superfluous bool).
Jeff Sternal
Never mind, I was being an idiot (see my comment on the original question). Thanks!
Dougc
+3  A: 

A lock is just not appropriate here, you'll want to signal an event. For example:

    public void CheckFinishedEventFiresTest() {
        var threadTest = new ThreadRunner();
        var finished = new ManualResetEvent(false);
        threadTest.Finished += delegate(object sender, EventArgs e) {
            finished.Set();
        };
        threadTest.StartThreadTest();
        threadTest.FinishThreadTest();
        Assert.IsTrue(finished.WaitOne(1000));
    }
Hans Passant
Thanks nobugz, that seems to solve the issue. Just FYI, I didn't seem to be able to use Monitor.Wait outside of a lock, so I replaced it with finished.WaitOne instead and all seems fine!
Dougc
Yes, I fumbled the code. Corrected.
Hans Passant
+2  A: 

I recently wrote a series of blog posts on unit testing event sequences for objects that publish both synchronous and asynchronous events. The posts describe a unit testing approach and framework, and provides the full source code with tests.

Using the framework tests can be written like so:

AsyncEventPublisher publisher = new AsyncEventPublisher();

Action test = () =>
{
    publisher.RaiseA();
    publisher.RaiseB();
    publisher.RaiseC();
};

var expectedSequence = new[] { "EventA", "EventB", "EventC" };

EventMonitor.Assert(test, publisher, expectedSequence, TimeoutMS);

The EventMonitor does all the heavy lifting and will run the test (action) and assert that events are raised in the expected sequence (expectedSequence). It handles asynchronous events, and prints out nice diagnostic messages on test failure.

There's a lot of detail in the posts describing the issues and approaches, and source code too:

http://gojisoft.com/blog/2010/04/22/event-sequence-unit-testing-part-1/

chibacity