How To Be Awesome: Enforcing ToDos with Specification Tests

Earlier today, I blogged about a Kobayashi Maru situation I found myself in with the code base which must not be named. I was stuck making the choice between completely destabilizing our code base at the end of a sprint while trying to track down all the possible side effects of fixing the root cause of my problem or putting yet another little hack into our code.

Over lunch, I was describing the situation to Terry, one of our other developers. We came to the conclusion that we needed to go with the hack method to meet our obligations to the Scrum Master, but we should get the root cause of the issue in the backlog so we could resolve it later.

But this means that our hack would be in place in a fairly unobvious place and in 6 months when we get the item into the sprint backlog, we might fix the issue but forget to remove the hack.

Typically this is solved by adding a "TODO:" in the comments right by your hack to remind you to remove it in the future. Our code base is littered with them.

Notice the one from 2005. :-( And these are only the comments that happen to have "TODO:" in them. Obviously, we need a better way to remind ourselves to go back and take care of housekeeping tasks when we change the system.

Then Terry said, "Wouldn't it be awesome if we had a test that would fail if this condition changes?" and the light went off. We talked about it some more and as soon as I got back to the office, I created a new utility project called TestTools.HackToDos. I then wrote two specification style tests.

The first test demonstrates the low level call that causes the issue:

[TestClass]
public class When_Cleaning_A_DataTable_With_Null_Dates : GridViewHacksContext
{
  protected override void BecauseOf()
  {
    Tools.Instance().CleanData(dataWithNulls);
  }

  [TestMethod]
  public void The_Nulls_Will_Be_Converted_To_Invalid_Dates()
  {
    dataWithNulls.Rows[0].Field(0).ShouldEqual(DateTime.MinValue);
  }
}

The second test demonstrates what I would like to happen:

[TestClass]
public class When_Binding_GridView_To_DataView_With_Invalid_Dates : GridViewHacksContext
{
  protected override void BecauseOf()
  {
    Tools.Instance().CleanData(dataWithNulls);
    grid.BindData(dataWithNulls.DefaultView, layoutManager.Columns);
  }

  [TestMethod]
  public void The_Invaid_Date_Should_Be_Hacked_Out()
  {
    Assert.IsTrue(String.IsNullOrEmpty(grid.Rows[0].Cells[0].Text));
  }
}

Running these tests resulted in the first test passing and the second failing. I then introduced my "hack fix" as a part of my extension methods to GridView:

private static void hackOutBadDates(GridView grid)
{
  foreach (var row in grid.GridDataRows())
  {
    foreach (TableCell cell in row.Cells)
    {
      if (cell.Text.Contains(@"01/01/0001"))
        cell.Text = null;
    }
  }
}

Now both tests pass. And my lovely jQuery awesome grid now displays null dates correctly.

Now I can safely check this code in and feel like I performed my boy scout duty today without feeling dirty. In 6 months when someone on the team gets the sprint item to fix our goofy handling of null values in data tables, my first test is either going to fail or even fail to compile. That dev will go look at the test and find himself staring at this comment block:

/// TODO: this spec demonstrates a low level flaw in our dataaccess layer where
/// DateTime columns in a DataTable are set to a value of DateTime.MinValue
/// after every read. When we attempt to bind a Grid View to the resulting DataTable
/// null dates are displayed as "01/01/0001" which DateTime.Parse throws as invalid.
/// I have added a hack to our grid view extension methods that fixes this problem.
/// If these tests fails, it might be because we fixed the low level issue and
/// the hack should be removed along with these tests.

And there you have ToDos that are enforced by unit tests.