Partager via

Unit tests for a simple OnMoved event handler

  • Article

Sometimes even the simple stuff can have some interesting unit tests and corner cases. I had a class that's basically a "Thing" with an X,Y position and an OnMoved event. In C# :

     public class ThingMovedArgs {
        public ThingMovedArgs(Thing t, Point ptOld, Point ptNew);
    }

    class Thing
    {
        int X; // just get/set X
        int Y; // just get/set Y
        Point XY; // get/set X and Y together as a Point
        public event EventHandler<ThingMovedArgs> OnMoved;
    }

The OnMoved handler provides the old location and new location. Assume this is all single-threaded.

It looks so simple. Yet there are some pretty interesting corner cases that would actually be reasonable for a well-intentioned developer to get wrong.

 

Here are the tests I wrote and some of the "specification holes" they reveal :

  1. Obvious: Test that getting and setting X and Y work. Eg, "thing.X = x; Assert(thing.X == x)".

  2. There's no constructor here, so test for the default value of the properties.

  3. Test that the XY property cooperates with the X and Y properties. For example, if you set X and Y separately, you can retrieve them as expected via XY.  Ensure that (XY.X == X). In general, there's always a test surface around places where multiple properties are attached to a single data source. You need to ensure changes in one property are reflected as changes in the other.

  4. Test that the OnMoved handler is actually fired when changing each property and has the expected properties when fired in trivial cases.

  5. Test that the X,Y, and XY properties on the EventArgs instance have the "correct" value when queried inside of the OnMoved handler. Essentially, this is checking whether OnMove is fired before or after the property is updated. (Spec hole: It's debatable whether the "correct" value should be the old or new value. That redundancy may suggest that a better design is to remove whichever parameter on the ThingMovedArgs event is redundant with the XY property.)

  6. Test that setting the X,Y, and XY properties in the OnMoved handler does the "right" thing. (Spec hole: It's debatable what that should be: Fail? Fire a nested OnMoved callback? Not fire an OnMoved callback?).
       * This could be tempting to do as a way of "rejecting" a move.
       * This could also happen if an OnMoved handler is complex enough that it adjusts another object, and that object adjusts the original object.
       * This is an example of the problem that the whole world can change under you when you invoke a callback. See other design problems with callbacks.

  7. Test that if an exception is thrown from OnMoved, that it does the right thing. (Another spec hole).

  8. Test that OnMoved handler is still fired even when you set a property to itself.  (eg "thing.X = thing.X").   In this case, ptOld == ptNew. I think the naive implementation would just handle this case, but a test could be valuable in case the developer tries to get too clever. It's not obvious that that's the "best" behavior. This is a good example of where a Tester can hold the Developer accountable to ensure the code is well specified.

  9. Test that when setting XY, the OnMoved handler is only fired once and not twice. This would catch bugs in a naive buggy implementation of XY's setter. It may not be obvious why this is a bug. In this case, OnMoved() is fired with a point that the Thing is never actually at.

             Point XY // get/set X and Y together as a Point
        {
            set // BUG!!! Fires OnMoved 2 times instead of just once
            {
                X = value.X; // <-- fires OnMoved once for changing X
                Y = value.Y; // <-- fires OnMoved a 2nd time for changing Y
            }
        }

  10. Test that the OnMoved handler is NOT fired once you clear it.  This should be a freebe for the default implementation of events.

  11. Scenario test: Build a simple scenario test and ensure that it works end-to-end. In this case, I want 2 things (T1 and T2), and I always want T2 to be 10 pixels to the right of T1. So "T2.X = T1.X + 10;". To ensure this, T2 subscribes to T1's OnMove and updates itself as needed.

 

Anybody see others?

 

The lesson learned:
There's an infinite number of possible bugs in a sufficiently dumb implementation. I think a key here is that many of these failures:
- could occur with a bad corner-case from a reasonable attempt to implement the class.  If you got 100 developers to implement the class, you'd probably hit all of these problems. Or you could probably hit these cases with just 5 implementations if you just asked the developers to try and optimize their code. :)
- indicate spec holes.

Comments