Test Smells

Before you begin: Functions, collections, and assertions.

Test smells are clues that tests may be unreliable, confusing, or unnecessarily expensive to maintain. Examples include unexplained setup, dependence on test order, waiting for arbitrary time intervals, and assertions that break whenever implementation details change. A smell is a reason to investigate, not proof that a test is worthless. 1

Original version: two passing tests that fail together

These small test functions use ordinary assertions rather than a particular test framework. One adds an item. Another checks that a new cart starts empty. Both accidentally use the same list.

public static class Tests
{
    private static readonly List<string> cart = new List<string>();

    public static void AddItem()
    {
        cart.Add("Potion");
        if (!cart.Contains("Potion")) throw new Exception("Item missing.");
    }

    public static void NewCartIsEmpty()
    {
        if (cart.Count != 0) throw new Exception("Cart was not empty.");
    }
}

AddItem leaves a potion in the static list. Running NewCartIsEmpty afterward fails, even though the cart implementation has not changed. The second test never constructed a new cart at all.

cart = []


def test_add_item():
    cart.append("Potion")
    assert "Potion" in cart


def test_new_cart_is_empty():
    assert cart == []

test_add_item leaves a potion in the module-level list. Running the empty-cart test afterward fails. Running that test first succeeds. Test order has become an undocumented input.

Revised version: each test owns its starting state

public static class Tests
{
    public static void AddItem()
    {
        var cart = new List<string>();
        cart.Add("Potion");
        if (!cart.SequenceEqual(new[] { "Potion" }))
        {
            throw new Exception("Unexpected cart contents.");
        }
    }

    public static void NewCartIsEmpty()
    {
        var cart = new List<string>();
        if (cart.Count != 0) throw new Exception("Cart was not empty.");
    }
}

Each method creates its own list. The item test checks the whole expected contents, not merely whether one item happens to be present. These two tests can run in either order without changing one another’s state.

def test_add_item():
    cart = []
    cart.append("Potion")
    assert cart == ["Potion"]


def test_new_cart_is_empty():
    cart = []
    assert cart == []

Each function constructs its own list. The first checks the exact expected contents. The second really does examine a newly constructed cart. They no longer depend on execution order.

Make a failure explain something

Independent setup is one part of useful testing. A failing test should also identify the behavior that mattered. Prefer a name such as “rejects a second reward claim” to “test method 3,” and make expected values visible rather than hiding them in a large fixture.

Avoid tests that reproduce the implementation’s arithmetic to calculate their expected answer: the same mistake can appear on both sides of the assertion. Use concrete known outcomes and boundary cases. Assert internal calls only when those interactions are themselves important—for example, that a payment is not submitted twice.

Shared expensive infrastructure can be appropriate in integration tests, but each test still needs controlled data and cleanup. For asynchronous work, wait for an observable condition with a bound rather than sleeping and hoping the operation has finished.

The example tests only lists; it does not establish coverage of a real cart. Its purpose is to make test-order dependence visible, then remove it without changing the behavior under test.

Sources

Microsoft — Unit testing best practices, on readable, isolated, repeatable tests and avoiding logic in tests.