Do I have to unit test event handlers

Question

I am inclined to believe that only public interfaces should be tested, thereby covering testing private procedures. However, an interesting question arose yesterday - should there be one event handler? My gut instinct is that the logic should be stored in autonomous procedures called by the handlers themselves, but this is subjective and most likely will lead to closed procedures, and not to public testers. Should I be a modular event handler, and if so, what are the best methods for doing this?

Answer 1

In no case will I say that someone is "mistaken" for unit testing an event handler. Personally, I would go with the philosophy of "checking what can break," and never will.

The main thing that I saw sequentially with the event code is that the unit tests will not catch - the "On" method will look like this:

if (MyEventHandler != null) MyEventHandler(this, e); 

This is a race condition; MyEventHandler should be assigned to a variable before null checking.

The second common mistake: people passing the null value for the data parameter of the event "e"; it can be checked.

If you do not have a copy of the Frame Design Guide 2nd ed. Kvalina and Abrams, buy it now. He will tell you how to write the event code correctly, how to write the Dispose template correctly, and much more.

Answer 2

I believe that event handlers should be tested. If you follow the normal pattern of doing something line by line:

 public event EventHandler MyEvent; protected void OnMyEvent() { // Raise MyEvent here } 

Then, testing MyEvent is actually part of the OnMyEvent test, as the only "test" you will do is to verify that the event was raised properly.

As a rule, testing an event means subscribing to it and doing something that (or should not) raise it.

Answer 3

I read your question and did not know if you asked about the body of the handler or whether the handler correctly handled the event.

As you say, the body of the handler should simply call another method that you have already tested (if it is open).

I generally do not unit test posting event handlers if they do not change at runtime, since I will most likely catch in my event handlers developers and integrations that do not bind / do not bind at run time, are not connected and should have been .

Answer 4

TrueWill provides an example of a good implementation of an event and a method for raising it. This is a Microsoft Click class event. First of all, note that they use an EventHandlerList to store tasks in ...

 protected EventHandlerList Events { get { if (events == null) { events = new EventHandlerList(this); } return events; } } ... public event EventHandler Click { add { Events.AddHandler(EventClick, value); } remove { Events.RemoveHandler(EventClick, value); } } 

Now pay attention to the real OnClick promotion OnClick , it is probably very different from what you used to see ...

 protected virtual void OnClick(EventArgs e) { Contract.Requires(e != null); EventHandler handler = (EventHandler)Events[EventClick]; if (handler != null) handler(this, e); } 

... don't worry about the line Contract.Requires(e != null); that their contract management infrastructure, but notice that it pulls it from EventHandlerList , and then if this handler is not null , they will start it.

Another thing that is probably worth noting here is that you probably won't need to implement your events in exactly the same way, but the programming guide released by Microsoft actually raises the race condition in the second part of this guide that TrueWill indicates. You can find this guide here . This is actually one very good guide from Microsoft.

Now to your point , I believe that events should be tested, and here is the mechanism that I used in the past ...

 private ManualResetEvent _eventRaised = new ManualResetEvent(false); [TestMethod] public void TestSomething() { _eventRaised.Reset(); // hook up the event to the target being tested // NOW, in the event handler, issue _eventRaised.Set(); // do something to raise the event _eventRaised.WaitOne(); }