Unit Tests for getting the first Monday of the month

Question

I have an application that runs hourly, and I want to use it to also send an email at the start of the day of the first Monday of the month. I searched and found information to help me code what I needed, and all my tests pass, but I want to make sure that I am testing for all edge cases and that I am also being a good little programmer and writing good code.

I have the application being called from a task scheduler on the hour, sometimes some of the code doesn't get called right on the hour.

My Code for checking the date:

public bool CheckSendNewEvent()
{
    var now = DateTime.Now;
    const int SendTime = 8;
    var firstMonday = GetFirstMondayofMonth(now);
    var timeToSendOnFirstMonday = new DateTime(firstMonday.Year, firstMonday.Month, firstMonday.Day, SendTime, 0, 0);
    return now > timeToSendOnFirstMonday && now < (timeToSendOnFirstMonday.AddHours(1));
}

public static bool CheckSendNewEvent(DateTime now)
{
    const int SendTime = 8;
    var firstMonday = GetFirstMondayofMonth(now);
    var timeToSendOnFirstMonday = new DateTime(firstMonday.Year, firstMonday.Month, firstMonday.Day, SendTime, 0, 0);
    return now > timeToSendOnFirstMonday && now < (timeToSendOnFirstMonday.AddHours(1));
}

/// From http://stackoverflow.com/a/3284486/1214743
///<summary>Gets the first week day following a date.</summary>
///<param name="date">The date.</param>
///<param name="dayOfWeek">The day of week to return.</param>
///<returns>The first dayOfWeek day following date, or date if it is on dayOfWeek.</returns>
public static DateTime Next(DateTime date, DayOfWeek dayOfWeek)
{
    return date.AddDays((dayOfWeek < date.DayOfWeek ? 7 : 0) + dayOfWeek - date.DayOfWeek);
}

public static DateTime GetFirstMondayofMonth(DateTime now)
{
    var monday = new DateTime(now.Year, now.Month, 1);
    monday = Next(monday, DayOfWeek.Monday);
    return monday;
}

I hate to admit this, but I am pretty much a beginner when it comes to Unit Tests, and I think that my life would be much easier with them, so I want to make sure that I am starting out with good habits.

Some of these tests were written before and after reading some articles about naming tests, hence the reason for the bad naming consistency

So here are my tests:

[TestMethod]
public void CheckSendNewEvent_Test()
{
    var program = new Program();
    var checker = program.CheckSendNewEvent();
    Assert.AreEqual(false, checker);

}
[TestMethod]
public void CheckSendNewEvent_MinDate()
{
    var checker = AttorneyEventEmailer.Program.CheckSendNewEvent(DateTime.MinValue);
    Assert.AreEqual(false, checker);

}
[TestMethod]
public void CheckSendNewEvent_MaxDate()
{
    var checker = AttorneyEventEmailer.Program.CheckSendNewEvent(DateTime.MaxValue);
    Assert.AreEqual(false, checker);          
}

[TestMethod]
public void CheckSendNewEvent_201603010800()
{
    DateTime march1st2016 = new DateTime(2016, 3, 1, 8, 0, 0);
    var checker = AttorneyEventEmailer.Program.CheckSendNewEvent(march1st2016);
    Assert.AreEqual(false, checker);
}

[TestMethod]
public void CheckSendNewEvent_201603070800()
{
    DateTime march1st2016 = new DateTime(2016, 3, 7, 8, 0, 0);
    var checker = AttorneyEventEmailer.Program.CheckSendNewEvent(march1st2016);
    Assert.AreEqual(false, checker);
}

[TestMethod]
public void CheckSendNewEvent_20160301_0800()
{
    DateTime march1st2016_8AM = new DateTime(2016, 3, 1, 8, 0, 0);
    var checker = AttorneyEventEmailer.Program.CheckSendNewEvent(march1st2016_8AM);
    Assert.AreEqual(false, checker);
}

[TestMethod]
public void CheckSendNewEvent_20160301_080005()
{
    DateTime march1st2016_80005AM = new DateTime(2016, 3, 1, 8, 0, 5);
    var checker = AttorneyEventEmailer.Program.CheckSendNewEvent(march1st2016_80005AM);
    Assert.AreEqual(false, checker);
}

[TestMethod]
public void CheckSendNewEvent_20160307_080005()
{
    DateTime march7th2016_80005AM = new DateTime(2016, 3, 7, 8, 0, 5);
    var checker = AttorneyEventEmailer.Program.CheckSendNewEvent(march7th2016_80005AM);
    Assert.AreEqual(true, checker);
}


[TestMethod]
public void GetFirstMondayOfMonth_20150406_20150406()
{
    var date_20150406 = new DateTime(2015, 4, 6);
    var firstMonday = new DateTime(2015, 4, 6);
    var testResult = Program.GetFirstMondayofMonth(date_20150406);
    Assert.AreEqual(testResult, firstMonday);

}

[TestMethod]
public void GetFirstMondayOfMonth_20150401_20150406()
{
    var date_20150401 = new DateTime(2015, 4, 1);
    var firstMonday = new DateTime(2015, 4, 6);
    var testResult = Program.GetFirstMondayofMonth(date_20150401);
    Assert.AreEqual(testResult, firstMonday);
}

[TestMethod]
public void GetFirstMondayOfMonth_20150415_20150406()
{
    var date_20150415 = new DateTime(2015, 4, 15);
    var firstMonday = new DateTime(2015, 4, 6);
    var testResult = Program.GetFirstMondayofMonth(date_20150415);
    Assert.AreEqual(testResult, firstMonday);
}

Answer 1

public bool CheckSendNewEvent()
{
    var now = DateTime.Now;
    const int SendTime = 8;
    var firstMonday = GetFirstMondayofMonth(now);
    var timeToSendOnFirstMonday = new DateTime(firstMonday.Year, firstMonday.Month, firstMonday.Day, SendTime, 0, 0);
    return now > timeToSendOnFirstMonday && now < (timeToSendOnFirstMonday.AddHours(1));
}

public static bool CheckSendNewEvent(DateTime now)
{
    const int SendTime = 8;
    var firstMonday = GetFirstMondayofMonth(now);
    var timeToSendOnFirstMonday = new DateTime(firstMonday.Year, firstMonday.Month, firstMonday.Day, SendTime, 0, 0);
    return now > timeToSendOnFirstMonday && now < (timeToSendOnFirstMonday.AddHours(1));
}

This is basically one method where you provide a standard option. There are two choice you have here: either put the implementation in one method and create an overload like this:

public static bool CheckSendNewEvent(DateTime now)
{
    // Implementation
}

public static bool CheckSendNewEvent()
{
    return CheckSendNewEvent(DateTime.Now);
}

Or use a default value. However since DateTime.Now is not a constant, you'll have to use a little trick:

public static bool CheckSendNewEvent(DateTime? now = null)
{
    now = now ?? DateTime.Now;
    // rest of method
}

---

public static DateTime Next(DateTime date, DayOfWeek dayOfWeek)

I'd name it GetNextWeekDay or something like that.

---

public static DateTime GetFirstMondayofMonth(DateTime now)
{
    var monday = new DateTime(now.Year, now.Month, 1);
    monday = Next(monday, DayOfWeek.Monday);
    return monday;
}

You can shorten this to

public static DateTime GetFirstMondayofMonth(DateTime now)
{
    return Next(new DateTime(now.Year, now.Month, 1), DayOfWeek.Monday);
}

Perhaps it's worth considering making Next an extension method?

---

const int SendTime = 8;

I don't know what that means from just looking at it. I'd suggest a comment // 8 am to avoid confusion.

---

Assert.AreEqual(false, checker);

Use the appropriate methods: Assert.IsFalse(checker) -- this way you don't throw away information that might help you diagnose a failed test.

---

Assert.AreEqual(testResult, firstMonday);

The first argument is the expected value, the second is the result.

---

The DateTime API is an external system, just like a database, the filesystem or a network operation. In unit testing these are typically stubbed out so you work with data entirely under your control. In your case this isn't a problem because the only DateTime used is the one you pass in. However should this not be the case, you should inject a proper datetime service which you can stub from your tests (if you're following along: this will be part of post 3 of my blogseries).

---

In (unit) testing there is the Arrange, Act, Assert approach to each test. I prefer to explicitly separate these parts with a newline and even a comment.

---

The naming of the tests.. I wouldn't put the [ExpectedResult] part as a DateTime since it just forces you to compare it to the [Scenario]. I think plain English "ShouldReturnSameDate" will be more effective.

Answer 2

return now > timeToSendOnFirstMonday && now < (timeToSendOnFirstMonday.AddHours(1));

The > looks strange to me, because I would expect the method to return true when the time is exactly 8am -- that is, I think it should be >=. As it stands, it returns true one millisecond after 8am, but false at 8am exactly.

Whether or not it should return true when it's exactly 8am is of course up to you and your requirements, but if you want to return false for 8am, I think that line deserves a comment.

(If you do decide an event should be sent at 8am, be sure to update CheckSendNewEvent_201603070800 as it says that no event should be sent at 8am on the first Monday of March, 2016.)

Besides Jeroen's excellent points, I think the code can be simplified. Assuming we decide to include 8am, I would write

public static bool CheckSendNewEvent(DateTime dateTime)
{
    return dateTime.DayOfWeek == DayOfWeek.Monday &&
           dateTime.AddDays(-7).Month != dateTime.Month &&
           dateTime.Hour == 8;
}

DateTime is a strange beast, though. What if someone decides to give the time in UTC?

// Just after 8am on the first Monday of March, 2016, in Melbourne, Australia.
var dt = new DateTime(2016, 3, 6, 21, 0, 1, DateTimeKind.Utc);
Console.WriteLine(dt);
Console.WriteLine(dt.ToLocalTime());
Console.WriteLine(CheckSendNewEvent(dt));

On my machine, this prints

6/03/2016 9:00:01 PM
7/03/2016 8:00:01 AM
False

I realise someone passing in a UTC DateTime is unlikely, but it is something to keep in mind.

One option is to consider using NodaTime, which fixes some of the issues with DateTime.

public static bool CheckSendNewEvent(IClock clock)
{
    var dateTime = clock.Now.InZone(DateTimeZoneProviders.Bcl.GetSystemDefault());
    return dateTime.IsoDayOfWeek == IsoDayOfWeek.Monday &&
           dateTime.Date.PlusWeeks(-1).Month != dateTime.Month &&
           dateTime.Hour == 8;
}