Testing Anti-Patterns

TDD Can’t Save You From Yourself

Jim Kirkbride

--

I’m a big advocate of TDD. I believe it leads to cleaner, more maintainable code, and more extensible designs. That being said TDD is no silver bullet that will automatically make your life easier. I’ve discovered over the years that there are certain testing anti-patterns that can cause the cost of maintaining your test suites to increase dramatically, and these anti-patterns should be avoided whenever possible.

Lets take a look at some of these.

Fixtures

Fixtures are typically composed of sets of data that are used in one or more test suites. The usual flavor is data that is inserted into the database prior to the tests running, to be queried against in the tests. On its face the concept of fixtures seems to be an extension of DRY. The data is inserted once and never thought of again, or until we need to change it based on new requirements or test cases.

The problem here is two-fold: visibility and atomicity.

First, lets address the visibility issue. Examine the following code (Please excuse the poor formatted, Medium seems to be raw in this regard):

@Test
public void givenTwoWidgetsWhenTheWidgetsAreQueriedThenTheCountIsTwo() {
WidgetQuery widgetQuery = new WidgetQuery();
assertEquals(2, widgetQuery.count());
}

Great looking test, right? We’re using Given-When-Then naming, the test is only two lines long, and we have a single assert.

Well, yes and no. Where is the data coming from? Why should the test pass given what we have in that test? The insertion of the fixture data occurred before the test suite was ever run, and it presumably inserted the proper number of widgets.

The problem here is that the test is dependent on setup that occurs outside of the test suite entirely. In order to figure out what is happening the developer now needs to examine the fixture data, which is usually stored in another file entirely, creating the need to flip between the two files.

Next, atomicity.

Ideally tests should be atomic unto themselves, by which I mean two things. First, each test should be self-contained. There should be a clear setup, execution and assertion for each test (even if that setup occurs within the suite). A more transparent version of the same test above might be:

@Test
public void givenTwoWidgetsWhenTheWidgetsAreQueriedThenTheCountIsTwo() {
insertDefaultWidget();
insertDefaultWidget();
WidgetQuery widgetQuery = new WidgetQuery();
assertEquals(2, widgetQuery.count());
}

This test is much more clear about what is being tested. You can easily tell that we are inserting two widgets, which matches our expectations.

Secondly, the atomicity of tests should exist across the suite as well. This means that the data should not shared across tests, possibly to be polluted when a test runs, potentially causing the next test to fail. Test data should be inserted, executed against, and cleaned up transactionally to be prevent this behavior.

Everything Is A Mock

This is an anti-pattern in which most tests are unit tests, with little to no integration or acceptance tests. While the testing pyramid is certainly formed with unit tests at its base it should still be composed of higher level tests as it grows. Integration tests exist in order to insure that the mocked behaviors of the dependencies in the code under test behave the way we expect. This is especially important when the database is involved, or file I/O is occurring. Other issues integration tests can uncover include unexpected transactional behaviors, threading issues, etc.

Another manifestation of this anti-pattern is the mocking of data structures. Data structures should be real, and constructed as they are needed. There should be no need to mock a data structure (such as a struct in C++), as they have no behavior, and the test should expect the code to interact with it appropriately. If you find yourself saying “But it’s difficult to create my data structure!” it is a sure sign that your design needs to be re-examined.

Mocking should be restricted to external dependencies (compared to the current level of abstraction) and classes with behavior determined by external dependencies.

Everything Is An Acceptance Test

This anti-pattern occurs when your testing pyramid is inverted and most testing is done at the highest behavior level. It usually manifests itself when the underlying design is such that it is difficult to test anything in isolation. You have to spin everything up, either because the dependencies are not being injected (everything is being “newed-up” under the hood), or because there are many inter-dependencies (A relies on B, B relies on C, C relies on… etc, etc).

The problem here is that driving in new behavior via unit tests in lower layers of the system becomes difficult. Without unit tests at the appropriate layers it becomes cumbersome to test because you’re spinning up the entire system for each test.

Fewer unit tests also mean a higher chance of new defects creeping in simply because you aren’t providing enough coverage for all the possible scenarios. You also take a performance hit for these types of tests for the same reasons stated above; it simply takes more time and resources to spin up the test.

Extracted Assertions

This is similar to the “Fixture” anti-pattern, except its on the opposite end of the test, on the assertion side. In this scenario the expectations of a test have been extracted out to a method of their own.

Again, this comes from a desire to make the tests DRY. The problem becomes that the developer again needs to jump around in the code to check the assertion for a given test, rather than having the test be atomic and self-contained. Furthermore examination of the stack trace invariably leads you back to the same point of failure for many tests that use the same extracted method, rather than directly to the test you are actually interested in (Yes, you can still get to it, however it is buried in the trace). The emergence of this anti-pattern may also be an indication that there are duplicate test scenarios if you’re reusing the same set of assertions in multiple places.

Superfluous Setup Data

This is an example of having too much noise in your test setup data. This anti-pattern often occurs when testing queries or filters, in which you only expect to get a subset of the data back. The underlying idea is that, in order to be thorough, “extra” data should be present to show that the query or filter works as required.

Examine the following code:

@Test
public void givenMultipleWidgetsExistWhenQueriedByNameThenOnlyWidgetAFound() {
insertDefaultWidget("a");
insertDefaultWidget("b");
insertDefaultWidget("c");

WidgetQuery widgetQuery = new WidgetQuery();
List<Widget> results = widgetQuery.findByName("a");

assertEquals(1, results.size());
assertEquals("a", results[0].getName());
}

Given the expected behavior of the test only two pieces of data are needed; a widget named “a” and a widget not named “a”. The addition of a third widget is entirely superfluous to the test. This is obviously a contrived example, but you can imagine a more complex test when more complex data is required.

The bare minimum of data should be created for a test, and no more.

These are just a few examples of some common test anti-patterns, and they are the kind of techniques that can cause your codebase to slowly rot from the inside out. Most of them stem from either a misapplication of certain principles, such as DRY (which is generally great in production code, but needs to be approached more cautiously in testing), or a lack of experience or knowledge of testing in general.

As I said, TDD goes a long way towards cleaner, more maintainable code, but just as we need to be vigilant about what our production code looks like so too much we be vigilant about what our test code looks like. Clean, maintainable test code leads to clean, maintainable product code.

Happy testing!

Did you enjoy this article? If so please recommend it, and check out my other articles. If you have any thoughts about this article I’d love to hear from you. You can find me on Twitter as @jameskbride. Thanks for reading!

--

--