Test Code Smells

This section gives an overview of bad code smells that are specific for test code.

Smell 1: Mystery Guest

When a test uses external resources, such as a file containing test data, the test is no longer self-contained. Consequently, there is not enough information to understand the tested functionality, making it hard to use that test as documentation.

Moreover, using external resources introduces hidden dependencies: If some force changes or deletes such a resource, tests start failing. Chances for this increase when more tests use the same resource. The use of external resources can be eliminated using the refactoring Inline Resource (1). If external resources are needed, you can apply Setup External Resource (2) to remove hidden dependencies.

Smell 2: Resource Optimism

Test code that makes optimistic assumptions about the existence (or absence) and state of external resources (such as particular directories or database tables) can cause nondeterministic behavior in test outcomes. The situation in which tests run fine at one time and fail miserably another time is not a situation you want to find yourself in. Use Setup External Resource (2) to allocate and/or initialize all resources that are used.

Smell 3: Test Run War

Such wars arise when the tests run fine as long as you are the only one testing but fail when more programmers run them. This is most likely caused by resource interference: Some tests in your suite allocate resources, such as temporary files, that are also used by others. Apply Make Resource Unique (3) to overcome interference.

Smell 4: General Fixture

In the JUnit framework, a programmer can write a setUp method that will be executed before each test method to create a fixture for the tests to run in.

Things start to smell when the setUp fixture is too general and different tests access only part of the fixture. Such setUp methods are harder to read and understand. Moreover, they may make tests run more slowly (because they do unnecessary work). The danger of having tests that take too much time to complete is that testing starts interfering with the rest of the programming process, and programmers eventually may not run the tests at all.

The solution is to use setUp only for that part of the fixture that is shared by all tests using Fowler's Extract Method (F:110) and put the rest of the fixture in the method that uses it using Inline Method (F:117). If, for example, two different groups of tests require different fixtures, consider setting these up in separate methods that are explicitly invoked for each test, or spin off two separate test classes using Extract Class (F:149).

Smell 5: Eager Test

When a test method checks several methods of the object to be tested, it is hard to read and understand, and therefore more difficult to use as documentation. Moreover, it makes tests more dependent on each other and harder to maintain.

The solution is simple: Separate the test code into test methods that test only one method using Fowler's Extract Method (F:110), using a meaningful name highlighting the purpose of the test. Note that splitting into smaller methods can slow down the tests because of increased setup/teardown overhead.

Smell 6: Lazy Test

This occurs when several test methods check the same method using the same fixture (but, for example, check the values of different instance variables). Such tests often have meaning only when we consider them together, so they are easier to use when joined using Inline Method (F:117).

Smell 7: Assertion Roulette

"Guess what's wrong?" This smell comes from having a number of assertions in a test method that have no explanation. If one of the assertions fails, you do not know which one it is. Use Add Assertion Explanation (5) to remove this smell.

Smell 8: Indirect Testing

A test class is supposed to test its counterpart in the production code. It starts to smell when a test class contains methods that actually perform tests on other objects (for example, because there are references to them in the class that is to be tested). Such indirection can be moved to the appropriate test class by applying Extract Method (F:110) followed by Move Method (F:142) on that part of the test. The fact that this smell arises also indicates that there might be problems with data hiding in the production code.

Note that opinions differ on indirect testing. Some people do not consider it a smell but a way to guard tests against changes in the "lower" classes. We feel that there are more losses than gains to this approach: It is much harder to test anything that can break in an object from a higher level. Moreover, understanding and debugging indirect tests is much harder.

Smell 9: For Testers Only

When a production class contains methods that are used only by test methods, these methods either are not needed and can be removed or are needed only to set up a fixture for testing. Depending on the functionality of those methods, you may not want them in production code where others can use them. If this is the case, apply Extract Subclass (F:330) to move these methods from the class to a (new) subclass in the test code, and use that subclass to perform the tests on. You will often find that these methods have names or comments stressing that they should be used only for testing.

Fear of this smell may lead to another undesirable situation: a class without a corresponding test class. This happens when a developer does not know how to test the class without adding methods that are specifically needed for the test and does not want to pollute the production class with test code. Creating a separate subclass helps to deal with this problem.

Smell 10: Sensitive Equality

It is fast and easy to write equality checks using the toString method. A typical way is to compute an actual result and map it to a string, which is then compared to a string literal representing the expected value. Such tests, however, may depend on many irrelevant details, such as commas, quotes, and spaces. Whenever the toString method for an object is changed, tests start failing. The solution is to replace toString equality checks by real equality checks using Introduce Equality Method (6).

Smell 11: Test Code Duplication

Test code may contain undesirable duplication. In particular, the parts that set up test fixtures are susceptible to this problem. Solutions are similar to those for normal code duplication, as described in [Fowler1999, p. 76]. The most common case for test code is duplication of code in the same test class. This can be removed using Extract Method (F:110). For duplication across test classes, it may be helpful to mirror the class hierarchy of the production code into the test class hierarchy. A word of caution, however: Moving duplicated code from two separate classes to a common class can introduce (unwanted) dependencies between tests.

A special case of code duplication is test implication: Tests A and B cover the same production code, and A fails if and only if B fails. A typical example occurs when the production code gets refactored: Before this refactoring, A and B covered different code, but afterward they deal with the same code, and it is not necessary anymore to maintain both tests.