CODEWITHSUNDEEP
c#unit-testingtdd
Sam Shiles
Sam Shiles

Reputation: 11249

Writing more unit tests after TDD refactor

This is something we've discussed over and over and people's opinions seem to differ quite a lot on this.

The basic question, when doing TDD, should you add additional unit tests after the refactor step of the cycle. I'm not talking about your next test to start your next cycle, but rather tests to cover any changes which have come about due to the refactoring.

This might best be explained with a real life example. After the green of a TDD cycle we have the following code:

    public bool ShouldVerifyDomain
    {
        get
        {
            return this.Orders.SelectMany(x => x.Items).Any(x => x.Product.RequiresDomainVerification);
        }
    }

Now, I look at this and think, hmmmm, that linq statement could be a bit tidier, a bit easier to read, not violate Demeter quite so much, let's refactor that. So I create the following on Order:

 public bool HasItemsThatRequireDomainVerification
 {
     get
     {
          return this.Items.Any(x => x.Product.RequiresCascadeDomainVerification);
     }
 }

And modified ShouldVerifyDomain to:

  public bool ShouldVerifyDomain
  {
      get
      {
           return this.Orders.Any(x => x.HasItemsThatRequireDomainVerification);
      }
  }

OK, that's looking a little better, I'm happier with that. Lets move on to the next test on my list...but...wait, we're now testing the property HasItemsThatRequireDomainVerification via a property on another object....is that a true unit test or should I add a test(s) to directly test HasItemsThatRequireDomainVerification.

The way I feel? I don't see that it would add much value. I think it would add to the maintenance burden of the suite, take time and not really give us any more confidence when it came to making future changes.

What might it give us? "Documentation" of the public interface of Order.

Thoughts?

Upvotes: 3

Views: 331

Answers (3)

Brad Rem
Brad Rem

Reputation: 6026

You haven't changed behaviour, only syntax. The code still works the same way, it's just written differently. I think that as long as it still works the same way, your unit tests remain reliable.

I think that we end up going down a rabbit hole if refactoring would demand that we start testing the new code we refactored. When would it end?

Upvotes: 2

David
David

Reputation: 218798

Did your refactor step add or change functionality? If so, then it was an invalid refactor step. You should back out those changes and add a test for the new functionality first.

However, in your example, I don't think that's necessarily the case. All you did was something very similar to extracting a method. You consolidated existing logic into another location and called that from the existing location. The existing tests still test this.

After the refactor, if you're concerned about needing to add more tests then the first place you should look is your test coverage. If you're still at 100% (or as close to it as you were before the refactor) then you're probably still good. If, on the other hand, the refactor added code paths which aren't covered by the tests, then you have some choices:

  • Does your code need those code paths? If so, the tests were insufficient. You should back out the refactor, add failing tests for the new code paths, and then add the new code paths.
  • If your code doesn't need those code paths, then why are they there? Get rid of them.

What you're asking is very similar to an age-old question about test coverage that's been asked in many forms:

  • Should I test private members?
  • Should I write an individual test for every method?
  • Should every member of every object have a test?

As with everything, the answer is always "it depends." All of the code should be tested, but every line of code doesn't need its own test. For example, suppose I have a property on a class:

public class Customer
{
    public string Name { get; set; }
}

Do I need to write a test which instantiates a Customer, writes a Name value to it, and then asserts that it can read back that same value? Clearly not. If that fails then there's something deeply wrong. However, should this line of code be covered by tests? Absolutely. Somewhere there should be a test which makes use of a Customer's Name. If there isn't, if no test in the system uses this property, then either the tests are incomplete or this property isn't actually needed by the system and should be removed.

To put it another way, you're not really testing the code when you write tests. You're testing the functionality of the system. The code which achieves that functionality is separate from and parallel to the tests. The two don't need to know a whole lot of details about each other. If the externally visible functionality of something changes, the tests should change to match (and verify) it. If the externally visible functionality hasn't changed, the tests shouldn't have to change either. They should still validate that same functionality.

Upvotes: 8

Skyp
Skyp

Reputation: 1197

When you are doing TDD, your tests should be at the level of functionality "functional tests", so as long as the functionality do not change, you should not change your tests.

Changing the implementation or refactoring are considered details in TDD, as long as the input and outputs of functionality are the same.

TDD should not lead you to have a 100% coverage.

If on the other hand you are using your unit tests as code explanation or want to have 100% coverage (we are speaking here of unit tests, because they should target only one piece of code), then those unit tests should change each time the implementation is adapted to cover all the cases, but this is not the goal of TDD.

Upvotes: 4

Related Questions