3

I've just written a unit test for this function, which loops through a collection of dates and sets properties equal to true or false depending on whether they're before or after a given comparison date:

public void CheckHistory(int months)
{
    var endDate = DateTime.Today.AddMonths(months);
    Dictionary<Order, bool> orders = new Dictionary<Order, bool>();

    foreach (var kvp in this.Orders)
    {
        if (kvp.Key.Date >= endDate)
        {
            orders.Add(kvp.Key, true);
        }
        else
        {
            orders.Add(kvp.Key, false);
        }
    }
    this.OrderHistories = orders;
}

So here's the test I wrote:

public void Assert_CheckHistory_SelectsCorrectDates()
{
    MyViewModel vm = GetVmWithMockRepository();
    vm.OrderHistories = new Dictionary<OrderHistory, bool>();

    OrderHistory ohOld = new OrderHistory();
    ohOld.MailingDate = DateTime.Today.AddMonths(-12);
    vm.OrderHistories.Add(ohOld, false);

    OrderHistory ohNew = new OrderHistory();
    ohNew.MailingDate = DateTime.Today.AddMonths(-3);
    vm.OrderHistories.Add(ohNew, false);

    vm.CheckOrderHist(-6);

    int selectedOrders = vm.OrderHistories.Where(o => o.Value == true).Count();
    Assert.AreEqual(1, selectedOrders, "Unexpected number of selected Order Histories");
}

Nothing wrong there. Test passes and all is good with the world.

However, I'm haunted by a nagging feeling that I'm not actually testing anything useful, and am just writing tests for the sake out it.

I get this a lot. A creeping paranoia that the tests I'm writing are incomplete in the sense that while they cover the lines of code in the target function, they don't really trap any likely problems and are therefore just a maintenance overhead.

Is that sample test worthwhile? Is even a badly-designed test worth worthwhile over no test at all? And most of all are there any principles to help programmers identify whether a test is useful or not, or to guide them in constructing useful tests in the future?

To be clear, I'm adding tests to an existing application. Going test-first in true TDD style isn't possible.

4 Answers 4

8

To quote the "Way of Testivus",

A bad test is better than no test

Making sure that your test tells you that the code works is the point of the test. When you are writing tests for code that has already been written, you are setting things up so that you can refactor it and be confident that you have not broken any current functionality. If the test fails for no apparent reason, then the test is bad and should be fixed. As long the test ensures the functionality of your application, the test has some value.

Writing these tests gives you confidence in your ability to refactor that code later on. Because if you have done it right, the test will still pass. You are giving yourself the confidence to change that code and not have to worry about breaking any old functionality.

Uncle Bob had a recent blog entry regarding this:

If you have a test suite that you trust so much that you are willing to deploy the system based solely on those tests passing; and if that test suite can be executed in seconds, or minutes, then you can quickly and easily clean the code without fear.

Your example test isn't all that good but it gives you more confidence that any changes you make will not affect the functionality. You want to check that for a given state the method will output a specific response. You trying to test what the code does rather than what the code is. For legacy code like this, you will end up with tests that are more "functional" rather than "unit".

Does having this test give you more confidence that any changes to the application did not result in the code being broken? If not, then your test is not useful and you should get rid of it or change it. Untested code will paralyze any useful changes that you can make later.

"We can't make that change because we don't know everywhere that it is being used!" Your tests make that argument moot. You know that the code all works because all the tests pass.

Tests also do not ensure bug-free code. Your code may still have bugs but when they are discovered and you add a new test duplicating it. You ensure that that bug NEVER happens again because then that test will fail. And as long as you are running your tests often, this bug will always be checked for.

1
  • 1
    That's a magnificent link. The advice you give isn't bad either :)
    – Bob Tway
    Commented May 7, 2014 at 14:25
5

So... Let me suggest a refactoring:

public void CheckHistory(int months)
{
    var endDate = DateTime.Today.AddMonths(months);
    Dictionary<Order, bool> orders = new Dictionary<Order, bool>();

    foreach (var kvp in this.Orders)
    {
        orders.Add(kvp.Key, kvp.Key.Date < endDate);
    }
    this.OrderHistories = orders;
}

Nice, right? Well, maybe nice, but not right. Because I (let's pretend inadvertently) flipped the direction of the inequality operator. Would your test have caught that? Actually (I think) no, because the number before & after is the same, and you're only testing that the count is 1. So you should probably think about enhancing it a little bit. But in terms of the value of having the unit test, of having that kind of test coverage: Yes, it's valuable, because it lets you refactor safely.

1
  • 3
    Thanks for this. I feel compelled to point out that my actual test contains several dates and would have caught your change - I just simplified it for the post :) Also that the user can change the truthy/falsy so I do need the if statement. But I'm just being defensive - you've used the sample I gave to make some great points.
    – Bob Tway
    Commented May 7, 2014 at 14:18
2

The value of a regression test is not in succeeding when things are well - you could get the same effect by painting a green rectangle on your monitor! Its value is in failing when things are wrong, and doing so as early as possible, before the cost of correcting the problem becomes too big.

The easiest way of ensuring that it does in fact fail when things are wrong is to write it first and verify that it fails. Pick any situation that occurs to you that could go wrong with date calculations, write a test that checks this, run it on a stub implementation and watch it fail. Then you can write the business code that handles this case and watch it succeed. This gives you two vital assurances:

  1. Your business logic is correct
  2. If, at some time in the future, someone changes the business code so that it handles this case wrongly, you will know immediately.

Coding test-first is not the only way of ensuring that your tests are appropriate and reliable - it's just the easiest and smoothest I can think of. Many people are initially turned off by the seemingly out-of-order of steps, but if you consider what a test is really supposed to do, I find it rather obvious.

2
  • Thanks for your answer. However in this instance I've inherited an entire application without any tests and am in the process of adding coverage. Whatever the benefits of test-first, that avenue is closed to me. I'm looking for support on how to structure tests when you have to write them after the fact.
    – Bob Tway
    Commented May 7, 2014 at 13:54
  • 1
    Alright, you have two problems then: (1) finding likely sources of incorrect behaviour (2) verifying that the code does not exhibit it. The second can be dealt with - it's sometimes even useful to deliberately introduce a flaw, verify that the test catches it, and then remove it again. I agree that the first is much harder, and not much pithy advice can be given. Depending on how entrenched the application is, it may be more useful to test that the application keeps behaving exactly as it does now, by capturing stimulus-response pairs and verifying that they do not change. Commented May 7, 2014 at 13:59
1

I see that you stumble upon a common issues when doing testing.

A teacher of mine was often joking what usually happens in the lab for software engineering - a student creates 3-4 test cases and if they don't break the application it was alright and they were ready for the next assignment.

A common misconception about testing is that it is supposed to cover test cases that do not break your software. It is actually the exact opposite - with testing you try to break your application and see what happens.

Another problem is the way test cases are defined and their number. For 5 cases that your application passes there are maybe 10000000 ones that break it. Of course since we have a limited time to spend on testing we don't need to and it is actually in most cases impossible to cover all test cases for a certain problem (especially the more complex ones).

That is why tools such as QuickCheck were created. With QuickCheck (available not only for Haskell, which it was originally created for, but also for many other languages) you define the logic that a certain function has to fulfill and then you generated as many test cases as you want to. This is how testing should be done.

From your code I will assume that you are using C#. There is no QuickCheck for C# (sadly) but you have the concept of invariants and contracts plus Pex, which enhances the whole unit testing experience a lot.

0

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.