views:

179

answers:

8

I have this method which is dependent on current date. It checks if today is Sun, Mon, Tue or Wed, then it gives 5 days of lead time for arrival of shipped items. If its Thur, Fri or Sat then it gives 6 days of lead time to account for the weekend.

private DateTime GetEstimatedArrivalDate()
{
    DateTime estimatedDate; 
    if (DateTime.Now.DayOfWeek >= DayOfWeek.Thursday)
    {
        estimatedDate = DateTime.Now.Date.AddDays(6);
    }
    else
    {
        estimatedDate = DateTime.Now.Date.AddDays(5);
    }
    return estimatedDate; 
}

The actual estimation logic is more complex. I have simplified it for the purpose of this question. My question is how do I write a unit test for something like this which depends on todays date?

A: 

Seems like there are a limited enough number of cases that you could test them each explicitly. The method depends on today's date, but the output depends only on the day of week, and every date has a day of week.

John at CashCommons
+15  A: 

You need to pass the current date in as a parameter:

private DateTime GetEstimatedArrivalDate(DateTime currentDate)
{
    DateTime estimatedDate; 
    if (currentDate.DayOfWeek >= DayOfWeek.Thursday)
    {
        estimatedDate = currentDate.AddDays(6);
    }
    else
    {
        estimatedDate = currentDate.AddDays(5);
    }
    return estimatedDate; 
}

In real code you call it like this:

DateTime estimatedDate = GetEstimatedArrivalDate(DateTime.Now.Date);

Then you can test it as follows:

DateTime actual = GetEstimatedArrivalDate(new DateTime(2010, 2, 10));
DateTime expected = ...;
// etc...

Note that this also fixes a potential bug in your program where the date changes between consecutive calls to DateTime.Now.

Mark Byers
I just posted the same answer. You beat me to the speed :)
Romain
fixed the if for you.
Paul Creasey
+1 for the last line...
Partha Choudhury
Thanks for pointing out the bug. I like your solution. But notice that this method is private. I can certainly make it public for the purpose of testing, but then I loose my nice encapsulation. I think I am okay with that since this is a small enough function.
Rohit Agarwal
A: 

You could pass in a delegate that returns DateTime.Now during normal execution, and then in your test pass in another delegate that returns a fixed date, and assert your result based on that.

Luhmann
A: 

One "common" way of doing so is to "fake" the current system date (that can be done in several ways) and then test your code on "known" dates.

Another interesting way is to change your implementation slightly to:

private DateTime GetEstimatedArrivalDate(DateTime forDate)
{
    return GetEstimatedArrivalDate(DateTime.Now);
}

private DateTime GetEstimatedArrivalDate(DateTime forDate)
{
    DateTime estimatedDate; 
    if (forDate.DayOfWeek >= DayOfWeek.Thursday)
    {
        estimatedDate = forDate.Date.AddDays(6);
    }
    else
    {
        estimatedDate = forDate.Date.AddDays(5);
    }
    return estimatedDate; 
}

And then use the method with a parameter to test on "immediate" dates.

Romain
+1  A: 

I'll give the controversial answer, don't test it.

The logic is trivial and it has zero dependencies, i believe in good code coverage but not when it increases complexity for no real gain.

Paul Creasey
That sounds reasonable, but this is exactly the kind of method that I would expect to change unexpectedly. If the company changes their shipping policies, starts offering tiered services, or switches carriers. Now technically, you could start testing it once it gets complicated, but I tend to think that this is a significant hinge in the application, and it should be under test so that absolutely everyone knows the moment it changes. Just in case.
jcdyer
Bah. What if we decide to swap the positions of Thursday and Friday during the week? You'd *never know this code was broken* until it was *too late!* And if we decide to start counting days *backward*, well, then, I don't even know *what* to tell you if you can't show me a blinking red "test failed" light. ;)
Aaronaught
I agree with other commenters. The logic in this method is actually non-trival. I made it simple just for the purposes of showing it as an example for my question. I do want to test it thoroughly.
Rohit Agarwal
+5  A: 

Generally speaking, you'd want to abstract the method of obtaining the current date and time behind an interface, eg:

public interface IDateTimeProvider
{
    DateTime Now { get; }
}

The real service would be:

public class DateTimeProvider: IDateTimeProvider
{
    public DateTime Now
    {
        get
        {
            return DateTime.Now;
        }
    }
}

And a test service would be:

public class TestDateTimeProvider: IDateTimeProvider
{
    private DateTime timeToProvide;
    public TestDateTimeProvider(DateTime timeToProvide)
    {
        this.timeToProvide = timeToProvide;
    }

    public DateTime Now
    {
        get
        {
            return timeToProvide;
        }
    }
}

For services that require the current time, have them take an IDateTimeProvider as a dependency. For the real thing, pass a new DateTimeProvider(); when you're a component, pass in a new TestDateTimeProvider(timeToTestFor).

Erik Forbes
I'll give this a +1because your absolutely right but at the same time i feel it's a bit over the top just for testing a trival method. I admire your dedication to coverage though, and i'd probably adopt this if there were either lots of methods or some complex method that needed testing :)
Paul Creasey
It may be trivial now, but things can easily change. Anyway, +1 as I'd certainly make an adapter around DateTime classes to enable testing. Plus, for a TDDer, this would be the normal route.
Finglas
On a side note, would it not be better to use a mocking framework, rather than creating a "test service" that you now need to maintain as well? It seems a little unnessersary?
David Kiff
Erik thanks for your answer. But I agree with Paul. It might be a bit of overhead for my case.
Rohit Agarwal
No worries. =) @David - perhaps it is, but then I tend to stay away from mocking frameworks when it's a simple matter of mocking it up manually. As for maintenance, it shouldn't be any more difficult than adding a mapping into your IOC container and adding a constructor parameter to the classes that require it.I'm just playing devil's advocate at this point however. Mark's solution is probably the best if you only have a few such methods - any more than a few, however, and passing in the current datetime will get tedious.
Erik Forbes
+2  A: 

Make your class take an IClock parameter (via constructor or property)

interface IClock
{
    DateTime Now { get; }
}

You can then use a fake implementation for testing

class FakeClock : IClock
{
    DateTime Now { get; set }
}

and a real implementation the rest of the time.

class SystemClock : IClock
{
    DateTime Now { get { return DateTime.Now; } }
}
Ben Lings
A: 

I would suggest doing this as Mark suggests, but with the addition of a overloaded call for production use that takes no parameter and uses DateTime.Now

private DateTime GetEstimatedArrivalDate()
{
    return GetEstimatedArrivalDate(DateTime.Now);
}

private DateTime GetEstimatedArrivalDate(DateTime currentDate)
{
    DateTime estimatedDate; 
    if (currentDate.DayOfWeek >= DayOfWeek.Thursday)
    {
        estimatedDate = currentDate.AddDays(6);
    }
    else
    {
        estimatedDate = currentDate.AddDays(5);
    }
    return estimatedDate; 
}
C. Ross