Unit Testing Smells: What Are Your Tests Telling You?

Erik DietrichFri, 09/29/2017 - 13:08

The title of this post makes reference to "test smells," which might seem a curious turn of phrase.  It's a bit of a riff on the idea of code smells.  So, to understand test smells, let's first understand code smells.

Jeff Atwood once wrote a Coding Horror post on the subject.  He calls them "warning signs in your own code" and cites Wikipedia's definition, which describes them as symptoms.  What makes Jeff's take so interesting is his suggestion that you should develop a "code nose."  By this, he means that you should develop a subconscious sense of code that doesn't seem quite right, somehow.

When reasoning about code, bugs make things easy.  Either the code behaves as expected or not.  Smells, on the other hand, create ambiguity.  There's nothing technically wrong with the code, but experienced developers recognize a higher probability for eventual mischief with certain structures.

Personally, I drive the notion home by evoking a ubiquitous saying: where there's smoke, there's fire.  When you encounter a code smell, you smell smoke.  And while smoke doesn't necessarily mean fire, it does so frequently enough that conventional wisdom assumes you'll find it.  So a code smell usually (but not necessarily) indicates underlying problems.  The correlation is strong enough that you should further investigate.

Unit Testing Smells

The term "code smell" has become widely adopted enough in the industry to command its own taxonomy.  This even includes categories of smell, such as "bloaters" and "couplers."

But I submit that we can borrow it and apply it more surgically to particular categories of code.  You thus might have ORM smells, MVC smells and, yes, unit testing smells.  After all, unit test code is simply a specific type of code that you write.  Others might argue differently, but I'd also say that it's every bit as important as production code.

Beyond that, though, unit test code can give you some unique insight into the nature of your production code.  I say this because unit test code uses your production code, by definition.  In doing this, it offers you a window into what it's like to set up, consume, and tear down the constructs you create in your code.  You can see what this is like.

Building on that, I'll talk about some unit testing smells.  What are some things that happen in your unit tests that you should take as probable indicators of deeper issues?  And what are those issues?

Tests Are Difficult to Write

My first unit testing smell might surprise you.  As a trainer and consultant, I spend time with different organizations, helping them get up to speed with unit testing.  And while the practice has both nuance and a learning curve, it's not that hard to get right.

At least, it's not that hard to get right when you have testable code.  And --- spoiler alert --- most organizations struggle to have testable code.  Many things, like global variables, can make your code extremely hard to test.

Unit testing neophytes often assume that they're doing the testing wrong.  But more often than not, they're trying to test problematic, monolithic code.  So if you have an exorbitantly difficult time writing tests, even as a newbie, you should take a critical look at the code you're trying to test.  It usually indicates excessive coupling.

You Do Weird Things to Get At The Code Under Test

When I look at a client's automated testing attempts, I often see two flavors of this next smell.  In languages that support it, they use reflection schemes to invoke and test private methods.  In C#, they make these methods internal and use the "Internals Visible To" attribute to expose the test methods to the unit test assembly.  Both amount to jumping through hoops to dig at private details of the code, paralleling the inappropriate intimacy code smell.

First of all, this is complicated and results in brittle unit tests.  But perhaps more importantly, it indicates that you're testing implementation details instead of behavior.  You use class and visibility to encapsulate and hide implementation details so that you can vary them as needed.  When you crack those classes open and test their details, you lose this flexibility.

When you find yourself testing this way, it means that you're probably creating iceberg classes.  In other words, you're encapsulating too much all in one place.  You may need to take some of those private methods and make them public methods on another class that you pull out of there.

Excessive Mocking

According to most unit testing/TDD experts, isolation is one of the defining characteristics of unit tests (as opposed to integration tests or end to end tests).  Unit tests have much narrower focus and concern themselves only with the behavior of the class/method under test.

To achieve this isolation, test writers use a concept called mocking.  In short, you do this by having classes use dependency injection and depend on interfaces.  In production, you use one kind of interface, but at test time, you create a "mock" version of the dependency and use that.  You typically hear this in the context of "let's mock out the file system and the database."  Instead of actually writing things to a database, you use an impostor dependency that you control.

Some mocking is an important part of any test suite because it makes testing your code much easier.  But excessive mocking usually indicates design problems.  You may have too many dependencies in the class under test, or you may be testing at the wrong level of abstraction.

Context Logic in Production Code

If you've programmed for a while, you've probably seen something like this.

public static void SaveToDatabase(Customer customerToWrite)

{

    if (AreWeTesting)

        WriteWithMockDatabase(customerToWrite);

    else

        Write(customerToWrite);

}

I call this context logic because your production code becomes aware of the context in which it is used.  This makes your code really brittle, and, frankly, it adds clutter.  When you're hunting down some critical defect under the gun, you don't want to reason about what happens at test time.

As far as smells go, this indicates a rigid design.  Usually, it indicates a design that could benefit from the aforementioned dependency injection.  Rather than set a state flag, you could just inject a mock and leave the code alone.

Slow Running Tests

You can look for smells that go beyond the structure of your tests as well.  For instance, examine them at runtime for slowness.  If you have unit tests that take tens of milliseconds or more to run, you have a smell.

You want your unit tests to run really fast.  If they run quickly, people will run them all the time and they will serve as the safety net they should.  But if they slow your development work to a crawl, the team will stop running them.  They'll rot.

You have a unit testing smell with a slow running test.  It may mean that you're doing something that's not really a unit test, such as hitting a database or writing a file.  Or it might be that you've found a really inefficient piece of production code that you need to address.

Intermittent Test Failures

I'll round out the list with one of the most maddening scenarios for people with test suites.  I'm talking about the situation where you have some pesky unit test that fails once in a blue moon.  Martin Fowler doesn't mince words when talking about this unit testing smell.  He calls them a "virulent infection that can completely ruin your entire test suite."

These tests have a specific kind of smell: non-determinism.  You run them with the same inputs over and over again, and usually, but not always, they produce the same output.  What does that actually tell you?  Nothing definitive.

It could be that you've written a bad test.  Maybe you've generated a random number and failed the test if you get 12.  And that's the better scenario.  Because a worse scenario is that you have code whose behavior you don't understand.

When you find an intermittently failing unit test, don't just jiggle the handle on it like a quirky toilet until it does what you want.  Figure out exactly when and why it fails and make things deterministic.  This will take you a while and it will frustrate you, but it will be worth it.

Listen to Your Test Suite

Unit testing smells tell you a story.  Sometimes, they tell you that you're not writing the best tests.  But in my experience, they tell you stories about your production code far more often than you'd think at first blush.

Don't ignore them and don't put off listening to them.  See what they say about your production code and then go fix it.  Just like any other code smell, the longer you ignore them, the more technical debt you accumulate.