Naming tests

I’m a sucker for clean, minimalistic code, that is easy to read, understand and maintain.

When it comes to “regular” code, most developers are fairly good about this, and we have a lot of opinionated content and examples, of code smells and patterns, but I find that when it comes to tests we tend to neglect this. Tests are often a hot mess with a plethora of problems, problems that discourage people to write tests because they are simply more of a headache than what they are worth.

• Badly named or badly organized tests
• Tests that don’t serve a purpose, or test the wrong thing
• Tests that are hard to understand
• Tests that are hard to maintain, and act as a barrier to refactoring instead of enabling it

At work, I have started a bi-weekly “gathering” where me and some like minded people get together and discuss interesting technical things. Lately, one of the topics we have discussed is writing good tests, and I thought I would share some of the things we have discussed, starting with some thoughts on naming.

Naming tests

For the purposes of this, I’ll use the example of the Banking kata, as it is very simple to understand.

public class BankAccount
{
    public int Balance { get; private set; }
    public BankAccount() { }
    public void Deposit(int amount) { }
    public void Withdraw(int amount) { }
}

A not uncommon way to approach writing unit tests for this class would be to write a test for each method, and then a test for each edge case. This would result in a test class that looks something like this:

public class BankAccountTests {
    [Fact]
    public void TestConstructor()
    {
        var sut = new BankAccount();
        sut.Balance.Should().Be(0);
    }

    [Fact]
    public void TestDeposit()
    {
        var sut = new BankAccount();
        sut.Deposit(100);
        sut.Balance.Should().Be(100);
        sut.Deposit(200);
        sut.Balance.Should().Be(300);
    }

    [Fact]
    public void TestWithdraw()
    {
        var sut = new BankAccount();
        sut.Deposit(100);
        sut.Withdraw(20);
        sut.Balance.Should().Be(80);
        Action action1 = () => sut.Withdraw(90);
        action1.Should().Throw<InvalidOperationException>();
        Action action2 = () => sut.Withdraw(-10);
        action2.Should().Throw<InvalidOperationException>();
    }
}

This strategy has a number of issues.

1. The name of the tests says very little about what it is we are testing, and what the expected result is. If we see this test fail, we have no idea what the problem is, and as someone reviewing this code, we would have to go into the test to see what it is testing.
2. We are testing multiple scenarios in each test, which makes it hard to understand both what is being tested, and which scenario is failing if the test fails.

From a readability perspective, I am also allergic to the use of sut (Subject Under Test), it makes it a lot harder to read the code than if we would have used a more descriptive name.

So, ideally, we want to…

• split the tests such that each test tests its own scenario
• have test names that reflect what we are testing and what the expected result is
• avoid testing things that we don’t need to test, such as .net internals, or even implementation details internal to the method TBH.

The Microsoft way and Gherkin

Microsoft’s unit testing best practices suggest that you should name your tests in the format UnitOfWork_StateUnderTest_ExpectedBehavior. An example of a test for the Withdraw method would then be

[Fact]
public void Withdraw_WhenAmountIsLessThanBalance_ShouldDecreaseBalanceByAmount()
{
    // arrange
    var account = new BankAccount();
    account.Deposit(100);

    // act
    account.Withdraw(20);

    // assert
    account.Balance.Should().Be(80);
}

A variation on this theme, is using Gherkin naming

[Fact]
public void GivenAnAccountWithABalance_WhenCallingWithdrawWithLessThanTheBalance_TheBalanceShouldBeDecreasedByTheAmount()
{
    // arrange
    var account = new BankAccount();
    account.Deposit(100);

    // act
    account.Withdraw(20);

    // assert
    account.Balance.Should().Be(80);
}

On the positive side:

• we are now testing one scenario per test
• it is pretty clear what we are testing, and what the expected result is

I have also sneaked in the arrange/act/assert pattern to make it even clearer what we are testing. I find that this pattern is helpful both when reading tests but also to help guide when writing tests to make sure that we are only testing one concept per test.

On the negative side:

• OMG! The test names are so long that they are unreadable. I have to scroll horizontally to read the entire name. This is not a good thing.
• The mix of using underscores and camel casing is very jarring, and makes it very hard to read.
• Since we specifically name the function Withdraw in the test name, if we were to rename the Withdraw method, we might be left with code drift as these test names will not be updated automatically.
• These schemes lock you in to a specific naming convention. When using these conventions I often find developers struggling with what to put where and where to place the underscores.
• The given, when, then scheme also has a lot of redundancy that makes the names even longer.

A small detour about words that we should avoid

There are plenty of words that often make it into tests, that I think we should avoid. The use of the word should above made me think of this, but there are plenty of others.

• Correct as in ShouldCorrectlyCalculateBalance or ShouldCorrectlyHandleNegativeAmounts. What is correct? What is the expected result? What is the scenario we are testing? The same obviously goes for Valid, Invalid, Good, Bad, etc. We need to be specific about what correct means in this context.
• Can as in CanCalculateBalance or CanHandleNegativeAmounts. This is another fluff word that only makes the test name longer and vaguer. Be direct and specific, ThrowsAnArgumentExceptionIfANegativeAmountIsWithdrawn
• Should as in ShouldThrowAnArgumentExceptionIfANegativeAmountIsWithdrawn. We are not testing wishes or desires, we are testing absolutes, the test either passes or fails. Skip the should, and just say ThrowsAnArgumentExceptionIfANegativeAmountIsWithdrawn
• Test as in ThrowsAnArgumentExceptionIfANegativeAmountIsWithdrawnTest. Unless this is required by the test framework, there is no need to add the word Test to our tests. I sometimes add the word Tests to the test class, ex. BankAccountTests but this is mainly to avoid naming conflicts with the class under test, and to make it clear that this is a test class.

A better way

I vividly recall listening to a presentation by Kevlin Henney on naming, that completely made me change my thinking about test names. A lot of my ramblings so far are honestly a respectful rip off of his ideas from the excellent talk Test smells and fragrances along with some lessons I learned myself, the hard way. Everything he says about naming in general resonates immensely with me.

He suggested that we are allowed to use snake_casing in our test names, even in C# ?!?!?!

Blasphemy, are we really allowed, what is this sorcery you speak of.

He said something akin to “if you are writing legal contracts or text messages you may be writing them both in English, but you would use completely different forms, to better suit your needs”. The same goes for code, we should use the tools at our disposal to make our code as readable as possible.

And… if we are to be honest, snake_casing_is_a_lot_more_readable ThanCamelCasingOrPascalCasingWhenTheSentencesGetLonger.

We can also group our tests (into nested classes if need be, nested classes in C#, really?!?!). Grouping our tests not only allows us to shorten our test names. As an added bonus, it also allows us to group functionality which can be helpful as we can use the same setup for a group of tests.

Applying this to our example, we could end up with something like this:

public class BankAccountTests
{
    public class A_new_account
    {
        [Fact]
        public void has_a_balance_of_zero() {}
    }

    public class Depositing_to_an_account
    {
        [Fact]
        public void adds_the_deposit_to_the_account_balance() {}
        [Fact]
        public void throws_an_exception_if_the_amount_is_negative() {}
    }

    public class Withdrawing_from_an_account
    {
        [Fact]
        public void subtracts_the_withdrawal_from_the_account_balance() {}
        [Fact]
        public void throws_an_exception_if_the_amount_is_negative() {}
        [Fact]
        public void throws_an_exception_if_the_amount_is_greater_than_the_balance() {}
    }
}

When we collapse the functions, both in the code and also in the test explorer, we end up with a spec of the functionality, and the intentions behind the implementation.

Conclusion

I know, as with many testing examples, this was a very simple example, but I have found that it extrapolates well to more complex scenarios.

When I write code, usually, the first thing I do, is pair up with a colleague, look through the story, and write these test skeletons of the functionality we are about to implement. That way we can iron out our understanding of the functionality before we even write one line of code. And as a bonus, when someone comes to review the story, they can just browse the tests first to ensure that we have covered all the scenarios.

I have a lot of ideas around readability and maintainability of the actual test code as well, but I’ll save that for another post. I would be very interested in your comments though, so whether you agree or disagree or have other tips or experiences, I would love to hear from you on twitter (X) @tessferrandez.