When should you remove or change a unit test?

skinapi

As a rule, a passing test should never be removed. That's because passing tests serve as the regression tests for our maintenance work.
They are there to make sure that when we change code we don t break anything else that s already working. Anywhere. That's also why you don't want to change a passing test unless the change is just to make it more readable.

But sometimes we might get some failing tests even though the change was absolutely reasonable. That usually means we ve encountered conflicting requirements. How does that happen? Here s how it might sound:

A year ago company X needed a feature of their calculator to be able to parse any amount of numbers, as long as they are positive. A number of tests were written to satisfy this requirement. Here s one:

[ExpectedException(typeof(Exception), Negatives not allowed )]

Void Sum_Negative1stNumberThrowsExcpeiton()

{

     Sum( -1,1,2 );

}

Today, the VP decided that a new feature is required to have negative numbers allowed in the calculator. A developer not knowing about the previous requirement builds a test case such as this and makes it pass:

Void Sum_Negative1stNumberCalculatesCorrectly()

{

     Int sumResult = Sum( -1,1,2 );

     Assert.AreEqual(2,sumResult);

}

Alas, the earlier test fails. It broke due to a requirement change. Now s the time for that developer to realize that an older requirement has changed. That test is indeed no longer valid and served no real purpose. It was written for a requirement that is no longer valid.

There are two possible routed to go here:

-         Delete the failing test after verifying that it is longer valid

-         Change the old test

o       Either to test the new requirement

o       Or to test the older requirement under new settings

The highlighted line above indicates that this is an important case. Sometimes a test is still valid even if it uses non-valid ways to accomplish its task. A good example would be to have a Person class with a constructor that takes one parameter.

A test that creates this class to test some functionality of it is using this constructor.

Years later another requirement comes along and another parameter is added to the constructor of the class.

Suddenly the old test breaks, even though it has nothing to do with the constructor other than it needs to create the class in order to test its functionality.

Removing the test then becomes a bad thing because you lose testing of some important functionality. In that case you might  want to change the creation of the class in your old test so that you can keep using it for the purpose it was built for.

Assuming you have 200 failing tests just because they use that old constructor you might be in for a problem maintaining your tests. That s why you should always remove duplication in your tests just as you would in production code.