Testing Cross Cutting Concerns

So imagine you've been asked to implement the following requirement:

When a to-do list item is marked as complete, the CompletedOn date time property shall be set to the current time.

That doesn't sound so hard to implement, but how can I test it? I don't know precisely what the value of the CompletedOn property "should be" since I don't know precisely when the Complete setter will run.

Method #1: Fuzzy Logic

One option would be to see if the value is "close enough":

var todo = new TodoItem("Test item");



var testStart = DateTime.Now;

todo.Complete = true;

var testStop = DateTime.Now;



Assert.IsTrue(todo.CompletedOn >= testStart && todo.CompletedOn <= testStop,

    "CompletedOn not set to correct timestamp.");

There's a lot to dislike about this technique. The test could fail if it runs when the system clock moves backwards, e.g., when daylight savings time ends. It's imprecise, and clearly won't work in cases where we need a total order across multiple timestamps.

On the other hand, it's good enough to start writing an implementation, and with that in hand we can consider a better test.

Implementing TodoItem

Here's a first pass at an implementation:

public class TodoItem

{

    public DateTime? CompletedOn { get; private set; }



    private bool _complete;

    public bool Complete

    {

        get { return _complete; }

        set

        {

             _complete = value;

             CompletedOn = value ? DateTime.Now ? null;

        }

    }

    // ... rest of class

Method #2: Ambient Context

In his excellent book, Dependency Injection in .NET (I'll write a full review someday, but the book was just updated and I'm still reading that), Mark Seeman suggests replacing the call to DateTime.Now in the implementation with a separate singleton which I can control. The implementation would change to:

public class TodoItem

{

    public DateTime? CompletedOn { get; private set; }



    private bool _complete;

    public bool Complete

    {

        get { return _complete; }

        set

        {

             _complete = value;

             CompletedOn = value ? TimeProvider.Current.Now ? null;

        }

    }

    // ... rest of class

...and the test becomes:

var todo = new TodoItem("Test item");

var expectedTime = new DateTime(2011, 01, 01, 1, 2, 3);

TimeProvider.Current = new OneTimeOnlyProvider { Now = expectedTime } ;



todo.Complete = true;



Assert.AreEqual(expectedTime, todo.CompletedOn, "CompletedOn not set to correct timestamp.");

The test is quite a bit better than the first attempt. I'm now testing for a specific value.

On the other hand, the readability of the code suffers somewhat. Most .NET programmers immediately know what DateTime.Now means, whereas with TimeProvider.Current.Now they have to guess. Also, developers on the team must learn to use a "non-standard" method of obtaining the current time.

Should legacy code, which might not even require a specific value of DateTime.Now for its tests be updated to use this "new" method, just for the sake of consistency? What if that changes the performance characteristics of the code? It's hard to dismiss this concern as "premature optimization" when we don't want to change the functionality of the legacy code. This leaves me with the choice of a possibly (slightly) risky change to existing code or inconsistency in the code base.

At any rate, this seems like a good solution, but it's not the only one.

Method #3: Moles

It's pretty common to mock or stub methods not directly related to the system under test. Most mocking libraries don't support this for static methods, but Moles, from Microsoft research, does.

Moles is a kind of monkeypatching for .NET; you can replace just about any method at runtime.

With Moles, we stick with the original implementation of the system under test and instead change the test:

var todo = new TodoItem("Test item");

var expectedTime = new DateTime(2011, 01, 01, 1, 2, 3);

MDateTime.NowGet = () => { return expectedTime; }; // this replaces all calls to DateTime.Now



todo.Complete = true;



Assert.AreEqual(expectedTime, todo.CompletedOn, "CompletedOn not set to correct timestamp.");

Again, it's a better test than method #1.

Is it better than #2? Well, it's certainly different. Moles replaces all calls to System.DateTime.Now in the code under test, whether or not I've intended this. In my sample code, there's only one. But in the real world the code under test is generally far more complicated.

Interestingly, the technique in use here -- replacing a standard method with one we supply -- is the same for Ambient Context and Moles. However the place where we make this redirection is quite different. Although the new method is defined in the test project in both cases, the actual rewiring is done in the test with Moles, whereas with Ambient Context it's a source code change.

Moles is clearly useful in cases where you need to change a dependency of code which is not your own. Imagine trying to test non-MVC ASP.NET code which depends on ASP.NET methods which internally refer to HttpContext.Current. Indeed, this was painful even in early versions of MVC, although not so much in MVC 3.

But that's not the case with this code: I have a direct dependency on DateTime.Now, not an indirect dependency. So in this case I'm not forced to use something like Moles.

Moles is pre-release (though it is currently being "productized"), un-supported, and closed source. That, by itself, will disqualify its use from many projects. On the other hand, it's required for Pex, which is useful in its own right.

Method #4: Constructor Injection

We could of course use normal DI for the time feature

public class TodoItem

{

    public DateTime? CompletedOn { get; private set; }



    private ITimeProvider _timeProvider;



    private bool _complete;

    public bool Complete

    {

        get { return _complete; }

        set

        {

             _complete = value;

             CompletedOn = value ? _timeProvider.Now ? null;

        }

    }

    // ... rest of class

...and the test becomes:

var expectedTime = new DateTime(2011, 01, 01, 1, 2, 3);

var todo = new TodoItem("Test item", new OneTimeOnlyProvider { Now = expectedTime });

todo.Complete = true;



Assert.AreEqual(expectedTime, todo.CompletedOn, "CompletedOn not set to correct timestamp.");

The test is simple and the code is clean. For this specific case it's arguably the best option.

However, if a significant fraction of all types start requiring a time provider constructor argument, especially if it's mostly a transient reference to pass along to another type's constructor, then it's not a good choice.

Conclusions?

The "fuzzy logic" method is better than no test at all, but that's about the best I can say for it. Moles can do things which no other method can, but that's not required for this case. The Ambient Context method works fine here, but raises questions about how to apply it consistently. I can't tell you which method is best to use; I'm still trying to work that out for myself! Your comments would be welcome.

Thanks to Mark Seeman for answering my questions on Twitter.