Branching tests by Mark Seemann

Is it ever okay to branch and loop in a unit test?

When I coach development organisations about unit testing and test-driven development, there's often a sizeable group of developers who don't see the value of unit testing. Some of the arguments they typically use are worth considering.

A common complaint is that it's difficult to see the wisdom in writing code to prevent defects in code. That's not an unreasonable objection.

We have scant scientific knowledge about software engineering, but the little we know suggests that the number of defects is proportional to lines of code. The more lines of code, the more defects.

If that's true, adding more code - even when it's test code - seems like a bad idea.

Reasons to trust test code #

First, we should consider the possibility that the correlation between lines of code and defects doesn't mean that defects are evenly distributed. As Adam Tornhill argues in Your Code as a Crime Scene, defects tend to cluster in hotspots.

You can have a large proportion of your code base which is, for all intents and purpose, bug-free, and hotspots where defects keep spawning.

If this is true, adding test code isn't a problem if you can keep it bug-free.

That, however, sounds like a chicken-and-the-egg kind of problem. How can you know that test code is bug-free without tests?

I've previously answered that question. In short, you can trust a test for two reasons:

• You've seen it fail (haven't you?)
• It's simple

That's what this article is actually about.

What's in a name? #

I was working with an online restaurant reservation system (example code), and had written this test:

[Theory]
[InlineData("2023-11-24 19:00", "juliad@example.net", "Julia Domna", 5)]
[InlineData("2024-02-13 18:15", "x@example.com", "Xenia Ng", 9)]
public async Task PostValidReservationWhenDatabaseIsEmpty(
    string at,
    string email,
    string name,
    int quantity)
{
    var db = new FakeDatabase();
    var sut = new ReservationsController(db);
 
    var dto = new ReservationDto
    {
        At = at,
        Email = email,
        Name = name,
        Quantity = quantity
    };
    await sut.Post(dto);
 
    var expected = new Reservation(
        DateTime.Parse(dto.At, CultureInfo.InvariantCulture),
        dto.Email,
        dto.Name,
        dto.Quantity);
    Assert.Contains(expected, db);
}

This is a state-based test that verifies that a valid reservation makes it to the database. The test has a cyclomatic complexity of 1, and I've seen it fail, so all is good. (It may, in fact, contain a future maintenance problem, but that's a topic for another article.)

The code shown here is part of the sample code base that accompanies my book Code That Fits in Your Head.

What constitutes a valid reservation? At the very least, we should demand that At is a valid date and time, and that Quantity is a positive number. The restaurant would like to be able to email a confirmation to the user, so an email address is also required. Email addresses are notoriously difficult to validate, so we'll just require that the the string isn't null.

What about the Name? I thought about this a bit and decided that, according to Postel's law, the system should accept null names. The name is only a convenience; the system doesn't need it, it's just there so that when you arrive at the restaurant, you can say "I have a reservation for Julia" instead of giving an email address to the maître d'hôtel. But then, if you didn't supply a name when you made the reservation, you can always state your email address when you arrive. To summarise, the name is just a convenience, not a requirement.

This decision meant that I ought to write a test case with a null name.

That turned out to present a problem. I'd defined the Reservation class so that it didn't accept null arguments, and I think that's the appropriate design. Null is just evil and has no place in my domain models.

That's not a problem in itself. In this case, I think it's acceptable to convert a null name to the empty string.

Copy and paste #

Allow me to summarise. If you consider the above unit test, I needed a third test case with a null name. In that case, expected should be a Reservation value with the name "". Not null, but "".

As far as I can tell, you can't easily express that in PostValidReservationWhenDatabaseIsEmpty without increasing its cyclomatic complexity. Based on the above introduction, that seems like a no-no.

What's the alternative? Should I copy the test and adjust the single line of code that differs? If I did, it would look like this:

[Theory]
[InlineData("2023-08-23 16:55", "kite@example.edu", null, 2)]
public async Task PostValidReservationWithNullNameWhenDatabaseIsEmpty(
    string at,
    string email,
    string name,
    int quantity)
{
    var db = new FakeDatabase();
    var sut = new ReservationsController(db);
 
    var dto = new ReservationDto
    {
        At = at,
        Email = email,
        Name = name,
        Quantity = quantity
    };
    await sut.Post(dto);
 
    var expected = new Reservation(
        DateTime.Parse(dto.At, CultureInfo.InvariantCulture),
        dto.Email,
        "",
        dto.Quantity);
    Assert.Contains(expected, db);
}

Apart from the values in the [InlineData] attribute and the method name, the only difference from PostValidReservationWhenDatabaseIsEmpty is that expected has a hard-coded name of "".

This is not acceptable.

There's a common misconception that the DRY principle doesn't apply to unit tests. I don't see why this should be true. The DRY principle exists because copy-and-paste code is difficult to maintain. Unit test code is also code that you have to maintain. All the rules about writing maintainable code also apply to unit test code.

Branching in test #

What's the alternative? One option (that shouldn't be easily dismissed) is to introduce a Test Helper to perform the conversion from a nullable name to a non-nullable name. Such a helper would have a cyclomatic complexity of 2, but could be unit tested in isolation. It might even turn out that it'd be useful in the production code.

Still, that seems like overkill, so I instead made the taboo move and added branching logic to the existing test to see how it'd look:

[Theory]
[InlineData("2023-11-24 19:00", "juliad@example.net", "Julia Domna", 5)]
[InlineData("2024-02-13 18:15", "x@example.com", "Xenia Ng", 9)]
[InlineData("2023-08-23 16:55", "kite@example.edu", null, 2)]
public async Task PostValidReservationWhenDatabaseIsEmpty(
    string at,
    string email,
    string name,
    int quantity)
{
    var db = new FakeDatabase();
    var sut = new ReservationsController(db);
 
    var dto = new ReservationDto
    {
        At = at,
        Email = email,
        Name = name,
        Quantity = quantity
    };
    await sut.Post(dto);
 
    var expected = new Reservation(
        DateTime.Parse(dto.At, CultureInfo.InvariantCulture),
        dto.Email,
        dto.Name ?? "",
        dto.Quantity);
    Assert.Contains(expected, db);
}

Notice that the expected name is now computed as dto.Name ?? "". Perhaps you think about branching instructions as relating exclusively to keywords such as if or switch, but the ?? operator is also a branching instruction. The test now has a cyclomatic complexity of 2.

Is that okay?

To branch or not to branch #

I think that in this case, it's okay to slightly increase the cyclomatic complexity of the test. It's not something I just pull out of my hat, though. I think it's possible to adjust the above heuristics to embrace this sort of variation.

To be clear, I consider this an advanced practice. If you're just getting started with unit testing, try to keep tests simple. Keep the cyclomatic complexity at 1.

Had I been in the above situation a couple of years ago, I might not have considered this option. About a year ago, though, I watched John Hughes' presentation Building on developers' intuitions to create effective property-based tests. When he, about 15 minutes in, wrote a test with a branching instruction, I remember becoming quite uncomfortable. This lasted for a while until I understood where he was going with it. It's truly an inspiring and illuminating talk; I highly recommend it.

How it relates to the problem presented here is through coverage. While the PostValidReservationWhenDatabaseIsEmpty test now has a cyclomatic complexity of 2, it's a parametrised test with three test cases. Two of these cover one branch, and the third covers the other.

What's more important is the process that produced the test. I added one test case at a time, and for each case, I saw the test fail.

Specifically, when I added the third test case with the null name, I first added the branching expression dto.Name ?? "" and ran the two existing tests. They still both passed, which bolstered my belief that they both exercised the left branch of that expression. I then added the third case and saw that it (and only it) failed. This supported my belief that the third case exercised the right branch of ??.

Branching in unit tests isn't something I do lightly. I still believe that it could make the test more vulnerable to future changes. I'm particularly worried about making a future change that might shift one or more of these test cases into false negatives in the form of tautological assertions.

Conclusion #

As you can tell, when I feel that I'm moving onto thin ice, I move deliberately. If there's one thing I've learned from decades of professional programming it's that my brain loves jumping to conclusions. Moving slowly and deliberately is my attempt at countering this tendency. I believe that it enables me to go faster in the long run.

I don't think that branching in unit tests should be common, but I believe that it may be occasionally valid. The key, I think, is to guarantee that each branch in the test is covered by a test case. The implication is that there must be at least as many test cases as the cyclomatic complexity. In other words, the test must be a parametrised test.

Comments

		[Theory]
		[InlineData("2023-11-24 19:00", "juliad@example.net", "Julia Domna", 5)]
		[InlineData("2024-02-13 18:15", "x@example.com",      "Xenia Ng",    9)]
		public async Task PostValidReservationWithNameWhenDatabaseIsEmpty
		           (string at,          string email,         string name,   int quantity) =>
			PostValidReservationWhenDatabaseIsEmpty(at, email, name, expectedName: name, quantity);

		[Fact]
		public async Task PostValidReservationWithoutNameWhenDatabaseIsEmpty() =>
			PostValidReservationWhenDatabaseIsEmpty(
				at          : "2023-11-24 19:00",
				email       : "juliad@example.net",
				name        : null,
				expectedName: "",
				quantity    : 5);

		private async Task PostValidReservationWhenDatabaseIsEmpty(
			    string at,
			    string email,
			    string name,
			    string expectedName,
			    int    quantity)
		{
		    var db = new FakeDatabase();
		    var sut = new ReservationsController(db);
		 
		    var dto = new ReservationDto
		    {
		        At       = at,
		        Email    = email,
		        Name     = name,
		        Quantity = quantity,
		    };
		    await sut.Post(dto);
		 
		    var expected = new Reservation(
		        DateTime.Parse(dto.At, CultureInfo.InvariantCulture),
		        dto.Email,
		        expectedName, // /!\ Not `dto.Name`,
		        dto.Quantity);
		    Assert.Contains(expected, db);
		}

				

private class PostValidReservationWhenDatabaseIsEmptyTestCases :
    TheoryData<ReservationDto, Reservation>
{
    public PostValidReservationWhenDatabaseIsEmptyTestCases()
    {
        AddWithName(new DateTime(2023, 11, 24, 19, 0, 0), "juliad@example.net", "Julia Domna", 5);
        AddWithName(new DateTime(2024, 2, 13, 18, 15, 0), "x@example.com", "Xenia Ng", 9);
        AddWithoutName(new DateTime(2023, 8, 23, 16, 55, 0), "kite@example.edu", 2);
    }
 
    private void AddWithName(DateTime at, string email, string name, int quantity)
    {
        Add(new ReservationDto
            {
                At = at.ToString("O"),
                Email = email,
                Name = name,
                Quantity = quantity
            },
            new Reservation(at, email, name, quantity));
    }
 
    private void AddWithoutName(DateTime at, string email, int quantity)
    {
        Add(new ReservationDto { At = at.ToString("O"), Email = email, Quantity = quantity },
            new Reservation(at, email, "", quantity));
    }
}
 
 
[Theory, ClassData(typeof(PostValidReservationWhenDatabaseIsEmptyTestCases))]
public async Task PostValidReservationWhenDatabaseIsEmpty(
    ReservationDto dto, Reservation expected)
{
    var db = new FakeDatabase();
    var sut = new ReservationsController(db);
 
    await sut.Post(dto);
 
    Assert.Contains(expected, db);
}

• ← Previous
• Archive
• Next →